From 5b1989f4d781e497e62b20cdc4cee8a568d0e0e7 Mon Sep 17 00:00:00 2001 From: UnownPlain <38232575+UnownPlain@users.noreply.github.com> Date: Mon, 18 Aug 2025 20:43:19 -0400 Subject: [PATCH 1/8] Release zip archived binaries (#2438) Adds zip archives to release workflow to improve compatibility (mainly older versions Windows which don't support `tar.gz` or `.zst` out of the box). Test release: https://github.com/UnownPlain/codex/releases/tag/rust-v0.0.0 Test run: https://github.com/UnownPlain/codex/actions/runs/16981943609 --- .github/workflows/rust-release.yml | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index d3e313b3cc..c30ffdf61c 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -117,10 +117,11 @@ jobs: dest="dist/${{ matrix.target }}" # For compatibility with environments that lack the `zstd` tool we - # additionally create a `.tar.gz` alongside every single binary that - # we publish. The end result is: + # additionally create a `.tar.gz` for all platforms and `.zip` for + # Windows alongside every single binary that we publish. The end result is: # codex-.zst (existing) # codex-.tar.gz (new) + # codex-.zip (only for Windows) # 1. Produce a .tar.gz for every file in the directory *before* we # run `zstd --rm`, because that flag deletes the original files. @@ -128,13 +129,20 @@ jobs: base="$(basename "$f")" # Skip files that are already archives (shouldn't happen, but be # safe). - if [[ "$base" == *.tar.gz ]]; then + if [[ "$base" == *.tar.gz || "$base" == *.zip ]]; then continue fi # Create per-binary tar.gz tar -C "$dest" -czf "$dest/${base}.tar.gz" "$base" + # Create zip archive for Windows binaries + # Must run from inside the dest dir so 7z won't + # embed the directory path inside the zip. + if [[ "${{ matrix.runner }}" == windows* ]]; then + (cd "$dest" && 7z a "${base}.zip" "$base") + fi + # Also create .zst (existing behaviour) *and* remove the original # uncompressed binary to keep the directory small. zstd -T0 -19 --rm "$dest/$base" From 7f216341657d2080c22b2618dbac854e2228f9a6 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 17:49:13 -0700 Subject: [PATCH 2/8] fix: eliminate ServerOptions.login_timeout and have caller use tokio::time::timeout() instead (#2395) https://github.com/openai/codex/pull/2373 introduced `ServerOptions.login_timeout` and `spawn_timeout_watcher()` to use an extra thread to manage the timeout for the login server. Now that we have asyncified the login stack, we can use `tokio::time::timeout()` from "outside" the login library to manage the timeout rather than having to a commit to a specific "timeout" concept from within. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2395). * #2399 * #2398 * #2396 * __->__ #2395 * #2394 * #2393 * #2389 --- codex-rs/login/src/server.rs | 115 +++++------------- codex-rs/login/tests/login_server_e2e.rs | 2 - .../mcp-server/src/codex_message_processor.rs | 20 ++- 3 files changed, 48 insertions(+), 89 deletions(-) diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index 419874e7a0..f33f1ae4fb 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -3,11 +3,7 @@ use std::io::{self}; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; -use std::sync::atomic::AtomicBool; -use std::sync::atomic::Ordering; -use std::sync::mpsc; use std::thread; -use std::time::Duration; use crate::AuthDotJson; use crate::get_auth_file; @@ -32,7 +28,6 @@ pub struct ServerOptions { pub port: u16, pub open_browser: bool, pub force_state: Option, - pub login_timeout: Option, } impl ServerOptions { @@ -44,7 +39,6 @@ impl ServerOptions { port: DEFAULT_PORT, open_browser: true, force_state: None, - login_timeout: None, } } } @@ -126,24 +120,8 @@ pub fn run_login_server( if opts.open_browser { let _ = webbrowser::open(&auth_url); } - let shutdown_notify: Arc = - shutdown_flag.unwrap_or_else(|| Arc::new(tokio::sync::Notify::new())); - let shutdown_notify_clone = shutdown_notify.clone(); - let timeout_flag = Arc::new(AtomicBool::new(false)); - - // Channel used to signal completion to timeout watcher. - let (done_tx, done_rx) = mpsc::channel::<()>(); - - if let Some(timeout) = opts.login_timeout { - spawn_timeout_watcher( - done_rx, - timeout, - shutdown_notify.clone(), - timeout_flag.clone(), - server.clone(), - ); - } + // Map blocking reads from server.recv() to an async channel. let (tx, mut rx) = tokio::sync::mpsc::channel::(16); let _server_handle = { let server = server.clone(); @@ -158,59 +136,52 @@ pub fn run_login_server( }) }; - let server_for_task = server.clone(); - let server_handle = tokio::spawn(async move { - loop { - tokio::select! { - _ = shutdown_notify.notified() => { - let _ = done_tx.send(()); - if timeout_flag.load(Ordering::SeqCst) { - return Err(io::Error::other("Login timed out")); - } else { + let shutdown_notify = shutdown_flag.unwrap_or_else(|| Arc::new(tokio::sync::Notify::new())); + let server_handle = { + let shutdown_notify = shutdown_notify.clone(); + let server = server.clone(); + tokio::spawn(async move { + loop { + tokio::select! { + _ = shutdown_notify.notified() => { return Err(io::Error::other("Login was not completed")); } - } - maybe_req = rx.recv() => { - let Some(req) = maybe_req else { - let _ = done_tx.send(()); - if timeout_flag.load(Ordering::SeqCst) { - return Err(io::Error::other("Login timed out")); - } else { + maybe_req = rx.recv() => { + let Some(req) = maybe_req else { return Err(io::Error::other("Login was not completed")); - } - }; + }; - let url_raw = req.url().to_string(); - let response = - process_request(&url_raw, &opts, &redirect_uri, &pkce, actual_port, &state).await; + let url_raw = req.url().to_string(); + let response = + process_request(&url_raw, &opts, &redirect_uri, &pkce, actual_port, &state).await; - let is_login_complete = matches!(response, HandledRequest::ResponseAndExit(_)); - match response { - HandledRequest::Response(r) | HandledRequest::ResponseAndExit(r) => { - let _ = tokio::task::spawn_blocking(move || req.respond(r)).await; + let is_login_complete = matches!(response, HandledRequest::ResponseAndExit(_)); + match response { + HandledRequest::Response(r) | HandledRequest::ResponseAndExit(r) => { + let _ = tokio::task::spawn_blocking(move || req.respond(r)).await; + } + HandledRequest::RedirectWithHeader(header) => { + let redirect = Response::empty(302).with_header(header); + let _ = tokio::task::spawn_blocking(move || req.respond(redirect)).await; + } } - HandledRequest::RedirectWithHeader(header) => { - let redirect = Response::empty(302).with_header(header); - let _ = tokio::task::spawn_blocking(move || req.respond(redirect)).await; - } - } - if is_login_complete { - shutdown_notify.notify_waiters(); - let _ = done_tx.send(()); - server_for_task.unblock(); - return Ok(()); + if is_login_complete { + shutdown_notify.notify_waiters(); + server.unblock(); + return Ok(()); + } } } } - } - }); + }) + }; Ok(LoginServer { - auth_url: auth_url.clone(), + auth_url, actual_port, server_handle, - shutdown_flag: shutdown_notify_clone, + shutdown_flag: shutdown_notify, server, }) } @@ -319,26 +290,6 @@ async fn process_request( } } -/// Spawns a detached thread that waits for either a completion signal on `done_rx` -/// or the specified `timeout` to elapse. If the timeout elapses first it marks -/// the `shutdown_flag`, records `timeout_flag`, and unblocks the HTTP server so -/// that the main server loop can exit promptly. -fn spawn_timeout_watcher( - done_rx: mpsc::Receiver<()>, - timeout: Duration, - shutdown_notify: Arc, - timeout_flag: Arc, - server: Arc, -) { - thread::spawn(move || { - if done_rx.recv_timeout(timeout).is_err() { - timeout_flag.store(true, Ordering::SeqCst); - shutdown_notify.notify_waiters(); - server.unblock(); - } - }); -} - fn build_authorize_url( issuer: &str, client_id: &str, diff --git a/codex-rs/login/tests/login_server_e2e.rs b/codex-rs/login/tests/login_server_e2e.rs index 09a447d565..ef387f575e 100644 --- a/codex-rs/login/tests/login_server_e2e.rs +++ b/codex-rs/login/tests/login_server_e2e.rs @@ -100,7 +100,6 @@ async fn end_to_end_login_flow_persists_auth_json() { port: 0, open_browser: false, force_state: Some(state), - login_timeout: None, }; let server = run_login_server(opts, None).unwrap(); let login_port = server.actual_port; @@ -159,7 +158,6 @@ async fn creates_missing_codex_home_dir() { port: 0, open_browser: false, force_state: Some(state), - login_timeout: None, }; let server = run_login_server(opts, None).unwrap(); let login_port = server.actual_port; diff --git a/codex-rs/mcp-server/src/codex_message_processor.rs b/codex-rs/mcp-server/src/codex_message_processor.rs index 7e5da55a32..00c8717c43 100644 --- a/codex-rs/mcp-server/src/codex_message_processor.rs +++ b/codex-rs/mcp-server/src/codex_message_processor.rs @@ -146,7 +146,6 @@ impl CodexMessageProcessor { let opts = LoginServerOptions { open_browser: false, - login_timeout: Some(LOGIN_CHATGPT_TIMEOUT), ..LoginServerOptions::new(config.codex_home.clone(), CLIENT_ID.to_string()) }; @@ -158,6 +157,7 @@ impl CodexMessageProcessor { let reply = match run_login_server(opts, None) { Ok(server) => { let login_id = Uuid::new_v4(); + let shutdown_handle = server.cancel_handle(); // Replace active login if present. { @@ -166,7 +166,7 @@ impl CodexMessageProcessor { existing.drop(); } *guard = Some(ActiveLogin { - shutdown_handle: server.cancel_handle(), + shutdown_handle: shutdown_handle.clone(), login_id, }); } @@ -180,9 +180,19 @@ impl CodexMessageProcessor { let outgoing_clone = self.outgoing.clone(); let active_login = self.active_login.clone(); tokio::spawn(async move { - let (success, error_msg) = match server.block_until_done().await { - Ok(()) => (true, None), - Err(err) => (false, Some(format!("Login server error: {err}"))), + let (success, error_msg) = match tokio::time::timeout( + LOGIN_CHATGPT_TIMEOUT, + server.block_until_done(), + ) + .await + { + Ok(Ok(())) => (true, None), + Ok(Err(err)) => (false, Some(format!("Login server error: {err}"))), + Err(_elapsed) => { + // Timeout: cancel server and report + shutdown_handle.cancel(); + (false, Some("Login timed out".to_string())) + } }; let notification = LoginChatGptCompleteNotification { login_id, From d5b42ba1ac89cbe7f694a28e3a6e6adb33bf0cdd Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 17:57:04 -0700 Subject: [PATCH 3/8] fix: make ShutdownHandle a private field of LoginServer (#2396) Folds the top-level `shutdown()` function into a method of `ShutdownHandle` and then simply stores `ShutdownHandle` on `LoginServer` since the two fields it contains were always being used together, anyway. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2396). * #2399 * #2398 * __->__ #2396 * #2395 * #2394 * #2393 * #2389 --- codex-rs/login/src/server.rs | 26 +++++++------------ .../mcp-server/src/codex_message_processor.rs | 4 +-- codex-rs/tui/src/onboarding/auth.rs | 2 +- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index f33f1ae4fb..f3256ee6c8 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -46,9 +46,8 @@ impl ServerOptions { pub struct LoginServer { pub auth_url: String, pub actual_port: u16, - shutdown_flag: Arc, server_handle: tokio::task::JoinHandle>, - server: Arc, + shutdown_handle: ShutdownHandle, } impl LoginServer { @@ -59,14 +58,11 @@ impl LoginServer { } pub fn cancel(&self) { - shutdown(&self.shutdown_flag, &self.server); + self.shutdown_handle.shutdown(); } pub fn cancel_handle(&self) -> ShutdownHandle { - ShutdownHandle { - shutdown_notify: self.shutdown_flag.clone(), - server: self.server.clone(), - } + self.shutdown_handle.clone() } } @@ -85,16 +81,12 @@ impl std::fmt::Debug for ShutdownHandle { } impl ShutdownHandle { - pub fn cancel(&self) { - shutdown(&self.shutdown_notify, &self.server); + pub fn shutdown(&self) { + self.shutdown_notify.notify_waiters(); + self.server.unblock(); } } -pub fn shutdown(shutdown_notify: &tokio::sync::Notify, server: &Server) { - shutdown_notify.notify_waiters(); - server.unblock(); -} - pub fn run_login_server( opts: ServerOptions, shutdown_flag: Option>, @@ -181,8 +173,10 @@ pub fn run_login_server( auth_url, actual_port, server_handle, - shutdown_flag: shutdown_notify, - server, + shutdown_handle: ShutdownHandle { + shutdown_notify, + server, + }, }) } diff --git a/codex-rs/mcp-server/src/codex_message_processor.rs b/codex-rs/mcp-server/src/codex_message_processor.rs index 00c8717c43..4f2b3bb693 100644 --- a/codex-rs/mcp-server/src/codex_message_processor.rs +++ b/codex-rs/mcp-server/src/codex_message_processor.rs @@ -66,7 +66,7 @@ struct ActiveLogin { impl ActiveLogin { fn drop(&self) { - self.shutdown_handle.cancel(); + self.shutdown_handle.shutdown(); } } @@ -190,7 +190,7 @@ impl CodexMessageProcessor { Ok(Err(err)) => (false, Some(format!("Login server error: {err}"))), Err(_elapsed) => { // Timeout: cancel server and report - shutdown_handle.cancel(); + shutdown_handle.shutdown(); (false, Some("Login timed out".to_string())) } }; diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 7166e349c3..490f85bff8 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -50,7 +50,7 @@ pub(crate) struct ContinueInBrowserState { impl Drop for ContinueInBrowserState { fn drop(&mut self) { if let Some(flag) = &self.shutdown_handle { - flag.cancel(); + flag.shutdown(); } } } From 146985f3ff3d3ec62c656f234874c3caefbf26a4 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 18:05:44 -0700 Subject: [PATCH 4/8] fix: reduce references to Server in codex-login crate (#2398) Updates the tokio task that monitors `shutdown_notify` and server requests to ensure that `server.unblock()` is always called, which means that `ShutdownHandle` only has to invoke `notify_waiters()`. Now `LoginServer` no longer has to maintain a reference to `Server`. The `Arc` only has two active references: the `thread::spawn()` for reading server messages and the `tokio::task()` that consumes them (and the shutdown message). Now when shutdown is called (or if login completes successfully), the `server.unblock()` call ensures the thread terminates cleanly, which in turn ensures `rx.recv()` in the `tokio::spawn()` returns `Err`, causing the `tokio::task()` to exit cleanly, as well. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2398). * #2399 * __->__ #2398 * #2396 * #2395 * #2394 * #2393 * #2389 --- codex-rs/login/src/server.rs | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index f3256ee6c8..e8af09fba0 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -66,24 +66,14 @@ impl LoginServer { } } -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct ShutdownHandle { shutdown_notify: Arc, - server: Arc, -} - -impl std::fmt::Debug for ShutdownHandle { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("ShutdownHandle") - .field("shutdown_notify", &self.shutdown_notify) - .finish() - } } impl ShutdownHandle { pub fn shutdown(&self) { self.shutdown_notify.notify_waiters(); - self.server.unblock(); } } @@ -133,14 +123,14 @@ pub fn run_login_server( let shutdown_notify = shutdown_notify.clone(); let server = server.clone(); tokio::spawn(async move { - loop { + let result = loop { tokio::select! { _ = shutdown_notify.notified() => { - return Err(io::Error::other("Login was not completed")); + break Err(io::Error::other("Login was not completed")); } maybe_req = rx.recv() => { let Some(req) = maybe_req else { - return Err(io::Error::other("Login was not completed")); + break Err(io::Error::other("Login was not completed")); }; let url_raw = req.url().to_string(); @@ -159,13 +149,16 @@ pub fn run_login_server( } if is_login_complete { - shutdown_notify.notify_waiters(); - server.unblock(); - return Ok(()); + break Ok(()); } } } - } + }; + + // Ensure that the server is unblocked so the thread dedicated to + // running `server.recv()` in a loop exits cleanly. + server.unblock(); + result }) }; @@ -173,10 +166,7 @@ pub fn run_login_server( auth_url, actual_port, server_handle, - shutdown_handle: ShutdownHandle { - shutdown_notify, - server, - }, + shutdown_handle: ShutdownHandle { shutdown_notify }, }) } From 2aad3a13b832f6ffe6a841ac340812f1f9da5035 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 18:15:50 -0700 Subject: [PATCH 5/8] fix: remove shutdown_flag param to run_login_server() (#2399) In practice, this was always passed in as `None`, so eliminated the param and updated all the call sites. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2399). * __->__ #2399 * #2398 * #2396 * #2395 * #2394 * #2393 * #2389 --- codex-rs/cli/src/login.rs | 2 +- codex-rs/login/src/server.rs | 7 ++----- codex-rs/login/tests/login_server_e2e.rs | 4 ++-- codex-rs/mcp-server/src/codex_message_processor.rs | 2 +- codex-rs/tui/src/onboarding/auth.rs | 2 +- 5 files changed, 7 insertions(+), 10 deletions(-) diff --git a/codex-rs/cli/src/login.rs b/codex-rs/cli/src/login.rs index fc40a0271f..36bbf2208d 100644 --- a/codex-rs/cli/src/login.rs +++ b/codex-rs/cli/src/login.rs @@ -14,7 +14,7 @@ use std::path::PathBuf; pub async fn login_with_chatgpt(codex_home: PathBuf) -> std::io::Result<()> { let opts = ServerOptions::new(codex_home, CLIENT_ID.to_string()); - let server = run_login_server(opts, None)?; + let server = run_login_server(opts)?; eprintln!( "Starting local login server on http://localhost:{}.\nIf your browser did not open, navigate to this URL to authenticate:\n\n{}", diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index e8af09fba0..32229484ff 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -77,10 +77,7 @@ impl ShutdownHandle { } } -pub fn run_login_server( - opts: ServerOptions, - shutdown_flag: Option>, -) -> io::Result { +pub fn run_login_server(opts: ServerOptions) -> io::Result { let pkce = generate_pkce(); let state = opts.force_state.clone().unwrap_or_else(generate_state); @@ -118,7 +115,7 @@ pub fn run_login_server( }) }; - let shutdown_notify = shutdown_flag.unwrap_or_else(|| Arc::new(tokio::sync::Notify::new())); + let shutdown_notify = Arc::new(tokio::sync::Notify::new()); let server_handle = { let shutdown_notify = shutdown_notify.clone(); let server = server.clone(); diff --git a/codex-rs/login/tests/login_server_e2e.rs b/codex-rs/login/tests/login_server_e2e.rs index ef387f575e..ceb0a94733 100644 --- a/codex-rs/login/tests/login_server_e2e.rs +++ b/codex-rs/login/tests/login_server_e2e.rs @@ -101,7 +101,7 @@ async fn end_to_end_login_flow_persists_auth_json() { open_browser: false, force_state: Some(state), }; - let server = run_login_server(opts, None).unwrap(); + let server = run_login_server(opts).unwrap(); let login_port = server.actual_port; // Simulate browser callback, and follow redirect to /success @@ -159,7 +159,7 @@ async fn creates_missing_codex_home_dir() { open_browser: false, force_state: Some(state), }; - let server = run_login_server(opts, None).unwrap(); + let server = run_login_server(opts).unwrap(); let login_port = server.actual_port; let client = reqwest::Client::new(); diff --git a/codex-rs/mcp-server/src/codex_message_processor.rs b/codex-rs/mcp-server/src/codex_message_processor.rs index 4f2b3bb693..1decf11da6 100644 --- a/codex-rs/mcp-server/src/codex_message_processor.rs +++ b/codex-rs/mcp-server/src/codex_message_processor.rs @@ -154,7 +154,7 @@ impl CodexMessageProcessor { Error(JSONRPCErrorError), } - let reply = match run_login_server(opts, None) { + let reply = match run_login_server(opts) { Ok(server) => { let login_id = Uuid::new_v4(); let shutdown_handle = server.cancel_handle(); diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 490f85bff8..facd06c64a 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -281,7 +281,7 @@ impl AuthModeWidget { fn start_chatgpt_login(&mut self) { self.error = None; let opts = ServerOptions::new(self.codex_home.clone(), CLIENT_ID.to_string()); - let server = run_login_server(opts, None); + let server = run_login_server(opts); match server { Ok(child) => { let auth_url = child.auth_url.clone(); From f49c934cd08ee2157dce3c9fbe70f4541bba116d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 Aug 2025 18:46:51 -0700 Subject: [PATCH 6/8] chore(deps): bump clap_complete from 4.5.56 to 4.5.57 in /codex-rs (#2403) --- codex-rs/Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 05e804c148..265857533f 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -561,9 +561,9 @@ dependencies = [ [[package]] name = "clap_complete" -version = "4.5.56" +version = "4.5.57" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67e4efcbb5da11a92e8a609233aa1e8a7d91e38de0be865f016d14700d45a7fd" +checksum = "4d9501bd3f5f09f7bbee01da9a511073ed30a80cd7a509f1214bb74eadea71ad" dependencies = [ "clap", ] From 97f995a749a2cd8b4edf68d59edf6d18f4363825 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 18 Aug 2025 20:22:48 -0700 Subject: [PATCH 7/8] Show login options when not signed in with ChatGPT (#2440) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Motivation: we have users who uses their API key although they want to use ChatGPT account. We want to give them the chance to always login with their account. This PR displays login options when the user is not signed in with ChatGPT. Even if you have set an OpenAI API key as an environment variable, you will still be prompted to log in with ChatGPT. We’ve also added a new flag, `always_use_api_key_signing` false by default, which ensures you are never asked to log in with ChatGPT and always defaults to using your API key. https://github.com/user-attachments/assets/b61ebfa9-3c5e-4ab7-bf94-395c23a0e0af After ChatGPT sign in: https://github.com/user-attachments/assets/d58b366b-c46a-428f-a22f-2ac230f991c0 --- README.md | 30 +++ codex-rs/chatgpt/src/chatgpt_token.rs | 3 +- codex-rs/cli/src/login.rs | 2 +- codex-rs/core/src/config.rs | 11 + codex-rs/core/src/conversation_manager.rs | 2 +- codex-rs/core/tests/client.rs | 204 ++++++++++++++++++ codex-rs/login/src/lib.rs | 42 +++- codex-rs/login/src/token_data.rs | 8 +- codex-rs/tui/src/app.rs | 87 +++++++- codex-rs/tui/src/lib.rs | 19 +- codex-rs/tui/src/onboarding/auth.rs | 54 ++++- .../tui/src/onboarding/onboarding_screen.rs | 11 +- 12 files changed, 440 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 596362a30a..f7428dc35e 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ - [Authenticate locally and copy your credentials to the "headless" machine](#authenticate-locally-and-copy-your-credentials-to-the-headless-machine) - [Connecting through VPS or remote](#connecting-through-vps-or-remote) - [Usage-based billing alternative: Use an OpenAI API key](#usage-based-billing-alternative-use-an-openai-api-key) + - [Forcing a specific auth method (advanced)](#forcing-a-specific-auth-method-advanced) - [Choosing Codex's level of autonomy](#choosing-codexs-level-of-autonomy) - [**1. Read/write**](#1-readwrite) - [**2. Read-only**](#2-read-only) @@ -165,6 +166,35 @@ Notes: - This command only sets the key for your current terminal session, which we recommend. To set it for all future sessions, you can also add the `export` line to your shell's configuration file (e.g., `~/.zshrc`). - If you have signed in with ChatGPT, Codex will default to using your ChatGPT credits. If you wish to use your API key, use the `/logout` command to clear your ChatGPT authentication. +#### Forcing a specific auth method (advanced) + +You can explicitly choose which authentication Codex should prefer when both are available. + +- To always use your API key (even when ChatGPT auth exists), set: + +```toml +# ~/.codex/config.toml +preferred_auth_method = "apikey" +``` + +Or override ad-hoc via CLI: + +```bash +codex --config preferred_auth_method="apikey" +``` + +- To prefer ChatGPT auth (default), set: + +```toml +# ~/.codex/config.toml +preferred_auth_method = "chatgpt" +``` + +Notes: + +- When `preferred_auth_method = "apikey"` and an API key is available, the login screen is skipped. +- When `preferred_auth_method = "chatgpt"` (default), Codex prefers ChatGPT auth if present; if only an API key is present, it will use the API key. Certain account types may also require API-key mode. + ### Choosing Codex's level of autonomy We always recommend running Codex in its default sandbox that gives you strong guardrails around what the agent can do. The default sandbox prevents it from editing files outside its workspace, or from accessing the network. diff --git a/codex-rs/chatgpt/src/chatgpt_token.rs b/codex-rs/chatgpt/src/chatgpt_token.rs index c674afbc57..f003c4392b 100644 --- a/codex-rs/chatgpt/src/chatgpt_token.rs +++ b/codex-rs/chatgpt/src/chatgpt_token.rs @@ -1,3 +1,4 @@ +use codex_login::AuthMode; use codex_login::CodexAuth; use std::path::Path; use std::sync::LazyLock; @@ -19,7 +20,7 @@ pub fn set_chatgpt_token_data(value: TokenData) { /// Initialize the ChatGPT token from auth.json file pub async fn init_chatgpt_token_from_auth(codex_home: &Path) -> std::io::Result<()> { - let auth = CodexAuth::from_codex_home(codex_home)?; + let auth = CodexAuth::from_codex_home(codex_home, AuthMode::ChatGPT)?; if let Some(auth) = auth { let token_data = auth.get_token_data().await?; set_chatgpt_token_data(token_data); diff --git a/codex-rs/cli/src/login.rs b/codex-rs/cli/src/login.rs index 36bbf2208d..72eb7b4faf 100644 --- a/codex-rs/cli/src/login.rs +++ b/codex-rs/cli/src/login.rs @@ -60,7 +60,7 @@ pub async fn run_login_with_api_key( pub async fn run_login_status(cli_config_overrides: CliConfigOverrides) -> ! { let config = load_config_or_exit(cli_config_overrides); - match CodexAuth::from_codex_home(&config.codex_home) { + match CodexAuth::from_codex_home(&config.codex_home, config.preferred_auth_method) { Ok(Some(auth)) => match auth.mode { AuthMode::ApiKey => match auth.get_token().await { Ok(api_key) => { diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 9c0c3c1cbe..f20912b22e 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -13,6 +13,7 @@ use crate::model_provider_info::built_in_model_providers; use crate::openai_model_info::get_model_info; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; +use codex_login::AuthMode; use codex_protocol::config_types::ReasoningEffort; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::SandboxMode; @@ -163,6 +164,9 @@ pub struct Config { /// The value for the `originator` header included with Responses API requests. pub internal_originator: Option, + + /// If set to `true`, the API key will be signed with the `originator` header. + pub preferred_auth_method: AuthMode, } impl Config { @@ -409,6 +413,9 @@ pub struct ConfigToml { pub internal_originator: Option, pub projects: Option>, + + /// If set to `true`, the API key will be signed with the `originator` header. + pub preferred_auth_method: Option, } #[derive(Deserialize, Debug, Clone, PartialEq, Eq)] @@ -672,6 +679,7 @@ impl Config { include_plan_tool: include_plan_tool.unwrap_or(false), include_apply_patch_tool: include_apply_patch_tool_val, internal_originator: cfg.internal_originator, + preferred_auth_method: cfg.preferred_auth_method.unwrap_or(AuthMode::ChatGPT), }; Ok(config) } @@ -1036,6 +1044,7 @@ disable_response_storage = true include_plan_tool: false, include_apply_patch_tool: false, internal_originator: None, + preferred_auth_method: AuthMode::ChatGPT, }, o3_profile_config ); @@ -1088,6 +1097,7 @@ disable_response_storage = true include_plan_tool: false, include_apply_patch_tool: false, internal_originator: None, + preferred_auth_method: AuthMode::ChatGPT, }; assert_eq!(expected_gpt3_profile_config, gpt3_profile_config); @@ -1155,6 +1165,7 @@ disable_response_storage = true include_plan_tool: false, include_apply_patch_tool: false, internal_originator: None, + preferred_auth_method: AuthMode::ChatGPT, }; assert_eq!(expected_zdr_profile_config, zdr_profile_config); diff --git a/codex-rs/core/src/conversation_manager.rs b/codex-rs/core/src/conversation_manager.rs index 48ccdddf35..2dc69be45a 100644 --- a/codex-rs/core/src/conversation_manager.rs +++ b/codex-rs/core/src/conversation_manager.rs @@ -40,7 +40,7 @@ impl Default for ConversationManager { impl ConversationManager { pub async fn new_conversation(&self, config: Config) -> CodexResult { - let auth = CodexAuth::from_codex_home(&config.codex_home)?; + let auth = CodexAuth::from_codex_home(&config.codex_home, config.preferred_auth_method)?; self.new_conversation_with_auth(config, auth).await } diff --git a/codex-rs/core/tests/client.rs b/codex-rs/core/tests/client.rs index 2f7977eeb6..075c496a6b 100644 --- a/codex-rs/core/tests/client.rs +++ b/codex-rs/core/tests/client.rs @@ -7,6 +7,7 @@ use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; use codex_core::protocol::Op; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; +use codex_login::AuthMode; use codex_login::CodexAuth; use core_test_support::load_default_config_for_test; use core_test_support::load_sse_fixture_with_id; @@ -54,6 +55,59 @@ fn assert_message_ends_with(request_body: &serde_json::Value, text: &str) { ); } +/// Writes an `auth.json` into the provided `codex_home` with the specified parameters. +/// Returns the fake JWT string written to `tokens.id_token`. +#[expect(clippy::unwrap_used)] +fn write_auth_json( + codex_home: &TempDir, + openai_api_key: Option<&str>, + chatgpt_plan_type: &str, + access_token: &str, + account_id: Option<&str>, +) -> String { + use base64::Engine as _; + use serde_json::json; + + let header = json!({ "alg": "none", "typ": "JWT" }); + let payload = json!({ + "email": "user@example.com", + "https://api.openai.com/auth": { + "chatgpt_plan_type": chatgpt_plan_type, + "chatgpt_account_id": account_id.unwrap_or("acc-123") + } + }); + + let b64 = |b: &[u8]| base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(b); + let header_b64 = b64(&serde_json::to_vec(&header).unwrap()); + let payload_b64 = b64(&serde_json::to_vec(&payload).unwrap()); + let signature_b64 = b64(b"sig"); + let fake_jwt = format!("{header_b64}.{payload_b64}.{signature_b64}"); + + let mut tokens = json!({ + "id_token": fake_jwt, + "access_token": access_token, + "refresh_token": "refresh-test", + }); + if let Some(acc) = account_id { + tokens["account_id"] = json!(acc); + } + + let auth_json = json!({ + "OPENAI_API_KEY": openai_api_key, + "tokens": tokens, + // RFC3339 datetime; value doesn't matter for these tests + "last_refresh": "2025-08-06T20:41:36.232376Z", + }); + + std::fs::write( + codex_home.path().join("auth.json"), + serde_json::to_string_pretty(&auth_json).unwrap(), + ) + .unwrap(); + + fake_jwt +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn includes_session_id_and_model_headers_in_request() { if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { @@ -311,6 +365,156 @@ async fn chatgpt_auth_sends_correct_request() { ); } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn prefers_chatgpt_token_when_config_prefers_chatgpt() { + if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { + println!( + "Skipping test because it cannot execute when network is disabled in a Codex sandbox." + ); + return; + } + + // Mock server + let server = MockServer::start().await; + + let first = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw(sse_completed("resp1"), "text/event-stream"); + + // Expect ChatGPT base path and correct headers + Mock::given(method("POST")) + .and(path("/v1/responses")) + .and(header_regex("Authorization", r"Bearer Access-123")) + .and(header_regex("chatgpt-account-id", r"acc-123")) + .respond_with(first) + .expect(1) + .mount(&server) + .await; + + let model_provider = ModelProviderInfo { + base_url: Some(format!("{}/v1", server.uri())), + ..built_in_model_providers()["openai"].clone() + }; + + // Init session + let codex_home = TempDir::new().unwrap(); + // Write auth.json that contains both API key and ChatGPT tokens for a plan that should prefer ChatGPT. + let _jwt = write_auth_json( + &codex_home, + Some("sk-test-key"), + "pro", + "Access-123", + Some("acc-123"), + ); + + let mut config = load_default_config_for_test(&codex_home); + config.model_provider = model_provider; + config.preferred_auth_method = AuthMode::ChatGPT; + + let conversation_manager = ConversationManager::default(); + let NewConversation { + conversation: codex, + .. + } = conversation_manager + .new_conversation(config) + .await + .expect("create new conversation"); + + codex + .submit(Op::UserInput { + items: vec![InputItem::Text { + text: "hello".into(), + }], + }) + .await + .unwrap(); + + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + // verify request body flags + let request = &server.received_requests().await.unwrap()[0]; + let request_body = request.body_json::().unwrap(); + assert!( + !request_body["store"].as_bool().unwrap(), + "store should be false for ChatGPT auth" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() { + if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { + println!( + "Skipping test because it cannot execute when network is disabled in a Codex sandbox." + ); + return; + } + + // Mock server + let server = MockServer::start().await; + + let first = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw(sse_completed("resp1"), "text/event-stream"); + + // Expect API key header, no ChatGPT account header required. + Mock::given(method("POST")) + .and(path("/v1/responses")) + .and(header_regex("Authorization", r"Bearer sk-test-key")) + .respond_with(first) + .expect(1) + .mount(&server) + .await; + + let model_provider = ModelProviderInfo { + base_url: Some(format!("{}/v1", server.uri())), + ..built_in_model_providers()["openai"].clone() + }; + + // Init session + let codex_home = TempDir::new().unwrap(); + // Write auth.json that contains both API key and ChatGPT tokens for a plan that should prefer ChatGPT, + // but config will force API key preference. + let _jwt = write_auth_json( + &codex_home, + Some("sk-test-key"), + "pro", + "Access-123", + Some("acc-123"), + ); + + let mut config = load_default_config_for_test(&codex_home); + config.model_provider = model_provider; + config.preferred_auth_method = AuthMode::ApiKey; + + let conversation_manager = ConversationManager::default(); + let NewConversation { + conversation: codex, + .. + } = conversation_manager + .new_conversation(config) + .await + .expect("create new conversation"); + + codex + .submit(Op::UserInput { + items: vec![InputItem::Text { + text: "hello".into(), + }], + }) + .await + .unwrap(); + + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + // verify request body flags + let request = &server.received_requests().await.unwrap()[0]; + let request_body = request.body_json::().unwrap(); + assert!( + request_body["store"].as_bool().unwrap(), + "store should be true for API key auth" + ); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn includes_user_instructions_message_in_request() { let server = MockServer::start().await; diff --git a/codex-rs/login/src/lib.rs b/codex-rs/login/src/lib.rs index 80fc0e821a..327daa7e9b 100644 --- a/codex-rs/login/src/lib.rs +++ b/codex-rs/login/src/lib.rs @@ -30,7 +30,8 @@ mod token_data; pub const CLIENT_ID: &str = "app_EMoamEEZ73f0CkXaXp7hrann"; pub const OPENAI_API_KEY_ENV_VAR: &str = "OPENAI_API_KEY"; -#[derive(Clone, Debug, PartialEq, Copy)] +#[derive(Clone, Debug, PartialEq, Copy, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] pub enum AuthMode { ApiKey, ChatGPT, @@ -63,8 +64,11 @@ impl CodexAuth { /// Loads the available auth information from the auth.json or /// OPENAI_API_KEY environment variable. - pub fn from_codex_home(codex_home: &Path) -> std::io::Result> { - load_auth(codex_home, true) + pub fn from_codex_home( + codex_home: &Path, + preferred_auth_method: AuthMode, + ) -> std::io::Result> { + load_auth(codex_home, true, preferred_auth_method) } pub async fn get_token_data(&self) -> Result { @@ -165,7 +169,11 @@ impl CodexAuth { } } -fn load_auth(codex_home: &Path, include_env_var: bool) -> std::io::Result> { +fn load_auth( + codex_home: &Path, + include_env_var: bool, + preferred_auth_method: AuthMode, +) -> std::io::Result> { // First, check to see if there is a valid auth.json file. If not, we fall // back to AuthMode::ApiKey using the OPENAI_API_KEY environment variable // (if it is set). @@ -201,7 +209,7 @@ fn load_auth(codex_home: &Path, include_env_var: bool) -> std::io::Result