fix: configure AgentIdentity AuthAPI base URL#19904
Conversation
efrazer-oai
force-pushed
the
dev/efrazer/agent-identity-authapi-base-url
branch
from
b427e8f to
8e394fa
Compare
efrazer-oai
force-pushed
the
dev/efrazer/agent-identity-authapi-base-url
branch
3 times, most recently
from
1740e94 to
e733e1e
Compare
shijie-oai
left a comment
shijie-oai
left a comment
There was a problem hiding this comment.
this is purely for testing against staging etc right? Just want to confirm because previously I did make it to be able to accept env variable but took it out cause did not really see the point of it but now I think we shuold. I am a bit hesitating with adding it to config.toml though but can go either way.
![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: 8e394fafaf
ℹ️ 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".
| dir.path().to_path_buf(), | ||
| /*enable_codex_api_key_env*/ false, | ||
| AuthCredentialsStoreMode::File, | ||
| /*chatgpt_base_url*/ None, | ||
| /*agent_identity_authapi_base_url*/ None, | ||
| ) |
There was a problem hiding this comment.
AuthManager::shared now takes both chatgpt_base_url and agent_identity_authapi_base_url, but this call still passes only four arguments. The login unit test module will not compile once tests are built, blocking codex-login test runs.
Useful? React with 👍 / 👎.
sayan-oai
left a comment
sayan-oai
left a comment
There was a problem hiding this comment.
agree with shijie's comment; if this is just for testing purposes it shouldnt be in config.toml. it should be an env var that can be set as needed
| enable_codex_api_key_env, | ||
| credentials_store_mode, | ||
| Some(chatgpt_base_url.clone()), | ||
| /*agent_identity_authapi_base_url*/ None, |
There was a problem hiding this comment.
question-- is there ever a legit case someone would want to override the agent_identity_authapi_base_url via cloud_requirements? doesnt seem like it but wanted to double check
There was a problem hiding this comment.
ack, set it to an env variable as discussed offline!
## Summary AgentIdentity auth previously registered the process task lazily behind a `OnceCell`. That meant the auth object could be constructed before its runtime task binding was known. This PR makes AgentIdentity auth load the runtime task at auth load time and stores the resulting process task id directly on the auth object. The model-provider call path can then read a concrete task id instead of handling a missing lazy value. ## Stack 1. [refactor: make auth loading async](#19762) (merged) 2. **This PR:** [refactor: load AgentIdentity runtime eagerly](#19763) 3. [fix: configure AgentIdentity AuthAPI base URL](#19904) 4. [feat: verify AgentIdentity JWTs with JWKS](#19764) ## Important call sites | Area | Change | | --- | --- | | `AgentIdentityAuth::load` | Registers the process task during auth loading and stores `process_task_id`. | | `CodexAuth::from_agent_identity_jwt` | Awaits AgentIdentity auth loading. | | model-provider auth | Reads a concrete `process_task_id` instead of an optional lazy value. | | AgentIdentity auth tests | Mock task registration now covers eager runtime allocation. | ## Design decisions AgentIdentity auth now treats task registration as part of constructing a usable auth object. That matches how callers use the value: once auth is present, the model-provider path expects the task-scoped assertion data to be ready. ## Testing Tests: targeted Rust auth test compilation, formatter, scoped Clippy fix, and Bazel lock check.
| #[test] | ||
| fn agent_identity_authapi_base_url_prefers_env_value() { | ||
| let _guard = EnvVarGuard::set( | ||
| CODEX_AGENT_IDENTITY_AUTHAPI_BASE_URL_ENV_VAR, | ||
| "https://authapi.example.test/api/accounts/", | ||
| ); | ||
| assert_eq!( | ||
| agent_identity_authapi_base_url(), | ||
| "https://authapi.example.test/api/accounts" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn agent_identity_authapi_base_url_uses_prod_authapi_by_default() { | ||
| let _guard = EnvVarGuard::remove(CODEX_AGENT_IDENTITY_AUTHAPI_BASE_URL_ENV_VAR); |
There was a problem hiding this comment.
might need to serialize these two given they mutate env
efrazer-oai
force-pushed
the
dev/efrazer/agent-identity-authapi-base-url
branch
from
dffde4b to
326798a
Compare
3 participants
Summary
AgentIdentity runtime loading currently registers tasks against a single hardcoded AuthAPI base URL. That works for production, but local and staging validation may need registration to target a different authapi-login-provider without baking internal staging service URLs into the OSS binary.
This PR adds a small config surface for
agent_identity_authapi_base_urland threads it through the existing auth-loading path as a direct argument. Explicit config wins. Without config, task registration keeps using the production AuthAPI URL, matching the current default behavior.Stack
refactor: make auth loading async(merged)refactor: load agent identity runtime eagerlyfix: configure AgentIdentity AuthAPI base URLfeat: verify agent identity JWTs with JWKSDesign decisions
chatgpt_base_urlremains separate and is used by the follow-up JWKS verification PR for fetching public keys from the ChatGPT backend route.Testing
Tests: targeted Rust checks, AgentIdentity auth tests, config schema regeneration, formatter/fix pass, and whitespace diff check.