Fix /review interrupt and TUI exit wedges (#18921)

Addresses #11267

## Summary
`/review` can be interrupted while it is still spawning the review
sub-agent. That spawn path lives in `codex-core` and did not observe the
task cancellation token until after `Codex::spawn` returned, so an
interrupted review could keep building a child session and leave the TUI
in a wedged state.

The TUI exit path also waited indefinitely for app-server
`thread/unsubscribe`, which made Ctrl+C look broken if the app-server
was already stuck. This makes interactive delegate startup
cancellation-aware and bounds the TUI shutdown-first unsubscribe wait
with a short UI escape-hatch timeout.

## Testing
I reproed the hang using the steps in the bug report. Confirmed hang no
longer exists after fix.
This commit is contained in:
Eric Traut
2026-04-23 13:28:12 -07:00
committed by GitHub
Unverified
parent cccc1b618e
commit 3f8c06e457
3 changed files with 44 additions and 2 deletions
+2 -1
View File
@@ -97,7 +97,8 @@ pub(crate) async fn run_codex_thread_interactive(
analytics_events_client: Some(parent_session.services.analytics_events_client.clone()),
thread_store: Arc::clone(&parent_session.services.thread_store),
}))
.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;
+26
View File
@@ -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) =
+16 -1
View File
@@ -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,
@@ -1656,7 +1658,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)