diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 994f1a75ef..8c77689ba2 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1185,8 +1185,10 @@ impl ChatWidget { } pub(crate) fn refresh_iterm_session_status(&self) { + let status = self.desired_iterm_session_status(); if let Err(err) = crate::iterm_session_status::set_iterm_session_status( - self.desired_iterm_session_status(), + status, + self.iterm_session_detail(status), ) { tracing::debug!(error = %err, "failed to set iTerm2 session status"); } diff --git a/codex-rs/tui/src/chatwidget/status_surfaces.rs b/codex-rs/tui/src/chatwidget/status_surfaces.rs index eb9022d30a..7eef820958 100644 --- a/codex-rs/tui/src/chatwidget/status_surfaces.rs +++ b/codex-rs/tui/src/chatwidget/status_surfaces.rs @@ -332,6 +332,12 @@ impl ChatWidget { } } + pub(super) fn iterm_session_detail(&self, status: ItermSessionStatus) -> Option<&str> { + (status == ItermSessionStatus::Working && self.bottom_pane.is_task_running()) + .then_some(self.status_state.current_status.header.as_str()) + .filter(|detail| *detail != "Working") + } + pub(super) fn terminal_title_shows_action_required(&self) -> bool { self.terminal_title_requires_action() && self.terminal_title_uses_activity() } diff --git a/codex-rs/tui/src/chatwidget/tests/iterm_session_status.rs b/codex-rs/tui/src/chatwidget/tests/iterm_session_status.rs index 90d8ff1a07..d63e1e4fcf 100644 --- a/codex-rs/tui/src/chatwidget/tests/iterm_session_status.rs +++ b/codex-rs/tui/src/chatwidget/tests/iterm_session_status.rs @@ -54,3 +54,30 @@ async fn session_status_follows_existing_lifecycle_state() { [Idle, Working, Waiting, Working, Idle, Working, Idle] ); } + +#[tokio::test] +async fn session_status_detail_uses_only_foreground_activity() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let detail = + |chat: &ChatWidget, status| chat.iterm_session_detail(status).map(ToString::to_string); + + chat.bottom_pane.set_task_running(/*running*/ true); + let generic = detail(&chat, Working); + chat.set_status_header("Reading source".to_string()); + let working = detail(&chat, Working); + let waiting = detail(&chat, Waiting); + chat.bottom_pane.set_task_running(/*running*/ false); + let idle = detail(&chat, Idle); + let background = detail(&chat, Working); + + let details = [generic, working, waiting, idle, background] + .map(|detail| detail.unwrap_or_else(|| "-".to_string())) + .join("\n"); + insta::assert_snapshot!(details, @r" + - + Reading source + - + - + - + "); +} diff --git a/codex-rs/tui/src/iterm_session_status.rs b/codex-rs/tui/src/iterm_session_status.rs index f1a1005492..73b23dc036 100644 --- a/codex-rs/tui/src/iterm_session_status.rs +++ b/codex-rs/tui/src/iterm_session_status.rs @@ -1,9 +1,9 @@ //! Emits coarse Codex lifecycle state through iTerm2's OSC 21337 protocol. //! //! This first integration is intentionally limited to direct iTerm2 sessions. -//! The payload uses fixed strings, so it needs no dynamic text escaping or -//! throttling. A process-wide cache matches the terminal session's ownership -//! and prevents stale per-thread state when the active chat changes. +//! The payload includes bounded activity text from the existing TUI status. +//! A process-wide cache matches the terminal session's ownership and prevents +//! stale state when the active chat changes. use std::fmt; use std::io; @@ -24,29 +24,42 @@ static SESSION_STATUS_STATE: SessionStatusState = SessionStatusState::new(); static ITERM_SESSION_STATUS_SUPPORTED: OnceLock = OnceLock::new(); struct SessionStatusState { - last_emitted: Mutex, + last_emitted: Mutex, +} + +struct EmittedSessionStatus { + status: u8, + detail: String, } impl SessionStatusState { const fn new() -> Self { Self { - last_emitted: Mutex::new(NO_EMITTED_STATUS), + last_emitted: Mutex::new(EmittedSessionStatus { + status: NO_EMITTED_STATUS, + detail: String::new(), + }), } } - fn update(&self, status: u8, emit: impl FnOnce() -> io::Result<()>) -> io::Result<()> { + fn update( + &self, + status: u8, + detail: &str, + emit: impl FnOnce() -> io::Result<()>, + ) -> io::Result<()> { // Keep the terminal write and cache mutation together so panic-hook cleanup cannot // interleave with a draw from another thread. let mut last_emitted = self .last_emitted .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - if *last_emitted == status { + if last_emitted.status == status && last_emitted.detail == detail { return Ok(()); } - emit()?; - *last_emitted = status; + last_emitted.status = status; + detail.clone_into(&mut last_emitted.detail); Ok(()) } @@ -55,8 +68,8 @@ impl SessionStatusState { .last_emitted .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - if *last_emitted != NO_EMITTED_STATUS { - *last_emitted = INVALIDATED_EMITTED_STATUS; + if last_emitted.status != NO_EMITTED_STATUS { + last_emitted.status = INVALIDATED_EMITTED_STATUS; } } } @@ -87,7 +100,10 @@ impl ItermSessionStatus { } } -pub(crate) fn set_iterm_session_status(status: ItermSessionStatus) -> io::Result<()> { +pub(crate) fn set_iterm_session_status( + status: ItermSessionStatus, + detail: Option<&str>, +) -> io::Result<()> { if cfg!(test) { return Ok(()); } @@ -102,13 +118,14 @@ pub(crate) fn set_iterm_session_status(status: ItermSessionStatus) -> io::Result return Ok(()); } - SESSION_STATUS_STATE.update(status as u8, || { - execute!(stdout(), SetItermSessionStatus(status)) + let detail = sanitize_iterm_session_detail(detail.unwrap_or_default()); + SESSION_STATUS_STATE.update(status as u8, &detail, || { + execute!(stdout(), SetItermSessionStatus(status, &detail)) }) } pub(crate) fn clear_iterm_session_status() -> io::Result<()> { - SESSION_STATUS_STATE.update(NO_EMITTED_STATUS, || { + SESSION_STATUS_STATE.update(NO_EMITTED_STATUS, "", || { execute!(stdout(), ClearItermSessionStatus) }) } @@ -121,6 +138,21 @@ pub(crate) fn invalidate_iterm_session_status() { SESSION_STATUS_STATE.invalidate(); } +fn sanitize_iterm_session_detail(detail: &str) -> String { + let detail = crate::terminal_title::sanitize_terminal_title( + detail, + crate::terminal_title::TitleEncoding::Unicode, + ); + let mut escaped = String::with_capacity(detail.len()); + for ch in detail.chars() { + if matches!(ch, ';' | '\\') { + escaped.push('\\'); + } + escaped.push(ch); + } + escaped +} + fn iterm_session_status_supported( terminal: &TerminalInfo, stdout_is_terminal: bool, @@ -133,15 +165,16 @@ fn iterm_session_status_supported( } #[derive(Clone, Copy, Debug)] -struct SetItermSessionStatus(ItermSessionStatus); +struct SetItermSessionStatus<'a>(ItermSessionStatus, &'a str); -impl Command for SetItermSessionStatus { +impl Command for SetItermSessionStatus<'_> { fn write_ansi(&self, f: &mut impl fmt::Write) -> fmt::Result { write!( f, - "\x1b]21337;status={};indicator={};status-color=;detail=\x07", + "\x1b]21337;status={};indicator={};status-color=;detail={}\x07", self.0.label(), self.0.indicator(), + self.1, ) } diff --git a/codex-rs/tui/src/iterm_session_status_tests.rs b/codex-rs/tui/src/iterm_session_status_tests.rs index 770e5bd769..ac1e729d70 100644 --- a/codex-rs/tui/src/iterm_session_status_tests.rs +++ b/codex-rs/tui/src/iterm_session_status_tests.rs @@ -12,17 +12,18 @@ use super::NO_EMITTED_STATUS; use super::SessionStatusState; use super::SetItermSessionStatus; use super::iterm_session_status_supported; +use super::sanitize_iterm_session_detail; #[test] fn encodes_session_status_fields() { let mut encoded = Vec::new(); - for status in [ - ItermSessionStatus::Idle, - ItermSessionStatus::Working, - ItermSessionStatus::Waiting, + for (status, detail) in [ + (ItermSessionStatus::Idle, ""), + (ItermSessionStatus::Working, "Reading source"), + (ItermSessionStatus::Waiting, ""), ] { let mut value = String::new(); - SetItermSessionStatus(status) + SetItermSessionStatus(status, detail) .write_ansi(&mut value) .expect("encode iTerm2 session status"); encoded.push(value); @@ -36,6 +37,18 @@ fn encodes_session_status_fields() { insta::assert_snapshot!("iterm_session_status_fields", snapshot); } +#[test] +fn detail_is_safe_bounded_osc_text() { + assert_eq!( + sanitize_iterm_session_detail(" Read a\\b; c\n\u{202e}\x1b "), + "Read a\\\\b\\; c" + ); + assert_eq!( + sanitize_iterm_session_detail(&"x".repeat(/*n*/ 300)), + "x".repeat(/*n*/ 240) + ); +} + #[test] fn clear_empties_every_owned_field() { let mut encoded = String::new(); @@ -53,34 +66,39 @@ fn clear_empties_every_owned_field() { fn cache_changes_only_after_a_successful_write() { let state = SessionStatusState::new(); let mut emitted = Vec::new(); - state - .update(ItermSessionStatus::Working as u8, || { + .update(ItermSessionStatus::Working as u8, "Thinking", || { Err(io::Error::other("write failed")) }) .expect_err("failed write should be returned"); state - .update(ItermSessionStatus::Working as u8, || { + .update(ItermSessionStatus::Working as u8, "Thinking", || { emitted.push(ItermSessionStatus::Working as u8); Ok(()) }) .expect("retry status write"); state - .update(ItermSessionStatus::Working as u8, || { + .update(ItermSessionStatus::Working as u8, "Thinking", || { emitted.push(ItermSessionStatus::Waiting as u8); Ok(()) }) .expect("deduplicate status write"); + state + .update(ItermSessionStatus::Working as u8, "Reading", || { + emitted.push(ItermSessionStatus::Working as u8); + Ok(()) + }) + .expect("emit changed detail"); state.invalidate(); state - .update(ItermSessionStatus::Working as u8, || { + .update(ItermSessionStatus::Working as u8, "Thinking", || { emitted.push(ItermSessionStatus::Working as u8); Ok(()) }) .expect("re-emit status after terminal handoff"); state - .update(NO_EMITTED_STATUS, || { + .update(NO_EMITTED_STATUS, "", || { emitted.push(NO_EMITTED_STATUS); Ok(()) }) @@ -89,6 +107,7 @@ fn cache_changes_only_after_a_successful_write() { assert_eq!( emitted, [ + ItermSessionStatus::Working as u8, ItermSessionStatus::Working as u8, ItermSessionStatus::Working as u8, NO_EMITTED_STATUS, diff --git a/codex-rs/tui/src/snapshots/codex_tui__iterm_session_status__tests__iterm_session_status_fields.snap b/codex-rs/tui/src/snapshots/codex_tui__iterm_session_status__tests__iterm_session_status_fields.snap index b6dfa21dd2..9a5cd475f3 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__iterm_session_status__tests__iterm_session_status_fields.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__iterm_session_status__tests__iterm_session_status_fields.snap @@ -1,8 +1,8 @@ --- source: tui/src/iterm_session_status_tests.rs -assertion_line: 36 +assertion_line: 39 expression: snapshot --- \u{1b}]21337;status=Idle;indicator=#00d75f;status-color=;detail=\u{7} -\u{1b}]21337;status=Working;indicator=#ff9500;status-color=;detail=\u{7} +\u{1b}]21337;status=Working;indicator=#ff9500;status-color=;detail=Reading source\u{7} \u{1b}]21337;status=Waiting;indicator=#5f87ff;status-color=;detail=\u{7} diff --git a/codex-rs/tui/src/terminal_title.rs b/codex-rs/tui/src/terminal_title.rs index ff2195ee50..37a47b2e1e 100644 --- a/codex-rs/tui/src/terminal_title.rs +++ b/codex-rs/tui/src/terminal_title.rs @@ -36,7 +36,7 @@ use ratatui::crossterm::execute; const MAX_TERMINAL_TITLE_CHARS: usize = 240; #[derive(Clone, Copy)] -enum TitleEncoding { +pub(crate) enum TitleEncoding { Unicode, Screen, } @@ -126,7 +126,7 @@ impl Command for SetWindowTitle { /// This removes terminal control characters, strips invisible/bidi formatting /// characters, collapses any whitespace run into a single ASCII space, and /// truncates after [`MAX_TERMINAL_TITLE_CHARS`] emitted characters. -fn sanitize_terminal_title(title: &str, encoding: TitleEncoding) -> String { +pub(crate) fn sanitize_terminal_title(title: &str, encoding: TitleEncoding) -> String { let mut sanitized = String::new(); let mut chars_written = 0; let mut pending_space = false;