mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
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
This commit is contained in:
@@ -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");
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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
|
||||
-
|
||||
-
|
||||
-
|
||||
");
|
||||
}
|
||||
|
||||
@@ -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<bool> = OnceLock::new();
|
||||
|
||||
struct SessionStatusState {
|
||||
last_emitted: Mutex<u8>,
|
||||
last_emitted: Mutex<EmittedSessionStatus>,
|
||||
}
|
||||
|
||||
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,
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
+2
-2
@@ -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}
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user