From 7532f34699fca93fc52cbb1ae77d153a70488382 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Wed, 14 Jan 2026 09:52:34 +0000 Subject: [PATCH] fix: drop double waiting header in TUI (#9145) --- codex-rs/core/src/unified_exec/mod.rs | 2 - .../core/src/unified_exec/process_manager.rs | 38 ----- codex-rs/tui/src/chatwidget.rs | 152 ++++++++++-------- ...fied_exec_non_empty_then_empty_active.snap | 2 - ...ed_exec_waiting_multiple_empty_active.snap | 5 - codex-rs/tui/src/chatwidget/tests.rs | 66 ++------ codex-rs/tui/src/history_cell.rs | 100 ------------ 7 files changed, 103 insertions(+), 262 deletions(-) delete mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_waiting_multiple_empty_active.snap diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index ae1005407..e84b63098 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -130,8 +130,6 @@ impl Default for UnifiedExecProcessManager { struct ProcessEntry { process: Arc, - session_ref: Arc, - turn_ref: Arc, call_id: String, process_id: String, command: Vec, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 2e8015611..80f0a4841 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -10,12 +10,7 @@ use tokio::time::Duration; use tokio::time::Instant; use tokio_util::sync::CancellationToken; -use crate::bash::extract_bash_command; -use crate::codex::Session; -use crate::codex::TurnContext; use crate::exec_env::create_env; -use crate::protocol::BackgroundEventEvent; -use crate::protocol::EventMsg; use crate::protocol::ExecCommandSource; use crate::sandboxing::ExecEnv; use crate::sandboxing::SandboxPermissions; @@ -74,8 +69,6 @@ struct PreparedProcessHandles { output_buffer: OutputBuffer, output_notify: Arc, cancellation_token: CancellationToken, - session_ref: Arc, - turn_ref: Arc, command: Vec, process_id: String, } @@ -224,8 +217,6 @@ impl UnifiedExecProcessManager { Arc::clone(&transcript), ) .await; - - Self::emit_waiting_status(&context.session, &context.turn, &request.command).await; }; let original_token_count = approx_token_count(&text); @@ -259,8 +250,6 @@ impl UnifiedExecProcessManager { output_buffer, output_notify, cancellation_token, - session_ref, - turn_ref, command: session_command, process_id, .. @@ -325,10 +314,6 @@ impl UnifiedExecProcessManager { session_command: Some(session_command.clone()), }; - if response.process_id.is_some() { - Self::emit_waiting_status(&session_ref, &turn_ref, &session_command).await; - } - Ok(response) } @@ -382,8 +367,6 @@ impl UnifiedExecProcessManager { output_buffer, output_notify, cancellation_token, - session_ref: Arc::clone(&entry.session_ref), - turn_ref: Arc::clone(&entry.turn_ref), command: entry.command.clone(), process_id: entry.process_id.clone(), }) @@ -412,8 +395,6 @@ impl UnifiedExecProcessManager { ) { let entry = ProcessEntry { process: Arc::clone(&process), - session_ref: Arc::clone(&context.session), - turn_ref: Arc::clone(&context.turn), call_id: context.call_id.clone(), process_id: process_id.clone(), command: command.to_vec(), @@ -449,25 +430,6 @@ impl UnifiedExecProcessManager { ); } - async fn emit_waiting_status( - session: &Arc, - turn: &Arc, - command: &[String], - ) { - let command_display = if let Some((_, script)) = extract_bash_command(command) { - script.to_string() - } else { - command.join(" ") - }; - let message = format!("Waiting for `{command_display}`"); - session - .send_event( - turn.as_ref(), - EventMsg::BackgroundEvent(BackgroundEventEvent { message }), - ) - .await; - } - pub(crate) async fn open_session_with_exec_env( &self, env: &ExecEnv, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 97bab9875..450d96918 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -199,6 +199,28 @@ impl UnifiedExecWaitState { } } +#[derive(Clone, Debug)] +struct UnifiedExecWaitStreak { + process_id: String, + command_display: Option, +} + +impl UnifiedExecWaitStreak { + fn new(process_id: String, command_display: Option) -> Self { + Self { + process_id, + command_display: command_display.filter(|display| !display.is_empty()), + } + } + + fn update_command_display(&mut self, command_display: Option) { + if self.command_display.is_some() { + return; + } + self.command_display = command_display.filter(|display| !display.is_empty()); + } +} + fn is_unified_exec_source(source: ExecCommandSource) -> bool { matches!( source, @@ -373,6 +395,7 @@ pub(crate) struct ChatWidget { running_commands: HashMap, suppressed_exec_calls: HashSet, last_unified_wait: Option, + unified_exec_wait_streak: Option, task_complete_pending: bool, unified_exec_processes: Vec, /// Tracks whether codex-core currently considers an agent turn to be in progress. @@ -486,6 +509,26 @@ impl ChatWidget { self.bottom_pane .set_task_running(self.agent_turn_running || self.mcp_startup_status.is_some()); } + + fn restore_reasoning_status_header(&mut self) { + if let Some(header) = extract_first_bold(&self.reasoning_buffer) { + self.set_status_header(header); + } else if self.bottom_pane.is_task_running() { + self.set_status_header(String::from("Working")); + } + } + + fn flush_unified_exec_wait_streak(&mut self) { + let Some(wait) = self.unified_exec_wait_streak.take() else { + return; + }; + self.needs_final_message_separator = true; + let cell = history_cell::new_unified_exec_interaction(wait.command_display, String::new()); + self.app_event_tx + .send(AppEvent::InsertHistoryCell(Box::new(cell))); + self.restore_reasoning_status_header(); + } + fn flush_answer_stream_with_separator(&mut self) { if let Some(mut controller) = self.stream_controller.take() && let Some(cell) = controller.finalize() @@ -610,6 +653,12 @@ impl ChatWidget { // (between **/**) as the chunk header. Show this header as status. self.reasoning_buffer.push_str(&delta); + if self.unified_exec_wait_streak.is_some() { + // Unified exec waiting should take precedence over reasoning-derived status headers. + self.request_redraw(); + return; + } + if let Some(header) = extract_first_bold(&self.reasoning_buffer) { // Update the shimmer header to the extracted reasoning chunk header. self.set_status_header(header); @@ -656,13 +705,14 @@ impl ChatWidget { fn on_task_complete(&mut self, last_agent_message: Option) { // If a stream is currently active, finalize it. self.flush_answer_stream_with_separator(); - self.flush_wait_cell(); + self.flush_unified_exec_wait_streak(); // Mark task stopped and request redraw now that all content is in history. self.agent_turn_running = false; self.update_task_running_state(); self.running_commands.clear(); self.suppressed_exec_calls.clear(); self.last_unified_wait = None; + self.unified_exec_wait_streak = None; self.request_redraw(); // If there is a queued user message, send exactly one now to begin the next turn. @@ -982,50 +1032,38 @@ impl ChatWidget { .find(|process| process.key == ev.process_id) .map(|process| process.command_display.clone()); if ev.stdin.is_empty() { - // Empty stdin means we are still waiting on background output; keep a live shimmer cell. - if let Some(wait_cell) = self.active_cell.as_mut().and_then(|cell| { - cell.as_any_mut() - .downcast_mut::() - }) && wait_cell.matches(command_display.as_deref()) - { - // Same process still waiting; update command display if it shows up late. - if wait_cell.update_command_display(command_display) { - self.bump_active_cell_revision(); + // Empty stdin means we are polling for background output. + // Surface this in the status header (single "waiting" surface) instead of the transcript. + self.bottom_pane.ensure_status_indicator(); + self.bottom_pane.set_interrupt_hint_visible(true); + let header = if let Some(command) = &command_display { + format!("Waiting for background terminal · {command}") + } else { + "Waiting for background terminal".to_string() + }; + self.set_status_header(header); + match &mut self.unified_exec_wait_streak { + Some(wait) if wait.process_id == ev.process_id => { + wait.update_command_display(command_display); + } + Some(_) => { + self.flush_unified_exec_wait_streak(); + self.unified_exec_wait_streak = + Some(UnifiedExecWaitStreak::new(ev.process_id, command_display)); + } + None => { + self.unified_exec_wait_streak = + Some(UnifiedExecWaitStreak::new(ev.process_id, command_display)); } - self.request_redraw(); - return; } - let has_non_wait_active = matches!( - self.active_cell.as_ref(), - Some(active) - if active - .as_any() - .downcast_ref::() - .is_none() - ); - if has_non_wait_active { - // Do not preempt non-wait active cells with a wait entry. - return; - } - self.flush_wait_cell(); - self.active_cell = Some(Box::new(history_cell::new_unified_exec_wait_live( - command_display, - self.config.animations, - ))); - self.bump_active_cell_revision(); self.request_redraw(); } else { - if let Some(wait_cell) = self.active_cell.as_ref().and_then(|cell| { - cell.as_any() - .downcast_ref::() - }) { - // Convert the live wait cell into a static "(waited)" entry before logging stdin. - let waited_command = wait_cell.command_display().or(command_display.clone()); - self.active_cell = None; - self.add_to_history(history_cell::new_unified_exec_interaction( - waited_command, - String::new(), - )); + if self + .unified_exec_wait_streak + .as_ref() + .is_some_and(|wait| wait.process_id == ev.process_id) + { + self.flush_unified_exec_wait_streak(); } self.add_to_history(history_cell::new_unified_exec_interaction( command_display, @@ -1060,6 +1098,14 @@ impl ChatWidget { fn on_exec_command_end(&mut self, ev: ExecCommandEndEvent) { if is_unified_exec_source(ev.source) { + if let Some(process_id) = ev.process_id.as_deref() + && self + .unified_exec_wait_streak + .as_ref() + .is_some_and(|wait| wait.process_id == process_id) + { + self.flush_unified_exec_wait_streak(); + } self.track_unified_exec_process_end(&ev); if !self.bottom_pane.is_task_running() { return; @@ -1555,6 +1601,7 @@ impl ChatWidget { running_commands: HashMap::new(), suppressed_exec_calls: HashSet::new(), last_unified_wait: None, + unified_exec_wait_streak: None, task_complete_pending: false, unified_exec_processes: Vec::new(), agent_turn_running: false, @@ -1646,6 +1693,7 @@ impl ChatWidget { running_commands: HashMap::new(), suppressed_exec_calls: HashSet::new(), last_unified_wait: None, + unified_exec_wait_streak: None, task_complete_pending: false, unified_exec_processes: Vec::new(), agent_turn_running: false, @@ -2072,34 +2120,12 @@ impl ChatWidget { } fn flush_active_cell(&mut self) { - self.flush_wait_cell(); if let Some(active) = self.active_cell.take() { self.needs_final_message_separator = true; self.app_event_tx.send(AppEvent::InsertHistoryCell(active)); } } - // Only flush a live wait cell here; other active cells must finalize via their end events. - fn flush_wait_cell(&mut self) { - // Wait cells are transient: convert them into "(waited)" history entries if present. - // Leave non-wait active cells intact so their end events can finalize them. - let Some(active) = self.active_cell.take() else { - return; - }; - let Some(wait_cell) = active - .as_any() - .downcast_ref::() - else { - self.active_cell = Some(active); - return; - }; - self.needs_final_message_separator = true; - let cell = - history_cell::new_unified_exec_interaction(wait_cell.command_display(), String::new()); - self.app_event_tx - .send(AppEvent::InsertHistoryCell(Box::new(cell))); - } - pub(crate) fn add_to_history(&mut self, cell: impl HistoryCell + 'static) { self.add_boxed_history(Box::new(cell)); } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_non_empty_then_empty_active.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_non_empty_then_empty_active.snap index bd83ca4e3..93aac7d84 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_non_empty_then_empty_active.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_non_empty_then_empty_active.snap @@ -4,5 +4,3 @@ expression: active_combined --- ↳ Interacted with background terminal · just fix └ pwd - -• Waiting for background terminal · just fix diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_waiting_multiple_empty_active.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_waiting_multiple_empty_active.snap deleted file mode 100644 index 1467b9a94..000000000 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_waiting_multiple_empty_active.snap +++ /dev/null @@ -1,5 +0,0 @@ ---- -source: tui/src/chatwidget/tests.rs -expression: active_blob(&chat) ---- -• Waiting for background terminal · just fix diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 0c5f6b852..96ec39f50 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -414,6 +414,7 @@ async fn make_chatwidget_manual( running_commands: HashMap::new(), suppressed_exec_calls: HashSet::new(), last_unified_wait: None, + unified_exec_wait_streak: None, task_complete_pending: false, unified_exec_processes: Vec::new(), agent_turn_running: false, @@ -1317,62 +1318,23 @@ async fn unified_exec_end_after_task_complete_is_suppressed() { } #[tokio::test] -async fn unified_exec_wait_cell_revision_updates_on_late_command_display() { +async fn unified_exec_wait_status_header_updates_on_late_command_display() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; - chat.active_cell = Some(Box::new(crate::history_cell::new_unified_exec_wait_live( - None, - chat.config.animations, - ))); chat.unified_exec_processes.push(UnifiedExecProcessSummary { key: "proc-1".to_string(), command_display: "sleep 5".to_string(), }); - let before = chat.active_cell_revision; chat.on_terminal_interaction(TerminalInteractionEvent { call_id: "call-1".to_string(), process_id: "proc-1".to_string(), stdin: String::new(), }); - assert_eq!(chat.active_cell_revision, before.wrapping_add(1)); - let lines = chat - .active_cell_transcript_lines(80) - .expect("active cell lines"); - let blob = lines_to_single_string(&lines); - assert!( - blob.contains("sleep 5"), - "expected command display to render: {blob:?}" - ); -} - -#[tokio::test] -async fn unified_exec_wait_cell_revision_updates_on_replacement() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; - chat.active_cell = Some(Box::new(crate::history_cell::new_unified_exec_wait_live( - Some("old command".to_string()), - chat.config.animations, - ))); - chat.unified_exec_processes.push(UnifiedExecProcessSummary { - key: "proc-2".to_string(), - command_display: "new command".to_string(), - }); - - let before = chat.active_cell_revision; - chat.on_terminal_interaction(TerminalInteractionEvent { - call_id: "call-2".to_string(), - process_id: "proc-2".to_string(), - stdin: String::new(), - }); - - assert_eq!(chat.active_cell_revision, before.wrapping_add(1)); - let lines = chat - .active_cell_transcript_lines(80) - .expect("active cell lines"); - let blob = lines_to_single_string(&lines); - assert!( - blob.contains("new command"), - "expected replacement wait cell to render: {blob:?}" + assert!(chat.active_cell.is_none()); + assert_eq!( + chat.current_status_header, + "Waiting for background terminal · sleep 5" ); } @@ -1383,9 +1345,9 @@ async fn unified_exec_waiting_multiple_empty_snapshots() { terminal_interaction(&mut chat, "call-wait-1a", "proc-1", ""); terminal_interaction(&mut chat, "call-wait-1b", "proc-1", ""); - assert_snapshot!( - "unified_exec_waiting_multiple_empty_active", - active_blob(&chat) + assert_eq!( + chat.current_status_header, + "Waiting for background terminal · just fix" ); chat.handle_codex_event(Event { @@ -1426,15 +1388,15 @@ async fn unified_exec_non_empty_then_empty_snapshots() { terminal_interaction(&mut chat, "call-wait-3a", "proc-3", "pwd\n"); terminal_interaction(&mut chat, "call-wait-3b", "proc-3", ""); + assert_eq!( + chat.current_status_header, + "Waiting for background terminal · just fix" + ); let pre_cells = drain_insert_history(&mut rx); - let mut active_combined = pre_cells + let active_combined = pre_cells .iter() .map(|lines| lines_to_single_string(lines)) .collect::(); - if !active_combined.is_empty() { - active_combined.push('\n'); - } - active_combined.push_str(&active_blob(&chat)); assert_snapshot!("unified_exec_non_empty_then_empty_active", active_combined); chat.handle_codex_event(Event { diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 34925ee50..d4c45736e 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -25,7 +25,6 @@ use crate::render::line_utils::line_to_static; use crate::render::line_utils::prefix_lines; use crate::render::line_utils::push_owned_lines; use crate::render::renderable::Renderable; -use crate::shimmer::shimmer_spans; use crate::style::user_message_style; use crate::text_formatting::format_and_truncate_tool_result; use crate::text_formatting::truncate_text; @@ -469,98 +468,6 @@ pub(crate) fn new_unified_exec_interaction( UnifiedExecInteractionCell::new(command_display, stdin) } -#[derive(Debug)] -// Live-only wait cell that shimmers while we poll; flushes into a static entry later. -pub(crate) struct UnifiedExecWaitCell { - command_display: Option, - animations_enabled: bool, - start_time: Instant, -} - -impl UnifiedExecWaitCell { - pub(crate) fn new(command_display: Option, animations_enabled: bool) -> Self { - Self { - command_display: command_display.filter(|display| !display.is_empty()), - animations_enabled, - start_time: Instant::now(), - } - } - - pub(crate) fn matches(&self, command_display: Option<&str>) -> bool { - let command_display = command_display.filter(|display| !display.is_empty()); - match (self.command_display.as_deref(), command_display) { - (Some(current), Some(incoming)) => current == incoming, - _ => true, - } - } - - /// Update the command display once. - /// - /// Unified exec can start without a stable command string, and later correlate a process id to - /// a user-facing `command_display`. This method records that first non-empty command display and - /// returns whether it changed the cell; callers use the `true` case to invalidate any cached - /// transcript rendering (for example, the transcript overlay live tail). - pub(crate) fn update_command_display(&mut self, command_display: Option) -> bool { - let command_display = command_display.filter(|display| !display.is_empty()); - if self.command_display.is_some() || command_display.is_none() { - return false; - } - self.command_display = command_display; - true - } - - pub(crate) fn command_display(&self) -> Option { - self.command_display.clone() - } -} - -impl HistoryCell for UnifiedExecWaitCell { - fn display_lines(&self, width: u16) -> Vec> { - if width == 0 { - return Vec::new(); - } - let wrap_width = width as usize; - - let mut header_spans = vec!["• ".dim()]; - if self.animations_enabled { - header_spans.extend(shimmer_spans("Waiting for background terminal")); - } else { - header_spans.push("Waiting for background terminal".bold()); - } - if let Some(command) = &self.command_display - && !command.is_empty() - { - header_spans.push(" · ".dim()); - header_spans.push(command.clone().dim()); - } - let header = Line::from(header_spans); - - let mut out: Vec> = Vec::new(); - let header_wrapped = word_wrap_line(&header, RtOptions::new(wrap_width)); - push_owned_lines(&header_wrapped, &mut out); - out - } - - fn desired_height(&self, width: u16) -> u16 { - self.display_lines(width).len() as u16 - } - - fn transcript_animation_tick(&self) -> Option { - if !self.animations_enabled { - return None; - } - // Match `App`'s frame scheduling cadence for transcript overlay live-tail animation. - Some((self.start_time.elapsed().as_millis() / 50) as u64) - } -} - -pub(crate) fn new_unified_exec_wait_live( - command_display: Option, - animations_enabled: bool, -) -> UnifiedExecWaitCell { - UnifiedExecWaitCell::new(command_display, animations_enabled) -} - #[derive(Debug)] struct UnifiedExecProcessesCell { processes: Vec, @@ -1862,13 +1769,6 @@ mod tests { ); } - #[test] - fn unified_exec_wait_cell_renders_wait() { - let cell = new_unified_exec_wait_live(None, false); - let lines = render_transcript(&cell); - assert_eq!(lines, vec!["• Waiting for background terminal"],); - } - #[test] fn ps_output_empty_snapshot() { let cell = new_unified_exec_processes_output(Vec::new());