From ba057e43758cb19beead2af9df19d10756cc4100 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 4 Nov 2026 16:58:37 +0000 Subject: [PATCH] Drain queued side-conversation events before switching focus (#59458) ## Why Pressing `Ctrl-/` immediately after `/btw` must not switch focus before the queued side-conversation prompt is dispatched. ## What changed Wait for queued app events to drain before reading terminal input when the displayed thread belongs to a parent/side conversation pair, even when side retention is disabled. This keeps queued prompts and transcript updates ahead of focus changes. ## Testing Extend the side-conversation startup test to toggle focus at the earliest allowed opportunity and verify that the turn starts on the side thread. Also check that a queued request to reopen the side conversation blocks terminal input until handled. GitOrigin-RevId: cb46636d35de97b02994ed68a32eb971cd7bf6db --- codex-rs/tui/src/app/startup.rs | 19 +++++++-- codex-rs/tui/src/app/tests.rs | 71 +++++++++++++++++++++++---------- 2 files changed, 65 insertions(+), 25 deletions(-) diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index c4fb378490..b1d0eab210 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -168,6 +168,21 @@ impl App { self.pending_thread_switch_resets != 0 && self.side_retention_enabled(tui) } + /// Drain a side pair's queued prompts and transcript updates before a key can switch focus. + pub(super) fn can_read_terminal_after_thread_switch( + &self, + tui: &tui::Tui, + app_event_rx: &mpsc::UnboundedReceiver, + ) -> bool { + self.pending_thread_switch_resets == 0 + && (app_event_rx.is_empty() + || (!self.side_retention_enabled(tui) + && self + .current_displayed_thread_id() + .and_then(|id| self.side_pair_key(id)) + .is_none())) + } + /// Keep the provisional loading frame until queued history reaches the owned transcript. /// Visible startup decisions and the agent overview must still render immediately. pub(super) fn render_startup_frame( @@ -1547,9 +1562,7 @@ See the Codex keymap documentation for supported actions and examples." } AppRunControl::Continue } - // Dispatch the current composer's queued submission before a focus key can switch threads. - event = tui_events.next(), if app.pending_thread_switch_resets == 0 - && (!app.side_retention_enabled(tui) || app_event_rx.is_empty()) + event = tui_events.next(), if app.can_read_terminal_after_thread_switch(tui, &app_event_rx) && (app.reconnect.offline || !block_terminal_input_for_pending_startup_events) => { if let Some(event) = event { if (matches!( diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index b698b65d91..e7c7b17878 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -3329,6 +3329,16 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<() "[tui]\nstatus_line = ['model-with-reasoning', 'thread-name']\n", )?; let config = app.chat_widget.config_ref().clone(); + let model = + app_test_support::create_mock_responses_server_repeating_assistant("Side answer").await; + std::fs::write( + config.codex_home.join("config.toml"), + // Each thread reloads this footer. Omit the platform-specific working directory. + format!( + "openai_base_url = '{}/v1'\n[tui]\nstatus_line = ['model-with-reasoning']", + model.uri() + ), + )?; let parent_thread_id = ThreadId::from_string( &app_test_support::create_fake_rollout( config.codex_home.as_path(), @@ -3381,7 +3391,7 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<() &mut tui, &mut app_server, parent_thread_id, - /*user_message*/ None, + Some("Side question".into()), )) .await?; @@ -3399,8 +3409,27 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<() .is_closed ); + // Give Ctrl-/ its earliest opportunity to switch back after /btw queues the prompt. + while !app.can_read_terminal_after_thread_switch(&tui, &app_event_rx) { + let event = app_event_rx.try_recv()?; + boxed_future!(app.handle_event(&mut tui, &mut app_server, event)).await?; + } + macro_rules! keys { + ($($key:expr),+) => {{ + for key in [$($key),+] { + boxed_future!(app.handle_tui_event(&mut tui, &mut app_server, TuiEvent::Key(key))).await?; + } + drain_app_events(&mut app, &mut tui, &mut app_server, &mut app_event_rx).await?; + }}; + } + let toggle = KeyEvent::new(KeyCode::Char('/'), KeyModifiers::CONTROL); + for expected in [parent_thread_id, side_thread_id] { + keys!(toggle); + assert_eq!(app.active_thread_id, Some(expected)); + } + let mut saw_thread_started = false; - for _ in 0..20 { + loop { let event = time::timeout( std::time::Duration::from_secs(/*secs*/ 2), app_server.next_event(), @@ -3410,11 +3439,19 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<() .expect("app-server event stream should remain open"); if let codex_app_server_client::AppServerEvent::ServerNotification(notification) = event.event - && let ServerNotification::ThreadStarted(notification) = notification.as_ref() - && notification.thread.id == side_thread_id.to_string() { - saw_thread_started = true; - break; + match notification.as_ref() { + ServerNotification::ThreadStarted(started) + if started.thread.id == side_thread_id.to_string() => + { + saw_thread_started = true; + } + ServerNotification::TurnStarted(started) => { + assert_eq!(started.thread_id, side_thread_id.to_string()); + break; + } + _ => {} + } } } @@ -3433,16 +3470,15 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<() .await?; app.select_agents_overview_thread(&mut tui, &mut app_server, parent_thread_id) .await?; - Box::pin(app.handle_start_side( - &mut tui, - &mut app_server, + drain_app_events(&mut app, &mut tui, &mut app_server, &mut app_event_rx).await?; + app.app_event_tx.send(AppEvent::StartSide { parent_thread_id, - /*user_message*/ None, - )) - .await?; + user_message: None, + }); + assert!(!app.can_read_terminal_after_thread_switch(&tui, &app_event_rx)); + drain_app_events(&mut app, &mut tui, &mut app_server, &mut app_event_rx).await?; assert_eq!(app.active_thread_id, Some(side_thread_id)); assert_eq!(app.side_threads.len(), 1); - drain_app_events(&mut app, &mut tui, &mut app_server, &mut app_event_rx).await?; enable_side_retention(&mut app)?; tui.set_owned_screen(/*owned*/ true)?; read_side_history(&mut app, &mut tui, "child")?; @@ -3533,14 +3569,6 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<() )) ); - macro_rules! keys { - ($($key:expr),+) => {{ - for key in [$($key),+] { - boxed_future!(app.handle_tui_event(&mut tui, &mut app_server, TuiEvent::Key(key))).await?; - } - drain_app_events(&mut app, &mut tui, &mut app_server, &mut app_event_rx).await?; - }}; - } let leader = KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL); let [minus, repeat, release] = [ KeyEventKind::Press, @@ -3605,7 +3633,6 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<() }; let inactive = composer_gutter(&mut app, &mut tui)?; - let toggle = KeyEvent::new(KeyCode::Char('/'), KeyModifiers::CONTROL); keys!(toggle); assert_eq!(app.current_displayed_thread_id(), Some(side_thread_id)); let wide = side_frame(&mut app, &mut tui, size)?;