Skip to content

[rmcp-client] Add Codex-owned OAuth recovery#30091

Closed
stevenlee-oai wants to merge 6 commits into
dev/stevenlee/mcp-oauth-stack-5-cancellation-safe-refreshfrom
dev/stevenlee/mcp-oauth-stack-6-codex-recovery
Closed

[rmcp-client] Add Codex-owned OAuth recovery#30091
stevenlee-oai wants to merge 6 commits into
dev/stevenlee/mcp-oauth-stack-5-cancellation-safe-refreshfrom
dev/stevenlee/mcp-oauth-stack-6-codex-recovery

Conversation

@stevenlee-oai

@stevenlee-oai stevenlee-oai commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Codex Thread 019edd6d-6f14-74e2-853c-345d1803d4a6

Important

This PR belongs to the superseded MCP OAuth stack. Please review and merge the replacement stack beginning with openai/codex#30292. This PR remains available only as historical/reference context.

Replacement review order:

  1. openai/codex#30292 — shared File/Secrets store locking
  2. openai/codex#30293 — authoritative serialized refresh
  3. openai/codex#30294 — Codex-owned transport refresh and one-shot 401 recovery
  4. openai/codex#30295 — login/logout transaction serialization
  5. openai/codex#30296 — diagnostic-only Auto store drift reporting

This is part 6 of an eight-PR stack that prevents concurrent MCP OAuth refreshers from replaying a rotating refresh token or overwriting newer credentials.

Review order

  1. openai/codex#29017 — refresh transaction and lifecycle-local store pinning
  2. openai/codex#29020 — authoritative reread after lock acquisition
  3. openai/codex#29019 — serialize login and logout with refresh
  4. openai/codex#29021 — lock aggregate File and Secrets stores
  5. openai/codex#30090 — retain refreshed credentials after a persistence failure
  6. openai/codex#30091 — add Codex-owned proactive and 401 recovery (this PR)
  7. openai/codex#29018 — make Codex the exclusive refresh owner for all RMCP traffic
  8. openai/codex#30089 — end-to-end transport recovery coverage

Why this layer exists

Codex needs the machinery to proactively refresh expired credentials and recover once from a rejected access token before RMCP can safely be restricted to request-only credentials. This layer introduces that machinery while deliberately retaining the legacy RMCP refresh path so this intermediate tip remains independently correct.

What changes

  • Add Codex-owned preflight refresh before public MCP operations.
  • Add one-shot 401 recovery and transport reconstruction for startup and public operations.
  • Attribute a delayed 401 to the access token actually sent, so a second request sent with A adopts/retries with B rather than rotating B again.
  • Carry forward omitted refresh-token and scope fields from the previous credential snapshot.
  • Keep full credentials in RMCP and retain legacy post-operation persistence in this layer only; part 7 removes both atomically when every transport path has Codex recovery.

Decisions reviewers should preserve across the stack

  • This PR intentionally has overlapping Codex and RMCP capability for intermediate-tip correctness. Exclusive ownership begins only in part 7.
  • Delayed 401 recovery compares the rejected access token with current credentials; it must not sample only the current token when recovery starts.
  • OAuth refresh time is outside the caller's MCP timeout. The caller deadline still decides whether the public request itself may be retried.
  • A successful B is memory-authoritative before persistence; persistence failure is logged and not retried.
  • Auto remains lifecycle-local and never hot-switches after construction.
  • The original openai/codex#28647 and its branch are untouched.

Review focus

Review proactive refresh, access-token attribution, one-shot retry, and timeout accounting. Also verify the intentional compatibility bridge: this tip must still supply RMCP full credentials and persist any RMCP-owned refresh until part 7 lands.

Non-goals

  • Making Codex the exclusive refresh owner; that is part 7.
  • Covering RMCP-owned SSE/response/DELETE traffic; that wrapper also lands in part 7.
  • Persistence retries or durable Auto selection.

Validation

  • just test -p codex-rmcp-client: 106 passed, 5 skipped.
  • Coverage includes startup recovery, proactive refresh, one-shot 401 recovery, delayed rejected-token attribution, and omitted-field carry-forward.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5d97a9d5dd

ℹ️ 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".

Comment thread codex-rs/rmcp-client/src/rmcp_client.rs
Comment thread codex-rs/rmcp-client/src/rmcp_client.rs
@stevenlee-oai stevenlee-oai force-pushed the dev/stevenlee/mcp-oauth-stack-5-cancellation-safe-refresh branch from b04b694 to f800bd9 Compare June 25, 2026 18:45
@stevenlee-oai stevenlee-oai force-pushed the dev/stevenlee/mcp-oauth-stack-6-codex-recovery branch from 2bb8dc9 to 0e139f8 Compare June 25, 2026 18:45
@stevenlee-oai

Copy link
Copy Markdown
Contributor Author

[from Codex]: Superseded by the compact five-PR stack. Codex-owned proactive and 401 recovery is now reviewed atomically in #29018 with the RMCP transport ownership switch. New order: #29017#29019#29021#29018#30089.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant