mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
feat(tui): show tab activity detail
Co-authored-by: George Nachman <gnachman@gmail.com>
This commit is contained in:
@@ -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<String> {
|
||||
self.is_tool_suggestion()
|
||||
.then(|| "Tool suggestion".to_string())
|
||||
}
|
||||
}
|
||||
|
||||
impl crate::render::renderable::Renderable for AppLinkView {
|
||||
|
||||
@@ -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<String> {
|
||||
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<String> {
|
||||
// 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 {
|
||||
|
||||
@@ -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<String> {
|
||||
None
|
||||
}
|
||||
|
||||
/// Return the next time-based redraw this view needs while it is active.
|
||||
fn next_frame_delay(&self) -> Option<std::time::Duration> {
|
||||
None
|
||||
|
||||
@@ -1646,6 +1646,10 @@ impl BottomPaneView for McpServerElicitationOverlay {
|
||||
true
|
||||
}
|
||||
|
||||
fn tab_status_detail(&self) -> Option<String> {
|
||||
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()
|
||||
{
|
||||
|
||||
@@ -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<String>) {
|
||||
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<String>) {
|
||||
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<String> {
|
||||
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<String>, 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<Vec<SkillMetadata>>) {
|
||||
@@ -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<TabStatus> {
|
||||
pub(crate) fn last_tab_status_for_test(&self) -> Option<(TabStatus, Option<String>)> {
|
||||
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<String>)> {
|
||||
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::<AppEvent>();
|
||||
|
||||
@@ -1295,6 +1295,11 @@ impl BottomPaneView for RequestUserInputOverlay {
|
||||
true
|
||||
}
|
||||
|
||||
fn tab_status_detail(&self) -> Option<String> {
|
||||
// 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();
|
||||
|
||||
@@ -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<TabStatus>,
|
||||
last_status: Option<(TabStatus, Option<String>)>,
|
||||
last_emit_at: Option<Instant>,
|
||||
current_activity: Option<String>,
|
||||
last_activity: Option<String>,
|
||||
}
|
||||
|
||||
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<String>) -> 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<Duration> {
|
||||
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<String>), 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<TabStatus> {
|
||||
self.last_status
|
||||
pub(super) fn last_status(&self) -> Option<(TabStatus, Option<String>)> {
|
||||
self.last_status.clone()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "tab_status_state_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
61
codex-rs/tui/src/bottom_pane/tab_status_state_tests.rs
Normal file
61
codex-rs/tui/src/bottom_pane/tab_status_state_tests.rs
Normal file
@@ -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))
|
||||
);
|
||||
}
|
||||
@@ -565,6 +565,8 @@ pub(crate) struct ChatWidget {
|
||||
clipboard_lease: Option<crate::clipboard_copy::ClipboardLease>,
|
||||
copy_last_response_binding: Vec<KeyBinding>,
|
||||
running_commands: HashMap<String, RunningCommand>,
|
||||
/// Orders concurrent commands for deterministic tab-status detail.
|
||||
running_command_seq: u64,
|
||||
collab_agent_metadata: HashMap<ThreadId, AgentMetadata>,
|
||||
pending_collab_spawn_requests: HashMap<String, multi_agents::SpawnRequestSummary>,
|
||||
suppressed_exec_calls: HashSet<String>,
|
||||
|
||||
@@ -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<String, RunningCommand>,
|
||||
) -> Option<String> {
|
||||
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;
|
||||
|
||||
44
codex-rs/tui/src/chatwidget/command_lifecycle_tests.rs
Normal file
44
codex-rs/tui/src/chatwidget/command_lifecycle_tests.rs
Normal file
@@ -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())
|
||||
);
|
||||
}
|
||||
@@ -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(),
|
||||
|
||||
@@ -9,6 +9,8 @@ pub(super) struct RunningCommand {
|
||||
pub(super) command: Vec<String>,
|
||||
pub(super) parsed_cmd: Vec<ParsedCommand>,
|
||||
pub(super) source: ExecCommandSource,
|
||||
/// Monotonic insertion order for deterministic tab-status selection.
|
||||
pub(super) start_order: u64,
|
||||
}
|
||||
|
||||
pub(super) struct UnifiedExecProcessSummary {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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<String> {
|
||||
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::<Vec<_>>();
|
||||
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::<Vec<_>>().join(" ");
|
||||
if collapsed.chars().count() <= MAX_UNKNOWN_COMMAND_CHARS {
|
||||
return collapsed;
|
||||
}
|
||||
format!(
|
||||
"{}…",
|
||||
collapsed
|
||||
.chars()
|
||||
.take(MAX_UNKNOWN_COMMAND_CHARS)
|
||||
.collect::<String>()
|
||||
)
|
||||
}
|
||||
|
||||
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<String>);
|
||||
|
||||
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)]
|
||||
|
||||
@@ -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('…'));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user