diff --git a/codex-rs/tui/src/chatwidget/command_lifecycle.rs b/codex-rs/tui/src/chatwidget/command_lifecycle.rs index 6f13d37987..794dfff73c 100644 --- a/codex-rs/tui/src/chatwidget/command_lifecycle.rs +++ b/codex-rs/tui/src/chatwidget/command_lifecycle.rs @@ -284,7 +284,7 @@ impl ChatWidget { .active_cell .as_mut() .and_then(|c| c.as_any_mut().downcast_mut::()) - && let Some(new_exec) = cell.with_added_call( + && cell.add_call( id.clone(), command.clone(), parsed_cmd.clone(), @@ -292,7 +292,6 @@ impl ChatWidget { /*interaction_input*/ None, ) { - *cell = new_exec; self.bump_active_cell_revision(); } else { self.flush_active_cell(); @@ -383,13 +382,11 @@ impl ChatWidget { let output = if is_unified_exec_interaction { CommandOutput { exit_code, - formatted_output: String::new(), aggregated_output: String::new(), } } else { CommandOutput { exit_code, - formatted_output: aggregated_output.clone(), aggregated_output, } }; diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_active.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_active.snap new file mode 100644 index 0000000000..a59d560379 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_active.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/chatwidget/tests/app_server.rs +expression: active +--- +• Running printf 'stdout\nstderr\n' + └ stdout + stderr diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_interrupted.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_interrupted.snap new file mode 100644 index 0000000000..73d2f0379c --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_interrupted.snap @@ -0,0 +1,8 @@ +--- +source: tui/src/chatwidget/tests/app_server.rs +expression: completed +--- +$ printf 'stdout\nstderr\n' +stdout +stderr +✗ (1) • diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_transcript.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_transcript.snap new file mode 100644 index 0000000000..beb04a6dca --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_transcript.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/chatwidget/tests/app_server.rs +expression: lines_to_single_string(&transcript) +--- +$ printf 'stdout\nstderr\n' +stdout +stderr diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index 1181120aaa..a0728e017f 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -850,6 +850,57 @@ async fn live_app_server_command_execution_strips_shell_wrapper() { ); } +#[tokio::test] +async fn live_app_server_command_output_delta_transcript_snapshot() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.on_task_started(); + begin_exec(&mut chat, "cmd-1", "printf 'stdout\\nstderr\\n'"); + + for delta in ["stdout\n", "stderr\n"] { + chat.handle_server_notification( + ServerNotification::CommandExecutionOutputDelta( + codex_app_server_protocol::CommandExecutionOutputDeltaNotification { + thread_id: "thread-1".to_string(), + turn_id: "turn-1".to_string(), + item_id: "cmd-1".to_string(), + delta: delta.to_string(), + }, + ), + /*replay_kind*/ None, + ); + } + + let active = active_blob(&chat); + assert_chatwidget_snapshot!("live_app_server_command_output_delta_active", active); + + let transcript = chat + .active_cell_transcript_lines(/*width*/ 80) + .expect("active exec transcript lines"); + assert_chatwidget_snapshot!( + "live_app_server_command_output_delta_transcript", + lines_to_single_string(&transcript) + ); + + handle_turn_interrupted(&mut chat, "turn-1"); + let mut completed = None; + while let Ok(event) = rx.try_recv() { + if let AppEvent::InsertHistoryCell(cell) = event { + let transcript = lines_to_single_string(&cell.transcript_lines(/*width*/ 80)); + if transcript.contains("printf 'stdout\\nstderr\\n'") { + completed = Some(transcript); + } + } + } + let completed = completed.expect("expected the interrupted command in history"); + let completed = regex_lite::Regex::new(r"(?m) • (?:\d+ms|\d+\.\d+s|\d+m \d+s)$") + .expect("valid duration regex") + .replace(&completed, " • "); + assert_chatwidget_snapshot!( + "live_app_server_command_output_delta_interrupted", + completed + ); +} + #[tokio::test] async fn live_app_server_collab_wait_items_render_history() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/exec_cell/model.rs b/codex-rs/tui/src/exec_cell/model.rs index cd3f56166c..11f0e9d210 100644 --- a/codex-rs/tui/src/exec_cell/model.rs +++ b/codex-rs/tui/src/exec_cell/model.rs @@ -11,16 +11,14 @@ use std::time::Instant; use codex_app_server_protocol::CommandExecutionSource as ExecCommandSource; use codex_protocol::parse_command::ParsedCommand; -#[derive(Clone, Debug, Default)] +#[derive(Debug, Default)] pub(crate) struct CommandOutput { pub(crate) exit_code: i32, /// The aggregated stderr + stdout interleaved. pub(crate) aggregated_output: String, - /// The formatted output of the command, as seen by the model. - pub(crate) formatted_output: String, } -#[derive(Debug, Clone)] +#[derive(Debug)] pub(crate) struct ExecCall { pub(crate) call_id: String, pub(crate) command: Vec, @@ -46,14 +44,14 @@ impl ExecCell { } } - pub(crate) fn with_added_call( - &self, + pub(crate) fn add_call( + &mut self, call_id: String, command: Vec, parsed: Vec, source: ExecCommandSource, interaction_input: Option, - ) -> Option { + ) -> bool { let call = ExecCall { call_id, command, @@ -65,12 +63,10 @@ impl ExecCell { interaction_input, }; if self.is_exploring_cell() && Self::is_exploring_call(&call) { - Some(Self { - calls: [self.calls.clone(), vec![call]].concat(), - animations_enabled: self.animations_enabled, - }) + self.calls.push(call); + true } else { - None + false } } @@ -95,23 +91,21 @@ impl ExecCell { } pub(crate) fn should_flush(&self) -> bool { - !self.is_exploring_cell() && self.calls.iter().all(|c| c.output.is_some()) + !self.is_exploring_cell() && self.calls.iter().all(|c| c.duration.is_some()) } pub(crate) fn mark_failed(&mut self) { for call in self.calls.iter_mut() { - if call.output.is_none() { + if call.duration.is_none() { let elapsed = call .start_time .map(|st| st.elapsed()) .unwrap_or_else(|| Duration::from_millis(0)); call.start_time = None; call.duration = Some(elapsed); - call.output = Some(CommandOutput { - exit_code: 1, - formatted_output: String::new(), - aggregated_output: String::new(), - }); + call.output + .get_or_insert_with(CommandOutput::default) + .exit_code = 1; } } } @@ -121,13 +115,13 @@ impl ExecCell { } pub(crate) fn is_active(&self) -> bool { - self.calls.iter().any(|c| c.output.is_none()) + self.calls.iter().any(|c| c.duration.is_none()) } pub(crate) fn active_start_time(&self) -> Option { self.calls .iter() - .find(|c| c.output.is_none()) + .find(|c| c.duration.is_none()) .and_then(|c| c.start_time) } diff --git a/codex-rs/tui/src/exec_cell/render.rs b/codex-rs/tui/src/exec_cell/render.rs index bf17277b91..55a9a625bf 100644 --- a/codex-rs/tui/src/exec_cell/render.rs +++ b/codex-rs/tui/src/exec_cell/render.rs @@ -128,13 +128,11 @@ pub(crate) fn output_lines( } }; - let src = aggregated_output; - let lines: Vec<&str> = src.lines().collect(); - let total = lines.len(); + let total = aggregated_output.lines().count(); let mut out: Vec> = Vec::new(); let head_end = total.min(line_limit); - for (i, raw) in lines[..head_end].iter().enumerate() { + for (i, raw) in aggregated_output.lines().take(head_end).enumerate() { let mut line = ansi_escape_line(raw); let prefix = if !include_prefix { "" @@ -166,7 +164,7 @@ pub(crate) fn output_lines( } else { head_end }; - for raw in lines[tail_start..].iter() { + for raw in aggregated_output.lines().skip(tail_start) { let mut line = ansi_escape_line(raw); if include_prefix { line.spans.insert(0, " ".into()); @@ -221,25 +219,24 @@ impl HistoryCell for ExecCell { if !call.is_unified_exec_interaction() { let wrap_width = width.max(1) as usize; let wrap_opts = RtOptions::new(wrap_width); - for unwrapped in output.formatted_output.lines().map(ansi_escape_line) { + for unwrapped in output.aggregated_output.lines().map(ansi_escape_line) { let wrapped = adaptive_wrap_line(&unwrapped, wrap_opts.clone()); push_owned_lines(&wrapped, &mut lines); } } - let duration = call - .duration - .map(format_duration) - .unwrap_or_else(|| "unknown".to_string()); - let mut result: Line = if output.exit_code == 0 { - Line::from("✓".green().bold()) - } else { - Line::from(vec![ - "✗".red().bold(), - format!(" ({})", output.exit_code).into(), - ]) - }; - result.push_span(format!(" • {duration}").dim()); - lines.push(result); + if let Some(duration) = call.duration { + let duration = format_duration(duration); + let mut result: Line = if output.exit_code == 0 { + Line::from("✓".green().bold()) + } else { + Line::from(vec![ + "✗".red().bold(), + format!(" ({})", output.exit_code).into(), + ]) + }; + result.push_span(format!(" • {duration}").dim()); + lines.push(result); + } } } lines @@ -275,38 +272,32 @@ impl ExecCell { }, ])); - let mut calls = self.calls.clone(); + let mut calls = self.calls.as_slice(); let mut out_indented = Vec::new(); - while !calls.is_empty() { - let mut call = calls.remove(0); - if call - .parsed - .iter() - .all(|parsed| matches!(parsed, ParsedCommand::Read { .. })) - { - while let Some(next) = calls.first() { - if next - .parsed - .iter() - .all(|parsed| matches!(parsed, ParsedCommand::Read { .. })) - { - call.parsed.extend(next.parsed.clone()); - calls.remove(0); - } else { - break; - } - } - } - + while let Some((call, remaining)) = calls.split_first() { let reads_only = call .parsed .iter() .all(|parsed| matches!(parsed, ParsedCommand::Read { .. })); + let group_len = if reads_only { + 1 + remaining + .iter() + .take_while(|next| { + next.parsed + .iter() + .all(|parsed| matches!(parsed, ParsedCommand::Read { .. })) + }) + .count() + } else { + 1 + }; + let (group, remaining) = calls.split_at(group_len); + calls = remaining; let call_lines: Vec<(&str, Vec>)> = if reads_only { - let names = call - .parsed + let names = group .iter() + .flat_map(|call| &call.parsed) .map(|parsed| match parsed { ParsedCommand::Read { name, .. } => name.clone(), _ => unreachable!(), @@ -367,7 +358,9 @@ impl ExecCell { panic!("Expected exactly one call in a command display cell"); }; let layout = EXEC_DISPLAY_LAYOUT; - let success = call.output.as_ref().map(|o| o.exit_code == 0); + let success = call + .duration + .and_then(|_| call.output.as_ref().map(|o| o.exit_code == 0)); let bullet = match success { Some(true) => "•".green().bold(), Some(false) => "•".red().bold(), @@ -736,7 +729,6 @@ mod tests { let output = CommandOutput { exit_code: 0, aggregated_output, - formatted_output: String::new(), }; let width = 20; let layout = EXEC_DISPLAY_LAYOUT; @@ -862,7 +854,6 @@ mod tests { let output = CommandOutput { exit_code: 0, aggregated_output: (1..=7).map(|n| n.to_string()).join("\n"), - formatted_output: String::new(), }; let rendered: Vec = output_lines( @@ -879,14 +870,39 @@ mod tests { .map(render_line_text) .collect(); - assert!( - rendered - .iter() - .any(|line| line.contains("… +3 lines (ctrl + t to view transcript)")), - "expected logical truncation to include transcript hint, got: {rendered:?}" + assert_eq!( + rendered, + vec![ + "1", + "2", + "… +3 lines (ctrl + t to view transcript)", + "6", + "7", + ] ); } + #[test] + fn output_lines_handles_newline_dense_output_without_materializing_every_line() { + let output = CommandOutput { + exit_code: 0, + aggregated_output: "\n".repeat(100_000), + }; + + let rendered = output_lines( + Some(&output), + OutputLinesParams { + line_limit: 5, + only_err: false, + include_angle_pipe: false, + include_prefix: false, + }, + ); + + assert_eq!(rendered.lines.len(), 11); + assert_eq!(rendered.omitted, Some(99_990)); + } + #[test] fn command_truncation_ellipsis_does_not_include_transcript_hint() { let truncated = ExecCell::limit_lines_from_start( @@ -1036,7 +1052,6 @@ mod tests { parsed: Vec::new(), output: Some(CommandOutput { exit_code: 0, - formatted_output: String::new(), aggregated_output: url.to_string(), }), source: ExecCommandSource::UserShell, @@ -1073,7 +1088,6 @@ mod tests { parsed: Vec::new(), output: Some(CommandOutput { exit_code: 0, - formatted_output: url.to_string(), aggregated_output: url.to_string(), }), source: ExecCommandSource::Agent, diff --git a/codex-rs/tui/src/history_cell/patches.rs b/codex-rs/tui/src/history_cell/patches.rs index 7892b7c4d7..61c6da151e 100644 --- a/codex-rs/tui/src/history_cell/patches.rs +++ b/codex-rs/tui/src/history_cell/patches.rs @@ -45,7 +45,6 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell { let output = output_lines( Some(&CommandOutput { exit_code: 1, - formatted_output: String::new(), aggregated_output: stderr, }), OutputLinesParams { diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index 157761d4f6..76439d6a95 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -1699,34 +1699,30 @@ fn coalesces_reads_across_multiple_calls() { // Call 1: Search only cell.complete_call("c1", CommandOutput::default(), Duration::from_millis(1)); // Call 2: Read A - cell = cell - .with_added_call( - "c2".into(), - vec!["bash".into(), "-lc".into(), "echo".into()], - vec![ParsedCommand::Read { - name: "shimmer.rs".into(), - cmd: "cat shimmer.rs".into(), - path: "shimmer.rs".into(), - }], - ExecCommandSource::Agent, - /*interaction_input*/ None, - ) - .unwrap(); + assert!(cell.add_call( + "c2".into(), + vec!["bash".into(), "-lc".into(), "echo".into()], + vec![ParsedCommand::Read { + name: "shimmer.rs".into(), + cmd: "cat shimmer.rs".into(), + path: "shimmer.rs".into(), + }], + ExecCommandSource::Agent, + /*interaction_input*/ None, + )); cell.complete_call("c2", CommandOutput::default(), Duration::from_millis(1)); // Call 3: Read B - cell = cell - .with_added_call( - "c3".into(), - vec!["bash".into(), "-lc".into(), "echo".into()], - vec![ParsedCommand::Read { - name: "status_indicator_widget.rs".into(), - cmd: "cat status_indicator_widget.rs".into(), - path: "status_indicator_widget.rs".into(), - }], - ExecCommandSource::Agent, - /*interaction_input*/ None, - ) - .unwrap(); + assert!(cell.add_call( + "c3".into(), + vec!["bash".into(), "-lc".into(), "echo".into()], + vec![ParsedCommand::Read { + name: "status_indicator_widget.rs".into(), + cmd: "cat status_indicator_widget.rs".into(), + path: "status_indicator_widget.rs".into(), + }], + ExecCommandSource::Agent, + /*interaction_input*/ None, + )); cell.complete_call("c3", CommandOutput::default(), Duration::from_millis(1)); let lines = cell.display_lines(/*width*/ 80); @@ -1918,7 +1914,6 @@ fn stderr_tail_more_than_five_lines_snapshot() { &call_id, CommandOutput { exit_code: 1, - formatted_output: String::new(), aggregated_output: stderr, }, Duration::from_millis(1), @@ -1966,7 +1961,6 @@ fn ran_cell_multiline_with_stderr_snapshot() { &call_id, CommandOutput { exit_code: 1, - formatted_output: String::new(), aggregated_output: stderr, }, Duration::from_millis(5), diff --git a/codex-rs/tui/src/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index 640438557b..cf263dd9a8 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -1226,7 +1226,6 @@ mod tests { CommandOutput { exit_code: 0, aggregated_output: "src\nREADME.md\n".into(), - formatted_output: "src\nREADME.md\n".into(), }, Duration::from_millis(420), );