mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
2e849703cd8a2eaebff094ef2a5108a1393d02ed
11 Commits
-
[codex] import token_data from codex-login directly (#15903)
## Why `token_data` is owned by `codex-login`, but `codex-core` was still re-exporting it. That let callers pull auth token types through `codex-core`, which keeps otherwise unrelated crates coupled to `codex-core` and makes `codex-core` more of a build-graph bottleneck. ## What changed - remove the `codex-core` re-export of `codex_login::token_data` - update the remaining `codex-core` internals that used `crate::token_data` to import `codex_login::token_data` directly - update downstream callers in `codex-rs/chatgpt`, `codex-rs/tui_app_server`, `codex-rs/app-server/tests/common`, and `codex-rs/core/tests` to import `codex_login::token_data` directly - add explicit `codex-login` workspace dependencies and refresh lock metadata for crates that now depend on it directly ## Validation - `cargo test -p codex-chatgpt --locked` - `just argument-comment-lint` - `just bazel-lock-update` - `just bazel-lock-check` ## Notes - attempted `cargo test -p codex-core --locked` and `cargo test -p codex-core auth_refresh --locked`, but both ran out of disk while linking `codex-core` test binaries in the local environment
Michael Bolin ·
2026-03-26 13:34:02 -07:00 -
chore: stop app-server auth refresh storms after permanent token failure (#15530)
built from #14256. PR description from @etraut-openai: This PR addresses a hole in [PR 11802](https://github.com/openai/codex/pull/11802). The previous PR assumed that app server clients would respond to token refresh failures by presenting the user with an error ("you must log in again") and then not making further attempts to call network endpoints using the expired token. While they do present the user with this error, they don't prevent further attempts to call network endpoints and can repeatedly call `getAuthStatus(refreshToken=true)` resulting in many failed calls to the token refresh endpoint. There are three solutions I considered here: 1. Change the getAuthStatus app server call to return a null auth if the caller specified "refreshToken" on input and the refresh attempt fails. This will cause clients to immediately log out the user and return them to the log in screen. This is a really bad user experience. It's also a breaking change in the app server contract that could break third-party clients. 2. Augment the getAuthStatus app server call to return an additional field that indicates the state of "token could not be refreshed". This is a non-breaking change to the app server API, but it requires non-trivial changes for all clients to properly handle this new field properly. 3. Change the getAuthStatus implementation to handle the case where a token refresh fails by marking the AuthManager's in-memory access and refresh tokens as "poisoned" so it they are no longer used. This is the simplest fix that requires no client changes. I chose option 3. Here's Codex's explanation of this change: When an app-server client asks `getAuthStatus(refreshToken=true)`, we may try to refresh a stale ChatGPT access token. If that refresh fails permanently (for example `refresh_token_reused`, expired, or revoked), the old behavior was bad in two ways: 1. We kept the in-memory auth snapshot alive as if it were still usable. 2. Later auth checks could retry refresh again and again, creating a storm of doomed `/oauth/token` requests and repeatedly surfacing the same failure. This is especially painful for app-server clients because they poll auth status and can keep driving the refresh path without any real chance of recovery. This change makes permanent refresh failures terminal for the current managed auth snapshot without changing the app-server API contract. What changed: - `AuthManager` now poisons the current managed auth snapshot in memory after a permanent refresh failure, keyed to the unchanged `AuthDotJson`. - Once poisoned, later refresh attempts for that same snapshot fail fast locally without calling the auth service again. - The poison is cleared automatically when auth materially changes, such as a new login, logout, or reload of different auth state from storage. - `getAuthStatus(includeToken=true)` now omits `authToken` after a permanent refresh failure instead of handing out the stale cached bearer token. This keeps the current auth method visible to clients, avoids forcing an immediate logout flow, and stops repeated refresh attempts for credentials that cannot recover. --------- Co-authored-by: Eric Traut <etraut@openai.com>
Celia Chen ·
2026-03-24 12:39:58 -07:00 -
chore: use access token expiration for proactive auth refresh (#15545)
Follow up to #15357 by making proactive ChatGPT auth refresh depend on the access token's JWT expiration instead of treating `last_refresh` age as the primary source of truth.
Celia Chen ·
2026-03-24 19:34:48 +00:00 -
Fix: proactive auth refresh to reload guarded disk state first (#15357)
## Summary Fix a managed ChatGPT auth bug where a stale Codex process could proactively refresh using an old in-memory refresh token even after another process had already rotated auth on disk. This changes the proactive `AuthManager::auth()` path to reuse the existing guarded `refresh_token()` flow instead of calling the refresh endpoint directly from cached auth state. ## Original Issue Users reported repeated `codexd` log lines like: ```text ERROR codex_core::auth: Failed to refresh token: error sending request for url (https://auth.openai.com/oauth/token) ``` In practice this showed up most often when multiple `codexd` processes were left running. Killing the extra processes stopped the noise, which suggested the issue was caused by stale auth state across processes rather than invalid user credentials. ## Diagnosis The bug was in the proactive refresh path used by `AuthManager::auth()`: - Process A could refresh successfully, rotate refresh token `R0` to `R1`, and persist the updated auth state plus `last_refresh` to disk. - Process B could keep an older auth snapshot cached in memory, still holding `R0` and the old `last_refresh`. - Later, when Process B called `auth()`, it checked staleness from its cached in-memory auth instead of first reloading from disk. - Because that cached `last_refresh` was stale, Process B would proactively call `/oauth/token` with stale refresh token `R0`. - On failure, `auth()` logged the refresh error but kept returning the same stale cached auth, so repeated `auth()` calls could keep retrying with dead state. This differed from the existing unauthorized-recovery flow, which already did the safer thing: guarded reload from disk first, then refresh only if the on-disk auth was unchanged. ## What Changed - Switched proactive refresh in `AuthManager::auth()` to: - do a pure staleness check on cached auth - call `refresh_token()` when stale - return the original cached auth on genuine refresh failure, preserving existing outward behavior - Removed the direct proactive refresh-from-cached-state path - Added regression tests covering: - stale cached auth with newer same-account auth already on disk - the same scenario even when the refresh endpoint would fail if called ## Why This Fix `refresh_token()` already contains the right cross-process safety behavior: - guarded reload from disk - same-account verification - skip-refresh when another process already changed auth Reusing that path makes proactive refresh consistent with unauthorized recovery and prevents stale processes from trying to refresh already-rotated tokens. ## Testing Test shape: - create a fresh temp `CODEX_HOME` from `~/.codex/auth.json` - force `last_refresh` to an old timestamp so proactive refresh is required - start two long-lived helper processes against the same auth file - start `B` first so it caches stale auth and sleeps - start `A` second so it refreshes first - point both at a local mock `/oauth/token` server - inspect whether `B` makes a second refresh request with the stale in-memory token, or reloads the rotated token from disk ### Before the fix The repro showed the bug clearly: the mock server saw two refreshes with the same stale token, `A` rotated to a new token, and `B` still returned the stale token instead of reloading from disk. ```text POST /oauth/token refresh_token=rt_j6s0... POST /oauth/token refresh_token=rt_j6s0... B:cached_before=rt_j6s0... B:cached_after=rt_j6s0... B:returned=rt_j6s0... A:cached_before=rt_j6s0... A:cached_after=rotated-refresh-token-logged-run-v2 A:returned=rotated-refresh-token-logged-run-v2 ``` ### After the fix After the fix, the mock server saw only one refresh request. `A` refreshed once, and `B` started with the stale token but reloaded and returned the rotated token. ```text POST /oauth/token refresh_token=rt_j6s0... B:cached_before=rt_j6s0... B:cached_after=rotated-refresh-token-fix-branch B:returned=rotated-refresh-token-fix-branch A:cached_before=rt_j6s0... A:cached_after=rotated-refresh-token-fix-branch A:returned=rotated-refresh-token-fix-branch ``` This shows the new behavior: `A` refreshes once, then `B` reuses the updated auth from disk instead of making a second refresh request with the stale token.
Celia Chen ·
2026-03-23 12:07:59 -07:00 -
Move auth code into login crate (#15150)
- Move the auth implementation and token data into codex-login. - Keep codex-core re-exporting that surface from codex-login for existing callers. --------- Co-authored-by: Codex <noreply@openai.com>
Ahmed Ibrahim ·
2026-03-19 18:58:17 -07:00 -
Fixed a hole in token refresh logic for app server (#11802)
We've continued to receive reports from users that they're seeing the error message "Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again." This PR fixes two holes in the token refresh logic that lead to this condition. Background: A previous change in token refresh introduced the `UnauthorizedRecovery` object. It implements a state machine in the core agent loop that first performs a load of the on-disk auth information guarded by a check for matching account ID. If it finds that the on-disk version has been updated by another instance of codex, it uses the reloaded auth tokens. If the on-disk version hasn't been updated, it issues a refresh request from the token authority. There are two problems that this PR addresses: Problem 1: We weren't doing the same thing for the code path used by the app server interface. This PR effectively replicates the `UnauthorizedRecovery` logic for that code path. Problem 2: The `UnauthorizedRecovery` logic contained a hole in the `ReloadOutcome::Skipped` case. Here's the scenario. A user starts two instances of the CLI. Instance 1 is active (working on a task), instance 2 is idle. Both instances have the same in-memory cached tokens. The user then runs `codex logout` or `codex login` to log in to a separate account, which overwrites the `auth.json` file. Instance 1 receives a 401 and refreshes its token, but it doesn't write the new token to the `auth.json` file because the account ID doesn't match. Instance 2 is later activated and presented with a new task. It immediately hits a 401 and attempts to refresh its token but fails because its cached refresh token is now invalid. To avoid this situation, I've changed the logic to immediately fail a token refresh if the user has since logged out or logged in to another account. This will still be seen as an error by the user, but the cause will be clearer. I also took this opportunity to clean up the names of existing functions to make their roles clearer. * `try_refresh_token` is renamed `request_chatgpt_token_refresh` * the existing `refresh_token` is renamed `refresh_token_from_authority` (there's a new higher-level function named `refresh_token` now) * `refresh_tokens` is renamed `refresh_and_persist_chatgpt_token`, and it now implicitly reloads * `update_tokens` is renamed `persist_tokens`
Eric Traut ·
2026-02-18 09:27:04 -08:00 -
chore: rename ChatGpt -> Chatgpt in type names (#10244)
When using ChatGPT in names of types, we should be consistent, so this renames some types with `ChatGpt` in the name to `Chatgpt`. From https://rust-lang.github.io/api-guidelines/naming.html: > In `UpperCamelCase`, acronyms and contractions of compound words count as one word: use `Uuid` rather than `UUID`, `Usize` rather than `USize` or `Stdin` rather than `StdIn`. In `snake_case`, acronyms and contractions are lower-cased: `is_xid_start`. This PR updates existing uses of `ChatGpt` and changes them to `Chatgpt`. Though in all cases where it could affect the wire format, I visually inspected that we don't change anything there. That said, this _will_ change the codegen because it will affect the spelling of type names. For example, this renames `AuthMode::ChatGPT` to `AuthMode::Chatgpt` in `app-server-protocol`, but the wire format is still `"chatgpt"`. This PR also updates a number of types in `codex-rs/core/src/auth.rs`.
Michael Bolin ·
2026-01-30 11:18:39 -08:00 -
feat(app-server): support external auth mode (#10012)
This enables a new use case where `codex app-server` is embedded into a parent application that will directly own the user's ChatGPT auth lifecycle, which means it owns the user’s auth tokens and refreshes it when necessary. The parent application would just want a way to pass in the auth tokens for codex to use directly. The idea is that we are introducing a new "auth mode" currently only exposed via app server: **`chatgptAuthTokens`** which consist of the `id_token` (stores account metadata) and `access_token` (the bearer token used directly for backend API calls). These auth tokens are only stored in-memory. This new mode is in addition to the existing `apiKey` and `chatgpt` auth modes. This PR reuses the shape of our existing app-server account APIs as much as possible: - Update `account/login/start` with a new `chatgptAuthTokens` variant, which will allow the client to pass in the tokens and have codex app-server use them directly. Upon success, the server emits `account/login/completed` and `account/updated` notifications. - A new server->client request called `account/chatgptAuthTokens/refresh` which the server can use whenever the access token previously passed in has expired and it needs a new one from the parent application. I leveraged the core 401 retry loop which typically triggers auth token refreshes automatically, but made it pluggable: - **chatgpt** mode refreshes internally, as usual. - **chatgptAuthTokens** mode calls the client via `account/chatgptAuthTokens/refresh`, the client responds with updated tokens, codex updates its in-memory auth, then retries. This RPC has a 10s timeout and handles JSON-RPC errors from the client. Also some additional things: - chatgpt logins are blocked while external auth is active (have to log out first. typically clients will pick one OR the other, not support both) - `account/logout` clears external auth in memory - Ensures that if `forced_chatgpt_workspace_id` is set via the user's config, we respect it in both: - `account/login/start` with `chatgptAuthTokens` (returns a JSON-RPC error back to the client) - `account/chatgptAuthTokens/refresh` (fails the turn, and on next request app-server will send another `account/chatgptAuthTokens/refresh` request to the client).
Owen Lin ·
2026-01-29 23:46:04 +00:00 -
Attempt to reload auth as a step in 401 recovery (#8880)
When authentication fails, first attempt to reload the auth from file and then attempt to refresh it.
pakrym-oai ·
2026-01-08 15:06:44 -08:00 -
Immutable CodexAuth (#8857)
Historically we started with a CodexAuth that knew how to refresh it's own tokens and then added AuthManager that did a different kind of refresh (re-reading from disk). I don't think it makes sense for both `CodexAuth` and `AuthManager` to be mutable and contain behaviors. Move all refresh logic into `AuthManager` and keep `CodexAuth` as a data object.
pakrym-oai ·
2026-01-08 11:43:56 -08:00 -
Improved token refresh handling to address "Re-connecting" behavior (#6231)
Currently, when the access token expires, we attempt to use the refresh token to acquire a new access token. This works most of the time. However, there are situations where the refresh token is expired, exhausted (already used to perform a refresh), or revoked. In those cases, the current logic treats the error as transient and attempts to retry it repeatedly. This PR changes the token refresh logic to differentiate between permanent and transient errors. It also changes callers to treat the permanent errors as fatal rather than retrying them. And it provides better error messages to users so they understand how to address the problem. These error messages should also help us further understand why we're seeing examples of refresh token exhaustion. Here is the error message in the CLI. The same text appears within the extension. <img width="863" height="38" alt="image" src="https://github.com/user-attachments/assets/7ffc0d08-ebf0-4900-b9a9-265064202f4f" /> I also correct the spelling of "Re-connecting", which shouldn't have a hyphen in it. Testing: I manually tested these code paths by adding temporary code to programmatically cause my refresh token to be exhausted (by calling the token refresh endpoint in a tight loop more than 50 times). I then simulated an access token expiration, which caused the token refresh logic to be invoked. I confirmed that the updated logic properly handled the error condition. Note: We earlier discussed the idea of forcefully logging out the user at the point where token refresh failed. I made several attempts to do this, and all of them resulted in a bad UX. It's important to surface this error to users in a way that explains the problem and tells them that they need to log in again. We also previously discussed deleting the auth.json file when this condition is detected. That also creates problems because it effectively changes the auth status from logged in to logged out, and this causes odd failures and inconsistent UX. I think it's therefore better not to delete auth.json in this case. If the user closes the CLI or VSCE and starts it again, we properly detect that the access token is expired and the refresh token is "dead", and we force the user to go through the login flow at that time. This should address aspects of #6191, #5679, and #5505
Eric Traut ·
2025-11-05 10:51:57 -08:00