Skip to content

[codex] Classify nested MCP authentication startup errors#30257

Merged
felixxia-oai merged 1 commit into
mainfrom
felixxia/fix-nested-mcp-reauth-classification
Jun 26, 2026
Merged

[codex] Classify nested MCP authentication startup errors#30257
felixxia-oai merged 1 commit into
mainfrom
felixxia/fix-nested-mcp-reauth-classification

Conversation

@felixxia-oai

Copy link
Copy Markdown
Contributor

Summary

  • classify authentication-required RMCP startup failures, including errors nested inside ClientInitializeError::TransportError
  • let codex-mcp consume that classification so the existing reauthenticationRequired startup failure reason is emitted
  • add a regression test that performs real startup with an expired persisted OAuth token and no refresh token

Why

Follow-up to #29877.

RMCP stores streamable HTTP initialization failures inside a dynamic transport error whose payload is not exposed through the standard Rust error source chain. The original anyhow::Error::chain() check therefore missed the nested AuthError::AuthorizationRequired seen during real MCP startup and emitted failureReason: null.

The transport-specific inspection now lives in codex-rmcp-client, while codex-mcp consumes only the domain-level authentication-required result. This classifier does not distinguish first-time login from reauthentication; the existing auth-state logic remains responsible for that distinction.

User impact

When stored MCP OAuth credentials are expired and cannot be refreshed, app clients now receive failureReason: "reauthenticationRequired" on the failed startup update and can show the reconnect action. First-time login and unrelated startup failures remain unchanged.

Validation

  • just test -p codex-rmcp-client --test streamable_http_oauth_startup identifies_expired_unrefreshable_token_startup_error
  • just test -p codex-mcp startup_outcome_error_identifies_authentication_required
  • just test -p codex-mcp mcp_startup_failure_reason_requires_existing_oauth_and_auth_failure
  • cargo build -p codex-cli --bin codex
  • local app-server probe emitted failureReason: "reauthenticationRequired"
  • manual end-to-end reconnect flow confirmed
  • just fmt

@felixxia-oai felixxia-oai requested a review from xl-openai June 26, 2026 15:36
@felixxia-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 66f16fe543

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@felixxia-oai felixxia-oai marked this pull request as ready for review June 26, 2026 15:58
@felixxia-oai felixxia-oai enabled auto-merge (squash) June 26, 2026 15:58
@itxaiohanglover

Copy link
Copy Markdown

Clean refactor! Extracting is_authentication_required_error into the rmcp-client crate makes it reusable. The nested auth classification logic is better as a standalone function.

@felixxia-oai felixxia-oai merged commit 526f495 into main Jun 26, 2026
31 checks passed
@felixxia-oai felixxia-oai deleted the felixxia/fix-nested-mcp-reauth-classification branch June 26, 2026 21:11
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 26, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants