diff --git a/codex-rs/tui/src/chatwidget/tests/history_replay.rs b/codex-rs/tui/src/chatwidget/tests/history_replay.rs index c48d188ad2..ef4208b9fc 100644 --- a/codex-rs/tui/src/chatwidget/tests/history_replay.rs +++ b/codex-rs/tui/src/chatwidget/tests/history_replay.rs @@ -1361,7 +1361,7 @@ async fn failed_repl_mcp_tool_call_preserves_status_and_result() { └ Script failed {"exit_code": 0, "output": "ready", "chunk_id": "chunk-1"} Script error: - permission denied + +1 line (ctrl + t to view transcript) "#); } assert_eq!( diff --git a/codex-rs/tui/src/exec_cell/render.rs b/codex-rs/tui/src/exec_cell/render.rs index 13574fe7fa..5867513a0b 100644 --- a/codex-rs/tui/src/exec_cell/render.rs +++ b/codex-rs/tui/src/exec_cell/render.rs @@ -13,6 +13,7 @@ use crate::motion::activity_indicator; use crate::render::highlight::highlight_bash_to_lines; use crate::render::line_utils::prefix_lines; use crate::render::line_utils::push_owned_lines; +use crate::tool_output::tool_output_preview; use crate::ui_consts::TRANSCRIPT_HINT; use crate::wrapping::RtOptions; use crate::wrapping::adaptive_wrap_line; @@ -130,7 +131,7 @@ pub(crate) fn output_lines( let head_end = total.min(line_limit).min(retained); for (i, raw) in output.lines().take(head_end).enumerate() { - let mut line = ansi_escape_line(raw.as_ref()); + let mut line = dimmed_output_line(raw.as_ref()); let prefix = if !include_prefix { "" } else if i == 0 && include_angle_pipe { @@ -138,10 +139,7 @@ pub(crate) fn output_lines( } else { " " }; - line.spans.insert(0, prefix.into()); - line.spans.iter_mut().for_each(|span| { - span.style = span.style.add_modifier(Modifier::DIM); - }); + line.spans.insert(0, prefix.dim()); out.push(line); } @@ -157,13 +155,10 @@ pub(crate) fn output_lines( let tail = output.lines().rev().take(tail_len).collect_vec(); for raw in tail.into_iter().rev() { - let mut line = ansi_escape_line(raw.as_ref()); + let mut line = dimmed_output_line(raw.as_ref()); if include_prefix { - line.spans.insert(0, " ".into()); + line.spans.insert(0, " ".dim()); } - line.spans.iter_mut().for_each(|span| { - span.style = span.style.add_modifier(Modifier::DIM); - }); out.push(line); } @@ -173,6 +168,23 @@ pub(crate) fn output_lines( } } +fn dimmed_output_line(raw: &str) -> Line<'static> { + let mut line = ansi_escape_line(raw); + for span in &mut line.spans { + span.style = span.style.add_modifier(Modifier::DIM); + } + line +} + +fn output_preview_lines(output: &CommandOutput, width: usize) -> Vec> { + let (total, _) = output.line_counts(); + tool_output_preview( + output.lines().map(|raw| dimmed_output_line(raw.as_ref())), + width, + total, + ) +} + fn activity_marker(start_time: Option, animations_enabled: bool) -> Span<'static> { activity_indicator( start_time, @@ -419,25 +431,29 @@ impl ExecCell { } if let Some(output) = call.output.as_ref() { - let line_limit = if call.is_user_shell_command() { - USER_SHELL_TOOL_CALL_MAX_LINES - } else { - TOOL_CALL_MAX_LINES - }; + if !call.is_user_shell_command() { + let preview = output_preview_lines(output, layout.output_block.wrap_width(width)); + let preview = if preview.is_empty() && !call.is_unified_exec_interaction() { + vec![Line::from("(no output)".dim())] + } else { + preview + }; + lines.extend(prefix_lines( + preview, + Span::from(layout.output_block.initial_prefix).dim(), + Span::from(layout.output_block.subsequent_prefix), + )); + return lines; + } let raw_output = output_lines( Some(output), OutputLinesParams { - line_limit, + line_limit: USER_SHELL_TOOL_CALL_MAX_LINES, only_err: false, include_angle_pipe: false, include_prefix: false, }, ); - let display_limit = if call.is_user_shell_command() { - USER_SHELL_TOOL_CALL_MAX_LINES - } else { - layout.output_max_lines - }; if raw_output.lines.is_empty() { if !call.is_unified_exec_interaction() { @@ -469,7 +485,7 @@ impl ExecCell { ); let trimmed_output = Self::truncate_lines_middle( &prefixed_output, - display_limit, + USER_SHELL_TOOL_CALL_MAX_LINES, width, raw_output.omitted, Some(Line::from( @@ -663,7 +679,6 @@ struct ExecDisplayLayout { command_continuation: PrefixedBlock, command_continuation_max_lines: usize, output_block: PrefixedBlock, - output_max_lines: usize, } impl ExecDisplayLayout { @@ -671,13 +686,11 @@ impl ExecDisplayLayout { command_continuation: PrefixedBlock, command_continuation_max_lines: usize, output_block: PrefixedBlock, - output_max_lines: usize, ) -> Self { Self { command_continuation, command_continuation_max_lines, output_block, - output_max_lines, } } } @@ -686,7 +699,6 @@ const EXEC_DISPLAY_LAYOUT: ExecDisplayLayout = ExecDisplayLayout::new( PrefixedBlock::new(" │ ", " │ "), /*command_continuation_max_lines*/ 2, PrefixedBlock::new(" └ ", " "), - /*output_max_lines*/ 5, ); #[cfg(test)] diff --git a/codex-rs/tui/src/history_cell/mcp.rs b/codex-rs/tui/src/history_cell/mcp.rs index a560a6b14c..58bc7d44c5 100644 --- a/codex-rs/tui/src/history_cell/mcp.rs +++ b/codex-rs/tui/src/history_cell/mcp.rs @@ -1,5 +1,5 @@ //! MCP tool-call, inventory, and output history cells. -//! Code-mode output previews share a rendered-row budget across all result blocks; +//! Tool output previews share a three-row budget across all result blocks; //! the expanded transcript retains the full text. use super::*; @@ -17,9 +17,12 @@ pub(crate) use computer_activity::ComputerActivityCell; use crate::style::StatusTone; use crate::style::accent_style; use crate::style::status_style; +use crate::text_formatting::format_json_compact; +use crate::tool_output::ToolOutputPreview; use codex_app_server_protocol::McpServerConnectionStatus; use result::McpResultKind; use result::McpToolResult; +use std::borrow::Cow; #[cfg(test)] #[path = "mcp_tests.rs"] @@ -203,6 +206,7 @@ impl McpToolCallCell { let mut detail_lines: Vec> = Vec::new(); // Reserve four columns for the tree prefix (" └ "/" ") and ensure the wrapper still has at least one cell to work with. let detail_wrap_width = (width as usize).saturating_sub(4).max(1); + let mut preview = ToolOutputPreview::new(detail_wrap_width, /*omitted*/ 0); if let Some(result) = &self.result { match result { @@ -212,55 +216,59 @@ impl McpToolCallCell { // Code-mode image blocks can carry text as well. Preserve that text // without losing the image indication when it replaces the summary. if node_repl && block.is_image && block.text().is_some() { - detail_lines.extend( - textwrap::wrap("Returned image", detail_wrap_width) - .into_iter() - .map(|line| Line::from(line.into_owned().dim())), - ); + if mode == McpToolCallRenderMode::Display { + preview.push_line(Line::from("Returned image".dim())); + } else { + detail_lines.extend( + textwrap::wrap("Returned image", detail_wrap_width) + .into_iter() + .map(|line| Line::from(line.into_owned().dim())), + ); + } } let text = if compact && status == Some(true) { let meaningful_output = block.text().and_then(|text| { if text.starts_with("Script completed\n") { return text .split_once("\nOutput:\n") - .map(|(_, output)| output.to_string()); + .map(|(_, output)| Cow::Borrowed(output)); } serde_json::from_str::(text) .ok() .filter(|output| output.exit_code == 0) - .map(|output| output.output) + .map(|output| Cow::Owned(output.output)) }); match meaningful_output { Some(output) if output.is_empty() => continue, - Some(output) => format_and_truncate_tool_result( - &output, - TOOL_CALL_MAX_LINES, - detail_wrap_width, - ), + Some(output) => format_json_compact(&output) + .map(Cow::Owned) + .unwrap_or(output), None => block.text().map_or_else( - || block.render(detail_wrap_width), + || block.render(), |text| { - format_and_truncate_tool_result( - text, - TOOL_CALL_MAX_LINES, - detail_wrap_width, - ) + format_json_compact(text) + .map(Cow::Owned) + .unwrap_or(Cow::Borrowed(text)) }, ), } } else if node_repl && let Some(output) = block.text() { if mode == McpToolCallRenderMode::Transcript { - output.trim_end_matches('\n').to_string() + Cow::Borrowed(output.trim_end_matches('\n')) } else { - format_and_truncate_tool_result( - output, - TOOL_CALL_MAX_LINES, - detail_wrap_width, - ) + format_json_compact(output) + .map(Cow::Owned) + .unwrap_or(Cow::Borrowed(output)) } } else { - block.render(detail_wrap_width) + block.render() }; + if mode == McpToolCallRenderMode::Display { + for line in text.lines() { + preview.push_line(Line::from(line.dim())); + } + continue; + } for segment in text.split('\n') { let line = Line::from(segment.to_string().dim()); let wrapped = adaptive_wrap_line( @@ -276,43 +284,25 @@ impl McpToolCallCell { } Err(err) => { let err_text = format!("Error: {err}"); - let err_text = if node_repl && mode == McpToolCallRenderMode::Transcript { - err_text + if mode == McpToolCallRenderMode::Display { + for line in err_text.lines() { + preview.push_line(Line::from(line.dim())); + } } else { - format_and_truncate_tool_result( - &err_text, - TOOL_CALL_MAX_LINES, - width as usize, - ) - }; - let err_line = Line::from(err_text.dim()); - let wrapped = adaptive_wrap_line( - &err_line, - RtOptions::new(detail_wrap_width) - .initial_indent("".into()) - .subsequent_indent(" ".into()), - ); - detail_lines.extend(wrapped.iter().map(line_to_static)); + let err_line = Line::from(err_text.dim()); + let wrapped = adaptive_wrap_line( + &err_line, + RtOptions::new(detail_wrap_width).subsequent_indent(" ".into()), + ); + detail_lines.extend(wrapped.iter().map(line_to_static)); + } } } } - if compact { - // Adaptive wrapping keeps URLs intact; split overlong preview rows before counting - // them so a URL cannot exceed the budget. The transcript keeps the original URL. - detail_lines = crate::wrapping::word_wrap_lines(detail_lines, detail_wrap_width); - if detail_lines.len() > TOOL_CALL_MAX_LINES { - // Retain the tail so stdout cannot crowd out a trailing failure diagnostic. - let tail = detail_lines.split_off(detail_lines.len() - TOOL_CALL_MAX_LINES / 2); - detail_lines.truncate((TOOL_CALL_MAX_LINES - 1) / 2); - detail_lines.push(crate::line_truncation::truncate_line_to_width( - "… more · ctrl+t".dim().into(), - detail_wrap_width, - )); - detail_lines.extend(tail); - } + if mode == McpToolCallRenderMode::Display { + detail_lines = preview.finish(); } - if !detail_lines.is_empty() { let initial_prefix: Span<'static> = if inline_invocation { " └ ".dim() @@ -350,7 +340,7 @@ impl HistoryCell for McpToolCallCell { match result { Ok(McpToolResult { content, .. }) => { for block in content { - let text = block.render(RAW_TOOL_OUTPUT_WIDTH); + let text = block.render(); lines.extend(raw_lines_from_source(&text)); } } diff --git a/codex-rs/tui/src/history_cell/mcp_result.rs b/codex-rs/tui/src/history_cell/mcp_result.rs index e3102f5a8c..cc8408c516 100644 --- a/codex-rs/tui/src/history_cell/mcp_result.rs +++ b/codex-rs/tui/src/history_cell/mcp_result.rs @@ -3,8 +3,7 @@ //! The wire result can contain multi-megabyte image, audio, or resource bodies. Validate each //! block once with the MCP model, then retain only what history rendering actually displays. -use crate::exec_cell::TOOL_CALL_MAX_LINES; -use crate::text_formatting::format_and_truncate_tool_result; +use crate::text_formatting::format_json_compact; use base64::Engine; use codex_protocol::mcp::CallToolResult; use image::DynamicImage; @@ -127,14 +126,16 @@ impl McpContentBlock { } } - /// Applies width-dependent formatting to full text or fallback JSON without reparsing the MCP - /// block. Media and resource summaries retain their existing untruncated display form. - pub(super) fn render(&self, width: usize) -> String { + /// Formats the complete text or fallback JSON. The caller applies a single preview limit + /// across all blocks, while transcript and raw output retain the full result. + pub(super) fn render(&self) -> Cow<'_, str> { match &self.display { McpContentDisplay::Text(text) | McpContentDisplay::Json(text) => { - format_and_truncate_tool_result(text, TOOL_CALL_MAX_LINES, width) + format_json_compact(text) + .map(Cow::Owned) + .unwrap_or(Cow::Borrowed(text)) } - McpContentDisplay::Summary(summary) => summary.to_string(), + McpContentDisplay::Summary(summary) => Cow::Borrowed(summary), } } } diff --git a/codex-rs/tui/src/history_cell/mcp_tests.rs b/codex-rs/tui/src/history_cell/mcp_tests.rs index 27dc0f5149..2b1f6fccd1 100644 --- a/codex-rs/tui/src/history_cell/mcp_tests.rs +++ b/codex-rs/tui/src/history_cell/mcp_tests.rs @@ -133,6 +133,55 @@ fn result(content: Vec) -> CallToolResult { } } +#[test] +fn mcp_preview_shares_one_limit_across_blocks_and_preserves_transcript() { + let mut cell = new_active_mcp_tool_call( + "call-preview".into(), + McpInvocation { + server: "search".into(), + tool: "lookup".into(), + arguments: None, + }, + /*animations_enabled*/ false, + ); + cell.complete( + Duration::ZERO, + Ok(result(vec![ + json!({"type": "text", "text": "first\nsecond"}), + json!({"type": "text", "text": "third\nfourth\nfifth"}), + ])), + ); + let display = cell.display_lines(/*width*/ 80); + let transcript = cell.transcript_lines(/*width*/ 80); + insta::assert_snapshot!(format!( + "history:\n{}\n\ntranscript:\n{}", + display + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n"), + transcript + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n"), + )); + assert_eq!( + cell.raw_lines() + .iter() + .map(ToString::to_string) + .collect::>(), + [ + "Called search.lookup()", + "first", + "second", + "third", + "fourth", + "fifth" + ], + ); +} + #[test] fn code_mode_output_shares_a_row_budget_across_blocks() { let mut cell = new_active_mcp_tool_call( @@ -159,13 +208,12 @@ fn code_mode_output_shares_a_row_budget_across_blocks() { .map(ToString::to_string) .collect::>() .join("\n"); - insta::assert_snapshot!(display, @" + insta::assert_snapshot!(display, @r" • Inspect page └ Page title Navigation - … more · ctrl+t - Link - Footer + Main content + +3 lines (ctrl + t to view transcri… "); let transcript = cell .transcript_lines(/*width*/ 100) @@ -178,7 +226,7 @@ fn code_mode_output_shares_a_row_budget_across_blocks() { } #[test] -fn code_mode_output_keeps_trailing_failure_diagnostics() { +fn code_mode_output_preserves_trailing_failure_diagnostics_in_transcript() { let mut cell = new_active_mcp_tool_call( "browser-error".to_string(), McpInvocation { @@ -205,14 +253,20 @@ fn code_mode_output_keeps_trailing_failure_diagnostics() { .map(ToString::to_string) .collect::>() .join("\n"); - insta::assert_snapshot!(display, @" + insta::assert_snapshot!(display, @r" • Inspect page └ Script failed Page title - … more · ctrl+t - Script error: - permission denied + Navigation + +6 lines (ctrl + t to view transcri… "); + let transcript = cell + .transcript_lines(/*width*/ 80) + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n"); + assert!(transcript.ends_with(" Script error:\n permission denied")); } #[test] @@ -243,13 +297,13 @@ fn code_mode_output_row_budget_applies_after_wrapping_and_to_errors() { cell.complete(Duration::ZERO, completion); for width in [20, 40, 80] { let display = cell.display_lines(width); - assert_eq!(display.len(), 1 + TOOL_CALL_MAX_LINES); + assert_eq!(display.len(), 5); // Header, three output rows, and omission hint. assert!( display .iter() .all(|line| line.width() <= usize::from(width)) ); - assert_eq!(display[3].to_string(), " … more · ctrl+t"); + assert!(display.last().unwrap().to_string().starts_with(" +")); let transcript = cell .transcript_lines(width) .iter() @@ -269,7 +323,7 @@ fn code_mode_output_row_budget_applies_after_wrapping_and_to_errors() { } #[test] -fn projected_content_preserves_width_dependent_rendering() { +fn projected_content_preserves_full_rendering() { let text = "{\"result\": [1, 2, 3], \"text\": \"long output 🦀\"}"; let malformed = json!({"type": "image", "data": PNG}); let invalid_metadata = @@ -290,28 +344,25 @@ fn projected_content_preserves_width_dependent_rendering() { McpResultKind::Standard, ); - for width in [1, 8, 40, 120, RAW_TOOL_OUTPUT_WIDTH] { - let format_text = - |text: &str| format_and_truncate_tool_result(text, TOOL_CALL_MAX_LINES, width); - assert_eq!( - projected - .content - .iter() - .map(|block| block.render(width)) - .collect::>(), - vec![ - format_text(text), - "Returned image".to_string(), - "