mirror of
https://github.com/openai/codex.git
synced 2026-09-08 15:50:34 +00:00
Show successful TUI commands individually (#41893)
## What changed - Emit a separate history cell for each completed command instead of grouping consecutive successes into a `Ran N commands` summary. - Preserve `Explored` grouping for related file reads, searches, and listings. - Replay completed commands as individual entries so their output and failure status remain visible. ## Testing - Update TUI tests and snapshots for live exploration and replayed commands. GitOrigin-RevId: 7dafe0b7846fc89784093791a6be137a1b1ab8ff
This commit is contained in:
committed by
copyberry
parent
e51b54b4c0
commit
32f48598a0
@@ -1203,9 +1203,6 @@ impl ChatWidget {
|
||||
pub(crate) fn pre_draw_tick(&mut self) {
|
||||
self.update_due_hook_visibility();
|
||||
self.schedule_hook_timer_if_needed();
|
||||
if self.bottom_pane.has_active_view() {
|
||||
self.flush_completed_command_activity();
|
||||
}
|
||||
self.bottom_pane.pre_draw_tick();
|
||||
if let Some(pet) = self.ambient_pet.as_ref() {
|
||||
pet.schedule_next_frame();
|
||||
|
||||
@@ -10,7 +10,6 @@ impl ChatWidget {
|
||||
let Some(wait) = self.unified_exec_wait_streak.take() else {
|
||||
return;
|
||||
};
|
||||
self.flush_completed_command_activity();
|
||||
self.transcript.needs_final_message_separator = true;
|
||||
let cell = history_cell::new_unified_exec_interaction(wait.command_display, String::new());
|
||||
self.app_event_tx
|
||||
@@ -362,7 +361,6 @@ impl ChatWidget {
|
||||
if self.suppressed_exec_calls.remove(&id) {
|
||||
return;
|
||||
}
|
||||
let was_running = running.is_some();
|
||||
let (command, parsed, source) = match running {
|
||||
Some(rc) => (rc.command, rc.parsed_cmd, rc.source),
|
||||
None => (event_command, event_parsed, source),
|
||||
@@ -371,33 +369,6 @@ impl ChatWidget {
|
||||
let is_unified_exec_interaction =
|
||||
matches!(source, ExecCommandSource::UnifiedExecInteraction);
|
||||
let is_user_shell = source == ExecCommandSource::UserShell;
|
||||
let retain_untracked_unified_exec = !was_running
|
||||
&& source == ExecCommandSource::UnifiedExecStartup
|
||||
&& self.transcript.active_cell.is_none();
|
||||
// Unified exec skips unknown start events, so group their successful completions here.
|
||||
if !was_running
|
||||
&& source == ExecCommandSource::UnifiedExecStartup
|
||||
&& let Some(cell) = self
|
||||
.transcript
|
||||
.active_cell
|
||||
.as_mut()
|
||||
.and_then(|cell| cell.as_any_mut().downcast_mut::<ExecCell>())
|
||||
&& !cell.is_active()
|
||||
&& cell.iter_calls().all(|call| {
|
||||
matches!(
|
||||
call.source,
|
||||
ExecCommandSource::Agent | ExecCommandSource::UnifiedExecStartup
|
||||
)
|
||||
})
|
||||
{
|
||||
cell.add_call(
|
||||
id.clone(),
|
||||
command.clone(),
|
||||
parsed.clone(),
|
||||
source,
|
||||
/*interaction_input*/ None,
|
||||
);
|
||||
}
|
||||
let end_target = match self.transcript.active_cell.as_ref() {
|
||||
Some(cell) => match cell.as_any().downcast_ref::<ExecCell>() {
|
||||
Some(exec_cell) if exec_cell.iter_calls().any(|call| call.call_id == id) => {
|
||||
@@ -424,10 +395,6 @@ impl ChatWidget {
|
||||
|
||||
match end_target {
|
||||
ExecEndTarget::ActiveTracked => {
|
||||
let has_active_hook = self
|
||||
.active_hook_cell
|
||||
.as_ref()
|
||||
.is_some_and(HookCell::has_visible_running_run);
|
||||
if let Some(cell) = self
|
||||
.transcript
|
||||
.active_cell
|
||||
@@ -436,7 +403,7 @@ impl ChatWidget {
|
||||
{
|
||||
let completed = cell.complete_call(&id, output, duration);
|
||||
debug_assert!(completed, "active exec cell should contain {id}");
|
||||
if cell.should_flush() || (has_active_hook && !cell.is_active()) {
|
||||
if cell.should_flush() {
|
||||
self.flush_active_cell();
|
||||
} else {
|
||||
self.bump_active_cell_revision();
|
||||
@@ -472,7 +439,7 @@ impl ChatWidget {
|
||||
);
|
||||
let completed = cell.complete_call(&id, output, duration);
|
||||
debug_assert!(completed, "new exec cell should contain {id}");
|
||||
if (!was_running && !retain_untracked_unified_exec) || cell.should_flush() {
|
||||
if cell.should_flush() {
|
||||
self.add_to_history(cell);
|
||||
} else {
|
||||
self.transcript.active_cell = Some(Box::new(cell));
|
||||
|
||||
@@ -36,10 +36,12 @@ impl ChatWidget {
|
||||
fn submit_shell_command(&mut self, command: &str) -> QueueDrain {
|
||||
let cmd = command.trim();
|
||||
if cmd.is_empty() {
|
||||
self.add_to_history(history_cell::new_info_event(
|
||||
USER_SHELL_COMMAND_HELP_TITLE.to_string(),
|
||||
Some(USER_SHELL_COMMAND_HELP_HINT.to_string()),
|
||||
));
|
||||
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
history_cell::new_info_event(
|
||||
USER_SHELL_COMMAND_HELP_TITLE.to_string(),
|
||||
Some(USER_SHELL_COMMAND_HELP_HINT.to_string()),
|
||||
),
|
||||
)));
|
||||
QueueDrain::Continue
|
||||
} else {
|
||||
self.submit_op(AppCommand::run_user_shell_command(cmd.to_string()));
|
||||
|
||||
@@ -25,7 +25,6 @@ impl ChatWidget {
|
||||
let should_pause_active_goal = self
|
||||
.bottom_pane
|
||||
.active_view_will_interrupt_turn_on_key_event(key_event);
|
||||
self.flush_completed_command_activity();
|
||||
self.bottom_pane.handle_key_event(key_event);
|
||||
if should_pause_active_goal {
|
||||
self.pause_active_goal_for_interrupt();
|
||||
@@ -176,9 +175,6 @@ impl ChatWidget {
|
||||
let had_modal_or_popup = !self.bottom_pane.no_modal_or_popup_active();
|
||||
let should_pause_active_goal =
|
||||
self.bottom_pane.should_interrupt_running_task(key_event);
|
||||
if key_event.code == KeyCode::Enter {
|
||||
self.flush_completed_command_activity();
|
||||
}
|
||||
let input_result = self.bottom_pane.handle_key_event(key_event);
|
||||
self.sync_backend_banner_view();
|
||||
if should_pause_active_goal {
|
||||
|
||||
@@ -156,26 +156,7 @@ impl ChatWidget {
|
||||
codex_app_server_protocol::CommandExecutionStatus::Completed
|
||||
| codex_app_server_protocol::CommandExecutionStatus::Failed,
|
||||
..
|
||||
} if from_replay => {
|
||||
if matches!(
|
||||
&item,
|
||||
ThreadItem::CommandExecution {
|
||||
status: codex_app_server_protocol::CommandExecutionStatus::Failed,
|
||||
..
|
||||
}
|
||||
) {
|
||||
self.flush_completed_command_activity();
|
||||
}
|
||||
if !self.transcript.active_cell.as_ref().is_some_and(|cell| {
|
||||
cell.as_any()
|
||||
.downcast_ref::<ExecCell>()
|
||||
.is_some_and(ExecCell::is_active)
|
||||
|| cell.as_any().is::<McpToolCallCell>()
|
||||
}) {
|
||||
self.handle_command_execution_started_now(item.clone());
|
||||
}
|
||||
self.handle_command_execution_completed_now(item);
|
||||
}
|
||||
} if from_replay => self.handle_command_execution_completed_now(item),
|
||||
item @ ThreadItem::CommandExecution { .. } => self.on_command_execution_completed(item),
|
||||
ThreadItem::FileChange {
|
||||
status: codex_app_server_protocol::PatchApplyStatus::InProgress,
|
||||
|
||||
@@ -145,7 +145,6 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
pub(super) fn dispatch_command(&mut self, cmd: SlashCommand) {
|
||||
self.flush_completed_command_activity();
|
||||
if !self.ensure_slash_command_allowed_in_side_conversation(cmd) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -2,4 +2,6 @@
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: active_blob(&chat)
|
||||
---
|
||||
• Ran 2 commands · ctrl + t to view transcript
|
||||
• Explored
|
||||
└ List ls -la
|
||||
Read foo.txt
|
||||
|
||||
@@ -2,4 +2,6 @@
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: active_blob(&chat)
|
||||
---
|
||||
• Ran 3 commands · ctrl + t to view transcript
|
||||
• Explored
|
||||
└ List ls -la
|
||||
Read foo.txt
|
||||
|
||||
@@ -2,4 +2,6 @@
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: active_blob(&chat)
|
||||
---
|
||||
• Ran 4 commands · ctrl + t to view transcript
|
||||
• Explored
|
||||
└ List ls -la
|
||||
Read foo.txt, bar.txt
|
||||
|
||||
@@ -1,82 +1,9 @@
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test]
|
||||
async fn compact_command_activity_groups_successes_and_preserves_full_transcript() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.on_task_started();
|
||||
|
||||
let first = begin_exec(&mut chat, "call-first", "printf first");
|
||||
end_exec(&mut chat, first, "first\n", "", /*exit_code*/ 0);
|
||||
|
||||
let second = begin_exec(&mut chat, "call-second", "printf second");
|
||||
insta::assert_snapshot!(active_blob(&chat), @r"• Ran 1 command · ctrl + t to view transcript
|
||||
• Running printf second
|
||||
");
|
||||
end_exec(&mut chat, second, "second\n", "", /*exit_code*/ 0);
|
||||
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
insta::assert_snapshot!(active_blob(&chat), @r"• Ran 2 commands · ctrl + t to view transcript
|
||||
");
|
||||
|
||||
let transcript = chat
|
||||
.active_cell_transcript_lines(/*width*/ 80)
|
||||
.expect("active transcript");
|
||||
let transcript = lines_to_single_string(&transcript);
|
||||
assert!(transcript.contains("$ printf first\nfirst\n"));
|
||||
assert!(transcript.contains("$ printf second\nsecond\n"));
|
||||
|
||||
chat.on_agent_message_delta("Finished\n".to_string());
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 2);
|
||||
assert_eq!(
|
||||
lines_to_single_string(&cells[0]),
|
||||
"• Ran 2 commands · ctrl + t to view transcript\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn compact_command_activity_groups_unified_exec_startup_commands() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.on_task_started();
|
||||
|
||||
let first = begin_unified_exec_startup(&mut chat, "call-first", "proc-first", "printf first");
|
||||
end_exec(&mut chat, first, "first\n", "", /*exit_code*/ 0);
|
||||
|
||||
let run_id = "pre-tool-use:0:/tmp/hooks.json";
|
||||
handle_hook_started(
|
||||
&mut chat,
|
||||
hook_run(
|
||||
run_id,
|
||||
AppServerHookEventName::PreToolUse,
|
||||
AppServerHookRunStatus::Running,
|
||||
"checking command policy",
|
||||
Vec::new(),
|
||||
),
|
||||
);
|
||||
handle_hook_completed(
|
||||
&mut chat,
|
||||
hook_run(
|
||||
run_id,
|
||||
AppServerHookEventName::PreToolUse,
|
||||
AppServerHookRunStatus::Completed,
|
||||
"checking command policy",
|
||||
Vec::new(),
|
||||
),
|
||||
);
|
||||
|
||||
let second =
|
||||
begin_unified_exec_startup(&mut chat, "call-second", "proc-second", "printf second");
|
||||
end_exec(&mut chat, second, "second\n", "", /*exit_code*/ 0);
|
||||
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
insta::assert_snapshot!(active_blob(&chat), @r"• Ran 2 commands · ctrl + t to view transcript
|
||||
");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replayed_command_completion_preserves_tracking_without_duplicate_starts() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.on_task_started();
|
||||
let mut item =
|
||||
begin_unified_exec_startup(&mut chat, "call-replay", "process-replay", "cat replay");
|
||||
@@ -96,12 +23,14 @@ async fn replayed_command_completion_preserves_tracking_without_duplicate_starts
|
||||
|
||||
assert!(chat.running_commands.is_empty());
|
||||
assert!(chat.unified_exec_processes.is_empty());
|
||||
let transcript = lines_to_single_string(
|
||||
&chat
|
||||
.active_cell_transcript_lines(/*width*/ 80)
|
||||
.expect("completed command remains visible"),
|
||||
let history = drain_insert_history(&mut rx)
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(
|
||||
history,
|
||||
vec!["• Ran cat replay\n └ (no output)\n".to_string()]
|
||||
);
|
||||
assert_eq!(transcript.matches("$ cat replay").count(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -145,103 +74,7 @@ async fn replayed_completion_preserves_unrelated_running_command() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn compact_command_activity_preserves_overlapping_reads_after_success() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.on_task_started();
|
||||
let prefix = begin_exec(&mut chat, "call-prefix", "printf before");
|
||||
end_exec(&mut chat, prefix, "before\n", "", /*exit_code*/ 0);
|
||||
let first = begin_exec(&mut chat, "call-first-read", "cat first.txt");
|
||||
let second = begin_exec(&mut chat, "call-second-read", "cat second.txt");
|
||||
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
chat.on_exec_command_output_delta("call-first-read", "streamed output\n");
|
||||
assert!(
|
||||
lines_to_single_string(
|
||||
&chat
|
||||
.active_cell_transcript_lines(/*width*/ 80)
|
||||
.expect("overlapping reads remain active")
|
||||
)
|
||||
.contains("streamed output")
|
||||
);
|
||||
end_exec(&mut chat, first, "first\n", "", /*exit_code*/ 0);
|
||||
end_exec(&mut chat, second, "second\n", "", /*exit_code*/ 0);
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
assert!(active_blob(&chat).contains("Ran 3 commands"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn compact_command_activity_keeps_failures_and_manual_shell_commands_visible() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.on_task_started();
|
||||
|
||||
let first = begin_exec(&mut chat, "call-first", "printf first");
|
||||
end_exec(&mut chat, first, "first\n", "", /*exit_code*/ 0);
|
||||
|
||||
let mut failed = begin_exec(&mut chat, "call-failed", "printf broken");
|
||||
if let AppServerThreadItem::CommandExecution {
|
||||
status,
|
||||
aggregated_output,
|
||||
..
|
||||
} = &mut failed
|
||||
{
|
||||
*status = AppServerCommandExecutionStatus::Declined;
|
||||
*aggregated_output = Some("broken\n".to_string());
|
||||
}
|
||||
handle_exec_end(&mut chat, failed);
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1);
|
||||
let failed_history = lines_to_single_string(&cells[0]);
|
||||
insta::assert_snapshot!(failed_history, @r"• Ran 1 command · ctrl + t to view transcript
|
||||
• Ran printf broken
|
||||
└ broken
|
||||
");
|
||||
|
||||
let manual = begin_exec_with_source(
|
||||
&mut chat,
|
||||
"call-manual",
|
||||
"printf manual",
|
||||
ExecCommandSource::UserShell,
|
||||
);
|
||||
end_exec(&mut chat, manual, "manual\n", "", /*exit_code*/ 0);
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1);
|
||||
let manual_history = lines_to_single_string(&cells[0]);
|
||||
assert!(manual_history.contains("You ran printf manual"));
|
||||
|
||||
for sources in [
|
||||
[
|
||||
ExecCommandSource::UnifiedExecInteraction,
|
||||
ExecCommandSource::Agent,
|
||||
],
|
||||
[
|
||||
ExecCommandSource::Agent,
|
||||
ExecCommandSource::UnifiedExecInteraction,
|
||||
],
|
||||
] {
|
||||
let first = begin_exec_with_source(&mut chat, "call-first", "cat foo.txt", sources[0]);
|
||||
let second = begin_exec_with_source(&mut chat, "call-second", "cat bar.txt", sources[1]);
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
end_exec(&mut chat, first, "content\n", "", /*exit_code*/ 0);
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
let transcript = lines_to_single_string(
|
||||
&chat
|
||||
.transcript
|
||||
.active_cell
|
||||
.as_ref()
|
||||
.expect("overlapping commands remain active")
|
||||
.transcript_lines(/*width*/ 80),
|
||||
);
|
||||
assert!(transcript.contains("foo.txt"));
|
||||
assert!(transcript.contains("bar.txt"));
|
||||
end_exec(&mut chat, second, "content\n", "", /*exit_code*/ 0);
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1);
|
||||
assert!(lines_to_single_string(&cells[0]).contains("bar.txt"));
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn compact_command_activity_keeps_overlapping_commands_active_after_failure() {
|
||||
async fn failed_exploration_keeps_overlapping_commands_active_until_all_finish() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.on_task_started();
|
||||
|
||||
@@ -266,13 +99,22 @@ async fn compact_command_activity_keeps_overlapping_commands_active_after_failur
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1);
|
||||
let history = lines_to_single_string(&cells[0]);
|
||||
assert!(history.contains("Ran ls missing"));
|
||||
assert!(history.contains("Ran cat foo.txt"));
|
||||
assert!(history.contains("Ran cat bar.txt"));
|
||||
insta::assert_snapshot!(history, @r"
|
||||
• Explored
|
||||
└ List missing
|
||||
Read foo.txt, bar.txt
|
||||
");
|
||||
|
||||
let later = begin_exec(&mut chat, "call-after-failure", "cat later.txt");
|
||||
end_exec(&mut chat, later, "later\n", "", /*exit_code*/ 0);
|
||||
insta::assert_snapshot!(active_blob(&chat), @r"
|
||||
• Explored
|
||||
└ Read later.txt
|
||||
");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn compact_command_activity_groups_replayed_successes_without_hiding_declines() {
|
||||
async fn replayed_commands_preserve_individual_output_and_failure_status() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let cwd = chat.config.cwd.clone();
|
||||
let replayed_command =
|
||||
@@ -337,12 +179,12 @@ async fn compact_command_activity_groups_replayed_successes_without_hiding_decli
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(cells.len(), 3);
|
||||
assert_eq!(
|
||||
lines_to_single_string(&cells[0].display_lines(/*width*/ 80)),
|
||||
"• Ran 2 commands · ctrl + t to view transcript\n"
|
||||
);
|
||||
let transcript = lines_to_single_string(&cells[0].transcript_lines(/*width*/ 80));
|
||||
assert_eq!(cells.len(), 4);
|
||||
let transcript = cells
|
||||
.iter()
|
||||
.map(|cell| lines_to_single_string(&cell.transcript_lines(/*width*/ 80)))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
insta::assert_snapshot!(transcript, @r"$ printf first
|
||||
first
|
||||
✓ • 5ms
|
||||
@@ -350,78 +192,15 @@ first
|
||||
$ printf second
|
||||
second
|
||||
✓ • 5ms
|
||||
|
||||
$ printf failure
|
||||
failure
|
||||
✗ (7) • 5ms
|
||||
|
||||
$ printf declined
|
||||
declined
|
||||
✗ (1) • 5ms
|
||||
");
|
||||
assert!(
|
||||
lines_to_single_string(&cells[1].display_lines(/*width*/ 80))
|
||||
.contains("Ran printf failure")
|
||||
);
|
||||
assert!(
|
||||
lines_to_single_string(&cells[2].display_lines(/*width*/ 80))
|
||||
.contains("Ran printf declined")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn compact_command_activity_bounds_completed_groups_without_flushing_active_calls() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.on_task_started();
|
||||
|
||||
for index in 0..32 {
|
||||
let command = begin_exec(&mut chat, &format!("call-{index}"), "printf bounded");
|
||||
end_exec(&mut chat, command, "bounded\n", "", /*exit_code*/ 0);
|
||||
}
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1);
|
||||
assert_eq!(
|
||||
lines_to_single_string(&cells[0]),
|
||||
"• Ran 32 commands · ctrl + t to view transcript\n"
|
||||
);
|
||||
assert!(chat.transcript.active_cell.is_none());
|
||||
|
||||
let commands = (0..33)
|
||||
.map(|index| begin_exec(&mut chat, &format!("call-{index}"), "cat foo.txt"))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
for command in commands {
|
||||
end_exec(&mut chat, command, "content\n", "", /*exit_code*/ 0);
|
||||
}
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1);
|
||||
assert!(lines_to_single_string(&cells[0]).contains("Ran 33 commands"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn compact_command_activity_flushes_before_user_attention() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.on_task_started();
|
||||
|
||||
let first = begin_exec(&mut chat, "call-attention", "printf attention");
|
||||
end_exec(&mut chat, first, "attention\n", "", /*exit_code*/ 0);
|
||||
let second = begin_exec(&mut chat, "call-followup", "printf followup");
|
||||
end_exec(&mut chat, second, "followup\n", "", /*exit_code*/ 0);
|
||||
|
||||
chat.handle_request_user_input_now(ToolRequestUserInputParams {
|
||||
thread_id: "thread-1".to_string(),
|
||||
item_id: "input-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
questions: Vec::new(),
|
||||
is_blocking: true,
|
||||
auto_resolution_ms: None,
|
||||
});
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1);
|
||||
assert!(lines_to_single_string(&cells[0]).contains("Ran 2 commands"));
|
||||
|
||||
let later = begin_exec(&mut chat, "call-after-request", "printf later");
|
||||
end_exec(&mut chat, later, "later\n", "", /*exit_code*/ 0);
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
chat.pre_draw_tick();
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1);
|
||||
assert!(lines_to_single_string(&cells[0]).contains("Ran printf later"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -460,17 +239,11 @@ async fn exec_approval_emits_proposed_command_and_decision_history() {
|
||||
chat.render(area, &mut buf);
|
||||
assert_chatwidget_snapshot!("exec_approval_modal_exec", format!("{buf:?}"));
|
||||
|
||||
let command = begin_exec(&mut chat, "call-during-approval", "printf waiting");
|
||||
end_exec(&mut chat, command, "waiting\n", "", /*exit_code*/ 0);
|
||||
|
||||
// Approve via keyboard and verify the preceding command stays before the decision.
|
||||
// Approve via keyboard and verify a concise decision history line is added
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE));
|
||||
let mut cells = drain_insert_history(&mut rx).into_iter();
|
||||
assert!(
|
||||
lines_to_single_string(&cells.next().expect("completed command"))
|
||||
.contains("Ran printf waiting")
|
||||
);
|
||||
let decision = cells.next().expect("expected decision cell in history");
|
||||
let decision = drain_insert_history(&mut rx)
|
||||
.pop()
|
||||
.expect("expected decision cell in history");
|
||||
assert_chatwidget_snapshot!(
|
||||
"exec_approval_history_decision_approved_short",
|
||||
lines_to_single_string(&decision)
|
||||
@@ -738,11 +511,12 @@ async fn exec_history_cell_shows_working_then_completed() {
|
||||
end_exec(&mut chat, begin, "done", "", /*exit_code*/ 0);
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert!(
|
||||
cells.is_empty(),
|
||||
"successful commands wait for the next boundary"
|
||||
);
|
||||
let blob = active_blob(&chat);
|
||||
// Exec end now finalizes and flushes the exec cell immediately.
|
||||
assert_eq!(cells.len(), 1, "expected finalized exec cell to flush");
|
||||
// Inspect the flushed exec cell rendering.
|
||||
let lines = &cells[0];
|
||||
let blob = lines_to_single_string(lines);
|
||||
// New behavior: no glyph markers; ensure command is shown and no panic.
|
||||
assert!(
|
||||
blob.contains("• Ran"),
|
||||
"expected summary header present: {blob:?}"
|
||||
@@ -865,7 +639,7 @@ async fn exec_end_without_begin_does_not_flush_unrelated_running_exploring_cell(
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_end_without_begin_groups_completed_agent_and_unified_commands() {
|
||||
async fn exec_end_without_begin_flushes_completed_unrelated_exploring_cell() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.on_task_started();
|
||||
|
||||
@@ -877,9 +651,30 @@ async fn exec_end_without_begin_groups_completed_agent_and_unified_commands() {
|
||||
let orphan = begin_unified_exec_startup(&mut chat, "call-after", "proc-1", "echo after");
|
||||
end_exec(&mut chat, orphan, "after\n", "", /*exit_code*/ 0);
|
||||
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
insta::assert_snapshot!(active_blob(&chat), @r"• Ran 2 commands · ctrl + t to view transcript
|
||||
");
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(
|
||||
cells.len(),
|
||||
2,
|
||||
"completed exploring cell should flush before the orphan entry"
|
||||
);
|
||||
let first = lines_to_single_string(&cells[0]);
|
||||
let second = lines_to_single_string(&cells[1]);
|
||||
assert!(
|
||||
first.contains("• Explored"),
|
||||
"expected flushed exploring cell: {first:?}"
|
||||
);
|
||||
assert!(
|
||||
first.contains("List ls -la"),
|
||||
"expected flushed exploring cell: {first:?}"
|
||||
);
|
||||
assert!(
|
||||
second.contains("• Ran echo after"),
|
||||
"expected orphan end entry after flush: {second:?}"
|
||||
);
|
||||
assert!(
|
||||
chat.transcript.active_cell.is_none(),
|
||||
"both entries should be finalized"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -938,11 +733,9 @@ async fn exec_history_shows_unified_exec_startup_commands() {
|
||||
/*exit_code*/ 0,
|
||||
);
|
||||
|
||||
assert!(
|
||||
drain_insert_history(&mut rx).is_empty(),
|
||||
"successful startup commands wait for the next boundary"
|
||||
);
|
||||
let blob = active_blob(&chat);
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1, "expected finalized exec cell to flush");
|
||||
let blob = lines_to_single_string(&cells[0]);
|
||||
assert!(
|
||||
blob.contains("• Ran echo unified exec startup"),
|
||||
"expected startup command to render: {blob:?}"
|
||||
@@ -1524,18 +1317,16 @@ async fn disabled_slash_command_while_task_running_snapshot() {
|
||||
// Build a chat widget and simulate an active task
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
handle_turn_started(&mut chat, "turn-1");
|
||||
let command = begin_exec(&mut chat, "call-before-error", "printf before");
|
||||
end_exec(&mut chat, command, "before\n", "", /*exit_code*/ 0);
|
||||
|
||||
// Resume remains available during MCP startup, but not while an agent turn is active.
|
||||
chat.bottom_pane
|
||||
.set_composer_text("/resume".to_string(), Vec::new(), Vec::new());
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
chat.dispatch_command(SlashCommand::Resume);
|
||||
|
||||
// Drain history and snapshot the rendered error line(s)
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 2);
|
||||
assert!(lines_to_single_string(&cells[0]).contains("Ran printf before"));
|
||||
assert!(
|
||||
!cells.is_empty(),
|
||||
"expected an error message history cell to be emitted",
|
||||
);
|
||||
let blob = lines_to_single_string(cells.last().unwrap());
|
||||
assert_chatwidget_snapshot!("disabled_slash_command_while_task_running_snapshot", blob);
|
||||
}
|
||||
|
||||
@@ -834,21 +834,17 @@ async fn permissions_selection_emits_history_cell_when_selection_changes() {
|
||||
chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
||||
}
|
||||
chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true);
|
||||
chat.on_task_started();
|
||||
chat.dispatch_command(SlashCommand::Permissions);
|
||||
let command = begin_exec(&mut chat, "call-permissions", "printf before");
|
||||
end_exec(&mut chat, command, "before\n", "", /*exit_code*/ 0);
|
||||
chat.open_permissions_popup();
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(
|
||||
cells.len(),
|
||||
2,
|
||||
"expected command and permissions selection history cells"
|
||||
1,
|
||||
"expected one permissions selection history cell"
|
||||
);
|
||||
assert!(lines_to_single_string(&cells[0]).contains("Ran printf before"));
|
||||
let rendered = lines_to_single_string(&cells[1]);
|
||||
let rendered = lines_to_single_string(&cells[0]);
|
||||
assert!(
|
||||
rendered.contains("Permissions updated to"),
|
||||
"expected permissions selection history message, got: {rendered}"
|
||||
|
||||
@@ -282,7 +282,6 @@ impl ChatWidget {
|
||||
|
||||
pub(crate) fn handle_exec_approval_now(&mut self, ev: ExecApprovalRequestEvent) {
|
||||
self.flush_answer_stream_with_separator();
|
||||
self.flush_completed_command_activity();
|
||||
let command = shlex::try_join(ev.command.iter().map(String::as_str))
|
||||
.unwrap_or_else(|_| ev.command.join(" "));
|
||||
self.notify(Notification::ExecApprovalRequested { command });
|
||||
@@ -311,7 +310,6 @@ impl ChatWidget {
|
||||
|
||||
pub(crate) fn handle_apply_patch_approval_now(&mut self, ev: ApplyPatchApprovalRequestEvent) {
|
||||
self.flush_answer_stream_with_separator();
|
||||
self.flush_completed_command_activity();
|
||||
|
||||
let changed_paths = ev.changes.keys().cloned().collect();
|
||||
let request = ApprovalRequest::ApplyPatch(ApplyPatchApprovalRequest {
|
||||
@@ -341,7 +339,6 @@ impl ChatWidget {
|
||||
params: McpServerElicitationRequestParams,
|
||||
) {
|
||||
self.flush_answer_stream_with_separator();
|
||||
self.flush_completed_command_activity();
|
||||
|
||||
self.notify(Notification::ElicitationRequested {
|
||||
server_name: params.server_name.clone(),
|
||||
@@ -398,7 +395,6 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
pub(crate) fn push_approval_request(&mut self, request: ApprovalRequest) {
|
||||
self.flush_completed_command_activity();
|
||||
self.bottom_pane
|
||||
.push_approval_request(request, &self.config.features);
|
||||
self.set_ambient_pet_notification(
|
||||
@@ -412,7 +408,6 @@ impl ChatWidget {
|
||||
&mut self,
|
||||
request: McpServerElicitationFormRequest,
|
||||
) {
|
||||
self.flush_completed_command_activity();
|
||||
self.bottom_pane
|
||||
.push_mcp_server_elicitation_request(request);
|
||||
self.set_ambient_pet_notification(
|
||||
@@ -424,7 +419,6 @@ impl ChatWidget {
|
||||
|
||||
pub(crate) fn handle_request_user_input_now(&mut self, ev: ToolRequestUserInputParams) {
|
||||
self.flush_answer_stream_with_separator();
|
||||
self.flush_completed_command_activity();
|
||||
let question_count = ev.questions.len();
|
||||
let summary = Notification::user_input_request_summary(&ev.questions);
|
||||
let title = match (question_count, summary.as_deref()) {
|
||||
@@ -443,7 +437,6 @@ impl ChatWidget {
|
||||
|
||||
pub(crate) fn handle_request_permissions_now(&mut self, ev: RequestPermissionsEvent) {
|
||||
self.flush_answer_stream_with_separator();
|
||||
self.flush_completed_command_activity();
|
||||
let request = ApprovalRequest::Permissions(PermissionsApprovalRequest {
|
||||
thread_id: self.thread_id.unwrap_or_default(),
|
||||
thread_label: None,
|
||||
|
||||
@@ -143,7 +143,6 @@ impl ChatWidget {
|
||||
self.request_pending_usage_output_insertion_after_stream_shutdown();
|
||||
}
|
||||
self.flush_unified_exec_wait_streak();
|
||||
self.flush_completed_command_activity();
|
||||
if !from_replay {
|
||||
self.collect_runtime_metrics_delta();
|
||||
let runtime_metrics =
|
||||
|
||||
@@ -14,8 +14,6 @@ use codex_app_server_protocol::CommandExecutionSource as ExecCommandSource;
|
||||
use codex_protocol::parse_command::ParsedCommand;
|
||||
use itertools::Either;
|
||||
|
||||
const MAX_GROUPED_COMMANDS: usize = 32;
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
pub(crate) struct CommandOutput {
|
||||
pub(crate) exit_code: i32,
|
||||
@@ -106,38 +104,7 @@ impl ExecCell {
|
||||
duration: None,
|
||||
interaction_input,
|
||||
};
|
||||
let has_failed_call = self.calls.iter().any(|existing| {
|
||||
existing
|
||||
.output
|
||||
.as_ref()
|
||||
.is_some_and(|output| output.exit_code != 0)
|
||||
});
|
||||
if (self.calls.len() >= MAX_GROUPED_COMMANDS && !self.is_active())
|
||||
|| (!Self::is_groupable_source(call.source) && !self.is_active())
|
||||
|| (has_failed_call && !self.is_active())
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
let continues_exploration = Self::is_exploring_call(&call)
|
||||
&& (self.is_exploring_cell()
|
||||
|| self.calls.last().is_some_and(|existing| {
|
||||
existing.duration.is_none() && Self::is_exploring_call(existing)
|
||||
}))
|
||||
&& (self.is_active()
|
||||
|| self
|
||||
.calls
|
||||
.iter()
|
||||
.all(|existing| Self::is_groupable_source(existing.source)));
|
||||
let continues_compact_group = self.calls.iter().all(|existing| {
|
||||
Self::is_groupable_source(existing.source)
|
||||
&& existing.duration.is_some()
|
||||
&& existing
|
||||
.output
|
||||
.as_ref()
|
||||
.is_some_and(|output| output.exit_code == 0)
|
||||
});
|
||||
if continues_exploration || continues_compact_group {
|
||||
if self.is_exploring_cell() && Self::is_exploring_call(&call) {
|
||||
self.calls.push(call);
|
||||
true
|
||||
} else {
|
||||
@@ -167,30 +134,13 @@ impl ExecCell {
|
||||
|
||||
pub(crate) fn should_flush(&self) -> bool {
|
||||
if self.calls.iter().any(|call| {
|
||||
!Self::is_groupable_source(call.source)
|
||||
|| call
|
||||
.output
|
||||
.as_ref()
|
||||
.is_some_and(|output| output.exit_code != 0)
|
||||
call.output
|
||||
.as_ref()
|
||||
.is_some_and(|output| output.exit_code != 0)
|
||||
}) {
|
||||
return !self.is_active();
|
||||
}
|
||||
|
||||
if self.calls.len() >= MAX_GROUPED_COMMANDS {
|
||||
return !self.is_active();
|
||||
}
|
||||
|
||||
if self.calls.iter().all(|call| {
|
||||
Self::is_groupable_source(call.source)
|
||||
&& call.duration.is_some()
|
||||
&& call
|
||||
.output
|
||||
.as_ref()
|
||||
.is_some_and(|output| output.exit_code == 0)
|
||||
}) {
|
||||
return false;
|
||||
}
|
||||
|
||||
!self.is_exploring_cell() && self.calls.iter().all(|c| c.duration.is_some())
|
||||
}
|
||||
|
||||
@@ -260,13 +210,6 @@ impl ExecCell {
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
fn is_groupable_source(source: ExecCommandSource) -> bool {
|
||||
matches!(
|
||||
source,
|
||||
ExecCommandSource::Agent | ExecCommandSource::UnifiedExecStartup
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
impl ExecCall {
|
||||
|
||||
@@ -185,9 +185,7 @@ fn activity_marker(start_time: Option<Instant>, animations_enabled: bool) -> Spa
|
||||
|
||||
impl HistoryCell for ExecCell {
|
||||
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
|
||||
if self.calls.len() > 1 && (!self.is_exploring_cell() || !self.is_active()) {
|
||||
self.compact_group_display_lines(width)
|
||||
} else if self.is_exploring_cell() {
|
||||
if self.is_exploring_cell() {
|
||||
self.exploring_display_lines(width)
|
||||
} else {
|
||||
self.command_display_lines(width)
|
||||
@@ -246,42 +244,6 @@ impl HistoryCell for ExecCell {
|
||||
}
|
||||
|
||||
impl ExecCell {
|
||||
fn compact_group_display_lines(&self, width: u16) -> Vec<Line<'static>> {
|
||||
let completed_commands = self
|
||||
.calls
|
||||
.iter()
|
||||
.take_while(|call| {
|
||||
matches!(
|
||||
call.source,
|
||||
ExecCommandSource::Agent | ExecCommandSource::UnifiedExecStartup
|
||||
) && call.duration.is_some()
|
||||
&& call
|
||||
.output
|
||||
.as_ref()
|
||||
.is_some_and(|output| output.exit_code == 0)
|
||||
})
|
||||
.count();
|
||||
let mut lines = Vec::new();
|
||||
if completed_commands > 0 {
|
||||
let noun = if completed_commands == 1 {
|
||||
"command"
|
||||
} else {
|
||||
"commands"
|
||||
};
|
||||
lines.push(Line::from(vec![
|
||||
"•".green().bold(),
|
||||
" ".into(),
|
||||
format!("Ran {completed_commands} {noun}").bold(),
|
||||
" · ".dim(),
|
||||
TRANSCRIPT_HINT.dim(),
|
||||
]));
|
||||
}
|
||||
for call in &self.calls[completed_commands..] {
|
||||
lines.extend(self.command_call_display_lines(width, call));
|
||||
}
|
||||
lines
|
||||
}
|
||||
|
||||
fn output_ellipsis_text(omitted: usize) -> String {
|
||||
format!("… +{omitted} lines ({TRANSCRIPT_HINT})")
|
||||
}
|
||||
@@ -391,10 +353,6 @@ impl ExecCell {
|
||||
let [call] = &self.calls.as_slice() else {
|
||||
panic!("Expected exactly one call in a command display cell");
|
||||
};
|
||||
self.command_call_display_lines(width, call)
|
||||
}
|
||||
|
||||
fn command_call_display_lines(&self, width: u16, call: &ExecCall) -> Vec<Line<'static>> {
|
||||
let layout = EXEC_DISPLAY_LAYOUT;
|
||||
let success = call
|
||||
.duration
|
||||
@@ -407,7 +365,7 @@ impl ExecCell {
|
||||
let is_interaction = call.is_unified_exec_interaction();
|
||||
let title = if is_interaction {
|
||||
""
|
||||
} else if call.duration.is_none() {
|
||||
} else if self.is_active() {
|
||||
"Running"
|
||||
} else if call.is_user_shell_command() {
|
||||
"You ran"
|
||||
|
||||
@@ -2,4 +2,6 @@
|
||||
source: tui/src/history_cell.rs
|
||||
expression: rendered
|
||||
---
|
||||
• Ran 3 commands · ctrl + t to view transcript
|
||||
• Explored
|
||||
└ Search shimmer_spans
|
||||
Read shimmer.rs, status_indicator_widget.rs
|
||||
|
||||
Reference in New Issue
Block a user