MCP OAuth: persist_if_needed() overwrites the stored refresh_token with the provider's partial refresh response

Start in oauth.rs at persist_if_needed(), then compare its credential persistence with refreshed_tokens() in oauth/refresh_transaction.rs. Review the persist_if_needed() call sites in rmcp_client.rs and reproduce with a provider whose refresh response omits refresh_token. Done means the durable credential retains the existing refresh token and later refreshes do not require reconnection.

Version

codex-cli 0.146.0-alpha.3.1 (the binary bundled in ChatGPT.app 26.721.41059, /Applications/ChatGPT.app/Contents/Resources/codex)

Platform

Darwin 25.5.0 arm64 arm

What issue are you seeing?

A remote HTTP MCP server keeps showing "Authentication expired. Reconnect to keep using this MCP server." every ~15 minutes, even though the server issued a refresh token valid for 30 days.

The stored keychain credential (Codex MCP Credentials / <server>|<hash>) ends up as:

{"server_name":"...","url":"...","client_id":"...",
 "token_response":{"access_token":"<redacted>","token_type":"bearer","expires_in":900},
 "expires_at":1784969933943}

— the refresh_token that the authorization-code exchange returned is gone. Other MCP servers in the same keychain (Notion, Linear, Cloudflare) all retain refresh_token, so this is specific to how this provider's refresh response is handled.

Root cause

The affected provider omits refresh_token from its refresh-grant response. That is legal per RFC 6749 §6 — the client must keep the previously issued one.

oauth/refresh_transaction.rs::refreshed_tokens() already handles this correctly for the proactive, expiry-driven refresh path:

fn refreshed_tokens(mut token_response: OAuthTokenResponse, previous: &StoredOAuthTokens, ...) -> StoredOAuthTokens {
    if token_response.refresh_token().is_none() {
        token_response.set_refresh_token(previous.token_response.0.refresh_token().cloned());
    }
    ...
}

But oauth.rs::persist_if_needed() is a second write path with no such merge — it reads whatever RMCP's AuthorizationManager currently holds and saves it verbatim:

pub(crate) async fn persist_if_needed(&self) -> Result<()> {
    let (client_id, maybe_credentials) = { ... guard.get_credentials().await }?;
    match maybe_credentials {
        Some(credentials) => {
            let new_token_response = WrappedOAuthTokenResponse(credentials.clone());
            ...
            self.inner.credential_store.save(&DefaultKeyringStore, &self.inner.server_name, &stored)?;

When RMCP's AuthClient performs its own reactive refresh (e.g. on a 401 during handshake), the manager is left holding the provider's partial response with no refresh token, and persist_if_needed() — called from several places in rmcp_client.rs (~lines 487, 756, 963, 1184), including after initialize — writes that partial response over the durable credential. From then on refresh_if_needed() short-circuits at the refresh_token().is_none_or(...) guard and returns AuthorizationRequired, so the user is asked to reconnect every time the 15-minute access token expires. The loss is permanent until a full re-authorization, and repeats immediately after it.

The comment in refresh_transaction.rs ("so that hook cannot overwrite durable credentials with the provider's partial response") shows the hazard is already understood for the refresh path; persist_if_needed() looks like the remaining gap.

What steps can reproduce the bug?

1. Configure an HTTP MCP server whose refresh-grant response omits refresh_token and whose access tokens are short-lived. Self-hosted Activepieces CE 0.86.3 (https://<host>/mcp/platform) reproduces it out of the box — access token TTL 900 s, refresh response = {access_token, token_type, expires_in} only.
2. Authorize the server from Codex. The initial exchange returns a refresh_token and everything works.
3. Keep the session alive past the first access-token expiry.
4. Read the keychain entry: refresh_token is no longer present, and Codex begins asking to reconnect on every subsequent expiry.

What is the expected behavior?

persist_if_needed() should apply the same carry-forward as refreshed_tokens() — never persist a token response that drops a refresh_token/scopes the durable credential already has — so that providers which legally omit refresh_token on refresh keep working until the refresh token itself expires.

Additional information

Verified against the sources at tag rust-v0.146.0-alpha.3.1. The carry-forward in refreshed_tokens() was introduced in 6962a2e ("Serialize MCP OAuth credential refreshes", #32229); persist_if_needed() was not covered by that change.