Conversation
Fix moved auth test imports, add auth-crate test deps, and expose testing constructors used by codex-core. Co-authored-by: Codex <noreply@openai.com>
Make auth testing constructors public for codex-core callers across the new crate boundary. Co-authored-by: Codex <noreply@openai.com>
Add a Bazel package marker for the extracted auth crate so the new source package is visible to Bazel. Co-authored-by: Codex <noreply@openai.com>
Define the auth source filegroup expected by the parent Bazel package after extracting auth into a nested crate. Co-authored-by: Codex <noreply@openai.com>
Declare the extracted auth package as a Bazel Rust crate target so codex-core can depend on it under Bazel. Co-authored-by: Codex <noreply@openai.com>
Point the extracted auth Bazel crate at its nonstandard root-level Rust sources so the :auth library target is generated. Co-authored-by: Codex <noreply@openai.com>
Pass the extracted auth config to enforce_login_restrictions at the remaining Rust callsites. Co-authored-by: Codex <noreply@openai.com>
Construct AuthConfig explicitly at the remaining login restriction callsites after the auth split. Co-authored-by: Codex <noreply@openai.com>
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Line 455 in 380d83f
run_main still calls enforce_login_restrictions(&config), but this commit removed that API in favor of enforce_login_restrictions_with_client. With no in-scope replacement at this callsite, codex-tui no longer compiles.
ℹ️ 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".
Switch the remaining TUI login restriction callsite to the explicit client path so codex-tui builds after removing the no-client auth wrapper. Co-authored-by: Codex <noreply@openai.com>
|
@codex review this |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Move the codex-terminal dev-dependency to the mcp test support crate that imports it so CI can compile mcp_test_support again. Co-authored-by: Codex <noreply@openai.com>
codex-rs/core/src/default_client.rs
Outdated
| #[cfg(test)] | ||
| #[path = "default_client_tests.rs"] | ||
| mod tests; | ||
| pub use codex_client::CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR; |
There was a problem hiding this comment.
should I reexport like that or make other crates depend on the new auth crate?
There was a problem hiding this comment.
I think reexporting makes more sense to keep auth centralized
Instantiate the environment before fetching the filesystem so app-server compiles under Bazel again. Co-authored-by: Codex <noreply@openai.com>
Instantiate the exec-server environment before fetching the filesystem so the PR merge ref compiles under Bazel. Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
d6c58be to
60192f2
Compare
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
- Move the auth implementation and token data into . - Keep re-exporting that surface from for existing callers. Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
8d19906 to
a717396
Compare
Co-authored-by: Codex <noreply@openai.com>
|
@codex review this |
Co-authored-by: Codex <noreply@openai.com>
Move the telemetry auth-mode conversion back into codex-core callsites to avoid an orphan impl in codex-login, and include the stale Cargo.lock cleanup from the core dependency removal. Co-authored-by: Codex <noreply@openai.com>
Import AuthMode from the auth module in codex-core; it is not re-exported at the crate root. Co-authored-by: Codex <noreply@openai.com>
ba52b1f to
cfbec84
Compare
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa2b69dc78
ℹ️ 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".
codex-rs/login/src/server.rs
Outdated
| "originator".to_string(), | ||
| originator().value.as_str().to_string(), | ||
| std::env::var("CODEX_INTERNAL_ORIGINATOR_OVERRIDE") | ||
| .unwrap_or_else(|_| "codex_cli_rs".to_string()), |
There was a problem hiding this comment.
Preserve runtime originator in OAuth authorize URL
build_authorize_url now derives originator only from CODEX_INTERNAL_ORIGINATOR_OVERRIDE or a hardcoded codex_cli_rs. That ignores runtime originators set via set_default_originator (used by non-CLI entrypoints), so login requests are mislabeled. This changes behavior from pre-move code and can break client attribution or backend logic keyed on the originator query parameter.
Useful? React with 👍 / 👎.
codex-rs/login/src/auth/manager.rs
Outdated
| fn build_auth_http_client() -> CodexHttpClient { | ||
| let inner = | ||
| build_reqwest_client_with_custom_ca(reqwest::Client::builder()).unwrap_or_else(|error| { | ||
| tracing::warn!(error = %error, "failed to build auth reqwest client"); | ||
| reqwest::Client::new() | ||
| }); | ||
| CodexHttpClient::new(inner) |
There was a problem hiding this comment.
Reuse default HTTP client policy for token refresh
Auth refresh now uses a bare reqwest builder (build_auth_http_client) instead of the previous default client path. This drops default header wiring (originator/residency) and sandbox proxy behavior (no_proxy). In proxied or residency-constrained environments, refresh calls can fail or be routed with incorrect request metadata.
Useful? React with 👍 / 👎.
joshka-oai
left a comment
There was a problem hiding this comment.
LGTM generally - a few questions about the reasons for the changed behavior bits
| Some(auth) => match auth.auth_mode() { | ||
| CoreAuthMode::ApiKey => Some(Account::ApiKey {}), | ||
| CoreAuthMode::Chatgpt => { | ||
| CoreAuthMode::Chatgpt | CoreAuthMode::ChatgptAuthTokens => { |
There was a problem hiding this comment.
What's this change about?
There was a problem hiding this comment.
We have multiple enums and we should unify at some point
codex-rs/login/src/server.rs
Outdated
| "originator".to_string(), | ||
| originator().value.as_str().to_string(), | ||
| std::env::var("CODEX_INTERNAL_ORIGINATOR_OVERRIDE") | ||
| .unwrap_or_else(|_| "codex_cli_rs".to_string()), |
There was a problem hiding this comment.
What's the reasone for this change?
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 178a0dc31c
ℹ️ 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".
|
@codex review latest commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 178a0dc31c
ℹ️ 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".
| pub use auth::login_with_api_key; | ||
| pub use auth::logout; | ||
| pub use auth::read_openai_api_key_from_env; | ||
| pub use auth::save_auth; |
There was a problem hiding this comment.
Re-export missing auth helpers for compatibility
The commit message says codex-core should keep the old auth surface, but codex-login root no longer re-exports helpers like login_with_chatgpt_auth_tokens and read_codex_api_key_from_env. Internal code had to switch to codex_login::auth::..., so downstream callers using codex_core::auth::... will hit source-level breakage.
Useful? React with 👍 / 👎.
| pub use auth::logout; | ||
| pub use auth::read_openai_api_key_from_env; | ||
| pub use auth::save_auth; | ||
| pub use codex_app_server_protocol::AuthMode; |
There was a problem hiding this comment.
Preserve old AuthMode type at re-export boundary
codex-login now re-exports codex_app_server_protocol::AuthMode, which adds ChatgptAuthTokens. Existing callers that exhaustively match codex_core::auth::AuthMode will no longer compile (the repo itself added new match arms). That contradicts the stated goal of keeping existing caller compatibility.
Useful? React with 👍 / 👎.
Uh oh!
There was an error while loading. Please reload this page.