mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
fix(tui): cap elicitation tab detail
This commit is contained in:
@@ -110,8 +110,6 @@ impl ApprovalRequest {
|
||||
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)
|
||||
@@ -135,7 +133,10 @@ impl ApprovalRequest {
|
||||
server_name,
|
||||
message,
|
||||
..
|
||||
} => Some(format!("Elicitation from {server_name}: {message}")),
|
||||
} => Some(format!(
|
||||
"Elicitation from {server_name}: {}",
|
||||
crate::tab_status::oneline_truncated(message)
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -788,7 +788,6 @@ impl BottomPane {
|
||||
self.composer.sync_popups();
|
||||
self.maybe_show_delayed_approval_requests_at(now);
|
||||
self.schedule_active_view_frame();
|
||||
// Poll every frame to catch view-stack transitions and changing detail.
|
||||
self.refresh_tab_status_at(now);
|
||||
}
|
||||
|
||||
@@ -1014,7 +1013,6 @@ impl BottomPane {
|
||||
self.status.is_some()
|
||||
}
|
||||
|
||||
/// 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, Option<String>)> {
|
||||
self.tab_status.last_status()
|
||||
@@ -2013,6 +2011,10 @@ mod tests {
|
||||
Some((status, None))
|
||||
}
|
||||
|
||||
fn last_tab_class(pane: &BottomPane) -> Option<TabStatus> {
|
||||
pane.last_tab_status_for_test().map(|(status, _)| status)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tab_status_disabled_suppresses_emission() {
|
||||
// Refresh must honor the `tui.tab_status = false` opt-out: nothing
|
||||
@@ -2035,22 +2037,13 @@ mod tests {
|
||||
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().map(|(status, _)| status),
|
||||
Some(TabStatus::Working)
|
||||
);
|
||||
assert_eq!(last_tab_class(&pane), Some(TabStatus::Working));
|
||||
|
||||
pane.push_approval_request(exec_request(), &features);
|
||||
assert_eq!(
|
||||
pane.last_tab_status_for_test().map(|(status, _)| status),
|
||||
Some(TabStatus::Waiting)
|
||||
);
|
||||
assert_eq!(last_tab_class(&pane), Some(TabStatus::Waiting));
|
||||
|
||||
let _ = pane.on_ctrl_c();
|
||||
assert_eq!(
|
||||
pane.last_tab_status_for_test().map(|(status, _)| status),
|
||||
Some(TabStatus::Working)
|
||||
);
|
||||
assert_eq!(last_tab_class(&pane), Some(TabStatus::Working));
|
||||
|
||||
pane.set_task_running(/*running*/ false);
|
||||
assert_eq!(pane.last_tab_status_for_test(), tab_status(TabStatus::Idle));
|
||||
@@ -2115,18 +2108,12 @@ 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().map(|(status, _)| status),
|
||||
Some(TabStatus::Waiting)
|
||||
);
|
||||
assert_eq!(last_tab_class(&pane), 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().map(|(status, _)| status),
|
||||
Some(TabStatus::Working)
|
||||
);
|
||||
assert_eq!(last_tab_class(&pane), Some(TabStatus::Working));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -2142,23 +2129,20 @@ mod tests {
|
||||
pane.show_view(Box::new(DismissibleView::default()));
|
||||
pane.set_task_running(/*running*/ true);
|
||||
assert_eq!(
|
||||
pane.last_tab_status_for_test().map(|(status, _)| status),
|
||||
last_tab_class(&pane),
|
||||
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().map(|(status, _)| status),
|
||||
Some(TabStatus::Waiting)
|
||||
);
|
||||
assert_eq!(last_tab_class(&pane), 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().map(|(status, _)| status),
|
||||
last_tab_class(&pane),
|
||||
Some(TabStatus::Working),
|
||||
"pre_draw_tick must drop the tab out of Waiting once the \
|
||||
top of the stack is no longer action-required",
|
||||
|
||||
@@ -1296,7 +1296,6 @@ impl BottomPaneView for RequestUserInputOverlay {
|
||||
}
|
||||
|
||||
fn tab_status_detail(&self) -> Option<String> {
|
||||
// No concise per-request summary available; use a generic label.
|
||||
Some("Tool input request".to_string())
|
||||
}
|
||||
|
||||
|
||||
@@ -24,7 +24,8 @@ fn activity_rolls_forward_and_resets_for_a_new_turn() {
|
||||
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);
|
||||
let desired = (TabStatus::Working, Some("first".to_string()));
|
||||
state.refresh(desired.clone(), started_at);
|
||||
|
||||
assert_eq!(
|
||||
state.refresh_delay(
|
||||
@@ -40,17 +41,8 @@ fn throttle_defers_detail_changes_but_not_status_changes() {
|
||||
),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn equal_refresh_does_not_rearm_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,
|
||||
|
||||
@@ -1,24 +1,6 @@
|
||||
//! 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.
|
||||
//! Sanitization for untrusted terminal-title and tab-status 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.
|
||||
/// Whether a control or invisible formatting character is unsafe in OSC text.
|
||||
pub(crate) fn is_disallowed_osc_text_char(ch: char) -> bool {
|
||||
if ch.is_control() {
|
||||
return true;
|
||||
|
||||
Reference in New Issue
Block a user