From 96d238d0b95096c3dce32b31ebaef4650bcae1fc Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Thu, 8 Oct 2026 18:40:09 +0000 Subject: [PATCH] Include foreground activity in iTerm2 session status (#52226) ## Why The iTerm2 session status reports `Working` without describing the current activity. ## What changed Populate the OSC 21337 `detail` field from the TUI status header during foreground work, for example `Reading source`. Leave it empty for idle, waiting, background-only work, and the generic `Working` header. Reuse terminal-title text filtering and the 240-character limit, then escape semicolons and backslashes for the status payload. Include detail in the emission cache so activity changes update the terminal even when the lifecycle state stays the same. ## Testing Add coverage for foreground-only details, payload encoding, text filtering and truncation, and cache updates when the detail changes. GitOrigin-RevId: b27a287b80e0db52be6cd9d5d246f03897e2996b --- codex-rs/tui/src/chatwidget.rs | 4 +- .../tui/src/chatwidget/status_surfaces.rs | 6 ++ .../chatwidget/tests/iterm_session_status.rs | 27 ++++++++ codex-rs/tui/src/iterm_session_status.rs | 69 ++++++++++++++----- .../tui/src/iterm_session_status_tests.rs | 41 ++++++++--- ...s__tests__iterm_session_status_fields.snap | 4 +- codex-rs/tui/src/terminal_title.rs | 4 +- 7 files changed, 121 insertions(+), 34 deletions(-) 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;