Skip to content

Commit be54e27

Browse files
committed
docs(tui): document ChatWidget task-running invariant
ChatWidget treats the bottom-pane "task running" indicator as a derived UI-busy signal. The UI can be busy for two independent reasons: an agent turn (TurnStarted..complete/abort) and MCP startup (McpStartupUpdate..McpStartupComplete). Document the invariant that the bottom-pane indicator is driven by agent_turn_running || mcp_startup_status.is_some(), and that turn cleanup does not clear MCP startup tracking. Also add brief module headers to the ChatWidget test modules and clarify the MCP startup completion regression test intent.
1 parent adf3982 commit be54e27

File tree

4 files changed

+72
-6
lines changed

4 files changed

+72
-6
lines changed

codex-rs/tui/src/chatwidget.rs

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,12 @@
1414
//! cache key is designed to change when the active cell mutates in place or when its transcript
1515
//! output is time-dependent so the overlay can refresh its cached tail without rebuilding it on
1616
//! every draw.
17-
17+
//!
18+
//! The bottom pane exposes a single "task running" indicator that drives the spinner and interrupt
19+
//! hints. This module treats that indicator as derived UI-busy state: it is set while an agent turn
20+
//! is in progress and while MCP server startup is in progress. Those lifecycles are tracked
21+
//! independently (`agent_turn_running` and `mcp_startup_status`) and synchronized via
22+
//! `update_task_running_state`.
1823
use std::collections::HashMap;
1924
use std::collections::HashSet;
2025
use std::collections::VecDeque;
@@ -330,6 +335,12 @@ pub(crate) enum ExternalEditorState {
330335
Active,
331336
}
332337

