From 78249f7fcec6ff66f6b9277773e07b3e48257709 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 25 Feb 2026 13:55:02 -0800 Subject: [PATCH] codex: address PR review feedback (#12815) --- codex-rs/rmcp-client/src/oauth.rs | 55 ++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/codex-rs/rmcp-client/src/oauth.rs b/codex-rs/rmcp-client/src/oauth.rs index f7699a8dec..468ab30e9f 100644 --- a/codex-rs/rmcp-client/src/oauth.rs +++ b/codex-rs/rmcp-client/src/oauth.rs @@ -287,6 +287,7 @@ enum GuardedRefreshOutcome { ReloadedChanged(StoredOAuthTokens), ReloadedNoChange, MissingOrInvalid, + ReloadFailed, } impl OAuthPersistor { @@ -401,6 +402,7 @@ impl OAuthPersistor { self.persist_if_needed().await } GuardedRefreshOutcome::MissingOrInvalid => self.apply_runtime_credentials(None).await, + GuardedRefreshOutcome::ReloadFailed => Ok(()), } } @@ -416,22 +418,15 @@ impl OAuthPersistor { return GuardedRefreshOutcome::NoAction; } - let persisted_credentials = match load_oauth_tokens( + guarded_refresh_outcome_from_load_result( + cached_credentials, + load_oauth_tokens( + &self.inner.server_name, + &self.inner.url, + self.inner.store_mode, + ), &self.inner.server_name, - &self.inner.url, - self.inner.store_mode, - ) { - Ok(credentials) => credentials, - Err(error) => { - warn!( - "failed to reload OAuth tokens for server {}: {error}", - self.inner.server_name - ); - return GuardedRefreshOutcome::MissingOrInvalid; - } - }; - - determine_guarded_refresh_outcome(cached_credentials, persisted_credentials) + ) } async fn apply_runtime_credentials( @@ -472,6 +467,22 @@ impl OAuthPersistor { } } +fn guarded_refresh_outcome_from_load_result( + cached_credentials: &StoredOAuthTokens, + persisted_credentials: Result>, + server_name: &str, +) -> GuardedRefreshOutcome { + let persisted_credentials = match persisted_credentials { + Ok(credentials) => credentials, + Err(error) => { + warn!("failed to reload OAuth tokens for server {server_name}: {error}"); + return GuardedRefreshOutcome::ReloadFailed; + } + }; + + determine_guarded_refresh_outcome(cached_credentials, persisted_credentials) +} + const FALLBACK_FILENAME: &str = ".credentials.json"; const MCP_SERVER_TYPE: &str = "http"; @@ -1050,6 +1061,20 @@ mod tests { ); } + #[test] + fn guarded_refresh_outcome_keeps_state_recoverable_when_reload_fails() { + let error = anyhow::anyhow!("transient read failure"); + + assert_eq!( + super::guarded_refresh_outcome_from_load_result( + &sample_tokens(), + Err(error), + "test-server", + ), + super::GuardedRefreshOutcome::ReloadFailed, + ); + } + #[test] fn oauth_tokens_equal_for_refresh_ignores_only_expires_in() { let left = sample_tokens();