From d5b42ba1ac89cbe7f694a28e3a6e6adb33bf0cdd Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 17:57:04 -0700 Subject: [PATCH] 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 f33f1ae4..f3256ee6 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 00c8717c..4f2b3bb6 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 7166e349..490f85bf 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(); } } }