From f87ba04ec3cd386a92e775405b252a310b40bb0d Mon Sep 17 00:00:00 2001 From: tejsudha-openai Date: Wed, 14 Oct 2026 14:05:23 +0000 Subject: [PATCH] Report the reason for ChatGPT token refresh requests (#53991) ## What changed Include a `refresh_reason` diagnostic in ChatGPT OAuth token refresh requests to distinguish `proactive_expiry`, `scheduled`, `unauthorized_recovery`, and `forced` refreshes. Omit the field for gateway refresh requests. ## Testing Extend refresh tests to check the reported reasons, including scheduled refreshes when access token expiry is unavailable. GitOrigin-RevId: b0294a61ffc3089a66e7b132d4f45661a9e18d38 --- codex-rs/login/src/auth/manager.rs | 73 +++++++++++++++------- codex-rs/login/src/gateway_auth.rs | 1 + codex-rs/login/src/oauth/client.rs | 24 +++++++ codex-rs/login/src/oauth/client_tests.rs | 5 +- codex-rs/login/src/oauth/mod.rs | 1 + codex-rs/login/tests/suite/auth_refresh.rs | 57 +++++++++++++++++ 6 files changed, 137 insertions(+), 24 deletions(-) diff --git a/codex-rs/login/src/auth/manager.rs b/codex-rs/login/src/auth/manager.rs index cdc9c8d356..88c80f5af2 100644 --- a/codex-rs/login/src/auth/manager.rs +++ b/codex-rs/login/src/auth/manager.rs @@ -62,6 +62,7 @@ use crate::default_client::create_default_auth_client; use crate::oauth::ErrorBodyLimit; use crate::oauth::OAuthClient; use crate::oauth::OAuthError; +use crate::oauth::RefreshReason; use crate::oauth::RefreshTokenGrant; use crate::oauth::TokenEncoding; use crate::oauth::TokenEndpoint; @@ -1627,6 +1628,7 @@ fn persist_tokens( async fn request_chatgpt_token_refresh( refresh_token: String, client: &HttpClient, + refresh_reason: RefreshReason, ) -> Result { let client_id = oauth_client_id(); let endpoint = refresh_token_endpoint(); @@ -1644,6 +1646,7 @@ async fn request_chatgpt_token_refresh( .refresh(RefreshTokenGrant { refresh_token: &refresh_token, resource: None, + refresh_reason: Some(refresh_reason), }) .await { @@ -2011,14 +2014,18 @@ impl UnauthorizedRecovery { } } UnauthorizedRecoveryStep::RefreshToken => { - self.manager.refresh_token_from_authority().await?; + self.manager + .refresh_token_from_authority_with_reason(RefreshReason::UnauthorizedRecovery) + .await?; self.step = UnauthorizedRecoveryStep::Done; return Ok(UnauthorizedRecoveryStepResult { auth_state_changed: Some(true), }); } UnauthorizedRecoveryStep::ExternalRefresh => { - self.manager.refresh_token_from_authority().await?; + self.manager + .refresh_token_from_authority_with_reason(RefreshReason::UnauthorizedRecovery) + .await?; self.step = UnauthorizedRecoveryStep::Done; return Ok(UnauthorizedRecoveryStepResult { auth_state_changed: Some(true), @@ -2395,8 +2402,8 @@ impl AuthManager { } let auth = self.auth_cached()?; - if Self::should_refresh_proactively(&auth) - && let Err(err) = self.refresh_token().await + if let Some(reason) = Self::proactive_refresh_reason(&auth) + && let Err(err) = self.refresh_token_with_reason(reason).await { tracing::error!("Failed to refresh token: {}", err); return Some(auth); @@ -2843,6 +2850,13 @@ impl AuthManager { /// we can assume that the source already refreshed it. Otherwise, ask the /// token authority to refresh. pub async fn refresh_token(&self) -> Result<(), RefreshTokenError> { + self.refresh_token_with_reason(RefreshReason::Forced).await + } + + async fn refresh_token_with_reason( + &self, + reason: RefreshReason, + ) -> Result<(), RefreshTokenError> { let _refresh_guard = self.refresh_lock.acquire().await.map_err(|_| { RefreshTokenError::Permanent(RefreshTokenFailedError::new( RefreshTokenFailedReason::Other, @@ -2868,7 +2882,7 @@ impl AuthManager { tracing::info!("Skipping token refresh because auth changed after guarded reload."); Ok(()) } - ReloadOutcome::ReloadedNoChange => self.refresh_token_from_authority_impl().await, + ReloadOutcome::ReloadedNoChange => self.refresh_token_from_authority_impl(reason).await, ReloadOutcome::Skipped => { Err(RefreshTokenError::Permanent(RefreshTokenFailedError::new( RefreshTokenFailedReason::Other, @@ -2882,16 +2896,27 @@ impl AuthManager { /// it and update the shared cache. If the token refresh fails, returns the /// error to the caller. pub async fn refresh_token_from_authority(&self) -> Result<(), RefreshTokenError> { + self.refresh_token_from_authority_with_reason(RefreshReason::Forced) + .await + } + + async fn refresh_token_from_authority_with_reason( + &self, + reason: RefreshReason, + ) -> Result<(), RefreshTokenError> { let _refresh_guard = self.refresh_lock.acquire().await.map_err(|_| { RefreshTokenError::Permanent(RefreshTokenFailedError::new( RefreshTokenFailedReason::Other, REFRESH_TOKEN_UNKNOWN_MESSAGE.to_string(), )) })?; - self.refresh_token_from_authority_impl().await + self.refresh_token_from_authority_impl(reason).await } - async fn refresh_token_from_authority_impl(&self) -> Result<(), RefreshTokenError> { + async fn refresh_token_from_authority_impl( + &self, + reason: RefreshReason, + ) -> Result<(), RefreshTokenError> { tracing::info!("Refreshing token"); let attempted_auth = self.auth_cached(); @@ -2913,8 +2938,12 @@ impl AuthManager { "Token data is not available.", )) })?; - self.refresh_and_persist_chatgpt_token(chatgpt_auth, token_data.refresh_token) - .await + self.refresh_and_persist_chatgpt_token( + chatgpt_auth, + token_data.refresh_token, + reason, + ) + .await } Some( CodexAuth::ApiKey(_) @@ -2998,28 +3027,24 @@ impl AuthManager { .is_some_and(AuthMode::uses_codex_backend) } - fn should_refresh_proactively(auth: &CodexAuth) -> bool { + fn proactive_refresh_reason(auth: &CodexAuth) -> Option { let chatgpt_auth = match auth { CodexAuth::Chatgpt(chatgpt_auth) => chatgpt_auth, - _ => return false, + _ => return None, }; - let auth_dot_json = match chatgpt_auth.current_auth_json() { - Some(auth_dot_json) => auth_dot_json, - None => return false, - }; + let auth_dot_json = chatgpt_auth.current_auth_json()?; if let Some(tokens) = auth_dot_json.tokens.as_ref() && let Ok(Some(expires_at)) = parse_jwt_expiration(&tokens.access_token) { - return expires_at + return (expires_at <= Utc::now() - + chrono::Duration::minutes(CHATGPT_ACCESS_TOKEN_REFRESH_WINDOW_MINUTES); + + chrono::Duration::minutes(CHATGPT_ACCESS_TOKEN_REFRESH_WINDOW_MINUTES)) + .then_some(RefreshReason::ProactiveExpiry); } - let last_refresh = match auth_dot_json.last_refresh { - Some(last_refresh) => last_refresh, - None => return false, - }; - last_refresh < Utc::now() - chrono::Duration::days(TOKEN_REFRESH_INTERVAL) + let last_refresh = auth_dot_json.last_refresh?; + (last_refresh < Utc::now() - chrono::Duration::days(TOKEN_REFRESH_INTERVAL)) + .then_some(RefreshReason::Scheduled) } async fn refresh_external_auth( @@ -3090,8 +3115,10 @@ impl AuthManager { &self, auth: &ChatgptAuth, refresh_token: String, + reason: RefreshReason, ) -> Result<(), RefreshTokenError> { - let refresh_response = request_chatgpt_token_refresh(refresh_token, auth.client()).await?; + let refresh_response = + request_chatgpt_token_refresh(refresh_token, auth.client(), reason).await?; persist_tokens( auth.storage(), diff --git a/codex-rs/login/src/gateway_auth.rs b/codex-rs/login/src/gateway_auth.rs index 00871113ed..8ceb769e01 100644 --- a/codex-rs/login/src/gateway_auth.rs +++ b/codex-rs/login/src/gateway_auth.rs @@ -321,6 +321,7 @@ impl GatewayAuthManager { .refresh::(RefreshTokenGrant { refresh_token, resource: self.state.config.resource.as_deref(), + refresh_reason: None, }) .await { diff --git a/codex-rs/login/src/oauth/client.rs b/codex-rs/login/src/oauth/client.rs index f9a1d453d5..78de50c50e 100644 --- a/codex-rs/login/src/oauth/client.rs +++ b/codex-rs/login/src/oauth/client.rs @@ -39,6 +39,27 @@ pub(crate) struct AuthorizationCodeGrant<'a> { pub(crate) struct RefreshTokenGrant<'a> { pub refresh_token: &'a str, pub resource: Option<&'a str>, + pub refresh_reason: Option, +} + +/// Caller-reported diagnostics for AuthAPI; never evidence of token validity. +#[derive(Clone, Copy, Debug)] +pub(crate) enum RefreshReason { + ProactiveExpiry, + Scheduled, + UnauthorizedRecovery, + Forced, +} + +impl RefreshReason { + fn as_str(self) -> &'static str { + match self { + Self::ProactiveExpiry => "proactive_expiry", + Self::Scheduled => "scheduled", + Self::UnauthorizedRecovery => "unauthorized_recovery", + Self::Forced => "forced", + } + } } /// Shared OAuth protocol implementation; callers retain tokens, storage, and recovery decisions. @@ -85,6 +106,9 @@ impl<'a> OAuthClient<'a> { if let Some(resource) = grant.resource { parameters.push(("resource", resource)); } + if let Some(reason) = grant.refresh_reason { + parameters.push(("refresh_reason", reason.as_str())); + } self.exchange(¶meters, &[grant.refresh_token]).await } diff --git a/codex-rs/login/src/oauth/client_tests.rs b/codex-rs/login/src/oauth/client_tests.rs index 17e93ed5ce..8b2ae9b2bb 100644 --- a/codex-rs/login/src/oauth/client_tests.rs +++ b/codex-rs/login/src/oauth/client_tests.rs @@ -184,6 +184,7 @@ async fn rejection_preserves_status_and_omits_oversized_diagnostics() { .refresh::(RefreshTokenGrant { refresh_token, resource: Some("urn:gateway"), + refresh_reason: None, }) .await; let Err(OAuthError::Rejected(error)) = result else { @@ -227,6 +228,7 @@ async fn rejection_keeps_status_when_reading_the_body_fails() { .refresh::(RefreshTokenGrant { refresh_token: "refresh", resource: None, + refresh_reason: None, }) .await; let Err(OAuthError::Rejected(error)) = result else { @@ -287,7 +289,8 @@ async fn success_body_revocation_preserves_the_policy_error() { tokio::join!( form.refresh::(RefreshTokenGrant { refresh_token: "refresh", - resource: None + resource: None, + refresh_reason: None, }), server ) diff --git a/codex-rs/login/src/oauth/mod.rs b/codex-rs/login/src/oauth/mod.rs index 329e1be1e8..8ac9a559d0 100644 --- a/codex-rs/login/src/oauth/mod.rs +++ b/codex-rs/login/src/oauth/mod.rs @@ -16,6 +16,7 @@ pub(crate) use authorization::build_authorization_url; pub(crate) use authorization::generate_state; pub(crate) use client::AuthorizationCodeGrant; pub(crate) use client::OAuthClient; +pub(crate) use client::RefreshReason; pub(crate) use client::RefreshTokenGrant; pub(crate) use client::TokenEncoding; pub(crate) use client::TokenEndpoint; diff --git a/codex-rs/login/tests/suite/auth_refresh.rs b/codex-rs/login/tests/suite/auth_refresh.rs index 5e353de173..60b6d2a3fe 100644 --- a/codex-rs/login/tests/suite/auth_refresh.rs +++ b/codex-rs/login/tests/suite/auth_refresh.rs @@ -32,6 +32,7 @@ use tempfile::TempDir; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; +use wiremock::matchers::body_partial_json; use wiremock::matchers::method; use wiremock::matchers::path; @@ -119,6 +120,7 @@ async fn refresh_token_honors_respect_system_proxy() -> Result<()> { "client_id": "staging-client", "grant_type": "refresh_token", "refresh_token": INITIAL_REFRESH_TOKEN, + "refresh_reason": "forced", }) ); return Ok(()); @@ -224,6 +226,7 @@ async fn refresh_token_succeeds_updates_storage() -> Result<()> { "client_id": "staging-client", "grant_type": "refresh_token", "refresh_token": INITIAL_REFRESH_TOKEN, + "refresh_reason": "forced", }) ); @@ -333,6 +336,9 @@ async fn auth_refreshes_when_access_token_is_near_expiry() -> Result<()> { let server = MockServer::start().await; Mock::given(method("POST")) .and(path("/oauth/token")) + .and(body_partial_json( + json!({"refresh_reason": "proactive_expiry"}), + )) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "access_token": "new-access-token", "refresh_token": "new-refresh-token" @@ -388,6 +394,54 @@ async fn auth_refreshes_when_access_token_is_near_expiry() -> Result<()> { Ok(()) } +#[serial_test::serial(auth_env)] +#[tokio::test] +async fn auth_reports_scheduled_refresh_when_access_token_expiry_is_unavailable() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/oauth/token")) + .and(body_partial_json(json!({"refresh_reason": "scheduled"}))) + .respond_with(ResponseTemplate::new(/*s*/ 200).set_body_json(json!({ + "access_token": "new-access-token", + "refresh_token": "new-refresh-token" + }))) + .expect(/*r*/ 1) + .mount(&server) + .await; + + let ctx = RefreshTokenTestContext::new(&server).await?; + let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); + ctx.write_auth(&AuthDotJson { + auth_mode: Some(AuthMode::Chatgpt), + openai_api_key: None, + tokens: Some(initial_tokens.clone()), + last_refresh: Some(Utc::now() - Duration::days(9)), + agent_identity: None, + personal_access_token: None, + bedrock_api_key: None, + bedrock_access_keys: None, + }) + .await?; + + let refreshed = ctx + .auth_manager + .auth() + .await + .context("auth should refresh")?; + assert_eq!( + refreshed.get_token_data()?, + TokenData { + access_token: "new-access-token".to_string(), + refresh_token: "new-refresh-token".to_string(), + ..initial_tokens + } + ); + server.verify().await; + Ok(()) +} + #[serial_test::serial(auth_env)] #[tokio::test] async fn auth_skips_access_token_outside_refresh_window() -> Result<()> { @@ -1294,6 +1348,9 @@ async fn unauthorized_recovery_reloads_then_refreshes_tokens() -> Result<()> { let server = MockServer::start().await; Mock::given(method("POST")) .and(path("/oauth/token")) + .and(body_partial_json( + json!({"refresh_reason": "unauthorized_recovery"}), + )) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "access_token": "recovered-access-token", "refresh_token": "recovered-refresh-token"