From fa445a3435bc129a44c7fa62381998833d17a487 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Tue, 21 Apr 2026 18:15:44 -0700 Subject: [PATCH] Fix review interrupt and exit wedges Bug: #11267 --- codex-rs/core/src/codex_delegate.rs | 3 ++- codex-rs/core/src/codex_delegate_tests.rs | 26 +++++++++++++++++++++++ codex-rs/tui/src/app/event_dispatch.rs | 17 ++++++++++++++- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 858154eb77e3..fb104496d6e4 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -94,7 +94,8 @@ pub(crate) async fn run_codex_thread_interactive( parent_trace: None, analytics_events_client: Some(parent_session.services.analytics_events_client.clone()), })) - .await?; + .or_cancel(&cancel_token) + .await??; if parent_session.enabled(codex_features::Feature::GeneralAnalytics) { let thread_config = codex.thread_config_snapshot().await; let client_metadata = parent_session.app_server_client_metadata().await; diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index 3e685d92e91a..5fcf901f8159 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -153,6 +153,32 @@ async fn forward_ops_preserves_submission_trace_context() { .expect("forward_ops join error"); } +#[tokio::test] +async fn run_codex_thread_interactive_respects_pre_cancelled_spawn() { + let (parent_session, parent_ctx, _rx_events) = + crate::session::tests::make_session_and_context_with_rx().await; + let cancel_token = CancellationToken::new(); + cancel_token.cancel(); + + let result = timeout( + Duration::from_secs(/*secs*/ 1), + run_codex_thread_interactive( + parent_ctx.config.as_ref().clone(), + Arc::clone(&parent_session.services.auth_manager), + Arc::clone(&parent_session.services.models_manager), + parent_session, + parent_ctx, + cancel_token, + SubAgentSource::Review, + /*initial_history*/ None, + ), + ) + .await + .expect("cancelled delegate spawn should not hang"); + + assert!(matches!(result, Err(CodexErr::TurnAborted))); +} + #[tokio::test] async fn handle_request_permissions_uses_tool_call_id_for_round_trip() { let (parent_session, parent_ctx, rx_events) = diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index d4bfa45e23f2..af26e67ecdf4 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -5,6 +5,8 @@ use super::*; +const SHUTDOWN_FIRST_EXIT_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 2); + impl App { pub(super) async fn handle_event( &mut self, @@ -1640,7 +1642,20 @@ impl App { self.pending_shutdown_exit_thread_id = self.active_thread_id.or(self.chat_widget.thread_id()); if self.pending_shutdown_exit_thread_id.is_some() { - self.shutdown_current_thread(app_server).await; + // This is a UI escape-hatch budget, not a protocol + // deadline. A healthy local thread/unsubscribe round trip + // should finish comfortably inside two seconds, while a + // longer wait makes Ctrl+C feel broken when the app-server + // is already wedged. + if tokio::time::timeout( + SHUTDOWN_FIRST_EXIT_TIMEOUT, + self.shutdown_current_thread(app_server), + ) + .await + .is_err() + { + tracing::warn!("timed out waiting for app-server thread shutdown"); + } } self.pending_shutdown_exit_thread_id = None; AppRunControl::Exit(ExitReason::UserRequested)