refactor: route Codex auth through AuthProvider#18811
refactor: route Codex auth through AuthProvider#18811efrazer-oai wants to merge 2 commits intodev/efrazer/agent-identity-auth-runtimefrom
Conversation
af22d58 to
d21eaed
Compare
86413e2 to
0b61c82
Compare
d21eaed to
52b5ad3
Compare
0b61c82 to
52e547c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af22d58d06
ℹ️ 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".
52b5ad3 to
b51b093
Compare
b4f801e to
b718d3e
Compare
b51b093 to
f5befea
Compare
b718d3e to
b23a44f
Compare
f5befea to
690d32a
Compare
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/chatgpt/src/connectors.rs
Lines 27 to 31 in 690d32a
apps_enabled() now treats AgentIdentity as valid (uses_codex_backend), but it still builds auth with AuthManager::shared(...), which does not pass config.chatgpt_base_url. AgentIdentity runtime init can then target the default backend and fail in non-default deployments, causing this early gate to return false and skip connectors despite valid auth.
ℹ️ 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".
| authorization_header_value: Option<&str>, | ||
| ) -> HashMap<String, McpServerConfig> { | ||
| if config.apps_enabled && auth.is_some_and(CodexAuth::is_chatgpt_auth) { | ||
| if config.apps_enabled && auth.is_some_and(CodexAuth::uses_codex_backend) { |
There was a problem hiding this comment.
Key Codex Apps MCP cache by backend auth identity
After widening the apps gate to CodexAuth::uses_codex_backend, AgentIdentity now enters this path. However, cache identity is still derived from codex_apps_tools_cache_key() using get_token_data() (in mcp_connection_manager.rs), which is empty for AgentIdentity. That collapses cache keys to the same {None,None,false} value and can serve another account’s cached Apps tool list.
Useful? React with 👍 / 👎.
b23a44f to
4c2b315
Compare
efc466e to
37f3339
Compare
4c2b315 to
be0d99c
Compare
37f3339 to
dc19ee2
Compare
be0d99c to
0881c58
Compare
dc19ee2 to
6784881
Compare
0881c58 to
0f90a02
Compare
6784881 to
d393a91
Compare
## Summary This PR fully reverts the previously merged Agent Identity runtime integration from the old stack: https://github.com/openai/codex/pull/17387/changes It removes the Codex-side task lifecycle wiring, rollout/session persistence, feature flag plumbing, lazy `auth.json` mutation, background task auth paths, and request callsite changes introduced by that stack. This leaves the repo in a clean pre-AgentIdentity integration state so the follow-up PRs can reintroduce the pieces in smaller reviewable layers. ## Stack 1. This PR: full revert 2. #18871: move Agent Identity business logic into a crate 3. #18785: add explicit AgentIdentity auth mode and startup task allocation 4. #18811: migrate auth callsites through AuthProvider ## Testing Tests: targeted Rust checks, cargo-shear, Bazel lock check, and CI.
0f90a02 to
8bca584
Compare
6145ebf to
db51937
Compare
| /// Implementations must be cheap and non-blocking. Token refresh, task | ||
| /// registration, and other I/O must happen before request construction reaches | ||
| /// this trait. | ||
| pub trait AuthProvider: Send + Sync { |
There was a problem hiding this comment.
is this because of a dependency order?
8bca584 to
9679412
Compare
db51937 to
f15803b
Compare
9679412 to
c9e8046
Compare
f15803b to
ae57449
Compare
Summary
This PR moves Codex backend request authentication from direct bearer-token handling to
AuthProvider.The new
codex-auth-providercrate defines the shared request-auth trait.CodexAuth::provider()returns a provider that can apply all headers needed for the selected auth mode.This lets ChatGPT token auth and AgentIdentity auth share the same callsite path:
Reference old stack: https://github.com/openai/codex/pull/17387/changes
Callsite Migration
AuthProviderinstead of a raw token/headerCodexAuth::provider()uses_codex_backend()and keys caches from generic account gettersStack
CODEX_AGENT_IDENTITYTesting
Tests: targeted Rust checks, cargo-shear, Bazel lock check, and CI.