338+
/// Maintains the per-session UI state for the chat screen.
339+
///
340+
/// This type owns the state derived from a `codex_core::protocol` event stream (history cells,
341+
/// active streaming buffers, bottom-pane overlays, and transient status text). It is not
342+
/// responsible for running the agent itself; it only reflects progress by updating UI state and by
343+
/// sending `Op` requests back to codex-core.
333344
pub(crate) struct ChatWidget {
334345
app_event_tx: AppEventSender,
335346
codex_op_tx: UnboundedSender<Op>,
@@ -364,7 +375,16 @@ pub(crate) struct ChatWidget {
364375
last_unified_wait: Option<UnifiedExecWaitState>,
365376
task_complete_pending: bool,
366377
unified_exec_processes: Vec<UnifiedExecProcessSummary>,
378+
/// Tracks whether codex-core currently considers an agent turn to be in progress.
379+
///
380+
/// This is kept separate from `mcp_startup_status` so that MCP startup progress (or completion)
381+
/// can update the status header without accidentally clearing the spinner for an active turn.
367382
agent_turn_running: bool,
383+
/// Tracks per-server MCP startup state while startup is in progress.
384+
///
385+
/// The map is `Some(_)` from the first `McpStartupUpdate` until `McpStartupComplete`, and the
386+
/// bottom pane is treated as "running" while this is populated, even if no agent turn is
387+
/// currently executing.
368388
mcp_startup_status: Option<HashMap<String, McpStartupStatus>>,
369389
// Queue of interruptive UI events deferred during an active write cycle
370390
interrupts: InterruptManager,
@@ -458,11 +478,14 @@ fn create_initial_user_message(text: String, image_paths: Vec<PathBuf>) -> Optio
458478
}
459479

460480
impl ChatWidget {
481+
/// Synchronize the bottom-pane "task running" indicator with the current lifecycles.
482+
///
483+
/// The bottom pane only has one running flag, but this module treats it as a derived state of
484+
/// both the agent turn lifecycle and MCP startup lifecycle.
461485
fn update_task_running_state(&mut self) {
462486
self.bottom_pane
463487
.set_task_running(self.agent_turn_running || self.mcp_startup_status.is_some());
464488
}
465-
466489
fn flush_answer_stream_with_separator(&mut self) {
467490
if let Some(mut controller) = self.stream_controller.take()
468491
&& let Some(cell) = controller.finalize()
@@ -763,7 +786,10 @@ impl ChatWidget {
763786
self.rate_limit_snapshot = None;
764787
}
765788
}
766-
/// Finalize any active exec as failed and stop/clear running UI state.
789+
/// Finalize any active exec as failed and stop/clear agent-turn UI state.
790+
///
791+
/// This does not clear MCP startup tracking, because MCP startup can overlap with turn cleanup
792+
/// and should continue to drive the bottom-pane running indicator while it is in progress.
767793
fn finalize_turn(&mut self) {
768794
// Ensure any spinner is replaced by a red ✗ and flushed into history.
769795
self.finalize_active_cell_as_failed();

codex-rs/tui/src/chatwidget/tests.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
//! Exercises `ChatWidget` event handling and rendering invariants.
2+
//!
3+
//! These tests treat the widget as the adapter between `codex_core::protocol::EventMsg` inputs and
4+
//! the TUI output. Many assertions are snapshot-based so that layout regressions and status/header
5+
//! changes show up as stable, reviewable diffs.
6+
17
use super::*;
28
use crate::app_event::AppEvent;
39
use crate::app_event_sender::AppEventSender;

codex-rs/tui2/src/chatwidget.rs

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,12 @@
1414
//! cache key is designed to change when the active cell mutates in place or when its transcript
1515
//! output is time-dependent so the overlay can refresh its cached tail without rebuilding it on
1616
//! every draw.
17-
17+
//!
18+
//! The bottom pane exposes a single "task running" indicator that drives the spinner and interrupt
19+
//! hints. This module treats that indicator as derived UI-busy state: it is set while an agent turn
20+
//! is in progress and while MCP server startup is in progress. Those lifecycles are tracked
21+
//! independently (`agent_turn_running` and `mcp_startup_status`) and synchronized via
22+
//! `update_task_running_state`.
1823
use std::collections::HashMap;
1924
use std::collections::HashSet;
2025
use std::collections::VecDeque;
@@ -298,6 +303,12 @@ enum RateLimitSwitchPromptState {
298303
Shown,
299304
}
300305

306+
/// Maintains the per-session UI state for the chat screen.
307+
///
308+
/// This type owns the state derived from a `codex_core::protocol` event stream (history cells,
309+
/// active streaming buffers, bottom-pane overlays, and transient status text). It is not
310+
/// responsible for running the agent itself; it only reflects progress by updating UI state and by
311+
/// sending `Op` requests back to codex-core.
301312
pub(crate) struct ChatWidget {
302313
app_event_tx: AppEventSender,
303314
codex_op_tx: UnboundedSender<Op>,
@@ -331,7 +342,16 @@ pub(crate) struct ChatWidget {
331342
suppressed_exec_calls: HashSet<String>,
332343
last_unified_wait: Option<UnifiedExecWaitState>,
333344
task_complete_pending: bool,
345+
/// Tracks whether codex-core currently considers an agent turn to be in progress.
346+
///
347+
/// This is kept separate from `mcp_startup_status` so that MCP startup progress (or completion)
348+
/// can update the status header without accidentally clearing the spinner for an active turn.
334349
agent_turn_running: bool,
350+
/// Tracks per-server MCP startup state while startup is in progress.
351+
///
352+
/// The map is `Some(_)` from the first `McpStartupUpdate` until `McpStartupComplete`, and the
353+
/// bottom pane is treated as "running" while this is populated, even if no agent turn is
354+
/// currently executing.
335355
mcp_startup_status: Option<HashMap<String, McpStartupStatus>>,
336356
// Queue of interruptive UI events deferred during an active write cycle
337357
interrupts: InterruptManager,
@@ -424,11 +444,14 @@ fn create_initial_user_message(text: String, image_paths: Vec<PathBuf>) -> Optio
424444
}
425445

426446
impl ChatWidget {
447+
/// Synchronize the bottom-pane "task running" indicator with the current lifecycles.
448+
///
449+
/// The bottom pane only has one running flag, but this module treats it as a derived state of
450+
/// both the agent turn lifecycle and MCP startup lifecycle.
427451
fn update_task_running_state(&mut self) {
428452
self.bottom_pane
429453
.set_task_running(self.agent_turn_running || self.mcp_startup_status.is_some());
430454
}
431-
432455
fn flush_answer_stream_with_separator(&mut self) {
433456
if let Some(mut controller) = self.stream_controller.take()
434457
&& let Some(cell) = controller.finalize()
@@ -728,7 +751,10 @@ impl ChatWidget {
728751
self.rate_limit_snapshot = None;
729752
}
730753
}
731-
/// Finalize any active exec as failed and stop/clear running UI state.
754+
/// Finalize any active exec as failed and stop/clear agent-turn UI state.
755+
///
756+
/// This does not clear MCP startup tracking, because MCP startup can overlap with turn cleanup
757+
/// and should continue to drive the bottom-pane running indicator while it is in progress.
732758
fn finalize_turn(&mut self) {
733759
// Ensure any spinner is replaced by a red ✗ and flushed into history.
734760
self.finalize_active_cell_as_failed();

codex-rs/tui2/src/chatwidget/tests.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
//! Exercises `ChatWidget` event handling and rendering invariants.
2+
//!
3+
//! These tests treat the widget as the adapter between `codex_core::protocol::EventMsg` inputs and
4+
//! the TUI output. Many assertions are snapshot-based so that layout regressions and status/header
5+
//! changes show up as stable, reviewable diffs.
6+
17
use super::*;
28
use crate::app_event::AppEvent;
39
use crate::app_event_sender::AppEventSender;
@@ -2533,6 +2539,8 @@ async fn mcp_startup_complete_does_not_clear_running_task() {
25332539
}),
25342540
});
25352541

2542+
// The bottom pane has a single "task running" indicator even though MCP startup and an agent
2543+
// turn are tracked independently, so a startup completion event must not clear an active turn.
25362544
assert!(chat.bottom_pane.is_task_running());
25372545
assert!(chat.bottom_pane.status_indicator_visible());
25382546

0 commit comments

Comments
 (0)