diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index d62329024c0..25d1332271e 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -14,7 +14,12 @@ //! cache key is designed to change when the active cell mutates in place or when its transcript //! output is time-dependent so the overlay can refresh its cached tail without rebuilding it on //! every draw. - +//! +//! The bottom pane exposes a single "task running" indicator that drives the spinner and interrupt +//! hints. This module treats that indicator as derived UI-busy state: it is set while an agent turn +//! is in progress and while MCP server startup is in progress. Those lifecycles are tracked +//! independently (`agent_turn_running` and `mcp_startup_status`) and synchronized via +//! `update_task_running_state`. use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; @@ -330,6 +335,12 @@ pub(crate) enum ExternalEditorState { Active, } +/// Maintains the per-session UI state for the chat screen. +/// +/// This type owns the state derived from a `codex_core::protocol` event stream (history cells, +/// active streaming buffers, bottom-pane overlays, and transient status text). It is not +/// responsible for running the agent itself; it only reflects progress by updating UI state and by +/// sending `Op` requests back to codex-core. pub(crate) struct ChatWidget { app_event_tx: AppEventSender, codex_op_tx: UnboundedSender, @@ -364,6 +375,16 @@ pub(crate) struct ChatWidget { last_unified_wait: Option, task_complete_pending: bool, unified_exec_processes: Vec, + /// Tracks whether codex-core currently considers an agent turn to be in progress. + /// + /// This is kept separate from `mcp_startup_status` so that MCP startup progress (or completion) + /// can update the status header without accidentally clearing the spinner for an active turn. + agent_turn_running: bool, + /// Tracks per-server MCP startup state while startup is in progress. + /// + /// The map is `Some(_)` from the first `McpStartupUpdate` until `McpStartupComplete`, and the + /// bottom pane is treated as "running" while this is populated, even if no agent turn is + /// currently executing. mcp_startup_status: Option>, // Queue of interruptive UI events deferred during an active write cycle interrupts: InterruptManager, @@ -457,6 +478,14 @@ fn create_initial_user_message(text: String, image_paths: Vec) -> Optio } impl ChatWidget { + /// Synchronize the bottom-pane "task running" indicator with the current lifecycles. + /// + /// The bottom pane only has one running flag, but this module treats it as a derived state of + /// both the agent turn lifecycle and MCP startup lifecycle. + fn update_task_running_state(&mut self) { + self.bottom_pane + .set_task_running(self.agent_turn_running || self.mcp_startup_status.is_some()); + } fn flush_answer_stream_with_separator(&mut self) { if let Some(mut controller) = self.stream_controller.take() && let Some(cell) = controller.finalize() @@ -613,8 +642,9 @@ impl ChatWidget { // Raw reasoning uses the same flow as summarized reasoning fn on_task_started(&mut self) { + self.agent_turn_running = true; self.bottom_pane.clear_ctrl_c_quit_hint(); - self.bottom_pane.set_task_running(true); + self.update_task_running_state(); self.retry_status_header = None; self.bottom_pane.set_interrupt_hint_visible(true); self.set_status_header(String::from("Working")); @@ -628,7 +658,8 @@ impl ChatWidget { self.flush_answer_stream_with_separator(); self.flush_wait_cell(); // Mark task stopped and request redraw now that all content is in history. - self.bottom_pane.set_task_running(false); + self.agent_turn_running = false; + self.update_task_running_state(); self.running_commands.clear(); self.suppressed_exec_calls.clear(); self.last_unified_wait = None; @@ -755,12 +786,16 @@ impl ChatWidget { self.rate_limit_snapshot = None; } } - /// Finalize any active exec as failed and stop/clear running UI state. + /// Finalize any active exec as failed and stop/clear agent-turn UI state. + /// + /// This does not clear MCP startup tracking, because MCP startup can overlap with turn cleanup + /// and should continue to drive the bottom-pane running indicator while it is in progress. fn finalize_turn(&mut self) { // Ensure any spinner is replaced by a red ✗ and flushed into history. self.finalize_active_cell_as_failed(); // Reset running state and clear streaming buffers. - self.bottom_pane.set_task_running(false); + self.agent_turn_running = false; + self.update_task_running_state(); self.running_commands.clear(); self.suppressed_exec_calls.clear(); self.last_unified_wait = None; @@ -789,7 +824,7 @@ impl ChatWidget { } status.insert(ev.server, ev.status); self.mcp_startup_status = Some(status); - self.bottom_pane.set_task_running(true); + self.update_task_running_state(); if let Some(current) = &self.mcp_startup_status { let total = current.len(); let mut starting: Vec<_> = current @@ -845,7 +880,7 @@ impl ChatWidget { } self.mcp_startup_status = None; - self.bottom_pane.set_task_running(false); + self.update_task_running_state(); self.maybe_send_next_queued_input(); self.request_redraw(); } @@ -1522,6 +1557,7 @@ impl ChatWidget { last_unified_wait: None, task_complete_pending: false, unified_exec_processes: Vec::new(), + agent_turn_running: false, mcp_startup_status: None, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), @@ -1612,6 +1648,7 @@ impl ChatWidget { last_unified_wait: None, task_complete_pending: false, unified_exec_processes: Vec::new(), + agent_turn_running: false, mcp_startup_status: None, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 4567c56a927..0c5f6b852d4 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -1,3 +1,9 @@ +//! Exercises `ChatWidget` event handling and rendering invariants. +//! +//! These tests treat the widget as the adapter between `codex_core::protocol::EventMsg` inputs and +//! the TUI output. Many assertions are snapshot-based so that layout regressions and status/header +//! changes show up as stable, reviewable diffs. + use super::*; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; @@ -30,6 +36,7 @@ use codex_core::protocol::ExecCommandSource; use codex_core::protocol::ExecPolicyAmendment; use codex_core::protocol::ExitedReviewModeEvent; use codex_core::protocol::FileChange; +use codex_core::protocol::McpStartupCompleteEvent; use codex_core::protocol::McpStartupStatus; use codex_core::protocol::McpStartupUpdateEvent; use codex_core::protocol::Op; @@ -409,6 +416,7 @@ async fn make_chatwidget_manual( last_unified_wait: None, task_complete_pending: false, unified_exec_processes: Vec::new(), + agent_turn_running: false, mcp_startup_status: None, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), @@ -2953,6 +2961,32 @@ async fn mcp_startup_header_booting_snapshot() { assert_snapshot!("mcp_startup_header_booting", terminal.backend()); } +#[tokio::test] +async fn mcp_startup_complete_does_not_clear_running_task() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.handle_codex_event(Event { + id: "task-1".into(), + msg: EventMsg::TurnStarted(TurnStartedEvent { + model_context_window: None, + }), + }); + + assert!(chat.bottom_pane.is_task_running()); + assert!(chat.bottom_pane.status_indicator_visible()); + + chat.handle_codex_event(Event { + id: "mcp-1".into(), + msg: EventMsg::McpStartupComplete(McpStartupCompleteEvent { + ready: vec!["schaltwerk".into()], + ..Default::default() + }), + }); + + assert!(chat.bottom_pane.is_task_running()); + assert!(chat.bottom_pane.status_indicator_visible()); +} + #[tokio::test] async fn background_event_updates_status_header() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index 66eebde6b3c..bd12a6e9b80 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -14,7 +14,12 @@ //! cache key is designed to change when the active cell mutates in place or when its transcript //! output is time-dependent so the overlay can refresh its cached tail without rebuilding it on //! every draw. - +//! +//! The bottom pane exposes a single "task running" indicator that drives the spinner and interrupt +//! hints. This module treats that indicator as derived UI-busy state: it is set while an agent turn +//! is in progress and while MCP server startup is in progress. Those lifecycles are tracked +//! independently (`agent_turn_running` and `mcp_startup_status`) and synchronized via +//! `update_task_running_state`. use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; @@ -298,6 +303,12 @@ enum RateLimitSwitchPromptState { Shown, } +/// Maintains the per-session UI state for the chat screen. +/// +/// This type owns the state derived from a `codex_core::protocol` event stream (history cells, +/// active streaming buffers, bottom-pane overlays, and transient status text). It is not +/// responsible for running the agent itself; it only reflects progress by updating UI state and by +/// sending `Op` requests back to codex-core. pub(crate) struct ChatWidget { app_event_tx: AppEventSender, codex_op_tx: UnboundedSender, @@ -331,6 +342,16 @@ pub(crate) struct ChatWidget { suppressed_exec_calls: HashSet, last_unified_wait: Option, task_complete_pending: bool, + /// Tracks whether codex-core currently considers an agent turn to be in progress. + /// + /// This is kept separate from `mcp_startup_status` so that MCP startup progress (or completion) + /// can update the status header without accidentally clearing the spinner for an active turn. + agent_turn_running: bool, + /// Tracks per-server MCP startup state while startup is in progress. + /// + /// The map is `Some(_)` from the first `McpStartupUpdate` until `McpStartupComplete`, and the + /// bottom pane is treated as "running" while this is populated, even if no agent turn is + /// currently executing. mcp_startup_status: Option>, // Queue of interruptive UI events deferred during an active write cycle interrupts: InterruptManager, @@ -423,6 +444,14 @@ fn create_initial_user_message(text: String, image_paths: Vec) -> Optio } impl ChatWidget { + /// Synchronize the bottom-pane "task running" indicator with the current lifecycles. + /// + /// The bottom pane only has one running flag, but this module treats it as a derived state of + /// both the agent turn lifecycle and MCP startup lifecycle. + fn update_task_running_state(&mut self) { + self.bottom_pane + .set_task_running(self.agent_turn_running || self.mcp_startup_status.is_some()); + } fn flush_answer_stream_with_separator(&mut self) { if let Some(mut controller) = self.stream_controller.take() && let Some(cell) = controller.finalize() @@ -579,8 +608,9 @@ impl ChatWidget { // Raw reasoning uses the same flow as summarized reasoning fn on_task_started(&mut self) { + self.agent_turn_running = true; self.bottom_pane.clear_ctrl_c_quit_hint(); - self.bottom_pane.set_task_running(true); + self.update_task_running_state(); self.retry_status_header = None; self.bottom_pane.set_interrupt_hint_visible(true); self.set_status_header(String::from("Working")); @@ -593,7 +623,8 @@ impl ChatWidget { // If a stream is currently active, finalize it. self.flush_answer_stream_with_separator(); // Mark task stopped and request redraw now that all content is in history. - self.bottom_pane.set_task_running(false); + self.agent_turn_running = false; + self.update_task_running_state(); self.running_commands.clear(); self.suppressed_exec_calls.clear(); self.last_unified_wait = None; @@ -720,12 +751,16 @@ impl ChatWidget { self.rate_limit_snapshot = None; } } - /// Finalize any active exec as failed and stop/clear running UI state. + /// Finalize any active exec as failed and stop/clear agent-turn UI state. + /// + /// This does not clear MCP startup tracking, because MCP startup can overlap with turn cleanup + /// and should continue to drive the bottom-pane running indicator while it is in progress. fn finalize_turn(&mut self) { // Ensure any spinner is replaced by a red ✗ and flushed into history. self.finalize_active_cell_as_failed(); // Reset running state and clear streaming buffers. - self.bottom_pane.set_task_running(false); + self.agent_turn_running = false; + self.update_task_running_state(); self.running_commands.clear(); self.suppressed_exec_calls.clear(); self.last_unified_wait = None; @@ -754,7 +789,7 @@ impl ChatWidget { } status.insert(ev.server, ev.status); self.mcp_startup_status = Some(status); - self.bottom_pane.set_task_running(true); + self.update_task_running_state(); if let Some(current) = &self.mcp_startup_status { let total = current.len(); let mut starting: Vec<_> = current @@ -810,7 +845,7 @@ impl ChatWidget { } self.mcp_startup_status = None; - self.bottom_pane.set_task_running(false); + self.update_task_running_state(); self.maybe_send_next_queued_input(); self.request_redraw(); } @@ -1381,6 +1416,7 @@ impl ChatWidget { suppressed_exec_calls: HashSet::new(), last_unified_wait: None, task_complete_pending: false, + agent_turn_running: false, mcp_startup_status: None, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), @@ -1469,6 +1505,7 @@ impl ChatWidget { suppressed_exec_calls: HashSet::new(), last_unified_wait: None, task_complete_pending: false, + agent_turn_running: false, mcp_startup_status: None, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), diff --git a/codex-rs/tui2/src/chatwidget/tests.rs b/codex-rs/tui2/src/chatwidget/tests.rs index 32dce6b6480..41b27600091 100644 --- a/codex-rs/tui2/src/chatwidget/tests.rs +++ b/codex-rs/tui2/src/chatwidget/tests.rs @@ -1,3 +1,9 @@ +//! Exercises `ChatWidget` event handling and rendering invariants. +//! +//! These tests treat the widget as the adapter between `codex_core::protocol::EventMsg` inputs and +//! the TUI output. Many assertions are snapshot-based so that layout regressions and status/header +//! changes show up as stable, reviewable diffs. + use super::*; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; @@ -29,6 +35,7 @@ use codex_core::protocol::ExecCommandSource; use codex_core::protocol::ExecPolicyAmendment; use codex_core::protocol::ExitedReviewModeEvent; use codex_core::protocol::FileChange; +use codex_core::protocol::McpStartupCompleteEvent; use codex_core::protocol::McpStartupStatus; use codex_core::protocol::McpStartupUpdateEvent; use codex_core::protocol::Op; @@ -397,6 +404,7 @@ async fn make_chatwidget_manual( suppressed_exec_calls: HashSet::new(), last_unified_wait: None, task_complete_pending: false, + agent_turn_running: false, mcp_startup_status: None, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), @@ -2520,6 +2528,34 @@ async fn mcp_startup_header_booting_snapshot() { assert_snapshot!("mcp_startup_header_booting", terminal.backend()); } +#[tokio::test] +async fn mcp_startup_complete_does_not_clear_running_task() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.handle_codex_event(Event { + id: "task-1".into(), + msg: EventMsg::TurnStarted(TurnStartedEvent { + model_context_window: None, + }), + }); + + // The bottom pane has a single "task running" indicator even though MCP startup and an agent + // turn are tracked independently, so a startup completion event must not clear an active turn. + assert!(chat.bottom_pane.is_task_running()); + assert!(chat.bottom_pane.status_indicator_visible()); + + chat.handle_codex_event(Event { + id: "mcp-1".into(), + msg: EventMsg::McpStartupComplete(McpStartupCompleteEvent { + ready: vec!["schaltwerk".into()], + ..Default::default() + }), + }); + + assert!(chat.bottom_pane.is_task_running()); + assert!(chat.bottom_pane.status_indicator_visible()); +} + #[tokio::test] async fn background_event_updates_status_header() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;