From 6032d784eead8d82e2fbbef957fb2f04fa74684d Mon Sep 17 00:00:00 2001
From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com>
Date: Wed, 24 Sep 2025 13:36:01 -0700
Subject: [PATCH] improve MCP tool call styling (#3871)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

<img width="760" height="213" alt="Screenshot 2025-09-18 at 12 29 15 PM"
src="https://github.com/user-attachments/assets/48a205b7-b95a-4988-8c76-efceb998dee7"
/>
---
 codex-rs/tui/src/chatwidget.rs                | 151 +++---
 codex-rs/tui/src/chatwidget/tests.rs          |   6 +-
 codex-rs/tui/src/history_cell.rs              | 456 ++++++++++++++----
 ..._tests__active_mcp_tool_call_snapshot.snap |   6 +
 ...ompleted_mcp_tool_call_error_snapshot.snap |   7 +
 ...call_multiple_outputs_inline_snapshot.snap |   8 +
 ...p_tool_call_multiple_outputs_snapshot.snap |  11 +
 ...pleted_mcp_tool_call_success_snapshot.snap |   7 +
 ...cp_tool_call_wrapped_outputs_snapshot.snap |  14 +
 9 files changed, 523 insertions(+), 143 deletions(-)
 create mode 100644 codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__active_mcp_tool_call_snapshot.snap
 create mode 100644 codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_error_snapshot.snap
 create mode 100644 codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_multiple_outputs_inline_snapshot.snap
 create mode 100644 codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_multiple_outputs_snapshot.snap
 create mode 100644 codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_success_snapshot.snap
 create mode 100644 codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_wrapped_outputs_snapshot.snap

diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs
index 3ac33534..ec749f4b 100644
--- a/codex-rs/tui/src/chatwidget.rs
+++ b/codex-rs/tui/src/chatwidget.rs
@@ -76,6 +76,7 @@ use crate::history_cell::AgentMessageCell;
 use crate::history_cell::CommandOutput;
 use crate::history_cell::ExecCell;
 use crate::history_cell::HistoryCell;
+use crate::history_cell::McpToolCallCell;
 use crate::history_cell::PatchEventType;
 use crate::history_cell::RateLimitSnapshotDisplay;
 use crate::markdown::append_markdown;
@@ -191,7 +192,7 @@ pub(crate) struct ChatWidget {
     app_event_tx: AppEventSender,
     codex_op_tx: UnboundedSender<Op>,
     bottom_pane: BottomPane,
-    active_exec_cell: Option<ExecCell>,
+    active_cell: Option<Box<dyn HistoryCell>>,
     config: Config,
     auth_manager: Arc<AuthManager>,
     session_header: SessionHeader,
@@ -395,7 +396,7 @@ impl ChatWidget {
     /// Finalize any active exec as failed and stop/clear running UI state.
     fn finalize_turn(&mut self) {
         // Ensure any spinner is replaced by a red ✗ and flushed into history.
-        self.finalize_active_exec_cell_as_failed();
+        self.finalize_active_cell_as_failed();
         // Reset running state and clear streaming buffers.
         self.bottom_pane.set_task_running(false);
         self.running_commands.clear();
@@ -601,7 +602,7 @@ impl ChatWidget {
     #[inline]
     fn handle_streaming_delta(&mut self, delta: String) {
         // Before streaming agent content, flush any active exec cell group.
-        self.flush_active_exec_cell();
+        self.flush_active_cell();
 
         if self.stream_controller.is_none() {
             self.stream_controller = Some(StreamController::new(self.config.clone()));
@@ -621,16 +622,25 @@ impl ChatWidget {
             None => (vec![ev.call_id.clone()], Vec::new()),
         };
 
-        if self.active_exec_cell.is_none() {
-            // This should have been created by handle_exec_begin_now, but in case it wasn't,
-            // create it now.
-            self.active_exec_cell = Some(history_cell::new_active_exec_command(
+        let needs_new = self
+            .active_cell
+            .as_ref()
+            .map(|cell| cell.as_any().downcast_ref::<ExecCell>().is_none())
+            .unwrap_or(true);
+        if needs_new {
+            self.flush_active_cell();
+            self.active_cell = Some(Box::new(history_cell::new_active_exec_command(
                 ev.call_id.clone(),
                 command,
                 parsed,
-            ));
+            )));
         }
-        if let Some(cell) = self.active_exec_cell.as_mut() {
+
+        if let Some(cell) = self
+            .active_cell
+            .as_mut()
+            .and_then(|c| c.as_any_mut().downcast_mut::<ExecCell>())
+        {
             cell.complete_call(
                 &ev.call_id,
                 CommandOutput {
@@ -642,7 +652,7 @@ impl ChatWidget {
                 ev.duration,
             );
             if cell.should_flush() {
-                self.flush_active_exec_cell();
+                self.flush_active_cell();
             }
         }
     }
@@ -709,50 +719,68 @@ impl ChatWidget {
                 parsed_cmd: ev.parsed_cmd.clone(),
             },
         );
-        if let Some(exec) = &self.active_exec_cell {
-            if let Some(new_exec) = exec.with_added_call(
+        if let Some(cell) = self
+            .active_cell
+            .as_mut()
+            .and_then(|c| c.as_any_mut().downcast_mut::<ExecCell>())
+            && let Some(new_exec) = cell.with_added_call(
                 ev.call_id.clone(),
                 ev.command.clone(),
                 ev.parsed_cmd.clone(),
-            ) {
-                self.active_exec_cell = Some(new_exec);
-            } else {
-                // Make a new cell.
-                self.flush_active_exec_cell();
-                self.active_exec_cell = Some(history_cell::new_active_exec_command(
-                    ev.call_id.clone(),
-                    ev.command.clone(),
-                    ev.parsed_cmd,
-                ));
-            }
+            )
+        {
+            *cell = new_exec;
         } else {
-            self.active_exec_cell = Some(history_cell::new_active_exec_command(
+            self.flush_active_cell();
+
+            self.active_cell = Some(Box::new(history_cell::new_active_exec_command(
                 ev.call_id.clone(),
                 ev.command.clone(),
                 ev.parsed_cmd,
-            ));
+            )));
         }
 
-        // Request a redraw so the working header and command list are visible immediately.
         self.request_redraw();
     }
 
     pub(crate) fn handle_mcp_begin_now(&mut self, ev: McpToolCallBeginEvent) {
         self.flush_answer_stream_with_separator();
-        self.add_to_history(history_cell::new_active_mcp_tool_call(ev.invocation));
+        self.flush_active_cell();
+        self.active_cell = Some(Box::new(history_cell::new_active_mcp_tool_call(
+            ev.call_id,
+            ev.invocation,
+        )));
+        self.request_redraw();
     }
     pub(crate) fn handle_mcp_end_now(&mut self, ev: McpToolCallEndEvent) {
         self.flush_answer_stream_with_separator();
-        self.add_boxed_history(history_cell::new_completed_mcp_tool_call(
-            80,
-            ev.invocation,
-            ev.duration,
-            ev.result
-                .as_ref()
-                .map(|r| !r.is_error.unwrap_or(false))
-                .unwrap_or(false),
-            ev.result,
-        ));
+
+        let McpToolCallEndEvent {
+            call_id,
+            invocation,
+            duration,
+            result,
+        } = ev;
+
+        let extra_cell = match self
+            .active_cell
+            .as_mut()
+            .and_then(|cell| cell.as_any_mut().downcast_mut::<McpToolCallCell>())
+        {
+            Some(cell) if cell.call_id() == call_id => cell.complete(duration, result),
+            _ => {
+                self.flush_active_cell();
+                let mut cell = history_cell::new_active_mcp_tool_call(call_id, invocation);
+                let extra_cell = cell.complete(duration, result);
+                self.active_cell = Some(Box::new(cell));
+                extra_cell
+            }
+        };
+
+        self.flush_active_cell();
+        if let Some(extra) = extra_cell {
+            self.add_boxed_history(extra);
+        }
     }
 
     fn layout_areas(&self, area: Rect) -> [Rect; 3] {
@@ -760,7 +788,7 @@ impl ChatWidget {
         let remaining = area.height.saturating_sub(bottom_min);
 
         let active_desired = self
-            .active_exec_cell
+            .active_cell
             .as_ref()
             .map_or(0, |c| c.desired_height(area.width) + 1);
         let active_height = active_desired.min(remaining);
@@ -805,7 +833,7 @@ impl ChatWidget {
                 placeholder_text: placeholder,
                 disable_paste_burst: config.disable_paste_burst,
             }),
-            active_exec_cell: None,
+            active_cell: None,
             config: config.clone(),
             auth_manager,
             session_header: SessionHeader::new(config.model),
@@ -866,7 +894,7 @@ impl ChatWidget {
                 placeholder_text: placeholder,
                 disable_paste_burst: config.disable_paste_burst,
             }),
-            active_exec_cell: None,
+            active_cell: None,
             config: config.clone(),
             auth_manager,
             session_header: SessionHeader::new(config.model),
@@ -897,7 +925,7 @@ impl ChatWidget {
     pub fn desired_height(&self, width: u16) -> u16 {
         self.bottom_pane.desired_height(width)
             + self
-                .active_exec_cell
+                .active_cell
                 .as_ref()
                 .map_or(0, |c| c.desired_height(width) + 1)
     }
@@ -1115,10 +1143,9 @@ impl ChatWidget {
         }
     }
 
-    fn flush_active_exec_cell(&mut self) {
-        if let Some(active) = self.active_exec_cell.take() {
-            self.app_event_tx
-                .send(AppEvent::InsertHistoryCell(Box::new(active)));
+    fn flush_active_cell(&mut self) {
+        if let Some(active) = self.active_cell.take() {
+            self.app_event_tx.send(AppEvent::InsertHistoryCell(active));
         }
     }
 
@@ -1129,7 +1156,7 @@ impl ChatWidget {
     fn add_boxed_history(&mut self, cell: Box<dyn HistoryCell>) {
         if !cell.display_lines(u16::MAX).is_empty() {
             // Only break exec grouping if the cell renders visible lines.
-            self.flush_active_exec_cell();
+            self.flush_active_cell();
         }
         self.app_event_tx.send(AppEvent::InsertHistoryCell(cell));
     }
@@ -1350,7 +1377,7 @@ impl ChatWidget {
         if let Some(output) = review.review_output {
             self.flush_answer_stream_with_separator();
             self.flush_interrupt_queue();
-            self.flush_active_exec_cell();
+            self.flush_active_cell();
 
             if output.findings.is_empty() {
                 let explanation = output.overall_explanation.trim().to_string();
@@ -1419,12 +1446,16 @@ impl ChatWidget {
         }
     }
 
-    /// Mark the active exec cell as failed (✗) and flush it into history.
-    fn finalize_active_exec_cell_as_failed(&mut self) {
-        if let Some(cell) = self.active_exec_cell.take() {
-            let cell = cell.into_failed();
-            // Insert finalized exec into history and keep grouping consistent.
-            self.add_to_history(cell);
+    /// Mark the active cell as failed (✗) and flush it into history.
+    fn finalize_active_cell_as_failed(&mut self) {
+        if let Some(mut cell) = self.active_cell.take() {
+            // Insert finalized cell into history and keep grouping consistent.
+            if let Some(exec) = cell.as_any_mut().downcast_mut::<ExecCell>() {
+                exec.mark_failed();
+            } else if let Some(tool) = cell.as_any_mut().downcast_mut::<McpToolCallCell>() {
+                tool.mark_failed();
+            }
+            self.add_boxed_history(cell);
         }
     }
 
@@ -1899,12 +1930,16 @@ impl WidgetRef for &ChatWidget {
         let [_, active_cell_area, bottom_pane_area] = self.layout_areas(area);
         (&self.bottom_pane).render(bottom_pane_area, buf);
         if !active_cell_area.is_empty()
-            && let Some(cell) = &self.active_exec_cell
+            && let Some(cell) = &self.active_cell
         {
-            let mut active_cell_area = active_cell_area;
-            active_cell_area.y = active_cell_area.y.saturating_add(1);
-            active_cell_area.height -= 1;
-            cell.render_ref(active_cell_area, buf);
+            let mut area = active_cell_area;
+            area.y = area.y.saturating_add(1);
+            area.height = area.height.saturating_sub(1);
+            if let Some(exec) = cell.as_any().downcast_ref::<ExecCell>() {
+                exec.render_ref(area, buf);
+            } else if let Some(tool) = cell.as_any().downcast_ref::<McpToolCallCell>() {
+                tool.render_ref(area, buf);
+            }
         }
     }
 }
diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs
index fa3235b6..7805e07e 100644
--- a/codex-rs/tui/src/chatwidget/tests.rs
+++ b/codex-rs/tui/src/chatwidget/tests.rs
@@ -314,7 +314,7 @@ fn make_chatwidget_manual() -> (
         app_event_tx,
         codex_op_tx: op_tx,
         bottom_pane: bottom,
-        active_exec_cell: None,
+        active_cell: None,
         config: cfg.clone(),
         auth_manager,
         session_header: SessionHeader::new(cfg.model),
@@ -551,9 +551,9 @@ fn end_exec(chat: &mut ChatWidget, call_id: &str, stdout: &str, stderr: &str, ex
 
 fn active_blob(chat: &ChatWidget) -> String {
     let lines = chat
-        .active_exec_cell
+        .active_cell
         .as_ref()
-        .expect("active exec cell present")
+        .expect("active cell present")
         .display_lines(80);
     lines_to_single_string(&lines)
 }
diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs
index 5b6c4af3..a63da9ea 100644
--- a/codex-rs/tui/src/history_cell.rs
+++ b/codex-rs/tui/src/history_cell.rs
@@ -105,6 +105,10 @@ impl dyn HistoryCell {
     pub(crate) fn as_any(&self) -> &dyn Any {
         self
     }
+
+    pub(crate) fn as_any_mut(&mut self) -> &mut dyn Any {
+        self
+    }
 }
 
 #[derive(Debug)]
@@ -542,9 +546,7 @@ impl WidgetRef for &ExecCell {
 }
 
 impl ExecCell {
-    /// Convert an active exec cell into a failed, completed exec cell.
-    /// Any call without output is marked as failed with a red ✗.
-    pub(crate) fn into_failed(mut self) -> ExecCell {
+    pub(crate) fn mark_failed(&mut self) {
         for call in self.calls.iter_mut() {
             if call.output.is_none() {
                 let elapsed = call
@@ -561,7 +563,6 @@ impl ExecCell {
                 });
             }
         }
-        self
     }
 
     pub(crate) fn new(call: ExecCall) -> Self {
@@ -942,6 +943,179 @@ impl HistoryCell for CompositeHistoryCell {
     }
 }
 
+#[derive(Debug)]
+pub(crate) struct McpToolCallCell {
+    call_id: String,
+    invocation: McpInvocation,
+    start_time: Instant,
+    duration: Option<Duration>,
+    result: Option<Result<mcp_types::CallToolResult, String>>,
+}
+
+impl McpToolCallCell {
+    pub(crate) fn new(call_id: String, invocation: McpInvocation) -> Self {
+        Self {
+            call_id,
+            invocation,
+            start_time: Instant::now(),
+            duration: None,
+            result: None,
+        }
+    }
+
+    pub(crate) fn call_id(&self) -> &str {
+        &self.call_id
+    }
+
+    pub(crate) fn complete(
+        &mut self,
+        duration: Duration,
+        result: Result<mcp_types::CallToolResult, String>,
+    ) -> Option<Box<dyn HistoryCell>> {
+        let image_cell = try_new_completed_mcp_tool_call_with_image_output(&result)
+            .map(|cell| Box::new(cell) as Box<dyn HistoryCell>);
+        self.duration = Some(duration);
+        self.result = Some(result);
+        image_cell
+    }
+
+    fn success(&self) -> Option<bool> {
+        match self.result.as_ref() {
+            Some(Ok(result)) => Some(!result.is_error.unwrap_or(false)),
+            Some(Err(_)) => Some(false),
+            None => None,
+        }
+    }
+
+    pub(crate) fn mark_failed(&mut self) {
+        let elapsed = self.start_time.elapsed();
+        self.duration = Some(elapsed);
+        self.result = Some(Err("interrupted".to_string()));
+    }
+
+    fn render_content_block(block: &mcp_types::ContentBlock, width: usize) -> String {
+        match block {
+            mcp_types::ContentBlock::TextContent(text) => {
+                format_and_truncate_tool_result(&text.text, TOOL_CALL_MAX_LINES, width)
+            }
+            mcp_types::ContentBlock::ImageContent(_) => "<image content>".to_string(),
+            mcp_types::ContentBlock::AudioContent(_) => "<audio content>".to_string(),
+            mcp_types::ContentBlock::EmbeddedResource(resource) => {
+                let uri = match &resource.resource {
+                    EmbeddedResourceResource::TextResourceContents(text) => text.uri.clone(),
+                    EmbeddedResourceResource::BlobResourceContents(blob) => blob.uri.clone(),
+                };
+                format!("embedded resource: {uri}")
+            }
+            mcp_types::ContentBlock::ResourceLink(ResourceLink { uri, .. }) => {
+                format!("link: {uri}")
+            }
+        }
+    }
+}
+
+impl HistoryCell for McpToolCallCell {
+    fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
+        let mut lines: Vec<Line<'static>> = Vec::new();
+        let status = self.success();
+        let bullet = match status {
+            Some(true) => "•".green().bold(),
+            Some(false) => "•".red().bold(),
+            None => spinner(Some(self.start_time)),
+        };
+        let header_text = if status.is_some() {
+            "Called"
+        } else {
+            "Calling"
+        };
+
+        let invocation_line = line_to_static(&format_mcp_invocation(self.invocation.clone()));
+        let mut compact_spans = vec![bullet.clone(), " ".into(), header_text.bold(), " ".into()];
+        let mut compact_header = Line::from(compact_spans.clone());
+        let reserved = compact_header.width();
+
+        let inline_invocation =
+            invocation_line.width() <= (width as usize).saturating_sub(reserved);
+
+        if inline_invocation {
+            compact_header.extend(invocation_line.spans.clone());
+            lines.push(compact_header);
+        } else {
+            compact_spans.pop(); // drop trailing space for standalone header
+            lines.push(Line::from(compact_spans));
+
+            let opts = RtOptions::new((width as usize).saturating_sub(4))
+                .initial_indent("".into())
+                .subsequent_indent("    ".into());
+            let wrapped = word_wrap_line(&invocation_line, opts);
+            let body_lines: Vec<Line<'static>> = wrapped.iter().map(line_to_static).collect();
+            lines.extend(prefix_lines(body_lines, "  └ ".dim(), "    ".into()));
+        }
+
+        let mut detail_lines: Vec<Line<'static>> = Vec::new();
+
+        if let Some(result) = &self.result {
+            match result {
+                Ok(mcp_types::CallToolResult { content, .. }) => {
+                    if !content.is_empty() {
+                        for block in content {
+                            let text = Self::render_content_block(block, width as usize);
+                            for segment in text.split('\n') {
+                                let line = Line::from(segment.to_string().dim());
+                                let wrapped = word_wrap_line(
+                                    &line,
+                                    RtOptions::new((width as usize).saturating_sub(4))
+                                        .initial_indent("".into())
+                                        .subsequent_indent("    ".into()),
+                                );
+                                detail_lines.extend(wrapped.iter().map(line_to_static));
+                            }
+                        }
+                    }
+                }
+                Err(err) => {
+                    let err_line = Line::from(format!("Error: {err}").dim());
+                    let wrapped = word_wrap_line(
+                        &err_line,
+                        RtOptions::new((width as usize).saturating_sub(4))
+                            .initial_indent("".into())
+                            .subsequent_indent("    ".into()),
+                    );
+                    detail_lines.extend(wrapped.iter().map(line_to_static));
+                }
+            }
+        }
+
+        if !detail_lines.is_empty() {
+            let initial_prefix: Span<'static> = if inline_invocation {
+                "  └ ".dim()
+            } else {
+                "    ".into()
+            };
+            lines.extend(prefix_lines(detail_lines, initial_prefix, "    ".into()));
+        }
+
+        lines
+    }
+}
+
+impl WidgetRef for &McpToolCallCell {
+    fn render_ref(&self, area: Rect, buf: &mut Buffer) {
+        if area.height == 0 {
+            return;
+        }
+        let lines = self.display_lines(area.width);
+        let max_rows = area.height as usize;
+        let rendered = if lines.len() > max_rows {
+            lines[lines.len() - max_rows..].to_vec()
+        } else {
+            lines
+        };
+
+        Text::from(rendered).render(area, buf);
+    }
+}
+
 fn spinner(start_time: Option<Instant>) -> Span<'static> {
     const FRAMES: &[char] = &['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏'];
     let idx = start_time
@@ -951,11 +1125,11 @@ fn spinner(start_time: Option<Instant>) -> Span<'static> {
     ch.to_string().into()
 }
 
-pub(crate) fn new_active_mcp_tool_call(invocation: McpInvocation) -> PlainHistoryCell {
-    let title_line = Line::from(vec!["tool".magenta(), " running...".dim()]);
-    let lines: Vec<Line> = vec![title_line, format_mcp_invocation(invocation)];
-
-    PlainHistoryCell { lines }
+pub(crate) fn new_active_mcp_tool_call(
+    call_id: String,
+    invocation: McpInvocation,
+) -> McpToolCallCell {
+    McpToolCallCell::new(call_id, invocation)
 }
 
 pub(crate) fn new_web_search_call(query: String) -> PlainHistoryCell {
@@ -1003,79 +1177,6 @@ fn try_new_completed_mcp_tool_call_with_image_output(
     }
 }
 
-pub(crate) fn new_completed_mcp_tool_call(
-    num_cols: usize,
-    invocation: McpInvocation,
-    duration: Duration,
-    success: bool,
-    result: Result<mcp_types::CallToolResult, String>,
-) -> Box<dyn HistoryCell> {
-    if let Some(cell) = try_new_completed_mcp_tool_call_with_image_output(&result) {
-        return Box::new(cell);
-    }
-
-    let duration = format_duration(duration);
-    let status_str = if success { "success" } else { "failed" };
-    let title_line = Line::from(vec![
-        "tool".magenta(),
-        " ".into(),
-        if success {
-            status_str.green()
-        } else {
-            status_str.red()
-        },
-        format!(", duration: {duration}").dim(),
-    ]);
-
-    let mut lines: Vec<Line<'static>> = Vec::new();
-    lines.push(title_line);
-    lines.push(format_mcp_invocation(invocation));
-
-    match result {
-        Ok(mcp_types::CallToolResult { content, .. }) => {
-            if !content.is_empty() {
-                lines.push(Line::from(""));
-
-                for tool_call_result in content {
-                    let line_text = match tool_call_result {
-                        mcp_types::ContentBlock::TextContent(text) => {
-                            format_and_truncate_tool_result(
-                                &text.text,
-                                TOOL_CALL_MAX_LINES,
-                                num_cols,
-                            )
-                        }
-                        mcp_types::ContentBlock::ImageContent(_) => {
-                            // TODO show images even if they're not the first result, will require a refactor of `CompletedMcpToolCall`
-                            "<image content>".to_string()
-                        }
-                        mcp_types::ContentBlock::AudioContent(_) => "<audio content>".to_string(),
-                        mcp_types::ContentBlock::EmbeddedResource(resource) => {
-                            let uri = match resource.resource {
-                                EmbeddedResourceResource::TextResourceContents(text) => text.uri,
-                                EmbeddedResourceResource::BlobResourceContents(blob) => blob.uri,
-                            };
-                            format!("embedded resource: {uri}")
-                        }
-                        mcp_types::ContentBlock::ResourceLink(ResourceLink { uri, .. }) => {
-                            format!("link: {uri}")
-                        }
-                    };
-                    lines.push(Line::styled(
-                        line_text,
-                        Style::default().add_modifier(Modifier::DIM),
-                    ));
-                }
-            }
-        }
-        Err(e) => {
-            lines.push(vec!["Error: ".red().bold(), e.into()].into());
-        }
-    };
-
-    Box::new(PlainHistoryCell { lines })
-}
-
 #[allow(clippy::disallowed_methods)]
 pub(crate) fn new_warning_event(message: String) -> PlainHistoryCell {
     PlainHistoryCell {
@@ -1743,6 +1844,11 @@ mod tests {
     use codex_core::config::ConfigToml;
     use dirs::home_dir;
     use pretty_assertions::assert_eq;
+    use serde_json::json;
+
+    use mcp_types::CallToolResult;
+    use mcp_types::ContentBlock;
+    use mcp_types::TextContent;
 
     fn test_config() -> Config {
         Config::load_from_base_config_with_overrides(
@@ -1769,6 +1875,192 @@ mod tests {
         render_lines(&cell.transcript_lines())
     }
 
+    #[test]
+    fn active_mcp_tool_call_snapshot() {
+        let invocation = McpInvocation {
+            server: "search".into(),
+            tool: "find_docs".into(),
+            arguments: Some(json!({
+                "query": "ratatui styling",
+                "limit": 3,
+            })),
+        };
+
+        let cell = new_active_mcp_tool_call("call-1".into(), invocation);
+        let rendered = render_lines(&cell.display_lines(80)).join("\n");
+
+        insta::assert_snapshot!(rendered);
+    }
+
+    #[test]
+    fn completed_mcp_tool_call_success_snapshot() {
+        let invocation = McpInvocation {
+            server: "search".into(),
+            tool: "find_docs".into(),
+            arguments: Some(json!({
+                "query": "ratatui styling",
+                "limit": 3,
+            })),
+        };
+
+        let result = CallToolResult {
+            content: vec![ContentBlock::TextContent(TextContent {
+                annotations: None,
+                text: "Found styling guidance in styles.md".into(),
+                r#type: "text".into(),
+            })],
+            is_error: None,
+            structured_content: None,
+        };
+
+        let mut cell = new_active_mcp_tool_call("call-2".into(), invocation);
+        assert!(
+            cell.complete(Duration::from_millis(1420), Ok(result))
+                .is_none()
+        );
+
+        let rendered = render_lines(&cell.display_lines(80)).join("\n");
+
+        insta::assert_snapshot!(rendered);
+    }
+
+    #[test]
+    fn completed_mcp_tool_call_error_snapshot() {
+        let invocation = McpInvocation {
+            server: "search".into(),
+            tool: "find_docs".into(),
+            arguments: Some(json!({
+                "query": "ratatui styling",
+                "limit": 3,
+            })),
+        };
+
+        let mut cell = new_active_mcp_tool_call("call-3".into(), invocation);
+        assert!(
+            cell.complete(Duration::from_secs(2), Err("network timeout".into()))
+                .is_none()
+        );
+
+        let rendered = render_lines(&cell.display_lines(80)).join("\n");
+
+        insta::assert_snapshot!(rendered);
+    }
+
+    #[test]
+    fn completed_mcp_tool_call_multiple_outputs_snapshot() {
+        let invocation = McpInvocation {
+            server: "search".into(),
+            tool: "find_docs".into(),
+            arguments: Some(json!({
+                "query": "ratatui styling",
+                "limit": 3,
+            })),
+        };
+
+        let result = CallToolResult {
+            content: vec![
+                ContentBlock::TextContent(TextContent {
+                    annotations: None,
+                    text: "Found styling guidance in styles.md and additional notes in CONTRIBUTING.md.".into(),
+                    r#type: "text".into(),
+                }),
+                ContentBlock::ResourceLink(ResourceLink {
+                    annotations: None,
+                    description: Some("Link to styles documentation".into()),
+                    mime_type: None,
+                    name: "styles.md".into(),
+                    size: None,
+                    title: Some("Styles".into()),
+                    r#type: "resource_link".into(),
+                    uri: "file:///docs/styles.md".into(),
+                }),
+            ],
+            is_error: None,
+            structured_content: None,
+        };
+
+        let mut cell = new_active_mcp_tool_call("call-4".into(), invocation);
+        assert!(
+            cell.complete(Duration::from_millis(640), Ok(result))
+                .is_none()
+        );
+
+        let rendered = render_lines(&cell.display_lines(48)).join("\n");
+
+        insta::assert_snapshot!(rendered);
+    }
+
+    #[test]
+    fn completed_mcp_tool_call_wrapped_outputs_snapshot() {
+        let invocation = McpInvocation {
+            server: "metrics".into(),
+            tool: "get_nearby_metric".into(),
+            arguments: Some(json!({
+                "query": "very_long_query_that_needs_wrapping_to_display_properly_in_the_history",
+                "limit": 1,
+            })),
+        };
+
+        let result = CallToolResult {
+            content: vec![ContentBlock::TextContent(TextContent {
+                annotations: None,
+                text: "Line one of the response, which is quite long and needs wrapping.\nLine two continues the response with more detail.".into(),
+                r#type: "text".into(),
+            })],
+            is_error: None,
+            structured_content: None,
+        };
+
+        let mut cell = new_active_mcp_tool_call("call-5".into(), invocation);
+        assert!(
+            cell.complete(Duration::from_millis(1280), Ok(result))
+                .is_none()
+        );
+
+        let rendered = render_lines(&cell.display_lines(40)).join("\n");
+
+        insta::assert_snapshot!(rendered);
+    }
+
+    #[test]
+    fn completed_mcp_tool_call_multiple_outputs_inline_snapshot() {
+        let invocation = McpInvocation {
+            server: "metrics".into(),
+            tool: "summary".into(),
+            arguments: Some(json!({
+                "metric": "trace.latency",
+                "window": "15m",
+            })),
+        };
+
+        let result = CallToolResult {
+            content: vec![
+                ContentBlock::TextContent(TextContent {
+                    annotations: None,
+                    text: "Latency summary: p50=120ms, p95=480ms.".into(),
+                    r#type: "text".into(),
+                }),
+                ContentBlock::TextContent(TextContent {
+                    annotations: None,
+                    text: "No anomalies detected.".into(),
+                    r#type: "text".into(),
+                }),
+            ],
+            is_error: None,
+            structured_content: None,
+        };
+
+        let mut cell = new_active_mcp_tool_call("call-6".into(), invocation);
+        assert!(
+            cell.complete(Duration::from_millis(320), Ok(result))
+                .is_none()
+        );
+
+        let rendered = render_lines(&cell.display_lines(120)).join("\n");
+
+        insta::assert_snapshot!(rendered);
+    }
+
     #[test]
     fn session_header_includes_reasoning_level_when_present() {
         let cell = SessionHeaderHistoryCell::new(
diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__active_mcp_tool_call_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__active_mcp_tool_call_snapshot.snap
new file mode 100644
index 00000000..2c1067b2
--- /dev/null
+++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__active_mcp_tool_call_snapshot.snap
@@ -0,0 +1,6 @@
+---
+source: tui/src/history_cell.rs
+assertion_line: 1740
+expression: rendered
+---
+⠋ Calling search.find_docs({"query":"ratatui styling","limit":3})
diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_error_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_error_snapshot.snap
new file mode 100644
index 00000000..776f7920
--- /dev/null
+++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_error_snapshot.snap
@@ -0,0 +1,7 @@
+---
+source: tui/src/history_cell.rs
+assertion_line: 1817
+expression: rendered
+---
+• Called search.find_docs({"query":"ratatui styling","limit":3})
+  └ Error: network timeout
diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_multiple_outputs_inline_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_multiple_outputs_inline_snapshot.snap
new file mode 100644
index 00000000..6e6bccb3
--- /dev/null
+++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_multiple_outputs_inline_snapshot.snap
@@ -0,0 +1,8 @@
+---
+source: tui/src/history_cell.rs
+assertion_line: 1934
+expression: rendered
+---
+• Called metrics.summary({"metric":"trace.latency","window":"15m"})
+  └ Latency summary: p50=120ms, p95=480ms.
+    No anomalies detected.
diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_multiple_outputs_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_multiple_outputs_snapshot.snap
new file mode 100644
index 00000000..6f7fd0e8
--- /dev/null
+++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_multiple_outputs_snapshot.snap
@@ -0,0 +1,11 @@
+---
+source: tui/src/history_cell.rs
+assertion_line: 1850
+expression: rendered
+---
+• Called
+  └ search.find_docs({"query":"ratatui
+        styling","limit":3})
+    Found styling guidance in styles.md and
+        additional notes in CONTRIBUTING.md.
+    link: file:///docs/styles.md
diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_success_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_success_snapshot.snap
new file mode 100644
index 00000000..3c601c22
--- /dev/null
+++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_success_snapshot.snap
@@ -0,0 +1,7 @@
+---
+source: tui/src/history_cell.rs
+assertion_line: 1795
+expression: rendered
+---
+• Called search.find_docs({"query":"ratatui styling","limit":3})
+  └ Found styling guidance in styles.md
diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_wrapped_outputs_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_wrapped_outputs_snapshot.snap
new file mode 100644
index 00000000..7d9c1613
--- /dev/null
+++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__completed_mcp_tool_call_wrapped_outputs_snapshot.snap
@@ -0,0 +1,14 @@
+---
+source: tui/src/history_cell.rs
+assertion_line: 1891
+expression: rendered
+---
+• Called
+  └ metrics.get_nearby_metric({"query":"
+        very_long_query_that_needs_wrapp
+        ing_to_display_properly_in_the_h
+        istory","limit":1})
+    Line one of the response, which is
+        quite long and needs wrapping.
+    Line two continues the response with
+        more detail.