refactor: route Codex auth through AuthProvider#18811
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".
b23a44f to
4c2b315
Compare
efc466e to
37f3339
Compare
4c2b315 to
be0d99c
Compare
37f3339 to
dc19ee2
Compare
be0d99c to
0881c58
Compare
dc19ee2 to
6784881
Compare
| let token_data = | ||
| get_chatgpt_token_data().ok_or_else(|| anyhow::anyhow!("ChatGPT token not available"))?; | ||
| let cache_key = all_connectors_cache_key(config, &token_data); | ||
| let auth_manager = |
There was a problem hiding this comment.
we added a bit of boilerplate by removing init_chatgpt_token_from_auth. anyting worth doing?
There was a problem hiding this comment.
I added a different type of helper for this to avoid repetition; i think reasoning about the Auth object is easier than the OnceLock we had for auth data which felt a bit hidden.
df5a2df to
ed51cb2
Compare
| .map_err(std::io::Error::other) | ||
| }) | ||
| } | ||
| CodexAuth::ApiKey(_) => self.auth.api_key().map_or_else( |
There was a problem hiding this comment.
why does this partially reimplement bearer provider? can we add a separate provider for model identity and reuse bearer when appropriate/
| }) | ||
| } | ||
|
|
||
| pub fn is_workspace_account(&self) -> bool { |
There was a problem hiding this comment.
don't we already have this logic somewhere?
There was a problem hiding this comment.
Extracted it out into a helper under plan_type
| &self.record | ||
| } | ||
|
|
||
| pub fn process_task_id(&self) -> Option<&str> { |
There was a problem hiding this comment.
both pub field and pub method?
There was a problem hiding this comment.
Fixed, only pub getter now
ed51cb2 to
40daf07
Compare
## Summary This PR adds `codex-agent-identity` as an isolated crate for Agent Identity business logic. The crate owns: - AgentAssertion construction. - Agent task registration. - private-key assertion signing. - bounded blocking HTTP for task registration. It does not wire AgentIdentity into `auth.json`, `AuthManager`, rollout state, or request callsites. That integration happens in later PRs. Reference old stack: https://github.com/openai/codex/pull/17387/changes ## Stack 1. #18757: full revert 2. This PR: isolated Agent Identity crate 3. #18785: explicit AgentIdentity auth mode and startup task allocation 4. #18811: migrate Codex backend auth callsites through AuthProvider 5. #18904: accept AgentIdentity JWTs and load `CODEX_AGENT_IDENTITY` ## Testing Tests: targeted Rust checks, cargo-shear, Bazel lock check, and CI.
40daf07 to
06285d7
Compare
ce246ec to
cb93ab0
Compare
06285d7 to
b7bde9c
Compare
cb93ab0 to
316a875
Compare
b7bde9c to
a457bb3
Compare
## Summary This PR adds `CodexAuth::AgentIdentity` as an explicit auth mode. An AgentIdentity auth record is a standalone `auth.json` mode. When `AuthManager::auth().await` loads that mode, it registers one process-scoped task and stores it in runtime-only state on the auth value. Header creation stays synchronous after that because the task is initialized before callers receive the auth object. This PR also removes the old feature flag path. AgentIdentity is selected by explicit auth mode, not by a hidden flag or lazy mutation of ChatGPT auth records. Reference old stack: https://github.com/openai/codex/pull/17387/changes ## Design Decisions - AgentIdentity is a real auth enum variant because it can be the only credential in `auth.json`. - The process task is ephemeral runtime state. It is not serialized and is not stored in rollout/session data. - Account/user metadata needed by existing Codex backend checks lives on the AgentIdentity record for now. - `is_chatgpt_auth()` remains token-specific. - `uses_codex_backend()` is the broader predicate for ChatGPT-token auth and AgentIdentity auth. ## Stack 1. #18757: full revert 2. #18871: isolated Agent Identity crate 3. This PR: explicit AgentIdentity auth mode and startup task allocation 4. #18811: migrate Codex backend auth callsites through AuthProvider 5. #18904: accept AgentIdentity JWTs and load `CODEX_AGENT_IDENTITY` ## Testing Tests: targeted Rust checks, cargo-shear, Bazel lock check, and CI.
a457bb3 to
6c03831
Compare
| } | ||
| } | ||
|
|
||
| pub fn empty() -> Self { |
There was a problem hiding this comment.
why would we ever construct empty?
| retry_suffix_after_or(self.resets_at.as_ref()) | ||
| ), | ||
| Some(PlanType::Known( | ||
| Some(PlanType::Known(plan)) => match plan { |
There was a problem hiding this comment.
let's not churn this logic unless needed.
| | Self::Enterprise | ||
| | Self::Edu | ||
| ) | ||
| self.is_team_like() |
There was a problem hiding this comment.
do we need to change this?
| } | ||
|
|
||
| impl KnownPlan { | ||
| pub fn is_team_like(self) -> bool { |
There was a problem hiding this comment.
why do we have these methods in so many places? on KnownPlan and on PlanType
d339b11 to
ece4453
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. openai#18871: move Agent Identity business logic into a crate 3. openai#18785: add explicit AgentIdentity auth mode and startup task allocation 4. openai#18811: migrate auth callsites through AuthProvider ## Testing Tests: targeted Rust checks, cargo-shear, Bazel lock check, and CI.
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.