From 3a2dff2ebea5d0c832d0397850cba62a697deba6 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Thu, 4 Jun 2026 19:16:11 -0300 Subject: [PATCH] feat(tui): show tab activity detail Co-authored-by: George Nachman --- codex-rs/tui/src/bottom_pane/app_link_view.rs | 5 + .../tui/src/bottom_pane/approval_overlay.rs | 42 +++++ .../tui/src/bottom_pane/bottom_pane_view.rs | 6 + .../src/bottom_pane/mcp_server_elicitation.rs | 4 + codex-rs/tui/src/bottom_pane/mod.rs | 166 +++++++++++++++--- .../src/bottom_pane/request_user_input/mod.rs | 5 + .../tui/src/bottom_pane/tab_status_state.rs | 77 +++++++- .../src/bottom_pane/tab_status_state_tests.rs | 61 +++++++ codex-rs/tui/src/chatwidget.rs | 2 + .../tui/src/chatwidget/command_lifecycle.rs | 21 +++ .../src/chatwidget/command_lifecycle_tests.rs | 44 +++++ codex-rs/tui/src/chatwidget/constructor.rs | 1 + codex-rs/tui/src/chatwidget/exec_state.rs | 2 + codex-rs/tui/src/chatwidget/turn_runtime.rs | 10 +- codex-rs/tui/src/osc_text.rs | 22 ++- codex-rs/tui/src/osc_text_tests.rs | 2 +- codex-rs/tui/src/status_indicator_widget.rs | 5 +- codex-rs/tui/src/tab_status.rs | 138 ++++++++++++--- codex-rs/tui/src/tab_status_tests.rs | 106 ++++++++--- 19 files changed, 632 insertions(+), 87 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/tab_status_state_tests.rs create mode 100644 codex-rs/tui/src/chatwidget/command_lifecycle_tests.rs diff --git a/codex-rs/tui/src/bottom_pane/app_link_view.rs b/codex-rs/tui/src/bottom_pane/app_link_view.rs index bbd7f355c3..068b9e598d 100644 --- a/codex-rs/tui/src/bottom_pane/app_link_view.rs +++ b/codex-rs/tui/src/bottom_pane/app_link_view.rs @@ -777,6 +777,11 @@ impl BottomPaneView for AppLinkView { fn terminal_title_requires_action(&self) -> bool { self.is_tool_suggestion() } + + fn tab_status_detail(&self) -> Option { + self.is_tool_suggestion() + .then(|| "Tool suggestion".to_string()) + } } impl crate::render::renderable::Renderable for AppLinkView { diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index f318dd2103..b1db5a12aa 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -106,6 +106,39 @@ pub(crate) enum ApprovalRequest { } impl ApprovalRequest { + /// One-line summary suitable for the OSC 21337 `detail` field. + pub(crate) fn tab_status_summary(&self) -> Option { + match self { + ApprovalRequest::Exec { command, .. } => { + // Parse like the visible history cells so the detail reads + // "Run python3 …" / "Read foo.rs", not a raw shell line. + let parsed = codex_shell_command::parse_command::parse_command(command); + Some( + crate::tab_status::format_parsed_command_for_tab_status(&parsed) + .unwrap_or_else(|| { + crate::tab_status::format_command_for_tab_status(command) + }), + ) + } + ApprovalRequest::ApplyPatch { changes, .. } => { + let count = changes.len(); + let suffix = if count == 1 { "" } else { "s" }; + Some(format!("Apply patch ({count} file{suffix})")) + } + ApprovalRequest::Permissions { reason, .. } => Some( + reason + .as_deref() + .map(|r| format!("Permissions: {r}")) + .unwrap_or_else(|| "Permissions request".to_string()), + ), + ApprovalRequest::McpElicitation { + server_name, + message, + .. + } => Some(format!("Elicitation from {server_name}: {message}")), + } + } + fn thread_id(&self) -> ThreadId { match self { ApprovalRequest::Exec { thread_id, .. } @@ -601,6 +634,15 @@ impl BottomPaneView for ApprovalOverlay { fn terminal_title_requires_action(&self) -> bool { true } + + fn tab_status_detail(&self) -> Option { + // Read the live current_request: the overlay advances its queue + // internally (no new push_view), so this keeps the detail in lockstep + // with what the user sees. + self.current_request + .as_ref() + .and_then(ApprovalRequest::tab_status_summary) + } } impl Renderable for ApprovalOverlay { diff --git a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs index 7538d76ca7..fbf5ab1dcb 100644 --- a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs +++ b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs @@ -131,6 +131,12 @@ pub(crate) trait BottomPaneView: Renderable { false } + /// Optional one-line OSC detail for what this view is waiting on. + /// Stateful views should compute it from their current request. + fn tab_status_detail(&self) -> Option { + None + } + /// Return the next time-based redraw this view needs while it is active. fn next_frame_delay(&self) -> Option { None diff --git a/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs b/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs index 447ab8e6c3..5a39e758b5 100644 --- a/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs +++ b/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs @@ -1646,6 +1646,10 @@ impl BottomPaneView for McpServerElicitationOverlay { true } + fn tab_status_detail(&self) -> Option { + Some("MCP server request".to_string()) + } + fn on_ctrl_c(&mut self) -> CancellationEvent { if !self.current_field_is_select() && !self.composer.current_text_with_pending().is_empty() { diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 2e4e16ca5b..4ecbe630ad 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -309,13 +309,17 @@ impl BottomPane { self.tab_status.set_enabled(enabled); } - /// Compute the desired tab-status state from current task + modal state. - /// - /// Waiting preempts Working: a user-blocking modal is the more important - /// signal to surface. Waiting reads the same `terminal_title_requires_action` - /// signal as the title indicator, so the two can never disagree. - fn desired_tab_status(&self) -> TabStatus { - if self.terminal_title_requires_action() { + pub(crate) fn set_current_activity(&mut self, activity: Option) { + if self.tab_status.set_current_activity(activity) { + self.refresh_tab_status(); + } + } + + fn desired_tab_class(&self) -> TabStatus { + if self + .active_view() + .is_some_and(BottomPaneView::terminal_title_requires_action) + { TabStatus::Waiting } else if self.is_task_running || !self.unified_exec_footer.is_empty() { TabStatus::Working @@ -324,11 +328,66 @@ impl BottomPane { } } - /// Emit an OSC 21337 update if the desired state differs from what we - /// last wrote. Invoked at state-class transitions only. + fn desired_tab_status(&self) -> (TabStatus, Option) { + let class = self.desired_tab_class(); + let detail = match class { + TabStatus::Waiting => self + .active_view() + .and_then(BottomPaneView::tab_status_detail), + TabStatus::Working => self.working_tab_status_detail(), + TabStatus::Idle => self + .tab_status + .last_activity() + .map(|activity| format!("last: {activity}")), + }; + (class, detail) + } + + fn working_tab_status_detail(&self) -> Option { + let mut parts = Vec::new(); + if let Some(activity) = self.tab_status.current_activity() { + parts.push(activity.to_string()); + } + let push_if_new = |parts: &mut Vec, candidate: &str| { + let candidate = candidate.trim(); + if candidate.is_empty() + || parts + .iter() + .any(|part| part.contains(candidate) || candidate.contains(part.as_str())) + { + return; + } + parts.push(candidate.to_string()); + }; + if let Some(status) = self.status.as_ref() { + let header = status.header(); + if header != "Working" { + push_if_new(&mut parts, header); + } + if let Some(first_line) = status.details().and_then(|details| details.lines().next()) { + push_if_new(&mut parts, first_line); + } + } + if !parts.is_empty() { + return Some(parts.join(" • ")); + } + self.tab_status + .last_activity() + .map(|activity| format!("after: {activity}")) + } + fn refresh_tab_status(&mut self) { + self.refresh_tab_status_at(Instant::now()); + } + + fn refresh_tab_status_at(&mut self, now: Instant) { + let desired_class = self.desired_tab_class(); + if let Some(delay) = self.tab_status.refresh_delay(desired_class, now) { + self.frame_requester.schedule_frame_in(delay); + return; + } let desired = self.desired_tab_status(); - self.tab_status.refresh(desired); + self.tab_status.refresh(desired, now); } pub fn set_skills(&mut self, skills: Option>) { @@ -776,10 +835,8 @@ impl BottomPane { self.composer.sync_popups(); self.maybe_show_delayed_approval_requests_at(now); self.schedule_active_view_frame(); - // Poll every frame: the imperative refresh at push/task-state sites - // misses transitions where a pop into a non-empty stack changes the - // active view, or where a dismissal bypasses on_active_view_complete. - self.refresh_tab_status(); + // Poll every frame to catch view-stack transitions and changing detail. + self.refresh_tab_status_at(now); } fn schedule_active_view_frame(&self) { @@ -1004,9 +1061,9 @@ impl BottomPane { self.status.is_some() } - /// Test-only view of the last OSC 21337 tab status we wrote. + /// Test-only view of the last OSC 21337 status and detail we wrote. #[cfg(test)] - pub(crate) fn last_tab_status_for_test(&self) -> Option { + pub(crate) fn last_tab_status_for_test(&self) -> Option<(TabStatus, Option)> { self.tab_status.last_status() } @@ -1038,6 +1095,7 @@ impl BottomPane { if running { if !was_running { + self.tab_status.reset_for_new_turn(); if self.status.is_none() { self.status = Some(StatusIndicatorWidget::new( self.app_event_tx.clone(), @@ -1998,6 +2056,10 @@ mod tests { }) } + fn tab_status(status: TabStatus) -> Option<(TabStatus, Option)> { + Some((status, None)) + } + #[test] fn tab_status_disabled_suppresses_emission() { // Refresh must honor the `tui.tab_status = false` opt-out: nothing @@ -2017,19 +2079,28 @@ mod tests { let features = Features::with_defaults(); let mut pane = fresh_pane(); pane.refresh_tab_status(); - assert_eq!(pane.last_tab_status_for_test(), Some(TabStatus::Idle)); + assert_eq!(pane.last_tab_status_for_test(), tab_status(TabStatus::Idle)); pane.set_task_running(/*running*/ true); - assert_eq!(pane.last_tab_status_for_test(), Some(TabStatus::Working)); + assert_eq!( + pane.last_tab_status_for_test().map(|(status, _)| status), + Some(TabStatus::Working) + ); pane.push_approval_request(exec_request(), &features); - assert_eq!(pane.last_tab_status_for_test(), Some(TabStatus::Waiting)); + assert_eq!( + pane.last_tab_status_for_test().map(|(status, _)| status), + Some(TabStatus::Waiting) + ); let _ = pane.on_ctrl_c(); - assert_eq!(pane.last_tab_status_for_test(), Some(TabStatus::Working)); + assert_eq!( + pane.last_tab_status_for_test().map(|(status, _)| status), + Some(TabStatus::Working) + ); pane.set_task_running(/*running*/ false); - assert_eq!(pane.last_tab_status_for_test(), Some(TabStatus::Idle)); + assert_eq!(pane.last_tab_status_for_test(), tab_status(TabStatus::Idle)); } #[test] @@ -2091,12 +2162,18 @@ mod tests { let mut pane = fresh_pane(); pane.set_task_running(/*running*/ true); pane.push_approval_request(exec_request(), &features); - assert_eq!(pane.last_tab_status_for_test(), Some(TabStatus::Waiting)); + assert_eq!( + pane.last_tab_status_for_test().map(|(status, _)| status), + Some(TabStatus::Waiting) + ); // Simulate handle_paste's view-complete branch: clear then complete. pane.view_stack.clear(); pane.on_active_view_complete(); - assert_eq!(pane.last_tab_status_for_test(), Some(TabStatus::Working)); + assert_eq!( + pane.last_tab_status_for_test().map(|(status, _)| status), + Some(TabStatus::Working) + ); } #[test] @@ -2112,26 +2189,63 @@ mod tests { pane.show_view(Box::new(DismissibleView::default())); pane.set_task_running(/*running*/ true); assert_eq!( - pane.last_tab_status_for_test(), + pane.last_tab_status_for_test().map(|(status, _)| status), Some(TabStatus::Working), "non-action underlay shouldn't move us off Working", ); pane.push_approval_request(exec_request(), &features); - assert_eq!(pane.last_tab_status_for_test(), Some(TabStatus::Waiting)); + assert_eq!( + pane.last_tab_status_for_test().map(|(status, _)| status), + Some(TabStatus::Waiting) + ); // Cancel: DismissibleView remains underneath, so on_active_view_complete // does not fire and only the per-frame poll catches the transition. let _ = pane.on_ctrl_c(); pane.pre_draw_tick(); assert_eq!( - pane.last_tab_status_for_test(), + pane.last_tab_status_for_test().map(|(status, _)| status), Some(TabStatus::Working), "pre_draw_tick must drop the tab out of Waiting once the \ top of the stack is no longer action-required", ); } + #[test] + fn working_tab_status_combines_activity_and_status_detail() { + let mut pane = fresh_pane(); + pane.set_task_running(/*running*/ true); + pane.tab_status + .set_current_activity(Some("Run cargo test".to_string())); + pane.update_status( + "Thinking".to_string(), + Some("checking results\nignored line".to_string()), + StatusDetailsCapitalization::Preserve, + STATUS_DETAILS_DEFAULT_MAX_LINES, + ); + + assert_eq!( + pane.desired_tab_status(), + ( + TabStatus::Working, + Some("Run cargo test • Thinking • checking results".to_string()) + ) + ); + } + + #[test] + fn waiting_tab_status_uses_active_view_detail() { + let features = Features::with_defaults(); + let mut pane = fresh_pane(); + pane.set_task_running(/*running*/ true); + pane.push_approval_request(exec_request(), &features); + + let (status, detail) = pane.desired_tab_status(); + assert_eq!(status, TabStatus::Waiting); + assert!(detail.is_some_and(|detail| !detail.is_empty())); + } + #[test] fn ctrl_c_on_modal_consumes_without_showing_quit_hint() { let (tx_raw, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs b/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs index 02a8074237..37f8ebedc1 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs +++ b/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs @@ -1295,6 +1295,11 @@ impl BottomPaneView for RequestUserInputOverlay { true } + fn tab_status_detail(&self) -> Option { + // No concise per-request summary available; use a generic label. + Some("Tool input request".to_string()) + } + fn on_ctrl_c(&mut self) -> CancellationEvent { if self.confirm_unanswered_active() { self.close_unanswered_confirmation(); diff --git a/codex-rs/tui/src/bottom_pane/tab_status_state.rs b/codex-rs/tui/src/bottom_pane/tab_status_state.rs index 047519d719..0d283b621c 100644 --- a/codex-rs/tui/src/bottom_pane/tab_status_state.rs +++ b/codex-rs/tui/src/bottom_pane/tab_status_state.rs @@ -1,10 +1,18 @@ +use std::time::Duration; +use std::time::Instant; + use crate::tab_status::TabStatus; use crate::tab_status::set_tab_status; +const MIN_DETAIL_INTERVAL: Duration = Duration::from_millis(/*millis*/ 250); + /// Tracks the OSC 21337 state emitted by the bottom pane. pub(super) struct TabStatusState { enabled: bool, - last_status: Option, + last_status: Option<(TabStatus, Option)>, + last_emit_at: Option, + current_activity: Option, + last_activity: Option, } impl TabStatusState { @@ -12,6 +20,9 @@ impl TabStatusState { Self { enabled: true, last_status: None, + last_emit_at: None, + current_activity: None, + last_activity: None, } } @@ -19,19 +30,73 @@ impl TabStatusState { self.enabled = enabled; } - pub(super) fn refresh(&mut self, desired: TabStatus) { - if !self.enabled || self.last_status == Some(desired) { + pub(super) fn reset_for_new_turn(&mut self) { + self.current_activity = None; + self.last_activity = None; + } + + pub(super) fn set_current_activity(&mut self, activity: Option) -> bool { + let activity = activity + .map(|activity| activity.trim().to_string()) + .filter(|activity| !activity.is_empty()); + if self.current_activity == activity { + return false; + } + if activity.is_none() + && let Some(previous) = self.current_activity.take() + { + self.last_activity = Some(previous); + } + self.current_activity = activity; + true + } + + pub(super) fn current_activity(&self) -> Option<&str> { + self.current_activity.as_deref() + } + + pub(super) fn last_activity(&self) -> Option<&str> { + self.last_activity.as_deref() + } + + /// Returns the remaining throttle delay before detail may be recomputed. + /// Status-class transitions always bypass the throttle. + pub(super) fn refresh_delay(&self, desired_class: TabStatus, now: Instant) -> Option { + if !self.enabled + || self.last_status.as_ref().map(|(status, _)| *status) != Some(desired_class) + { + return None; + } + self.last_emit_at.and_then(|last_emit_at| { + let elapsed = now.saturating_duration_since(last_emit_at); + MIN_DETAIL_INTERVAL + .checked_sub(elapsed) + .filter(|delay| !delay.is_zero()) + }) + } + + pub(super) fn refresh(&mut self, desired: (TabStatus, Option), now: Instant) { + if !self.enabled { return; } - if let Err(err) = set_tab_status(desired) { + if self.last_status.as_ref() == Some(&desired) { + self.last_emit_at = Some(now); + return; + } + if let Err(err) = set_tab_status(desired.0, desired.1.as_deref()) { tracing::debug!(error = %err, "failed to set tab status"); return; } self.last_status = Some(desired); + self.last_emit_at = Some(now); } #[cfg(test)] - pub(super) fn last_status(&self) -> Option { - self.last_status + pub(super) fn last_status(&self) -> Option<(TabStatus, Option)> { + self.last_status.clone() } } + +#[cfg(test)] +#[path = "tab_status_state_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/bottom_pane/tab_status_state_tests.rs b/codex-rs/tui/src/bottom_pane/tab_status_state_tests.rs new file mode 100644 index 0000000000..b37c784d0d --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/tab_status_state_tests.rs @@ -0,0 +1,61 @@ +use std::time::Duration; +use std::time::Instant; + +use pretty_assertions::assert_eq; + +use super::TabStatusState; +use crate::tab_status::TabStatus; + +#[test] +fn activity_rolls_forward_and_resets_for_a_new_turn() { + let mut state = TabStatusState::new(); + assert!(state.set_current_activity(Some(" Run cargo test ".to_string()))); + assert_eq!(state.current_activity(), Some("Run cargo test")); + + assert!(state.set_current_activity(/*activity*/ None)); + assert_eq!(state.current_activity(), None); + assert_eq!(state.last_activity(), Some("Run cargo test")); + + state.reset_for_new_turn(); + assert_eq!(state.last_activity(), None); +} + +#[test] +fn throttle_defers_detail_changes_but_not_status_changes() { + let mut state = TabStatusState::new(); + let started_at = Instant::now(); + state.refresh((TabStatus::Working, Some("first".to_string())), started_at); + + assert_eq!( + state.refresh_delay( + TabStatus::Working, + started_at + Duration::from_millis(/*millis*/ 100) + ), + Some(Duration::from_millis(/*millis*/ 150)) + ); + assert_eq!( + state.refresh_delay( + TabStatus::Waiting, + started_at + Duration::from_millis(/*millis*/ 100) + ), + None + ); +} + +#[test] +fn equal_refresh_rearms_the_detail_throttle() { + let mut state = TabStatusState::new(); + let started_at = Instant::now(); + let unchanged_at = started_at + Duration::from_millis(/*millis*/ 300); + let desired = (TabStatus::Working, Some("cargo test".to_string())); + state.refresh(desired.clone(), started_at); + state.refresh(desired, unchanged_at); + + assert_eq!( + state.refresh_delay( + TabStatus::Working, + unchanged_at + Duration::from_millis(/*millis*/ 100) + ), + Some(Duration::from_millis(/*millis*/ 150)) + ); +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 5628d0b23d..a4c38d0d7a 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -565,6 +565,8 @@ pub(crate) struct ChatWidget { clipboard_lease: Option, copy_last_response_binding: Vec, running_commands: HashMap, + /// Orders concurrent commands for deterministic tab-status detail. + running_command_seq: u64, collab_agent_metadata: HashMap, pending_collab_spawn_requests: HashMap, suppressed_exec_calls: HashSet, diff --git a/codex-rs/tui/src/chatwidget/command_lifecycle.rs b/codex-rs/tui/src/chatwidget/command_lifecycle.rs index 701ed919e1..36e5ca9341 100644 --- a/codex-rs/tui/src/chatwidget/command_lifecycle.rs +++ b/codex-rs/tui/src/chatwidget/command_lifecycle.rs @@ -256,16 +256,21 @@ impl ChatWidget { // Ensure the status indicator is visible while the command runs. self.bottom_pane.ensure_status_indicator(); let parsed_cmd = self.annotate_skill_reads_in_parsed_cmd(parsed_cmd); + self.running_command_seq = self.running_command_seq.wrapping_add(1); self.running_commands.insert( id.clone(), RunningCommand { command: command.clone(), parsed_cmd: parsed_cmd.clone(), source, + start_order: self.running_command_seq, }, ); let is_wait_interaction = matches!(source, ExecCommandSource::UnifiedExecInteraction); let command_display = command.join(" "); + let activity = crate::tab_status::format_parsed_command_for_tab_status(&parsed_cmd) + .unwrap_or_else(|| crate::tab_status::format_command_for_tab_status(&command)); + self.bottom_pane.set_current_activity(Some(activity)); let should_suppress_unified_wait = is_wait_interaction && self .last_unified_wait @@ -355,6 +360,8 @@ impl ChatWidget { let aggregated_output = aggregated_output.unwrap_or_default(); let running = self.running_commands.remove(&id); + let next_activity = next_running_command_tab_detail(&self.running_commands); + self.bottom_pane.set_current_activity(next_activity); if self.suppressed_exec_calls.remove(&id) { return; } @@ -457,3 +464,17 @@ impl ChatWidget { } } } + +/// Formats the most-recently-started surviving command for tab status. +fn next_running_command_tab_detail( + running: &std::collections::HashMap, +) -> Option { + running.values().max_by_key(|rc| rc.start_order).map(|rc| { + crate::tab_status::format_parsed_command_for_tab_status(&rc.parsed_cmd) + .unwrap_or_else(|| crate::tab_status::format_command_for_tab_status(&rc.command)) + }) +} + +#[cfg(test)] +#[path = "command_lifecycle_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/chatwidget/command_lifecycle_tests.rs b/codex-rs/tui/src/chatwidget/command_lifecycle_tests.rs new file mode 100644 index 0000000000..6b7916664a --- /dev/null +++ b/codex-rs/tui/src/chatwidget/command_lifecycle_tests.rs @@ -0,0 +1,44 @@ +use std::collections::HashMap; + +use codex_app_server_protocol::CommandExecutionSource as ExecCommandSource; +use codex_protocol::parse_command::ParsedCommand; +use pretty_assertions::assert_eq; + +use super::RunningCommand; +use super::next_running_command_tab_detail; + +fn command(name: &str, start_order: u64) -> RunningCommand { + RunningCommand { + command: name.split_whitespace().map(ToString::to_string).collect(), + parsed_cmd: vec![ParsedCommand::Unknown { + cmd: name.to_string(), + }], + source: ExecCommandSource::Agent, + start_order, + } +} + +#[test] +fn picks_the_latest_command_then_falls_back_to_the_survivor() { + let mut running = HashMap::from([ + ( + "newer".to_string(), + command("cargo test", /*start_order*/ 2), + ), + ( + "older".to_string(), + command("cargo build", /*start_order*/ 1), + ), + ]); + + assert_eq!( + next_running_command_tab_detail(&running), + Some("Run cargo test".to_string()) + ); + running.remove("newer"); + + assert_eq!( + next_running_command_tab_detail(&running), + Some("Run cargo build".to_string()) + ); +} diff --git a/codex-rs/tui/src/chatwidget/constructor.rs b/codex-rs/tui/src/chatwidget/constructor.rs index 9b0f2daf0c..45f6aed38e 100644 --- a/codex-rs/tui/src/chatwidget/constructor.rs +++ b/codex-rs/tui/src/chatwidget/constructor.rs @@ -136,6 +136,7 @@ impl ChatWidget { clipboard_lease: None, copy_last_response_binding, running_commands: HashMap::new(), + running_command_seq: 0, collab_agent_metadata: HashMap::new(), pending_collab_spawn_requests: HashMap::new(), suppressed_exec_calls: HashSet::new(), diff --git a/codex-rs/tui/src/chatwidget/exec_state.rs b/codex-rs/tui/src/chatwidget/exec_state.rs index c8bb698cef..db0b5098ff 100644 --- a/codex-rs/tui/src/chatwidget/exec_state.rs +++ b/codex-rs/tui/src/chatwidget/exec_state.rs @@ -9,6 +9,8 @@ pub(super) struct RunningCommand { pub(super) command: Vec, pub(super) parsed_cmd: Vec, pub(super) source: ExecCommandSource, + /// Monotonic insertion order for deterministic tab-status selection. + pub(super) start_order: u64, } pub(super) struct UnifiedExecProcessSummary { diff --git a/codex-rs/tui/src/chatwidget/turn_runtime.rs b/codex-rs/tui/src/chatwidget/turn_runtime.rs index 3e413c82a4..5f4ec9a1d1 100644 --- a/codex-rs/tui/src/chatwidget/turn_runtime.rs +++ b/codex-rs/tui/src/chatwidget/turn_runtime.rs @@ -18,6 +18,12 @@ impl ChatWidget { self.refresh_status_surfaces(); } + /// Clears activity before switching to Idle so `last: ...` emits at once. + pub(super) fn finalize_turn_tab_status(&mut self) { + self.bottom_pane.set_current_activity(/*activity*/ None); + self.update_task_running_state(); + } + pub(super) fn collect_runtime_metrics_delta(&mut self) { if let Some(delta) = self.session_telemetry.runtime_metrics_summary() { self.apply_runtime_metrics_delta(delta); @@ -162,7 +168,7 @@ impl ChatWidget { self.input_queue.user_turn_pending_start = false; self.clear_active_hook_cell(); self.turn_lifecycle.finish(); - self.update_task_running_state(); + self.finalize_turn_tab_status(); self.running_commands.clear(); self.suppressed_exec_calls.clear(); self.last_unified_wait = None; @@ -305,7 +311,7 @@ impl ChatWidget { // Reset running state and clear streaming buffers. self.input_queue.user_turn_pending_start = false; self.turn_lifecycle.finish(); - self.update_task_running_state(); + self.finalize_turn_tab_status(); self.running_commands.clear(); self.suppressed_exec_calls.clear(); self.last_unified_wait = None; diff --git a/codex-rs/tui/src/osc_text.rs b/codex-rs/tui/src/osc_text.rs index 024d738d50..f573836df7 100644 --- a/codex-rs/tui/src/osc_text.rs +++ b/codex-rs/tui/src/osc_text.rs @@ -1,6 +1,24 @@ -//! Sanitization for untrusted terminal-title and tab-status OSC text. +//! Shared sanitization helpers for untrusted text placed inside OSC sequences. +//! +//! Several emitters (the terminal title in `terminal_title.rs`, the OSC 21337 +//! tab-status detail in `tab_status.rs`) assemble display strings from +//! untrusted sources such as model output, command argv, MCP server messages, +//! thread names, and project paths. Before that text goes into an OSC payload +//! it has to be stripped of two distinct hazards, and both modules need the +//! exact same rule so they cannot drift: +//! +//! - Control characters that could terminate or reshape the escape sequence +//! (BEL, ESC, the C1 controls, etc.). +//! - Bidi/invisible formatting codepoints that can visually reorder or hide +//! text (the family of issues described in the Trojan Source writeups). These +//! are not `char::is_control()`, so they have to be enumerated. -/// Whether a control or invisible formatting character is unsafe in OSC text. +/// Returns whether `ch` must be dropped from any OSC display payload. +/// +/// Covers both plain control characters and a curated set of invisible +/// formatting codepoints. The bidi entries cover the Trojan-Source-style +/// text-reordering controls that can make a string render misleadingly relative +/// to its underlying byte sequence. pub(crate) fn is_disallowed_osc_text_char(ch: char) -> bool { if ch.is_control() { return true; diff --git a/codex-rs/tui/src/osc_text_tests.rs b/codex-rs/tui/src/osc_text_tests.rs index fd67415180..a041fc52fa 100644 --- a/codex-rs/tui/src/osc_text_tests.rs +++ b/codex-rs/tui/src/osc_text_tests.rs @@ -15,7 +15,7 @@ fn rejects_controls_bidi_and_invisible_format_chars() { #[test] fn allows_ordinary_text() { - for ch in ['a', 'Z', '0', ' ', '/', '.', '\u{2026}', '\u{00E9}'] { + for ch in ['a', 'Z', '0', ' ', '/', '.', '…', 'é'] { assert!( !is_disallowed_osc_text_char(ch), "expected {ch:?} to be allowed" diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index 622c4f989e..11eb7a7e8b 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -140,12 +140,13 @@ impl StatusIndicatorWidget { .filter(|message| !message.is_empty()); } - #[cfg(test)] + /// The animated header: the dynamic verb (e.g. "Working", "Thinking"). pub(crate) fn header(&self) -> &str { &self.header } - #[cfg(test)] + /// The detail text shown under the header, if any (multi-line; the first + /// line is the best single-line summary). pub(crate) fn details(&self) -> Option<&str> { self.details.as_deref() } diff --git a/codex-rs/tui/src/tab_status.rs b/codex-rs/tui/src/tab_status.rs index 08b69f74eb..5c4daef772 100644 --- a/codex-rs/tui/src/tab_status.rs +++ b/codex-rs/tui/src/tab_status.rs @@ -1,13 +1,9 @@ //! OSC 21337 tab-status output helpers for the TUI. //! -//! This module owns the low-level write path: callers decide *when* the tab -//! status changes, this only knows how to write the sequence. The -//! `IsTerminal` gate matches the terminal-title module so non-TTY stdout -//! (e.g. piped `codex exec`) doesn't get escape sequences in captured output. -//! -//! OSC 21337 is an iTerm2 extension; other terminals ignore it. The payload -//! is a semicolon-separated list of `key=value` pairs. We emit `status` -//! (label), `indicator` (dot color), and `status-color` (text color). +//! Callers decide when the tab status changes; this module formats activity +//! detail and owns the low-level terminal write path. OSC 21337 values escape +//! `;` and `\`, and every field is emitted on every transition so iTerm clears +//! stale values. use std::fmt; use std::io; @@ -16,13 +12,13 @@ use std::io::stdout; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; +use codex_protocol::parse_command::ParsedCommand; use crossterm::Command; use ratatui::crossterm::execute; -/// Whether this process has ever written an OSC 21337 sequence. Gates the -/// shutdown-time `clear_tab_status` so that, when we never emitted, we don't -/// clobber a status set by another tool sharing the tab. static EMITTED: AtomicBool = AtomicBool::new(/*v*/ false); +const MAX_TAB_STATUS_DETAIL_CHARS: usize = 200; +const MAX_UNKNOWN_COMMAND_CHARS: usize = 80; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum TabStatus { @@ -40,8 +36,6 @@ impl TabStatus { } } - /// Dot color. Matches the palette other agent CLIs emit so multiple - /// tools read consistently in the tab bar. fn indicator(self) -> &'static str { match self { TabStatus::Working => "#ff9500", @@ -50,8 +44,6 @@ impl TabStatus { } } - /// Status-text color. Working/Waiting match the dot; Idle dims so the - /// colored dot stays the active affordance once codex is done. fn text_color(self) -> &'static str { match self { TabStatus::Working => "#ff9500", @@ -61,11 +53,70 @@ impl TabStatus { } } -pub(crate) fn set_tab_status(status: TabStatus) -> io::Result<()> { +pub(crate) fn format_command_for_tab_status(argv: &[String]) -> String { + crate::exec_command::strip_bash_lc_and_escape(argv) +} + +pub(crate) fn format_parsed_command_for_tab_status(parsed: &[ParsedCommand]) -> Option { + if parsed.is_empty() { + return None; + } + + if parsed + .iter() + .all(|command| matches!(command, ParsedCommand::Read { .. })) + { + let names = parsed + .iter() + .filter_map(|command| match command { + ParsedCommand::Read { name, .. } => Some(name.as_str()), + ParsedCommand::ListFiles { .. } + | ParsedCommand::Search { .. } + | ParsedCommand::Unknown { .. } => None, + }) + .collect::>(); + let (head, rest) = names.split_at(names.len().min(/*other*/ 3)); + let mut summary = format!("Read {}", head.join(", ")); + if !rest.is_empty() { + summary.push_str(", …"); + } + return Some(summary); + } + + match &parsed[0] { + ParsedCommand::Read { name, .. } => Some(format!("Read {name}")), + ParsedCommand::ListFiles { path, .. } => { + Some(format!("List {}", path.as_deref().unwrap_or("."))) + } + ParsedCommand::Search { query, path, cmd } => Some(match (query, path) { + (Some(query), Some(path)) => format!("Search \"{query}\" in {path}"), + (Some(query), None) => format!("Search \"{query}\""), + (None, Some(path)) => format!("Search in {path}"), + (None, None) => format!("Run {}", oneline_truncated(cmd)), + }), + ParsedCommand::Unknown { cmd } => Some(format!("Run {}", oneline_truncated(cmd))), + } +} + +fn oneline_truncated(value: &str) -> String { + let collapsed = value.split_whitespace().collect::>().join(" "); + if collapsed.chars().count() <= MAX_UNKNOWN_COMMAND_CHARS { + return collapsed; + } + format!( + "{}…", + collapsed + .chars() + .take(MAX_UNKNOWN_COMMAND_CHARS) + .collect::() + ) +} + +pub(crate) fn set_tab_status(status: TabStatus, detail: Option<&str>) -> io::Result<()> { if !stdout().is_terminal() { return Ok(()); } - execute!(stdout(), SetTabStatus(status))?; + execute!(stdout(), SetTabStatus(status, detail.map(sanitize_detail)))?; EMITTED.store(/*val*/ true, Ordering::Relaxed); Ok(()) } @@ -79,17 +130,58 @@ pub(crate) fn clear_tab_status() -> io::Result<()> { Ok(()) } -#[derive(Debug, Clone, Copy)] -struct SetTabStatus(TabStatus); +fn sanitize_detail(detail: &str) -> String { + let mut out = String::with_capacity(detail.len()); + let mut chars_written = 0; + let mut pending_space = false; + let mut truncated = false; + + for ch in detail.chars() { + if ch.is_whitespace() { + pending_space = !out.is_empty(); + continue; + } + if crate::osc_text::is_disallowed_osc_text_char(ch) { + continue; + } + if pending_space { + if chars_written >= MAX_TAB_STATUS_DETAIL_CHARS { + truncated = true; + break; + } + out.push(' '); + chars_written += 1; + pending_space = false; + } + if chars_written >= MAX_TAB_STATUS_DETAIL_CHARS { + truncated = true; + break; + } + if matches!(ch, ';' | '\\') { + out.push('\\'); + } + out.push(ch); + chars_written += 1; + } + if truncated { + out.push('…'); + } + out +} + +#[derive(Debug, Clone)] +struct SetTabStatus(TabStatus, Option); impl Command for SetTabStatus { fn write_ansi(&self, f: &mut impl fmt::Write) -> fmt::Result { + let detail = self.1.as_deref().unwrap_or(""); write!( f, - "\x1b]21337;status={};indicator={};status-color={}\x07", + "\x1b]21337;status={};indicator={};status-color={};detail={}\x07", self.0.label(), self.0.indicator(), - self.0.text_color() + self.0.text_color(), + detail ) } @@ -111,9 +203,7 @@ struct ClearTabStatus; impl Command for ClearTabStatus { fn write_ansi(&self, f: &mut impl fmt::Write) -> fmt::Result { - // Explicit empty values for every managed field so iTerm clears - // them rather than leaving stale content visible. - write!(f, "\x1b]21337;status=;indicator=;status-color=\x07") + write!(f, "\x1b]21337;status=;indicator=;status-color=;detail=\x07") } #[cfg(windows)] diff --git a/codex-rs/tui/src/tab_status_tests.rs b/codex-rs/tui/src/tab_status_tests.rs index 6081ad7e22..35afa4cdb2 100644 --- a/codex-rs/tui/src/tab_status_tests.rs +++ b/codex-rs/tui/src/tab_status_tests.rs @@ -1,49 +1,107 @@ +use std::path::PathBuf; + +use codex_protocol::parse_command::ParsedCommand; use crossterm::Command; use pretty_assertions::assert_eq; use super::ClearTabStatus; +use super::MAX_TAB_STATUS_DETAIL_CHARS; use super::SetTabStatus; use super::TabStatus; +use super::format_command_for_tab_status; +use super::format_parsed_command_for_tab_status; +use super::sanitize_detail; #[test] -fn working_emits_orange_with_matching_text_color() { - let mut out = String::new(); - SetTabStatus(TabStatus::Working) - .write_ansi(&mut out) +fn status_sequences_include_detail_and_clear_stale_fields() { + let mut working = String::new(); + SetTabStatus(TabStatus::Working, Some("exec cargo build".to_string())) + .write_ansi(&mut working) .expect("encode tab status"); assert_eq!( - out, - "\x1b]21337;status=Working;indicator=#ff9500;status-color=#ff9500\x07" + working, + "\x1b]21337;status=Working;indicator=#ff9500;status-color=#ff9500;detail=exec cargo build\x07" + ); + + let mut idle = String::new(); + SetTabStatus(TabStatus::Idle, /*detail*/ None) + .write_ansi(&mut idle) + .expect("encode tab status"); + assert_eq!( + idle, + "\x1b]21337;status=Idle;indicator=#00d75f;status-color=#888888;detail=\x07" + ); + + let mut clear = String::new(); + ClearTabStatus.write_ansi(&mut clear).expect("encode clear"); + assert_eq!( + clear, + "\x1b]21337;status=;indicator=;status-color=;detail=\x07" ); } #[test] -fn waiting_emits_blue_with_matching_text_color() { - let mut out = String::new(); - SetTabStatus(TabStatus::Waiting) - .write_ansi(&mut out) - .expect("encode tab status"); +fn detail_is_safe_bounded_osc_text() { + assert_eq!(sanitize_detail(" a\\;b\t\u{202E}c\x1b "), "a\\\\\\;b c"); + + let sanitized = sanitize_detail(&";".repeat(MAX_TAB_STATUS_DETAIL_CHARS + 1)); assert_eq!( - out, - "\x1b]21337;status=Waiting;indicator=#5f87ff;status-color=#5f87ff\x07" + sanitized.chars().count(), + MAX_TAB_STATUS_DETAIL_CHARS * 2 + 1 + ); + assert!(sanitized.ends_with('…')); +} + +#[test] +fn command_detail_strips_shell_wrapper() { + assert_eq!( + format_command_for_tab_status(&[ + "/opt/homebrew/bin/zsh".into(), + "-lc".into(), + "touch".into(), + "/tmp/foo".into(), + ]), + "touch /tmp/foo" ); } #[test] -fn idle_uses_dim_text_color() { - let mut out = String::new(); - SetTabStatus(TabStatus::Idle) - .write_ansi(&mut out) - .expect("encode tab status"); +fn parsed_detail_combines_reads_and_caps_the_list() { + let parsed = ["a.rs", "b.rs", "c.rs", "d.rs"].map(|name| ParsedCommand::Read { + cmd: format!("cat {name}"), + name: name.to_string(), + path: PathBuf::from(name), + }); assert_eq!( - out, - "\x1b]21337;status=Idle;indicator=#00d75f;status-color=#888888\x07" + format_parsed_command_for_tab_status(&parsed), + Some("Read a.rs, b.rs, c.rs, …".to_string()) ); } #[test] -fn clear_emits_empty_fields() { - let mut out = String::new(); - ClearTabStatus.write_ansi(&mut out).expect("encode clear"); - assert_eq!(out, "\x1b]21337;status=;indicator=;status-color=\x07"); +fn parsed_detail_uses_first_mixed_command_and_quotes_search() { + let parsed = [ + ParsedCommand::Search { + cmd: "rg needle src".into(), + query: Some("needle".into()), + path: Some("src".into()), + }, + ParsedCommand::Unknown { + cmd: "ignored".into(), + }, + ]; + assert_eq!( + format_parsed_command_for_tab_status(&parsed), + Some("Search \"needle\" in src".to_string()) + ); +} + +#[test] +fn unknown_command_detail_collapses_and_truncates() { + let command = format!(" echo\n{} ", "x".repeat(/*n*/ 100)); + let summary = format_parsed_command_for_tab_status(&[ParsedCommand::Unknown { cmd: command }]) + .expect("unknown command produces a summary"); + assert_eq!(summary.chars().count(), "Run ".chars().count() + 81); + assert!(summary.starts_with("Run echo ")); + assert!(summary.ends_with('…')); }