agent_ui: Surface saved terminal tool output after restart#59271
Open
dandv wants to merge 2 commits into
Open
Conversation
After a thread is reloaded from `threads.db`, `Thread::replay` recreates each tool call from the persisted `LanguageModelToolResult`. For the terminal tool the live `Terminal` GPUI entity is gone, so the call's `content` is rebuilt as markdown from the saved `raw_output` (via `acp_thread::ToolCall::update_fields`'s raw-output fallback). `render_any_tool_call` only routes to `render_terminal_tool_call` when `tool_call.terminals().next().is_some()`, so a replayed terminal call falls through to `render_tool_call`. The `is_terminal_tool` branch of that function rendered just `render_collapsible_command` for the header and skipped the `Disclosure` widget that the non-terminal branch attaches. The body (`tool_output_display`) is gated on `is_open`, which in this branch never flips: no `NewTerminal` view event fires for a replayed call so `expanded_tool_calls` stays empty, and there was no disclosure to toggle it. Net result: the saved output sat in `tool_call.content` but was unreachable from the UI, matching the issue's "no expand/collapse controls" symptom. Fix: once such a call has a result, render a header row above the command block mirroring the live card: the working directory (recovered from the tool's `cd` input) on the left and, on the right, a hover-revealed expand/collapse `Disclosure` plus the live card's failure indicator (red x with an "Exited with code N" tooltip, falling back to "Failed"/"Canceled"/"Rejected" from the call status when no exit code survived). A nonzero exit code isn't an error result for the terminal tool - it only survives a reload as a sentinel in the persisted output string - so a new `parse_failed_exit_code` helper recovers it, placed next to `process_content`, which writes that sentinel. The "Run Command" preview label now shows only while a command has no result yet (e.g. awaiting confirmation), matching the live card, which swaps it for the working dir once the command runs. Calls with a live `Terminal` entity keep going through `render_terminal_tool_call`. Note that this branch isn't replay-only: it also serves `Execute`-kind calls that never get a terminal entity - commands auto-denied by the permission layer (e.g. containing `$VAR` substitutions, rejected before terminal creation), and external ACP agents that report command output as plain content blocks. Those had the same unreachable-output bug, so they get the same treatment; when no `cd` input exists, the header shows "Ran command" in place of the working dir. Replayed cards intentionally start collapsed regardless of the `expand_terminal_card` setting, which only auto-expands terminals as they are created live. Tests: a regression test in `acp_thread` for the `update_fields` raw-output fallback the UI depends on, and a round-trip test for `parse_failed_exit_code` against `process_content`. Release Notes: - Fixed terminal tool output not being reachable after restarting Zed on a thread that previously ran a terminal command (zed-industries#57230). Restored threads now also show the command's working directory and exit status.
The fix for zed-industries#57230 (previous commit) depends on `Thread::replay` emitting, for tools without a `replay` impl like the terminal tool, a `ToolCall` event followed by a `ToolCallUpdate` carrying the persisted `raw_output` - that update is what `acp_thread::ToolCall::update_fields` turns back into displayable content. `test_mcp_tool_result_displayed_when_server_disconnected` covers only the tool-not-found branch of `replay_tool_call`; the tool-found branch the terminal tool takes was untested, so a change to its emission shape would break restored threads while the UI-side tests kept passing. Run the real `TerminalTool` against a `FakeThreadEnvironment` terminal that exits immediately with code 128, replay the thread, and assert the event order, the Completed status (a nonzero exit code is not an error result for the terminal tool), and that the persisted output round-trips through `parse_failed_exit_code` into the exit code the UI failure indicator shows. The test responds to the permission prompt explicitly instead of calling `always_allow_tools`: the settings-global override can be clobbered when the async settings-file load kicked off by `setup` completes after the override, which is seed-dependent (observed at seed 14 as an unexpected permission prompt that left the turn pending). Verified across `ITERATIONS=100` scheduler seeds. Release Notes: - N/A
f882267 to
1224f1a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
Fixes #57230: after restarting Zed, terminal tool calls in a reloaded thread showed only the command, and the output persisted in
threads.dbwas unreachable (no expand/collapse controls).Solution & Showcase
Before: when a thread was reloaded,
Thread::replayrecreated each terminal tool call without its liveTerminalentity, so rendering fell through to the generic tool-call path (render_tool_call), which never offered a way to open the output thatacp_threadhad already reconstructed from the persistedraw_output.After: once such a call has a result, the PR renders a header row above the command block that mirrors the UX of the live terminal card (behavior, label and icon position):
Expand.replayed.terminal.tool.output.webm
cd(e.g. external ACP agents) show "Ran command" instead.agent::parse_failed_exit_codehelper.Differences from the live cards:
expand_terminal_cardsetting, to mirror the current UX of agent threads after Zed restarts. Later we might add auto-expansion during in-thread search, or an UX affordance to expand all.The PR also serves two more paths that had the same unreachable-output bug:
Execute-kind calls that never get a terminal entity, i.e. commands denied by the user or auto-denied by the permission layer (due to containing$VARsubstitutions, for example)User-denied.command.webm
Testing
Automated tests:
acp_thread: regression test for theupdate_fieldsraw-output fallback the UI depends on (test_raw_output_update_populates_content_when_empty).agent: round-trip test forparse_failed_exit_codeagainstprocess_content.Manual test plan, all pass
Summary: replayed cards start collapsed; chevron and copy button appear on hover; output expands/collapses on click; failed commands show the red ✕ with the exit-code tooltip.
Setup
--user-data-dir), reopen the same thread.Replayed cards (after restart)
T-001 — Replayed cards start collapsed
Status: ✅ PASS pre-restructure 2026-06-11; ✅ re-verify
Repro: Reload the thread. Every terminal card shows the command but no output. This holds even with
agent.expand_terminal_card: true(default) in settings (that setting only auto-expands live terminals as they are created).T-002 — Header shows working dir, not "Run Command"
Status: ✅ PASS 2026-06-12
Repro: Each replayed terminal card has a header row above the command, showing the working directory (the tool's
cdinput) on the left — matching live cards. "Run Command" must not appear on any card that has a result.T-003 — Chevron appears on hover, top-right of the header row
Status: ✅ PASS 2026-06-12
Repro: Mouse over a replayed card. The expand/collapse chevron fades in at the top-right of the header row — same position as live cards. The copy button appears on the command row below it.
T-004 — Click chevron: expand and collapse
Status: ✅ PASS 2026-06-12
Repro: Click the chevron → saved output renders below the command (markdown code block). Click again → collapses. State persists while scrolling the thread (virtualized list re-renders).
T-005 — Copy button copies unfenced command
Status: ✅ PASS 2026-06-12
Repro: Hover the command row, click copy. Pasted text is the bare command — no ``` fences.
T-006 — Failed command: red ✕ with exit-code tooltip
Status: ✅ PASS 2026-06-12
Repro: On the replayed card of the failing command, a red ✕ shows at the right of the header row (next to the chevron, same position as live). Hovering it shows "Exited with code N". Expanding shows the saved output including the failure sentinel text.
T-007 — Auto-denied
$VARcommand after restartStatus: ✅ PASS 2026-06-12
Repro: The auto-denied command's card shows a red ✕ (status Failed; no exit-code tooltip — it never ran). Expanding shows the denial message ("…does not allow shell substitutions…").
T-008 — Long multi-line command layout
Status: ✅ PASS 2026-06-12
Repro: The multi-line command card renders without layout breakage; long working-dir paths scroll horizontally in the header instead of wrapping or pushing the chevron/✕ out of view.
Live cards (same session, no restart needed)
T-009 — Live terminal cards unaffected
Status: ✅ PASS 2026-06-12
Repro: In the reloaded thread, ask the agent to run another command. The new card uses the live terminal rendering: working-dir header, real terminal view on expand, stop button while running, elapsed time. Compare side by side with a replayed card.
T-010 — Awaiting-confirmation preview unchanged
Status: ✅ PASS 2026-06-12
Repro: With confirm-mode permissions, trigger a command needing approval. While waiting, the card shows the "Run Command" preview label (no header row, no chevron). After approval it becomes a live terminal card.
T-011 — Auto-denied
$VARcommand in a live sessionStatus: ✅ PASS 2026-06-12
Repro: Same as T-007 but observed live, without restarting: the denied call renders as a static card (no terminal) with a red ✕.
Edge cases
T-012 — User-stopped command after restart
Status: ✅ PASS 2026-06-12
Repro: Stop a long-running command via the stop button. After restart, its card shows output text ("the user intentionally interrupted this command…" / "…timed out…") but no ✕ — the persisted result is not an error and carries no exit code. Document, don't try to fix.
T-013 — User-timed-out command after restart
Status: ✅ PASS 2026-06-12
Repro: Let a long-running command hit a
timeout_ms. After restart, its card shows output text ("Command …timed out…") but no ✕ — the persisted result is not an error and carries no exit code. Document, don't try to fix.T-014 — Empty-output success after restart
Status: ✅ PASS 2026-06-12
Repro: Run
true(no output). After restart the card's saved output is "Command executed successfully." — chevron present, expanding shows that text.T-015 — Themes
Status: ✅ PASS 2026-06-12
Repro: Quick pass in light + dark themes: header bg, chevron hover, red ✕ legible; card border visible but not jarring.
T-016 — External ACP agent: "Ran command" fallback label
Status: ✅ PASS 2026-06-12 (Claude Code)
Repro: In a thread driven by an external ACP agent (e.g. Claude Code / Gemini CLI) that reports
Execute-kind tool calls as plain content blocks (no embedded terminal), completed command cards show a "Ran command" header label (past tense — nocdinput to display) with the chevron, and the output expands on click.Notes: Claude Code also produces genuine live-terminal cards (it creates embedded ACP terminals for some commands without passing a
cwd), which render via the unchanged live path and show its pre-existing "current directory" fallback label — out of this PR's scope. The label "jumping" between "Ran command" and "current directory" within one thread reflects those two mechanisms, not a bug in this change.T-017 — ✕ tooltip fallback when no exit code is recoverable
Status: ✅ PASS 2026-06-13 (fix shipped 2026-06-12; regression-guarded by
test_terminal_failure_tooltip)Repro: Hover the red ✕ on failed cards whose saved output is not the native terminal tool's failure sentinel:
$VARcommand (live or replayed) → tooltip "Failed".Rejectedstatus, but the denial becomes an error tool result whose finalFailedstatus update overwrites it — that's what is recorded and replayed. The expanded output still shows "Permission to run tool denied by user".)A tooltip must appear in every case; "Exited with code N" still wins whenever the native sentinel is present (T-006). For comparison, release v1.7.2 shows no ✕ and no tooltip at all on these cards, live or replayed.
Self-Review Checklist:
Release Notes: