diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 79b73a3335..cdf94210bd 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -701,7 +701,6 @@ impl Session { let _ = self.tx_event.send(event).await; } - #[allow(clippy::too_many_arguments)] async fn on_exec_command_end( &self, turn_diff_tracker: &mut TurnDiffTracker, @@ -721,6 +720,7 @@ impl Session { const MAX_STREAM_OUTPUT: usize = 5 * 1024; // 5KiB let stdout = stdout.text.chars().take(MAX_STREAM_OUTPUT).collect(); let stderr = stderr.text.chars().take(MAX_STREAM_OUTPUT).collect(); + let formatted_output = format_exec_output_str(output); let msg = if is_apply_patch { EventMsg::PatchApplyEnd(PatchApplyEndEvent { @@ -734,6 +734,7 @@ impl Session { call_id: call_id.to_string(), stdout, stderr, + formatted_output, duration: *duration, exit_code: *exit_code, }) @@ -2357,7 +2358,7 @@ async fn handle_container_exec_with_params( let ExecToolCallOutput { exit_code, .. } = &output; let is_success = *exit_code == 0; - let content = format_exec_output(output); + let content = format_exec_output(&output); ResponseInputItem::FunctionCallOutput { call_id: call_id.clone(), output: FunctionCallOutputPayload { @@ -2490,7 +2491,7 @@ async fn handle_sandbox_error( let ExecToolCallOutput { exit_code, .. } = &retry_output; let is_success = *exit_code == 0; - let content = format_exec_output(retry_output); + let content = format_exec_output(&retry_output); ResponseInputItem::FunctionCallOutput { call_id: call_id.clone(), @@ -2522,13 +2523,33 @@ async fn handle_sandbox_error( } } -/// Exec output is a pre-serialized JSON payload -fn format_exec_output(exec_output: ExecToolCallOutput) -> String { +fn format_exec_output_str(exec_output: &ExecToolCallOutput) -> String { let ExecToolCallOutput { exit_code, stdout, stderr, + .. + } = exec_output; + + let is_success = *exit_code == 0; + let output = if is_success { stdout } else { stderr }; + + let mut formatted_output = output.text.clone(); + if let Some(truncated_after_lines) = output.truncated_after_lines { + formatted_output.push_str(&format!( + "\n\n[Output truncated after {truncated_after_lines} lines: too many lines or bytes.]", + )); + } + + formatted_output +} + +/// Exec output is a pre-serialized JSON payload +fn format_exec_output(exec_output: &ExecToolCallOutput) -> String { + let ExecToolCallOutput { + exit_code, duration, + .. } = exec_output; #[derive(Serialize)] @@ -2546,20 +2567,12 @@ fn format_exec_output(exec_output: ExecToolCallOutput) -> String { // round to 1 decimal place let duration_seconds = ((duration.as_secs_f32()) * 10.0).round() / 10.0; - let is_success = exit_code == 0; - let output = if is_success { stdout } else { stderr }; - - let mut formatted_output = output.text; - if let Some(truncated_after_lines) = output.truncated_after_lines { - formatted_output.push_str(&format!( - "\n\n[Output truncated after {truncated_after_lines} lines: too many lines or bytes.]", - )); - } + let formatted_output = format_exec_output_str(exec_output); let payload = ExecOutput { output: &formatted_output, metadata: ExecMetadata { - exit_code, + exit_code: *exit_code, duration_seconds, }, }; diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 9a562cbd4d..1a42fd0c61 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -291,6 +291,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { stderr, duration, exit_code, + .. }) => { let exec_command = self.call_id_to_command.remove(&call_id); let (duration, call) = if let Some(ExecCommandBegin { command, .. }) = exec_command diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index fbe052bf95..f337f94fe6 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -674,6 +674,8 @@ pub struct ExecCommandEndEvent { pub exit_code: i32, /// The duration of the command execution. pub duration: Duration, + /// Formatted output from the command, as seen by the model. + pub formatted_output: String, } #[derive(Debug, Clone, Deserialize, Serialize)] diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index e24975f1c7..02ea6cb62e 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -9,6 +9,8 @@ use codex_core::protocol::TokenUsage; use codex_file_search::FileMatch; use crossterm::event::KeyEvent; use ratatui::buffer::Buffer; +use ratatui::layout::Constraint; +use ratatui::layout::Layout; use ratatui::layout::Rect; use ratatui::widgets::WidgetRef; @@ -95,8 +97,31 @@ impl BottomPane { } else { self.composer.desired_height(width) }; + let top_pad = if self.active_view.is_none() || self.status_view_active { + 1 + } else { + 0 + }; + view_height + .saturating_add(Self::BOTTOM_PAD_LINES) + .saturating_add(top_pad) + } + + fn layout(&self, area: Rect) -> Rect { + let top = if self.active_view.is_none() || self.status_view_active { + 1 + } else { + 0 + }; - view_height.saturating_add(Self::BOTTOM_PAD_LINES) + let [_, content, _] = Layout::vertical([ + Constraint::Max(top), + Constraint::Min(1), + Constraint::Max(BottomPane::BOTTOM_PAD_LINES), + ]) + .areas(area); + + content } pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { @@ -104,10 +129,11 @@ impl BottomPane { // status indicator shown while a task is running, or approval modal). // In these states the textarea is not interactable, so we should not // show its caret. - if self.active_view.is_some() { + if self.active_view.is_some() || self.status_view_active { None } else { - self.composer.cursor_pos(area) + let content = self.layout(area); + self.composer.cursor_pos(content) } } @@ -365,31 +391,12 @@ impl BottomPane { impl WidgetRef for &BottomPane { fn render_ref(&self, area: Rect, buf: &mut Buffer) { + let content = self.layout(area); + if let Some(view) = &self.active_view { - // Reserve bottom padding lines; keep at least 1 line for the view. - let avail = area.height; - if avail > 0 { - let pad = BottomPane::BOTTOM_PAD_LINES.min(avail.saturating_sub(1)); - let view_rect = Rect { - x: area.x, - y: area.y, - width: area.width, - height: avail - pad, - }; - view.render(view_rect, buf); - } + view.render(content, buf); } else { - let avail = area.height; - if avail > 0 { - let composer_rect = Rect { - x: area.x, - y: area.y, - width: area.width, - // Reserve bottom padding - height: avail - BottomPane::BOTTOM_PAD_LINES.min(avail.saturating_sub(1)), - }; - (&self.composer).render_ref(composer_rect, buf); - } + (&self.composer).render_ref(content, buf); } } } @@ -495,13 +502,13 @@ mod tests { let area = Rect::new(0, 0, 40, 3); let mut buf = Buffer::empty(area); (&pane).render_ref(area, &mut buf); - let mut row0 = String::new(); + let mut row1 = String::new(); for x in 0..area.width { - row0.push(buf[(x, 0)].symbol().chars().next().unwrap_or(' ')); + row1.push(buf[(x, 1)].symbol().chars().next().unwrap_or(' ')); } assert!( - row0.contains("Working"), - "expected Working header after denial: {row0:?}" + row1.contains("Working"), + "expected Working header after denial on row 1: {row1:?}" ); // Drain the channel to avoid unused warnings. @@ -523,14 +530,13 @@ mod tests { // Begin a task: show initial status. pane.set_task_running(true); - // Render and confirm the line contains the "Working" header. let area = Rect::new(0, 0, 40, 3); let mut buf = Buffer::empty(area); (&pane).render_ref(area, &mut buf); let mut row0 = String::new(); for x in 0..area.width { - row0.push(buf[(x, 0)].symbol().chars().next().unwrap_or(' ')); + row0.push(buf[(x, 1)].symbol().chars().next().unwrap_or(' ')); } assert!( row0.contains("Working"), @@ -563,12 +569,12 @@ mod tests { let mut buf = Buffer::empty(area); (&pane).render_ref(area, &mut buf); - // Top row contains the status header + // Row 1 contains the status header (row 0 is the spacer) let mut top = String::new(); for x in 0..area.width { - top.push(buf[(x, 0)].symbol().chars().next().unwrap_or(' ')); + top.push(buf[(x, 1)].symbol().chars().next().unwrap_or(' ')); } - assert_eq!(buf[(0, 0)].symbol().chars().next().unwrap_or(' '), '▌'); + assert_eq!(buf[(0, 1)].symbol().chars().next().unwrap_or(' '), '▌'); assert!( top.contains("Working"), "expected Working header on top row: {top:?}" @@ -605,7 +611,7 @@ mod tests { pane.set_task_running(true); - // Height=2 → pad shrinks to 1; bottom row is blank, top row has spinner. + // Height=2 → with spacer, spinner on row 1; no bottom padding. let area2 = Rect::new(0, 0, 20, 2); let mut buf2 = Buffer::empty(area2); (&pane).render_ref(area2, &mut buf2); @@ -615,13 +621,10 @@ mod tests { row0.push(buf2[(x, 0)].symbol().chars().next().unwrap_or(' ')); row1.push(buf2[(x, 1)].symbol().chars().next().unwrap_or(' ')); } + assert!(row0.trim().is_empty(), "expected spacer on row 0: {row0:?}"); assert!( - row0.contains("Working"), - "expected Working header on row 0: {row0:?}" - ); - assert!( - row1.trim().is_empty(), - "expected bottom padding on row 1: {row1:?}" + row1.contains("Working"), + "expected Working on row 1: {row1:?}" ); // Height=1 → no padding; single row is the spinner. diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 0e4bd85664..e3097c9162 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -105,6 +105,7 @@ pub(crate) struct ChatWidget { full_reasoning_buffer: String, session_id: Option, frame_requester: FrameRequester, + last_history_was_exec: bool, } struct UserMessage { @@ -376,6 +377,9 @@ impl ChatWidget { self.bottom_pane.set_task_running(false); self.task_complete_pending = false; } + // A completed stream indicates non-exec content was just inserted. + // Reset the exec header grouping so the next exec shows its header. + self.last_history_was_exec = false; self.flush_interrupt_queue(); } } @@ -401,6 +405,7 @@ impl ChatWidget { exit_code: ev.exit_code, stdout: ev.stdout.clone(), stderr: ev.stderr.clone(), + formatted_output: ev.formatted_output.clone(), }, )); @@ -408,9 +413,16 @@ impl ChatWidget { self.active_exec_cell = None; let pending = std::mem::take(&mut self.pending_exec_completions); for (command, parsed, output) in pending { - self.add_to_history(history_cell::new_completed_exec_command( - command, parsed, output, - )); + let include_header = !self.last_history_was_exec; + let cell = history_cell::new_completed_exec_command( + command, + parsed, + output, + include_header, + ev.duration, + ); + self.add_to_history(cell); + self.last_history_was_exec = true; } } } @@ -473,9 +485,11 @@ impl ChatWidget { exec.parsed.extend(ev.parsed_cmd); } _ => { + let include_header = !self.last_history_was_exec; self.active_exec_cell = Some(history_cell::new_active_exec_command( ev.command, ev.parsed_cmd, + include_header, )); } } @@ -565,6 +579,7 @@ impl ChatWidget { reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), session_id: None, + last_history_was_exec: false, } } @@ -713,13 +728,19 @@ impl ChatWidget { fn flush_active_exec_cell(&mut self) { if let Some(active) = self.active_exec_cell.take() { + self.last_history_was_exec = true; self.app_event_tx .send(AppEvent::InsertHistoryCell(Box::new(active))); } } fn add_to_history(&mut self, cell: impl HistoryCell + 'static) { + // Only break exec grouping if the cell renders visible lines. + let has_display_lines = !cell.display_lines().is_empty(); self.flush_active_exec_cell(); + if has_display_lines { + self.last_history_was_exec = false; + } self.app_event_tx .send(AppEvent::InsertHistoryCell(Box::new(cell))); } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index e43dfc7824..04dd3ad389 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -44,6 +44,31 @@ fn test_config() -> Config { .expect("config") } +// Backward-compat shim for older session logs that predate the +// `formatted_output` field on ExecCommandEnd events. +fn upgrade_event_payload_for_tests(mut payload: serde_json::Value) -> serde_json::Value { + if let Some(obj) = payload.as_object_mut() + && let Some(msg) = obj.get_mut("msg") + && let Some(m) = msg.as_object_mut() + { + let ty = m.get("type").and_then(|v| v.as_str()).unwrap_or(""); + if ty == "exec_command_end" && !m.contains_key("formatted_output") { + let stdout = m.get("stdout").and_then(|v| v.as_str()).unwrap_or(""); + let stderr = m.get("stderr").and_then(|v| v.as_str()).unwrap_or(""); + let formatted = if stderr.is_empty() { + stdout.to_string() + } else { + format!("{stdout}{stderr}") + }; + m.insert( + "formatted_output".to_string(), + serde_json::Value::String(formatted), + ); + } + } + payload +} + #[test] fn final_answer_without_newline_is_flushed_immediately() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); @@ -157,6 +182,7 @@ fn make_chatwidget_manual() -> ( full_reasoning_buffer: String::new(), session_id: None, frame_requester: crate::tui::FrameRequester::test_dummy(), + last_history_was_exec: false, }; (widget, rx, op_rx) } @@ -239,6 +265,7 @@ fn exec_history_cell_shows_working_then_completed() { stderr: String::new(), exit_code: 0, duration: std::time::Duration::from_millis(5), + formatted_output: "done".into(), }), }); @@ -250,8 +277,12 @@ fn exec_history_cell_shows_working_then_completed() { ); let blob = lines_to_single_string(&cells[0]); assert!( - blob.contains("Completed"), - "expected completed exec cell to show Completed header: {blob:?}" + blob.contains('✓'), + "expected completed exec cell to show success marker: {blob:?}" + ); + assert!( + blob.contains("echo done"), + "expected command text to be present: {blob:?}" ); } @@ -284,6 +315,7 @@ fn exec_history_cell_shows_working_then_failed() { stderr: "error".into(), exit_code: 2, duration: std::time::Duration::from_millis(7), + formatted_output: "".into(), }), }); @@ -295,11 +327,82 @@ fn exec_history_cell_shows_working_then_failed() { ); let blob = lines_to_single_string(&cells[0]); assert!( - blob.contains("Failed (exit 2)"), - "expected completed exec cell to show Failed header with exit code: {blob:?}" + blob.contains('✗'), + "expected failure marker present: {blob:?}" + ); + assert!( + blob.contains("false"), + "expected command text present: {blob:?}" ); } +#[test] +fn exec_history_extends_previous_when_consecutive() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + // First command + chat.handle_codex_event(Event { + id: "call-a".into(), + msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { + call_id: "call-a".into(), + command: vec!["bash".into(), "-lc".into(), "echo one".into()], + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + parsed_cmd: vec![ + codex_core::parse_command::ParsedCommand::Unknown { + cmd: "echo one".into(), + } + .into(), + ], + }), + }); + chat.handle_codex_event(Event { + id: "call-a".into(), + msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { + call_id: "call-a".into(), + stdout: "one".into(), + stderr: String::new(), + exit_code: 0, + duration: std::time::Duration::from_millis(5), + formatted_output: "one".into(), + }), + }); + let first_cells = drain_insert_history(&mut rx); + assert_eq!(first_cells.len(), 1, "first exec should insert history"); + + // Second command + chat.handle_codex_event(Event { + id: "call-b".into(), + msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { + call_id: "call-b".into(), + command: vec!["bash".into(), "-lc".into(), "echo two".into()], + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + parsed_cmd: vec![ + codex_core::parse_command::ParsedCommand::Unknown { + cmd: "echo two".into(), + } + .into(), + ], + }), + }); + chat.handle_codex_event(Event { + id: "call-b".into(), + msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { + call_id: "call-b".into(), + stdout: "two".into(), + stderr: String::new(), + exit_code: 0, + duration: std::time::Duration::from_millis(5), + formatted_output: "two".into(), + }), + }); + let second_cells = drain_insert_history(&mut rx); + assert_eq!(second_cells.len(), 1, "second exec should extend history"); + let first_blob = lines_to_single_string(&first_cells[0]); + let second_blob = lines_to_single_string(&second_cells[0]); + assert!(first_blob.contains('✓')); + assert!(second_blob.contains("echo two")); +} + #[tokio::test(flavor = "current_thread")] async fn binary_size_transcript_matches_ideal_fixture() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); @@ -340,7 +443,9 @@ async fn binary_size_transcript_matches_ideal_fixture() { match kind { "codex_event" => { if let Some(payload) = v.get("payload") { - let ev: Event = serde_json::from_value(payload.clone()).expect("parse"); + let ev: Event = + serde_json::from_value(upgrade_event_payload_for_tests(payload.clone())) + .expect("parse"); chat.handle_codex_event(ev); while let Ok(app_ev) = rx.try_recv() { match app_ev { diff --git a/codex-rs/tui/src/exec_command.rs b/codex-rs/tui/src/exec_command.rs index db3bf2fef4..1923fa3b69 100644 --- a/codex-rs/tui/src/exec_command.rs +++ b/codex-rs/tui/src/exec_command.rs @@ -9,13 +9,7 @@ pub(crate) fn escape_command(command: &[String]) -> String { pub(crate) fn strip_bash_lc_and_escape(command: &[String]) -> String { match command { - // exactly three items - [first, second, third] - // first two must be "bash", "-lc" - if first == "bash" && second == "-lc" => - { - third.clone() // borrow `third` - } + [first, second, third] if first == "bash" && second == "-lc" => third.clone(), _ => escape_command(command), } } diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 0cbb4888df..3e51ffd036 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -29,10 +29,10 @@ use ratatui::prelude::*; use ratatui::style::Color; use ratatui::style::Modifier; use ratatui::style::Style; +use ratatui::style::Stylize; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; -use shlex::try_join as shlex_try_join; use std::collections::HashMap; use std::io::Cursor; use std::path::PathBuf; @@ -46,6 +46,7 @@ pub(crate) struct CommandOutput { pub(crate) exit_code: i32, pub(crate) stdout: String, pub(crate) stderr: String, + pub(crate) formatted_output: String, } pub(crate) enum PatchEventType { @@ -104,6 +105,8 @@ pub(crate) struct ExecCell { pub(crate) parsed: Vec, pub(crate) output: Option, start_time: Option, + duration: Option, + include_header: bool, } impl HistoryCell for ExecCell { fn display_lines(&self) -> Vec> { @@ -112,15 +115,63 @@ impl HistoryCell for ExecCell { &self.parsed, self.output.as_ref(), self.start_time, + self.include_header, ) } + + fn transcript_lines(&self) -> Vec> { + let mut lines: Vec> = vec!["".into()]; + + let cmd_display = strip_bash_lc_and_escape(&self.command); + for (i, part) in cmd_display.lines().enumerate() { + if i == 0 { + lines.push(Line::from(vec!["$ ".magenta(), part.to_string().into()])); + } else { + lines.push(Line::from(vec![" ".into(), part.to_string().into()])); + } + } + + // Command output: include full stdout and stderr (no truncation) + if let Some(output) = self.output.as_ref() { + lines.extend(output.formatted_output.lines().map(ansi_escape_line)); + } + + if let Some(output) = self.output.as_ref() { + let duration = self + .duration + .map(format_duration) + .unwrap_or_else(|| "unknown".to_string()); + let mut result = 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 + } } impl WidgetRef for &ExecCell { fn render_ref(&self, area: Rect, buf: &mut Buffer) { + if area.height == 0 { + return; + } + let content_area = Rect { + x: area.x, + y: area.y, + width: area.width, + height: area.height, + }; Paragraph::new(Text::from(self.display_lines())) .wrap(Wrap { trim: false }) - .render(area, buf); + .render(content_area, buf); } } @@ -131,8 +182,8 @@ struct CompletedMcpToolCallWithImageOutput { impl HistoryCell for CompletedMcpToolCallWithImageOutput { fn display_lines(&self) -> Vec> { vec![ - Line::from("tool result (image output omitted)"), Line::from(""), + Line::from("tool result (image output omitted)"), ] } } @@ -179,6 +230,7 @@ pub(crate) fn new_session_info( }; let lines: Vec> = vec![ + Line::from(Span::from("")), Line::from(vec![ Span::raw(">_ ").dim(), Span::styled( @@ -194,17 +246,16 @@ pub(crate) fn new_session_info( Line::from(format!(" /status - {}", SlashCommand::Status.description()).dim()), Line::from(format!(" /approvals - {}", SlashCommand::Approvals.description()).dim()), Line::from(format!(" /model - {}", SlashCommand::Model.description()).dim()), - Line::from("".dim()), ]; PlainHistoryCell { lines } } else if config.model == model { PlainHistoryCell { lines: Vec::new() } } else { let lines = vec![ + Line::from(""), Line::from("model changed:".magenta().bold()), Line::from(format!("requested: {}", config.model)), Line::from(format!("used: {model}")), - Line::from(""), ]; PlainHistoryCell { lines } } @@ -212,9 +263,9 @@ pub(crate) fn new_session_info( pub(crate) fn new_user_prompt(message: String) -> PlainHistoryCell { let mut lines: Vec> = Vec::new(); + lines.push(Line::from("")); lines.push(Line::from("user".cyan().bold())); lines.extend(message.lines().map(|l| Line::from(l.to_string()))); - lines.push(Line::from("")); PlainHistoryCell { lines } } @@ -222,12 +273,15 @@ pub(crate) fn new_user_prompt(message: String) -> PlainHistoryCell { pub(crate) fn new_active_exec_command( command: Vec, parsed: Vec, + include_header: bool, ) -> ExecCell { ExecCell { command, parsed, output: None, start_time: Some(Instant::now()), + duration: None, + include_header, } } @@ -235,76 +289,61 @@ pub(crate) fn new_completed_exec_command( command: Vec, parsed: Vec, output: CommandOutput, + include_header: bool, + duration: Duration, ) -> ExecCell { ExecCell { command, parsed, output: Some(output), start_time: None, + duration: Some(duration), + include_header, } } -fn exec_duration(start: Instant) -> String { - format!("{}s", start.elapsed().as_secs()) -} - fn exec_command_lines( command: &[String], parsed: &[ParsedCommand], output: Option<&CommandOutput>, start_time: Option, + include_header: bool, ) -> Vec> { match parsed.is_empty() { - true => new_exec_command_generic(command, output, start_time), - false => new_parsed_command(command, parsed, output, start_time), + true => new_exec_command_generic(command, output, start_time, include_header), + false => new_parsed_command(command, parsed, output, start_time, include_header), } } fn new_parsed_command( - command: &[String], + _command: &[String], parsed_commands: &[ParsedCommand], output: Option<&CommandOutput>, start_time: Option, + include_header: bool, ) -> Vec> { let mut lines: Vec = Vec::new(); - match output { + // Leading spacer and header line above command list + if include_header { + lines.push(Line::from("")); + lines.push(Line::from(">_".magenta())); + } + + // Determine the leading status marker: spinner while running, ✓ on success, ✗ on failure. + let status_marker: Span<'static> = match output { None => { - let mut spans = vec!["⚙︎ Working".magenta().bold()]; - if let Some(st) = start_time { - let dur = exec_duration(st); - spans.push(format!(" • {dur}").dim()); - } - lines.push(Line::from(spans)); - } - Some(o) if o.exit_code == 0 => { - lines.push(Line::from(vec!["✓".green(), " Completed".into()])); - } - Some(o) => { - lines.push(Line::from(vec![ - "✗".red(), - format!(" Failed (exit {})", o.exit_code).into(), - ])); + // Animated braille spinner – choose frame based on elapsed time. + const FRAMES: &[char] = &['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; + let idx = start_time + .map(|st| ((st.elapsed().as_millis() / 100) as usize) % FRAMES.len()) + .unwrap_or(0); + let ch = FRAMES[idx]; + Span::raw(format!("{ch}")) } + Some(o) if o.exit_code == 0 => Span::styled("✓", Style::default().fg(Color::Green)), + Some(_) => Span::styled("✗", Style::default().fg(Color::Red)), }; - // Optionally include the complete, unaltered command from the model. - if std::env::var("SHOW_FULL_COMMANDS") - .map(|v| !v.is_empty()) - .unwrap_or(false) - { - let full_cmd = shlex_try_join(command.iter().map(|s| s.as_str())) - .unwrap_or_else(|_| command.join(" ")); - lines.push(Line::from(vec![ - Span::styled(" └ ", Style::default().add_modifier(Modifier::DIM)), - Span::styled( - full_cmd, - Style::default() - .add_modifier(Modifier::DIM) - .add_modifier(Modifier::ITALIC), - ), - ])); - } - - for (i, parsed) in parsed_commands.iter().enumerate() { + for parsed in parsed_commands.iter() { let text = match parsed { ParsedCommand::Read { name, .. } => format!("📖 {name}"), ParsedCommand::ListFiles { cmd, path } => match path { @@ -323,19 +362,25 @@ fn new_parsed_command( ParsedCommand::Unknown { cmd } => format!("⌨️ {cmd}"), ParsedCommand::Noop { cmd } => format!("🔄 {cmd}"), }; - - let first_prefix = if i == 0 { " └ " } else { " " }; + // Prefix: two spaces, marker, space. Continuations align under the text block. for (j, line_text) in text.lines().enumerate() { - let prefix = if j == 0 { first_prefix } else { " " }; - lines.push(Line::from(vec![ - Span::styled(prefix, Style::default().add_modifier(Modifier::DIM)), - line_text.to_string().dim(), - ])); + if j == 0 { + lines.push(Line::from(vec![ + " ".into(), + status_marker.clone(), + " ".into(), + line_text.to_string().light_blue(), + ])); + } else { + lines.push(Line::from(vec![ + " ".into(), + line_text.to_string().light_blue(), + ])); + } } } lines.extend(output_lines(output, true, false)); - lines.push(Line::from("")); lines } @@ -344,29 +389,44 @@ fn new_exec_command_generic( command: &[String], output: Option<&CommandOutput>, start_time: Option, + include_header: bool, ) -> Vec> { let mut lines: Vec> = Vec::new(); + // Leading spacer and header line above command list + if include_header { + lines.push(Line::from("")); + lines.push(Line::from(">_".magenta())); + } let command_escaped = strip_bash_lc_and_escape(command); - let mut cmd_lines = command_escaped.lines(); - if let Some(first) = cmd_lines.next() { - let mut spans: Vec = vec!["⚡ Running".magenta()]; - if let Some(st) = start_time { - let dur = exec_duration(st); - spans.push(format!(" • {dur}").dim()); + + // Determine marker: spinner while running, ✓/✗ when completed + let status_marker: Span<'static> = match output { + None => { + const FRAMES: &[char] = &['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; + let idx = start_time + .map(|st| ((st.elapsed().as_millis() / 100) as usize) % FRAMES.len()) + .unwrap_or(0); + let ch = FRAMES[idx]; + Span::raw(format!("{ch}")) } - spans.push(" ".into()); - spans.push(first.to_string().into()); - lines.push(Line::from(spans)); - } else { - let mut spans: Vec = vec!["⚡ Running".magenta()]; - if let Some(st) = start_time { - let dur = exec_duration(st); - spans.push(format!(" • {dur}").dim()); + Some(o) if o.exit_code == 0 => Span::styled("✓", Style::default().fg(Color::Green)), + Some(_) => Span::styled("✗", Style::default().fg(Color::Red)), + }; + + for (i, line) in command_escaped.lines().enumerate() { + if i == 0 { + lines.push(Line::from(vec![ + Span::raw(" "), + status_marker.clone(), + Span::raw(" "), + Span::raw(line.to_string()), + ])); + } else { + lines.push(Line::from(vec![ + Span::styled(" ", Style::default().add_modifier(Modifier::DIM)), + Span::raw(line.to_string()), + ])); } - lines.push(Line::from(spans)); - } - for cont in cmd_lines { - lines.push(Line::from(cont.to_string())); } lines.extend(output_lines(output, false, true)); @@ -377,9 +437,9 @@ fn new_exec_command_generic( pub(crate) fn new_active_mcp_tool_call(invocation: McpInvocation) -> PlainHistoryCell { let title_line = Line::from(vec!["tool".magenta(), " running...".dim()]); let lines: Vec = vec![ + Line::from(""), title_line, format_mcp_invocation(invocation.clone()), - Line::from(""), ]; PlainHistoryCell { lines } @@ -489,8 +549,6 @@ pub(crate) fn new_completed_mcp_tool_call( )); } } - - lines.push(Line::from("")); } Err(e) => { lines.push(Line::from(vec![ @@ -503,6 +561,8 @@ pub(crate) fn new_completed_mcp_tool_call( } }; + // Leading blank separator at the start of this cell + lines.insert(0, Line::from("")); Box::new(PlainHistoryCell { lines }) } @@ -512,6 +572,7 @@ pub(crate) fn new_status_output( session_id: &Option, ) -> PlainHistoryCell { let mut lines: Vec> = Vec::new(); + lines.push(Line::from("")); lines.push(Line::from("/status".magenta())); let config_entries = create_config_summary_entries(config); @@ -596,8 +657,6 @@ pub(crate) fn new_status_output( ])); } - lines.push(Line::from("")); - // 👤 Account (only if ChatGPT tokens exist), shown under the first block let auth_file = get_auth_file(&config.codex_home); if let Ok(auth) = try_read_auth_json(&auth_file) @@ -688,13 +747,13 @@ pub(crate) fn new_status_output( usage.blended_total().to_string().into(), ])); - lines.push(Line::from("")); PlainHistoryCell { lines } } /// Render a summary of configured MCP servers from the current `Config`. pub(crate) fn empty_mcp_output() -> PlainHistoryCell { let lines: Vec> = vec![ + Line::from(""), Line::from("/mcp".magenta()), Line::from(""), Line::from(vec!["🔌 ".into(), "MCP Tools".bold()]), @@ -709,7 +768,6 @@ pub(crate) fn empty_mcp_output() -> PlainHistoryCell { " to configure them.".into(), ]) .style(Style::default().add_modifier(Modifier::DIM)), - Line::from(""), ]; PlainHistoryCell { lines } @@ -782,7 +840,7 @@ pub(crate) fn new_mcp_tools_output( } pub(crate) fn new_error_event(message: String) -> PlainHistoryCell { - let lines: Vec> = vec![vec!["🖐 ".red().bold(), message.into()].into(), "".into()]; + let lines: Vec> = vec!["".into(), vec!["🖐 ".red().bold(), message.into()].into()]; PlainHistoryCell { lines } } @@ -797,6 +855,8 @@ pub(crate) fn new_plan_update(update: UpdatePlanArgs) -> PlainHistoryCell { let UpdatePlanArgs { explanation, plan } = update; let mut lines: Vec> = Vec::new(); + // Leading blank for separation + lines.push(Line::from("")); // Header with progress summary let total = plan.len(); let completed = plan @@ -887,8 +947,6 @@ pub(crate) fn new_plan_update(update: UpdatePlanArgs) -> PlainHistoryCell { } } - lines.push(Line::from("")); - PlainHistoryCell { lines } } @@ -908,16 +966,16 @@ pub(crate) fn new_patch_event( auto_approved: false, } => { let lines: Vec> = vec![ - Line::from("✏️ Applying patch".magenta().bold()), Line::from(""), + Line::from("✏️ Applying patch".magenta().bold()), ]; return PlainHistoryCell { lines }; } }; let mut lines: Vec> = create_diff_summary(title, &changes, event_type); - - lines.push(Line::from("")); + // Add leading blank separator for the cell + lines.insert(0, Line::from("")); PlainHistoryCell { lines } } @@ -934,14 +992,15 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell { exit_code: 1, stdout: String::new(), stderr, + formatted_output: String::new(), }), true, true, )); } - lines.push(Line::from("")); - + // Leading blank separator + lines.insert(0, Line::from("")); PlainHistoryCell { lines } } @@ -988,9 +1047,8 @@ pub(crate) fn new_patch_apply_success(stdout: String) -> PlainHistoryCell { lines.push(Line::from(format!("... +{remaining} lines")).dim()); } } - - lines.push(Line::from("")); - + // Leading blank separator + lines.insert(0, Line::from("")); PlainHistoryCell { lines } } @@ -999,9 +1057,9 @@ pub(crate) fn new_reasoning_block( config: &Config, ) -> TranscriptOnlyHistoryCell { let mut lines: Vec> = Vec::new(); + lines.push(Line::from("")); lines.push(Line::from("thinking".magenta().italic())); append_markdown(&full_reasoning_buffer, &mut lines, config); - lines.push(Line::from("")); TranscriptOnlyHistoryCell { lines } } @@ -1014,6 +1072,7 @@ fn output_lines( exit_code, stdout, stderr, + .. } = match output { Some(output) if only_err && output.exit_code == 0 => return vec![], Some(output) => output, @@ -1096,9 +1155,14 @@ mod tests { let parsed = vec![ParsedCommand::Unknown { cmd: "printf 'foo\nbar'".to_string(), }]; - let lines = exec_command_lines(&[], &parsed, None, None); - assert!(lines.len() >= 3); - assert_eq!(lines[1].spans[0].content, " └ "); - assert_eq!(lines[2].spans[0].content, " "); + let lines = exec_command_lines(&[], &parsed, None, None, true); + assert!(lines.len() >= 4); + // Leading spacer then header line + assert!(lines[0].spans.is_empty() || lines[0].spans[0].content.is_empty()); + assert_eq!(lines[1].spans[0].content, ">_"); + // First rendered command line starts with two-space + marker. + assert_eq!(lines[2].spans[0].content, " "); + // Continuation lines align under the text block. + assert_eq!(lines[3].spans[0].content, " "); } } diff --git a/codex-rs/tui/src/render/line_utils.rs b/codex-rs/tui/src/render/line_utils.rs index 7d11c54155..1cc9d5c95a 100644 --- a/codex-rs/tui/src/render/line_utils.rs +++ b/codex-rs/tui/src/render/line_utils.rs @@ -34,12 +34,3 @@ pub fn is_blank_line_spaces_only(line: &Line<'_>) -> bool { .iter() .all(|s| s.content.is_empty() || s.content.chars().all(|c| c == ' ')) } - -/// Consider a line blank if its spans are empty or all span contents are -/// whitespace when trimmed. -pub fn is_blank_line_trim(line: &Line<'_>) -> bool { - if line.spans.is_empty() { - return true; - } - line.spans.iter().all(|s| s.content.trim().is_empty()) -} diff --git a/codex-rs/tui/src/streaming/controller.rs b/codex-rs/tui/src/streaming/controller.rs index 5fb60e78a5..b9afa89392 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -70,17 +70,6 @@ impl StreamController { self.header.maybe_emit(out_lines) } - #[inline] - fn ensure_single_trailing_blank(lines: &mut Lines) { - if lines - .last() - .map(|l| !crate::render::line_utils::is_blank_line_trim(l)) - .unwrap_or(true) - { - lines.push(Line::from("")); - } - } - /// Begin an answer stream. Does not emit header yet; it is emitted on first commit. pub(crate) fn begin(&mut self, _sink: &impl HistorySink) { // Starting a new stream cancels any pending finish-from-previous-stream animation. @@ -138,7 +127,6 @@ impl StreamController { let mut lines_with_header: Lines = Vec::new(); self.emit_header_if_needed(&mut lines_with_header); lines_with_header.extend(out_lines); - Self::ensure_single_trailing_blank(&mut lines_with_header); sink.insert_history(lines_with_header); } diff --git a/codex-rs/tui/src/streaming/mod.rs b/codex-rs/tui/src/streaming/mod.rs index 1e670738f1..eaaeeff4be 100644 --- a/codex-rs/tui/src/streaming/mod.rs +++ b/codex-rs/tui/src/streaming/mod.rs @@ -64,6 +64,8 @@ impl HeaderEmitter { pub(crate) fn maybe_emit(&mut self, out_lines: &mut Vec>) -> bool { if !self.emitted_in_stream && !self.emitted_this_turn { + // Add a leading blank line before the header for visual spacing + out_lines.push(ratatui::text::Line::from("")); out_lines.push(render_header_line()); self.emitted_in_stream = true; self.emitted_this_turn = true; diff --git a/codex-rs/tui/src/user_approval_widget.rs b/codex-rs/tui/src/user_approval_widget.rs index deb2c8ff16..de22f12b7b 100644 --- a/codex-rs/tui/src/user_approval_widget.rs +++ b/codex-rs/tui/src/user_approval_widget.rs @@ -258,7 +258,7 @@ impl UserApprovalWidget { } fn send_decision_with_feedback(&mut self, decision: ReviewDecision, feedback: String) { - let mut lines: Vec> = Vec::new(); + let mut lines: Vec> = vec![Line::from("")]; match &self.approval_request { ApprovalRequest::Exec { command, .. } => { let cmd = strip_bash_lc_and_escape(command); @@ -327,7 +327,6 @@ impl UserApprovalWidget { lines.push(Line::from(l.to_string())); } } - lines.push(Line::from("")); self.app_event_tx.send(AppEvent::InsertHistoryLines(lines)); let op = match &self.approval_request { diff --git a/codex-rs/tui/tests/fixtures/ideal-binary-response.txt b/codex-rs/tui/tests/fixtures/ideal-binary-response.txt index 80aae5d0e4..62d6af570f 100644 --- a/codex-rs/tui/tests/fixtures/ideal-binary-response.txt +++ b/codex-rs/tui/tests/fixtures/ideal-binary-response.txt @@ -9,7 +9,8 @@ codex I’m going to scan the workspace and Cargo manifests to see build profiles and dependencies that impact binary size. Then I’ll summarize the main causes. -⚡ Running ls -la +>_ + ✓ ls -la └ total 6696 drwxr-xr-x@ 39 easong staff 1248 Aug 9 08:49 . drwxr-xr-x@ 29 easong staff 928 Aug 9 08:50 .. @@ -21,7 +22,7 @@ dependencies that impact binary size. Then I’ll summarize the main causes. drwxr-xr-x@ 6 easong staff 192 Aug 7 19:20 tui -rw-r--r--@ 1 easong staff 8639 Aug 6 23:30 typewriter.md -rw-r--r--@ 1 easong staff 7673 Aug 6 21:58 wrap-plan.md -⚡ Running sed -n '1,200p' Cargo.toml + ✓ sed -n '1,200p' Cargo.toml └ [workspace] members = [ "ansi-escape", @@ -34,7 +35,7 @@ dependencies that impact binary size. Then I’ll summarize the main causes. # ratatui = { path = "../../ratatui" } ratatui = { git = "https://github.com/nornagon/ratatui", branch = "nornagon-v0.29.0-patch" } -⚡ Running for d in ansi-escape apply-patch arg0 cli common core exec execpolicy + ✓ for d in ansi-escape apply-patch arg0 cli common core exec execpolicy file-search linux-sandbox login mcp-client mcp-server mcp-types ollama tui; do echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done └ --- ansi-escape/Cargo.toml @@ -48,7 +49,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done "signal", ] } tracing = { version -⚡ Running sed -n '1,200p' execpolicy/Cargo.toml + ✓ sed -n '1,200p' execpolicy/Cargo.toml └ [package] name = "codex-execpolicy" version = { workspace = true } @@ -60,7 +61,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done [dev-dependencies] tempfile = "3.13.0" -⚡ Running sed -n '1,200p' file-search/Cargo.toml + ✓ sed -n '1,200p' file-search/Cargo.toml └ [package] edition = "2024" name = "codex-file-search" @@ -72,7 +73,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done serde = { version = "1", features = ["derive"] } serde_json = "1.0.142" tokio = { version = "1", features = ["full"] } -⚡ Running sed -n '1,200p' linux-sandbox/Cargo.toml + ✓ sed -n '1,200p' linux-sandbox/Cargo.toml └ [package] edition = "2024" name = "codex-linux-sandbox" @@ -84,7 +85,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done "rt-multi-thread", "signal", ] } -⚡ Running sed -n '1,200p' login/Cargo.toml + ✓ sed -n '1,200p' login/Cargo.toml └ [package] edition = "2024" name = "codex-login" @@ -96,7 +97,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done [dev-dependencies] pretty_assertions = "1.4.1" tempfile = "3" -⚡ Running sed -n '1,200p' mcp-client/Cargo.toml + ✓ sed -n '1,200p' mcp-client/Cargo.toml └ [package] name = "codex-mcp-client" version = { workspace = true } @@ -108,7 +109,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done "sync", "time", ] } -⚡ Running sed -n '1,200p' mcp-server/Cargo.toml + ✓ sed -n '1,200p' mcp-server/Cargo.toml └ [package] edition = "2024" name = "codex-mcp-server" @@ -120,7 +121,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done tempfile = "3" tokio-test = "0.4" wiremock = "0.6" -⚡ Running sed -n '1,200p' mcp-types/Cargo.toml + ✓ sed -n '1,200p' mcp-types/Cargo.toml └ [package] edition = "2024" name = "mcp-types" @@ -132,7 +133,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done [dependencies] serde = { version = "1", features = ["derive"] } serde_json = "1" -⚡ Running sed -n '1,200p' ollama/Cargo.toml + ✓ sed -n '1,200p' ollama/Cargo.toml └ [package] edition = "2024" name = "codex-ollama" @@ -144,7 +145,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done [dev-dependencies] tempfile = "3" -⚡ Running sed -n '1,200p' tui/Cargo.toml + ✓ sed -n '1,200p' tui/Cargo.toml └ [package] edition = "2024" name = "codex-tui" @@ -156,7 +157,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done pretty_assertions = "1" rand = "0.8" vt100 = "0.16.2" -⚡ Running sed -n '1,200p' chatgpt/Cargo.toml + ✓ sed -n '1,200p' chatgpt/Cargo.toml └ [package] edition = "2024" name = "codex-chatgpt" @@ -168,6 +169,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done [dev-dependencies] tempfile = "3" + codex Here’s what’s driving size in this workspace’s binaries.