mirror of
https://github.com/openai/codex.git
synced 2026-05-30 07:50:17 +00:00
[codex-cli] Refresh near-expiry ChatGPT access tokens before requests (#23546)
## Summary - refresh managed ChatGPT auth during auth resolution when its access token is inside ChatGPT web's five-minute near-expiry window - cover refresh-window decisions while preserving the existing expired-token refresh path ## Why Codex already resolves managed ChatGPT auth before outbound requests and refreshes expired access tokens there. This change adjusts the existing predicate to refresh a still-valid access token once it is within the same five-minute refresh window used by ChatGPT web, avoiding a request with a token about to expire. A cross-process serialization follow-up was explored in #24663 and closed for now; we do not currently suspect cross-process refresh races are a root cause of the refresh errors under investigation. External-token, API-key, and Agent Identity auth modes remain unchanged. ## Validation - `bazel test //codex-rs/login:login-all-test` - `just fmt` runs Rust formatting successfully, then its Python SDK Ruff step cannot install `openai-codex-cli-bin==0.131.0a4` on this Linux environment because no compatible wheel is published.
This commit is contained in:
@@ -84,6 +84,7 @@ struct ChatgptAuthState {
|
||||
}
|
||||
|
||||
const TOKEN_REFRESH_INTERVAL: i64 = 8;
|
||||
const CHATGPT_ACCESS_TOKEN_REFRESH_WINDOW_MINUTES: i64 = 5;
|
||||
|
||||
const REFRESH_TOKEN_EXPIRED_MESSAGE: &str = "Your access token could not be refreshed because your refresh token has expired. Please log out and sign in again.";
|
||||
const REFRESH_TOKEN_REUSED_MESSAGE: &str = "Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again.";
|
||||
@@ -1421,15 +1422,15 @@ impl AuthManager {
|
||||
}
|
||||
|
||||
/// Current cached auth (clone). May be `None` if not logged in or load failed.
|
||||
/// For stale managed ChatGPT auth, first performs a guarded reload and then
|
||||
/// refreshes only if the on-disk auth is unchanged.
|
||||
/// For managed ChatGPT auth that needs a proactive refresh, first performs
|
||||
/// a guarded reload and then refreshes only if the on-disk auth is unchanged.
|
||||
pub async fn auth(&self) -> Option<CodexAuth> {
|
||||
if let Some(auth) = self.resolve_external_api_key_auth().await {
|
||||
return Some(auth);
|
||||
}
|
||||
|
||||
let auth = self.auth_cached()?;
|
||||
if Self::is_stale_for_proactive_refresh(&auth)
|
||||
if Self::should_refresh_proactively(&auth)
|
||||
&& let Err(err) = self.refresh_token().await
|
||||
{
|
||||
tracing::error!("Failed to refresh token: {}", err);
|
||||
@@ -1808,7 +1809,7 @@ impl AuthManager {
|
||||
)
|
||||
}
|
||||
|
||||
fn is_stale_for_proactive_refresh(auth: &CodexAuth) -> bool {
|
||||
fn should_refresh_proactively(auth: &CodexAuth) -> bool {
|
||||
let chatgpt_auth = match auth {
|
||||
CodexAuth::Chatgpt(chatgpt_auth) => chatgpt_auth,
|
||||
_ => return false,
|
||||
@@ -1821,7 +1822,9 @@ impl AuthManager {
|
||||
if let Some(tokens) = auth_dot_json.tokens.as_ref()
|
||||
&& let Ok(Some(expires_at)) = parse_jwt_expiration(&tokens.access_token)
|
||||
{
|
||||
return expires_at <= Utc::now();
|
||||
return expires_at
|
||||
<= Utc::now()
|
||||
+ chrono::Duration::minutes(CHATGPT_ACCESS_TOKEN_REFRESH_WINDOW_MINUTES);
|
||||
}
|
||||
let last_refresh = match auth_dot_json.last_refresh {
|
||||
Some(last_refresh) => last_refresh,
|
||||
|
||||
@@ -158,6 +158,102 @@ async fn refresh_token_refreshes_when_auth_is_unchanged() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[tokio::test]
|
||||
async fn auth_refreshes_when_access_token_is_near_expiry() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"access_token": "new-access-token",
|
||||
"refresh_token": "new-refresh-token"
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let ctx = RefreshTokenTestContext::new(&server).await?;
|
||||
let initial_last_refresh = Utc::now();
|
||||
let near_expiry_access_token = access_token_with_expiration(Utc::now() + Duration::minutes(4));
|
||||
let initial_tokens = build_tokens(&near_expiry_access_token, INITIAL_REFRESH_TOKEN);
|
||||
let initial_auth = AuthDotJson {
|
||||
auth_mode: Some(AuthMode::Chatgpt),
|
||||
openai_api_key: None,
|
||||
tokens: Some(initial_tokens.clone()),
|
||||
last_refresh: Some(initial_last_refresh),
|
||||
agent_identity: None,
|
||||
};
|
||||
ctx.write_auth(&initial_auth).await?;
|
||||
|
||||
let cached_auth = ctx
|
||||
.auth_manager
|
||||
.auth()
|
||||
.await
|
||||
.context("auth should be cached")?;
|
||||
|
||||
let refreshed_tokens = TokenData {
|
||||
access_token: "new-access-token".to_string(),
|
||||
refresh_token: "new-refresh-token".to_string(),
|
||||
..initial_tokens.clone()
|
||||
};
|
||||
let cached = cached_auth
|
||||
.get_token_data()
|
||||
.context("token data should refresh")?;
|
||||
assert_eq!(cached, refreshed_tokens);
|
||||
let stored = ctx.load_auth()?;
|
||||
let tokens = stored.tokens.as_ref().context("tokens should exist")?;
|
||||
assert_eq!(tokens, &refreshed_tokens);
|
||||
let refreshed_at = stored
|
||||
.last_refresh
|
||||
.as_ref()
|
||||
.context("last_refresh should be recorded")?;
|
||||
assert!(
|
||||
*refreshed_at >= initial_last_refresh,
|
||||
"last_refresh should advance"
|
||||
);
|
||||
|
||||
server.verify().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[tokio::test]
|
||||
async fn auth_skips_access_token_outside_refresh_window() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = MockServer::start().await;
|
||||
let ctx = RefreshTokenTestContext::new(&server).await?;
|
||||
let initial_last_refresh = Utc::now();
|
||||
let fresh_access_token = access_token_with_expiration(Utc::now() + Duration::minutes(6));
|
||||
let initial_tokens = build_tokens(&fresh_access_token, INITIAL_REFRESH_TOKEN);
|
||||
let initial_auth = AuthDotJson {
|
||||
auth_mode: Some(AuthMode::Chatgpt),
|
||||
openai_api_key: None,
|
||||
tokens: Some(initial_tokens.clone()),
|
||||
last_refresh: Some(initial_last_refresh),
|
||||
agent_identity: None,
|
||||
};
|
||||
ctx.write_auth(&initial_auth).await?;
|
||||
|
||||
let cached_auth = ctx
|
||||
.auth_manager
|
||||
.auth()
|
||||
.await
|
||||
.context("auth should be cached")?;
|
||||
|
||||
let cached = cached_auth
|
||||
.get_token_data()
|
||||
.context("token data should remain cached")?;
|
||||
assert_eq!(cached, initial_tokens);
|
||||
assert_eq!(ctx.load_auth()?, initial_auth);
|
||||
let requests = server.received_requests().await.unwrap_or_default();
|
||||
assert!(requests.is_empty(), "expected no refresh token requests");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_skips_refresh_when_auth_changed() -> Result<()> {
|
||||
|
||||
Reference in New Issue
Block a user