Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3eeca0cb6
ℹ️ 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".
| fn create_client() -> CodexHttpClient { | ||
| CodexHttpClient::new(reqwest::Client::new()) |
There was a problem hiding this comment.
Reuse default client for auth refresh requests
create_client now builds CodexHttpClient from reqwest::Client::new(), which drops the default client configuration previously used in this path (core/src/default_client.rs), including standard headers (originator/User-Agent/residency) and seatbelt no_proxy handling. As a result, OAuth refresh calls in request_chatgpt_token_refresh can behave differently from the rest of Codex networking and fail in environments that rely on those defaults (for example sandboxed/proxied setups or header-gated backends), turning a refactor into a runtime auth regression.
Useful? React with 👍 / 👎.
5fe7a7b to
75062a8
Compare
75062a8 to
ea36462
Compare
|
abandoning in favor of #15150 |
Why
codex-coresits late in the build graph, and its auth implementation was still a large chunk of code and unit tests living inside that crate. That kept auth work on the critical path forcodex-corebuilds and made thecodex-core --libtest binary more expensive to compile.This change moves that auth surface into its own workspace crate so it can compile and test independently, then removes the temporary
codex-coreauth/token-data reexports so downstream crates depend oncodex-authdirectly.codex-corekeeps only the small pieces it still owns: the config-aware login restriction logic and the helper that reads auth through the core default HTTP client path.What Changed
codex-authcrate and moved auth state, storage, token parsing, refresh logic, and their unit tests therecodex-rs/auth/BUILD.bazelso the new crate is addressable from Bazel as//codex-rs/auth:authcore/src/auth.rsandcore/src/token_data.rs; downstream crates and test-support crates now importcodex-authdirectly instead of going throughcodex-corecore/src/default_client_auth.rsandcore/src/login_restrictions.rsso auth reads still use the core default client behavior for originator, residency, user-agent, and sandboxno_proxy, while login restriction enforcement stays config-awareauth/src/lib.rsinto smaller modules solib.rsis mostly exports, and replaced the generic authtests.rsfile with file-specific test modules such asauth_storage_io_tests.rscodex-authdependency edges, includingcore_test_supportandapp_test_supportVerification
CARGO_TARGET_DIR=/tmp/codex-auth-core-check cargo check -p codex-auth -p codex-core -p codex-app-server -p codex-backend-client -p codex-chatgpt -p codex-cli -p codex-cloud-requirements -p codex-exec -p codex-login -p codex-mcp-server -p codex-tuiCARGO_TARGET_DIR=/tmp/codex-auth-core-check cargo test -p codex-auth --libCARGO_TARGET_DIR=/tmp/codex-auth-core-check cargo test -p codex-core --libCARGO_TARGET_DIR=/tmp/codex-auth-core-check cargo test -p codex-app-server --tests --no-runCARGO_TARGET_DIR=/tmp/codex-auth-core-check cargo test -p codex-login --tests --no-runcargo shearjust bazel-lock-check