diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 4b601e5728..913a61069a 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -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(); diff --git a/codex-rs/tui/src/chatwidget/command_lifecycle.rs b/codex-rs/tui/src/chatwidget/command_lifecycle.rs index ac231e0d6d..da9bd9a89a 100644 --- a/codex-rs/tui/src/chatwidget/command_lifecycle.rs +++ b/codex-rs/tui/src/chatwidget/command_lifecycle.rs @@ -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::()) - && !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::() { 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)); diff --git a/codex-rs/tui/src/chatwidget/input_submission.rs b/codex-rs/tui/src/chatwidget/input_submission.rs index 8571519c7f..8a4133e72c 100644 --- a/codex-rs/tui/src/chatwidget/input_submission.rs +++ b/codex-rs/tui/src/chatwidget/input_submission.rs @@ -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())); diff --git a/codex-rs/tui/src/chatwidget/interaction.rs b/codex-rs/tui/src/chatwidget/interaction.rs index e36dcb582d..24e5470063 100644 --- a/codex-rs/tui/src/chatwidget/interaction.rs +++ b/codex-rs/tui/src/chatwidget/interaction.rs @@ -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 { diff --git a/codex-rs/tui/src/chatwidget/replay.rs b/codex-rs/tui/src/chatwidget/replay.rs index f73093c9ca..037b58c25f 100644 --- a/codex-rs/tui/src/chatwidget/replay.rs +++ b/codex-rs/tui/src/chatwidget/replay.rs @@ -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::() - .is_some_and(ExecCell::is_active) - || cell.as_any().is::() - }) { - 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, diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index 6925ef5a6e..2856c4cd4a 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -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; } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step4_finish_cat_foo.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step4_finish_cat_foo.snap index 8d96a42c6d..de15e7ca24 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step4_finish_cat_foo.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step4_finish_cat_foo.snap @@ -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 diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step5_finish_sed_range.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step5_finish_sed_range.snap index 67f7695631..de15e7ca24 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step5_finish_sed_range.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step5_finish_sed_range.snap @@ -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 diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step6_finish_cat_bar.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step6_finish_cat_bar.snap index e562f50f22..2a8ed11b7f 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step6_finish_cat_bar.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step6_finish_cat_bar.snap @@ -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 diff --git a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs index 4b8f1b5258..396cffe6f5 100644 --- a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs +++ b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs @@ -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::>(); + 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::>(); - 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::>() + .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::>(); - - 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); } diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index 8a359534da..5ba2723577 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -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}" diff --git a/codex-rs/tui/src/chatwidget/tool_requests.rs b/codex-rs/tui/src/chatwidget/tool_requests.rs index 4beb0660f3..1b1fb4ec8e 100644 --- a/codex-rs/tui/src/chatwidget/tool_requests.rs +++ b/codex-rs/tui/src/chatwidget/tool_requests.rs @@ -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, diff --git a/codex-rs/tui/src/chatwidget/turn_runtime.rs b/codex-rs/tui/src/chatwidget/turn_runtime.rs index 3d9e698e1b..4fc542fc82 100644 --- a/codex-rs/tui/src/chatwidget/turn_runtime.rs +++ b/codex-rs/tui/src/chatwidget/turn_runtime.rs @@ -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 = diff --git a/codex-rs/tui/src/exec_cell/model.rs b/codex-rs/tui/src/exec_cell/model.rs index 64ace4c578..0401b8c7fe 100644 --- a/codex-rs/tui/src/exec_cell/model.rs +++ b/codex-rs/tui/src/exec_cell/model.rs @@ -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 { diff --git a/codex-rs/tui/src/exec_cell/render.rs b/codex-rs/tui/src/exec_cell/render.rs index 5326cf6efe..ed091515d3 100644 --- a/codex-rs/tui/src/exec_cell/render.rs +++ b/codex-rs/tui/src/exec_cell/render.rs @@ -185,9 +185,7 @@ fn activity_marker(start_time: Option, animations_enabled: bool) -> Spa impl HistoryCell for ExecCell { fn display_lines(&self, width: u16) -> Vec> { - 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> { - 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> { 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" diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__coalesces_reads_across_multiple_calls.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__coalesces_reads_across_multiple_calls.snap index a3ae56d7d2..fcfa31eede 100644 --- a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__coalesces_reads_across_multiple_calls.snap +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__coalesces_reads_across_multiple_calls.snap @@ -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