-
Notifications
You must be signed in to change notification settings - Fork 6.3k
fix: refresh expired MCP OAuth tokens #6209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
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.
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ 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". |
Repro: - Set up an HTTP MCP server with OAuth expiring tokens like Datadog - Use it for a bit until the token expires - Start a new codex session Expected results: - The token gets transparently refreshed and the MCP server continues to work Actual results: - You start getting 401s and need to do `codex mcp logout` and log back in.
…pired
The old load path in codex-rs/rmcp-client/src/oauth.rs only set expires_in when the stored expiry was in the future:
// codex-rs/rmcp-client/src/oauth.rs:369-374 (original)
if let Some(expires_at) = entry.expires_at
&& let Some(seconds) = expires_in_from_timestamp(expires_at)
{
let duration = Duration::from_secs(seconds);
token_response.set_expires_in(Some(&duration));
}
And expires_in_from_timestamp returned None for expired tokens:
// codex-rs/rmcp-client/src/oauth.rs:444-453 (original)
if expires_at <= now_ms {
None
} else {
Some((expires_at - now_ms) / 1000)
}
So when the stored token was already expired, set_expires_in was never called, RMCP saw “no expiry,” and it didn’t auto-refresh on handshake.
13da57b to
214ac14
Compare
|
Closing in favor of modelcontextprotocol/rust-sdk#509 |
Repro:
Expected results:
Actual results:
codex mcp logoutand log back in.