feat(auth): activate managed accounts in native Codex - #449
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3065063713
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let _state_guard = crate::services::state_coordination::acquire_restore_mutation_guard() | ||
| .await | ||
| .map_err(CodexOAuthError::IoError)?; |
There was a problem hiding this comment.
When a Codex-OAuth proxy request reaches either token getter, these new calls wait for the global state-mutation guard. ProxyService::stop_with_restore acquires that same guard before calling stop_server_unlocked, while ProxyServer::stop waits for Axum's graceful-shutdown task; therefore, if shutdown acquires the guard while an in-flight request is entering authentication, the request cannot finish and shutdown cannot release the guard. Avoid acquiring this guard on the request path, or release it before awaiting graceful server shutdown.
Useful? React with 👍 / 👎.
|
|
||
| let actions = [ | ||
| texts::tui_key_set_default().to_string(), | ||
| crate::t!("Use in Codex", "在 Codex 中使用").to_string(), |
There was a problem hiding this comment.
After expanding this menu to three actions, the renderer still selects selected.min(1), although the key handler now permits index 2 and maps it directly to ManagedAuthRemove. Pressing Down twice therefore leaves “Use in Codex” visibly highlighted while Enter immediately deletes the account without confirmation; update the rendered selection bound to include the third row.
Useful? React with 👍 / 👎.
| let auth: Value = | ||
| serde_json::from_slice(&std::fs::read(crate::codex_config::get_codex_auth_path()).ok()?) | ||
| .ok()?; |
There was a problem hiding this comment.
When CODEX_HOME points to a directory that has not been created yet, this path lookup ignores the environment override and falls back to ~/.codex; use_account explicitly creates the missing override directory before doing the same lookup, but status does not. Thus auth status --json and the TUI can report an account from the fallback home as active even though the configured Codex home is empty and the next Codex process will not use that login. Resolve the environment path directly or apply the same directory preparation without modifying credentials.
AGENTS.md reference: AGENTS.md:L67-L67
Useful? React with 👍 / 👎.
| let id_token = tokens.id_token.clone().or_else(|| { | ||
| account | ||
| .codex_auth | ||
| .as_ref() | ||
| .and_then(|a| a.pointer("/tokens/id_token")) | ||
| .and_then(|v| v.as_str()) | ||
| .map(str::to_owned) |
There was a problem hiding this comment.
For an account loaded from the pre-change refresh-token-only store, codex_auth is None, so this fallback cannot preserve its ID token. If that account's complete credentials exist only in a non-live official provider snapshot and a proxy/quota request refreshes it, the refresh path never imports that snapshot; when the OAuth response omits id_token (a case the new tests explicitly support), remember_token_response creates an incomplete bundle and reconcile_native_copies overwrites the previously complete provider snapshot with it. Import the matching provider credentials before refreshing, otherwise existing users can lose the snapshot needed by auth use and be forced to sign in again.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
1 participant
auth defaultselects the managed/proxy account but leaves standalone Codex signed in to its previous account. Addauth use <account-id>and a matching Use in Codex TUI action for the next Codex process.Activation preserves
config.toml, MCP and skills, updates the current official provider's auth snapshot and managed default, and reports the live Codex account separately in status. Existingauth defaultbehavior is unchanged. Validation respects the selected Codex profile and the official unified-history configuration; incompatible routing and credential stores are rejected before activation.Persist native credential bundles, reuse unexpired access tokens across processes, and serialize credential mutations. OAuth refresh updates copies sharing the rotated credential; credentials accepted from temporary/shared Codex launches are retained for later account switches. Failed activation restores the prior login and provider snapshot. Already-running Codex processes are outside the switching contract: restart Codex or launch a new process;
/newdoes not reload login.Related to #252 (direct Codex activation; automatic account failover remains separate).
Validation (Rust 1.91.1, isolated HOME/config directories):
cargo fmt --checkcargo check --locked --lib --no-default-featurescargo test --locked --lib --no-default-features --no-runauth_use,start_codex_shared, andsettings_commands: all 11 integration tests passed, including separate-process activation/status, refresh retention, retry after partial publication and capture serialization.cargo clippy --locked --all-targets -- -A clippy::reversed_empty_rangespassed; the allowance covers an unchanged upstream empty-range test atsrc/cli/tui/ui/home_chart.rs:806.Six short-timeout tests under
proxy::forwarder::tests::error_pathsalso fail on the unchanged upstream commit8a5614din a separate worktree on this machine. They are outside this patch. The final library run passed 4,340 tests (2 ignored), excluding only those six reproduced upstream failures.Release smoke: synthetic B/A/B account selection was accepted by separate invocations of the installed native
codex login status; config.toml remained byte-identical. This checks the native credential format without making requests with real accounts.