From a53510164769cfef262cda2bce0a1581c03cd64b Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Fri, 5 Jun 2026 15:22:49 -0300 Subject: [PATCH] fix(tui): bound cached tab activity --- .../tui/src/bottom_pane/tab_status_state.rs | 4 ++- .../src/bottom_pane/tab_status_state_tests.rs | 16 ++++++++++++ .../tui/src/chatwidget/command_lifecycle.rs | 26 +++++++++---------- codex-rs/tui/src/tab_status.rs | 6 ++--- codex-rs/tui/src/tab_status_tests.rs | 4 +-- 5 files changed, 37 insertions(+), 19 deletions(-) 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 986b494ffd..0de190c11e 100644 --- a/codex-rs/tui/src/bottom_pane/tab_status_state.rs +++ b/codex-rs/tui/src/bottom_pane/tab_status_state.rs @@ -2,6 +2,7 @@ use std::time::Duration; use std::time::Instant; use crate::tab_status::TabStatus; +use crate::tab_status::sanitize_detail; use crate::tab_status::set_tab_status; const MIN_DETAIL_INTERVAL: Duration = Duration::from_millis(/*millis*/ 250); @@ -79,10 +80,11 @@ impl TabStatusState { if !self.enabled { return; } + let desired = (desired.0, desired.1.as_deref().map(sanitize_detail)); if self.last_status.as_ref() == Some(&desired) { return; } - if let Err(err) = set_tab_status(desired.0, desired.1.as_deref()) { + if let Err(err) = set_tab_status(desired.0, desired.1.clone()) { tracing::debug!(error = %err, "failed to set tab status"); return; } 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 index f9a9248722..56bfbbbf5f 100644 --- a/codex-rs/tui/src/bottom_pane/tab_status_state_tests.rs +++ b/codex-rs/tui/src/bottom_pane/tab_status_state_tests.rs @@ -51,3 +51,19 @@ fn throttle_defers_detail_changes_but_not_status_changes() { None ); } + +#[test] +fn caches_only_bounded_sanitized_detail() { + let mut state = TabStatusState::new(); + state.refresh( + (TabStatus::Working, Some("x".repeat(/*n*/ 1_000))), + Instant::now(), + ); + assert_eq!( + state.last_status(), + Some(( + TabStatus::Working, + Some(format!("{}…", "x".repeat(/*n*/ 200))), + )) + ); +} diff --git a/codex-rs/tui/src/chatwidget/command_lifecycle.rs b/codex-rs/tui/src/chatwidget/command_lifecycle.rs index 36e5ca9341..0755561c4f 100644 --- a/codex-rs/tui/src/chatwidget/command_lifecycle.rs +++ b/codex-rs/tui/src/chatwidget/command_lifecycle.rs @@ -256,21 +256,8 @@ 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 @@ -285,6 +272,19 @@ impl ChatWidget { self.suppressed_exec_calls.insert(id); return; } + self.running_command_seq = self.running_command_seq.wrapping_add(/*rhs*/ 1); + self.running_commands.insert( + id.clone(), + RunningCommand { + command: command.clone(), + parsed_cmd: parsed_cmd.clone(), + source, + start_order: self.running_command_seq, + }, + ); + 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)); if let Some(cell) = self .transcript .active_cell diff --git a/codex-rs/tui/src/tab_status.rs b/codex-rs/tui/src/tab_status.rs index a26fac7819..a6ec5bc7e7 100644 --- a/codex-rs/tui/src/tab_status.rs +++ b/codex-rs/tui/src/tab_status.rs @@ -128,11 +128,11 @@ pub(crate) fn oneline_truncated(value: &str) -> String { out } -pub(crate) fn set_tab_status(status: TabStatus, detail: Option<&str>) -> io::Result<()> { +pub(crate) fn set_tab_status(status: TabStatus, detail: Option) -> io::Result<()> { if !stdout().is_terminal() { return Ok(()); } - execute!(stdout(), SetTabStatus(status, detail.map(sanitize_detail)))?; + execute!(stdout(), SetTabStatus(status, detail))?; EMITTED.store(/*val*/ true, Ordering::Relaxed); Ok(()) } @@ -146,7 +146,7 @@ pub(crate) fn clear_tab_status() -> io::Result<()> { Ok(()) } -fn sanitize_detail(detail: &str) -> String { +pub(crate) fn sanitize_detail(detail: &str) -> String { let mut out = String::with_capacity(detail.len().min(MAX_TAB_STATUS_DETAIL_CHARS * 2 + 1)); let mut chars_written = 0; let mut pending_space = false; diff --git a/codex-rs/tui/src/tab_status_tests.rs b/codex-rs/tui/src/tab_status_tests.rs index 17343391b8..9984efd256 100644 --- a/codex-rs/tui/src/tab_status_tests.rs +++ b/codex-rs/tui/src/tab_status_tests.rs @@ -19,9 +19,9 @@ fn status_sequences_include_detail_and_clear_stale_fields() { SetTabStatus(TabStatus::Working, Some("exec cargo build".to_string())) .write_ansi(&mut working) .expect("encode tab status"); - assert_eq!( + insta::assert_snapshot!( working, - "\x1b]21337;status=Working;indicator=#ff9500;status-color=#ff9500;detail=exec cargo build\x07" + @"\u{1b}]21337;status=Working;indicator=#ff9500;status-color=#ff9500;detail=exec cargo build\u{7}" ); let mut idle = String::new();