Skip to content

[rmcp-client] Classify OAuth refresh and storage failures correctly - #26519

Closed
anp-oai wants to merge 3 commits into
mainfrom
codex/oauth-refresh-errors-repro
Closed

[rmcp-client] Classify OAuth refresh and storage failures correctly#26519
anp-oai wants to merge 3 commits into
mainfrom
codex/oauth-refresh-errors-repro

Conversation

@anp-oai

@anp-oai anp-oai commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Intent

Make OAuth startup failures distinguish genuine reauthentication requirements from transient provider and credential-storage failures, while retaining rotated credentials across retryable write failures and startup cancellation.

User-visible behavior

Before

  • A revoked or missing refresh credential, a transient token endpoint failure, and a credential-store failure could all surface as unauthenticated startup or "log in again" guidance, even when reauthentication would not help.
  • A successful refresh could update the active credentials but lose the rotated credentials if persistence failed or startup was cancelled before the later save, leaving stale durable credentials for the next connection.

After

  • A genuine invalid_grant, AuthorizationRequired, or missing refresh token produces actionable reauthentication guidance. Provider descriptions associated with revoked credentials are intentionally not displayed.
  • Transient provider/network failures remain distinguishable token-endpoint startup errors, while credential-store read/write failures remain distinguishable storage errors with a bounded actionable cause instead of login guidance.
  • Successful refreshes immediately retain the fresh in-memory credentials and expiry. Persistence is attempted before the MCP handshake; failed writes remain dirty and retry later without forcing a redundant refresh.

Concrete implementation

  • Refresh expired persisted credentials inside the startup timeout and attempt durable persistence before the MCP handshake.
  • Track fresh in-memory and last-persisted credentials separately so failed writes remain dirty, retry later, and do not trigger a second refresh.
  • Classify AuthorizationRequired, missing refresh tokens, and an exact RMCP invalid_grant error code as reauthentication; preserve bounded transient and storage causes without leaking revoked-token descriptions.
  • Add deterministic coverage for refresh failure classes, timeout/cancellation durability, real file/keyring causes, and write recovery.

Validation not covered by CI

  • just test -p codex-rmcp-client --test streamable_http_oauth_startup
  • just test -p codex-rmcp-client --lib
  • just test -p codex-mcp
  • just fix -p codex-rmcp-client
  • just fix -p codex-mcp
  • just fmt

The full just test -p codex-rmcp-client run passed 67/68 tests; the unrelated streamable_http_remote test could not locate a built codex binary in this worktree.

Deferred scope

@anp-oai anp-oai changed the title [rmcp-client] cover OAuth refresh failure handling [rmcp-client] Classify OAuth refresh and storage failures correctly Jun 5, 2026
@anp-oai

anp-oai commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

handing this off for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant