Resolve and pin MCP OAuth credential stores#30293
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c432fcdffc
ℹ️ 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".
c432fcd to
38b9e10
Compare
c99bdc7 to
e4dcca6
Compare
38b9e10 to
c003752
Compare
c003752 to
c93f236
Compare
e4dcca6 to
f1e1c17
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
# Conflicts: # codex-rs/rmcp-client/src/oauth.rs
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
# Conflicts: # codex-rs/rmcp-client/src/oauth.rs
|
@codex review — Codex |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Codex Thread 019edd6d-6f14-74e2-853c-345d1803d4a6
Stack
Review and merge in order. Every layer is independently correct and documents its safe stopping point.
This PR is layer 2.
Why
Autois keyring-first with a File fallback, but re-evaluating that policy during transport reconstruction or persistence can make one MCP client read from one store and later write to another. With rotating refresh tokens, the second store may contain an older token. This layer makes the source selected at client startup explicit and keeps that authority stable for the client lifecycle.What this PR does
resolve_oauth_tokens, which returns both credentials and the concrete File or Keyring source that supplied them.load_oauth_tokens_from_store, the exact-store reread operation used after a source is selected.Autoand adopt another store.OAuthPersistorthe resolved store and keeps subsequent persistence and removal on that store.Explicit decisions and non-goals
CODEX_HOME.Automay choose File at the start of a later process if keyring availability changes. Once this client resolves, a selected-store failure is returned instead of hot-switching.CODEX_HOMEinstances remain independent even when they can access the same Direct keyring credential.Safe stopping point
This PR can merge alone. A single MCP client no longer hot-switches credential stores across transport rebuilds or persistence. Two processes can still refresh the same selected credential concurrently until layer 3.
Review size
The net layer is 7 files, +336/−129. It contains store resolution/pinning only; the per-credential lock and refresh transaction were moved to openai/codex#30416.
Validation
just test -p codex-rmcp-client(97 passed; expected environment skips)cargo shear