Skip to content

Made token refresh code resilient to missing id_token #5782

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 27, 2025
Merged

Made token refresh code resilient to missing id_token #5782

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
53a8b3d
Made token refresh code resilient to missing `id_token`
etraut-openai Oct 26, 2025
1a4960e
Fixed bug found in code review -- need to handle 401 failures when us…
etraut-openai Oct 27, 2025
9788e29
Fixed lint
etraut-openai Oct 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions codex-rs/core/src/auth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,14 +382,16 @@ pub fn write_auth_json(auth_file: &Path, auth_dot_json: &AuthDotJson) -> std::io

async fn update_tokens(
auth_file: &Path,
id_token: String,
id_token: Option<String>,
access_token: Option<String>,
refresh_token: Option<String>,
) -> std::io::Result<AuthDotJson> {
let mut auth_dot_json = try_read_auth_json(auth_file)?;

let tokens = auth_dot_json.tokens.get_or_insert_with(TokenData::default);
tokens.id_token = parse_id_token(&id_token).map_err(std::io::Error::other)?;
if let Some(id_token) = id_token {
tokens.id_token = parse_id_token(&id_token).map_err(std::io::Error::other)?;
}
if let Some(access_token) = access_token {
tokens.access_token = access_token;
}
Expand Down Expand Up @@ -445,7 +447,7 @@ struct RefreshRequest {

#[derive(Deserialize, Clone)]
struct RefreshResponse {
id_token: String,
id_token: Option<String>,
access_token: Option<String>,
refresh_token: Option<String>,
}
Expand Down Expand Up @@ -511,6 +513,35 @@ mod tests {
assert_eq!(auth_dot_json, same_auth_dot_json);
}

#[tokio::test]
async fn refresh_without_id_token() {
let codex_home = tempdir().unwrap();
let fake_jwt = write_auth_file(
AuthFileParams {
openai_api_key: None,
chatgpt_plan_type: "pro".to_string(),
chatgpt_account_id: None,
},
codex_home.path(),
)
.expect("failed to write auth file");

let auth_file = super::get_auth_file(codex_home.path());
let updated = super::update_tokens(
auth_file.as_path(),
None,
Some("new-access-token".to_string()),
Some("new-refresh-token".to_string()),
)
.await
.expect("update_tokens should succeed");

let tokens = updated.tokens.expect("tokens should exist");
assert_eq!(tokens.id_token.raw_jwt, fake_jwt);
assert_eq!(tokens.access_token, "new-access-token");
assert_eq!(tokens.refresh_token, "new-refresh-token");
}

#[test]
fn login_with_api_key_overwrites_existing_auth_json() {
let dir = tempdir().unwrap();
Expand Down
5 changes: 4 additions & 1 deletion codex-rs/core/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,10 @@ impl ModelClient {
&& let Some(manager) = auth_manager.as_ref()
&& manager.auth().is_some()
{
let _ = manager.refresh_token().await;
manager
.refresh_token()
Copy link
Collaborator

@pakrym-oai pakrym-oai Oct 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this throw for API keys? We have this in refresh_token

    pub async fn refresh_token(&self) -> Result<String, std::io::Error> {
        tracing::info!("Refreshing token");

        let token_data = self
            .get_current_token_data()
            .ok_or(std::io::Error::other("Token data is not available."))?;

Copy link
Collaborator

@pakrym-oai pakrym-oai Oct 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep:
image

Copy link
Collaborator

@pakrym-oai pakrym-oai Oct 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also prefix the message.

Copy link
Collaborator Author

@etraut-openai etraut-openai Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

.await
.map_err(|err| StreamAttemptError::Fatal(err.into()))?;
}

// The OpenAI Responses endpoint returns structured JSON bodies even for 4xx/5xx
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/codex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1855,7 +1855,7 @@ async fn run_turn(
// at a seemingly frozen screen.
sess.notify_stream_error(
turn_context.as_ref(),
format!("Re-connecting... {retries}/{max_retries}"),
format!("Reconnecting... {retries}/{max_retries}"),
)
.await;

Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/codex/compact.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ async fn run_compact_task_inner(
let delay = backoff(retries);
sess.notify_stream_error(
turn_context.as_ref(),
format!("Re-connecting... {retries}/{max_retries}"),
format!("Reconnecting... {retries}/{max_retries}"),
)
.await;
tokio::time::sleep(delay).await;
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/tui/src/chatwidget/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2222,7 +2222,7 @@ fn plan_update_renders_history_cell() {
fn stream_error_updates_status_indicator() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual();
chat.bottom_pane.set_task_running(true);
let msg = "Re-connecting... 2/5";
let msg = "Reconnecting... 2/5";
chat.handle_codex_event(Event {
id: "sub-1".into(),
msg: EventMsg::StreamError(StreamErrorEvent {
Expand Down
Loading