Skip to content
This repository was archived by the owner on Sep 4, 2026. It is now read-only.

feat(chat): give MCP and tool rows their status, detail and a live toggle - #390

Merged
ElbertePlinio merged 1 commit into
feat-parser-tool-detailfrom
feat-tool-detail-ui
Jul 26, 2026
Merged

feat(chat): give MCP and tool rows their status, detail and a live toggle#390
ElbertePlinio merged 1 commit into
feat-parser-tool-detailfrom
feat-tool-detail-ui

Conversation

@ElbertePlinio

Copy link
Copy Markdown
Member

Frontend half of #362 and #365. Behind the new default-off mcpToolDetail flag.

Stacked on #387, which is what makes these events exist. Retarget to main once that merges — and note stacked PRs get no CI in this repo, so this needs a rebase-and-push after #387 lands to earn its checks.

The wire always carried this; the store threw it away

lib/agentChat.ts declares status and detail on both mcpToolCall and toolUse. The timeline item type did not, so the reducer read them and discarded them, and ChatTimeline rendered:

<McpCard server={item.server} tool={item.tool} />

With detail undefined, canExpand() is false → spacer instead of a chevron, disabled button, no body. That is exactly the reported symptom: MCP pickforge-lanes / lanes_wait as one dead grey line.

What changed

  • Both item types carry status and detail, both optional — a row persisted before the parser learned to resolve non-Bash tools still renders rather than breaking on replay.
  • A completion carrying no result keeps the arg summary already on the row. Resolving must never blank what the row was showing; that has its own test.
  • Expansion moves into the hoisted RowExpansion map, the same contract CommandCard and ThinkingBubble use, so the toggle survives the virtualizer disposing the row on scroll-back. Chat transcript rows expand with no motion: only THINKING animates, CMD/TOOL/MCP/swarm/lanes snap open #372 listed that as a separate annoyance worth its own issue — it comes free here, because these cards had to take open/onToggle anyway.
  • Status renders through StatusPill, matching CommandCard rather than adding new chrome: bracket-cornered mono, never a filled chip.

Why the flag

A persisted row's shape changed and the parser only emits completions on newer builds, so main stays releasable while this is dark — same precedent as changesReview gating the receipt card. flags.test.ts enumerates the registry, so the new flag is asserted there rather than silently appearing.

Tests

Three new store tests, all verified load-bearing (removing the carry fails two of them):

  • an MCP call's status/detail carried, and the completion resolving in place rather than appending
  • a completion with detail: null keeping the existing arg summary
  • a generic tool reaching a terminal failed status

Validation

  • bun run test:unit1604 passed
  • bunx tsc --noEmit clean; bun run lint clean

The two new render branches are extracted out of renderItem because adding them inline pushed it past the repo's complexity cap.

Still not done for these two issues

Stated rather than implied:

Refs #362, #365

@ElbertePlinio
ElbertePlinio force-pushed the feat-parser-tool-detail branch from 53825a9 to 866360d Compare July 26, 2026 02:04
…ggle

Frontend half of #362 and #365, behind the new default-off `mcpToolDetail`
flag. Stacked on the parser slice, which is what makes these events exist.

The wire event has always carried `status` and `detail` — `lib/agentChat.ts`
declares both on `mcpToolCall` and `toolUse`. The timeline item type did not, so
the reducer read them and threw them away, and `ChatTimeline` rendered
`<McpCard server tool />` with no `detail` at all. With `detail` undefined
`canExpand()` is false, which is exactly the reported symptom: a disabled
chevron on a dead grey row.

Both item types now carry `status` and `detail`, both optional so a row
persisted before the parser learned to resolve non-Bash tools still renders.
A completion that carries no result keeps the arg summary already on the row —
resolving must never blank what the row was showing.

Expansion moves into the hoisted `RowExpansion` map, the same contract
`CommandCard` and `ThinkingBubble` use, so the toggle survives the virtualizer
disposing the row on scroll-back. That was listed in #372 as a separate
annoyance; it comes free here because these cards had to take `open`/`onToggle`
anyway.

Status renders through `StatusPill`, matching `CommandCard` rather than adding
new chrome — bracket-cornered mono, never a filled chip.

The two new render branches are extracted out of `renderItem` because adding
them inline pushed it past the complexity cap. `flags.test.ts` enumerates the
registry, so the new flag is asserted there rather than silently appearing.

Refs #362, #365
@ElbertePlinio
ElbertePlinio merged commit 98b0ce4 into feat-parser-tool-detail Jul 26, 2026
ElbertePlinio added a commit that referenced this pull request Jul 26, 2026
…ggle (#390)

Frontend half of #362 and #365, behind the new default-off `mcpToolDetail`
flag. Stacked on the parser slice, which is what makes these events exist.

The wire event has always carried `status` and `detail` — `lib/agentChat.ts`
declares both on `mcpToolCall` and `toolUse`. The timeline item type did not, so
the reducer read them and threw them away, and `ChatTimeline` rendered
`<McpCard server tool />` with no `detail` at all. With `detail` undefined
`canExpand()` is false, which is exactly the reported symptom: a disabled
chevron on a dead grey row.

Both item types now carry `status` and `detail`, both optional so a row
persisted before the parser learned to resolve non-Bash tools still renders.
A completion that carries no result keeps the arg summary already on the row —
resolving must never blank what the row was showing.

Expansion moves into the hoisted `RowExpansion` map, the same contract
`CommandCard` and `ThinkingBubble` use, so the toggle survives the virtualizer
disposing the row on scroll-back. That was listed in #372 as a separate
annoyance; it comes free here because these cards had to take `open`/`onToggle`
anyway.

Status renders through `StatusPill`, matching `CommandCard` rather than adding
new chrome — bracket-cornered mono, never a filled chip.

The two new render branches are extracted out of `renderItem` because adding
them inline pushed it past the complexity cap. `flags.test.ts` enumerates the
registry, so the new flag is asserted there rather than silently appearing.

Refs #362, #365
ElbertePlinio added a commit that referenced this pull request Jul 26, 2026
…lag-gated) (#387)

* fix(claude): let every tool row carry its args and reach a terminal status

Shared parser slice for #362 and #365 — both issues named `claude_stream.rs`'s
`RememberedTool` and the Bash-only guard in `map_tool_result` as one surface to
serialize rather than duplicate, so this does that half once.

**Nothing non-Bash ever completed.** `map_tool_result` returned `None` unless
the tool was Bash, so every other tool was emitted `InProgress` and never
updated — permanently "running" long after it finished. Lifting the guard alone
would not have helped: `RememberedTool` collapsed everything non-Bash to a bare
`Other`, discarding the name, so there was nothing left to route a completion
to. It now retains the tool's identity — Bash, an MCP server/tool pair, or a
named tool — and `map_tool_result` dispatches on it.

**MCP rows were starved on purpose.** The MCP branch emitted `detail: None`
while the non-MCP branch immediately below it attached
`compact_input_summary(&input)`. That is why an MCP call rendered as one dead
grey line: no args, no result, no completion, disabled chevron.

**The generic row starved on a four-key allowlist.** `file_path`,
`notebook_path`, `pattern`, `description` — `AskUserQuestion`, `Task`, `Skill`,
`Glob`, `ToolSearch` and `Read`-with-offset carry none of them, so `detail` was
`None` and the chevron was a dead button. Known keys still win (they are the
human-meaningful ones); anything else falls back to a compact rendering of the
whole input.

Both payloads are capped in Rust, not the view, because these rows are
persisted. Argument summaries take the FRONT of the string — the meaningful part
of an argument list is at its head, unlike a command's output tail — and are
capped far shorter, since they ride on a single collapsed row.

Deliberately NOT in this slice, so the two follow-on PRs stay reviewable:
`tool_progress` elapsed time and `system`/`status` compacting/requesting (#365
B1), which add a new event variant, and the frontend half of both issues — the
timeline item's `status`/`detail` fields, the itemId update path, and the card
wiring.

Refs #362, #365

* fix(chat): resolve an MCP row in place instead of appending a duplicate

The parser half of this change starts emitting completion events for MCP
calls. `reduceMcpToolCall` appended unconditionally — it had no itemId match
path, unlike its `reduceToolUse` sibling — so a resolving call would visibly
double instead of updating.

That makes the two halves inseparable: shipping the parser without this turns
'the row never finishes' into 'the row finishes twice'.

Refs #362

* feat(chat): give MCP and tool rows their status, detail and a live toggle (#390)

Frontend half of #362 and #365, behind the new default-off `mcpToolDetail`
flag. Stacked on the parser slice, which is what makes these events exist.

The wire event has always carried `status` and `detail` — `lib/agentChat.ts`
declares both on `mcpToolCall` and `toolUse`. The timeline item type did not, so
the reducer read them and threw them away, and `ChatTimeline` rendered
`<McpCard server tool />` with no `detail` at all. With `detail` undefined
`canExpand()` is false, which is exactly the reported symptom: a disabled
chevron on a dead grey row.

Both item types now carry `status` and `detail`, both optional so a row
persisted before the parser learned to resolve non-Bash tools still renders.
A completion that carries no result keeps the arg summary already on the row —
resolving must never blank what the row was showing.

Expansion moves into the hoisted `RowExpansion` map, the same contract
`CommandCard` and `ThinkingBubble` use, so the toggle survives the virtualizer
disposing the row on scroll-back. That was listed in #372 as a separate
annoyance; it comes free here because these cards had to take `open`/`onToggle`
anyway.

Status renders through `StatusPill`, matching `CommandCard` rather than adding
new chrome — bracket-cornered mono, never a filled chip.

The two new render branches are extracted out of `renderItem` because adding
them inline pushed it past the complexity cap. `flags.test.ts` enumerates the
registry, so the new flag is asserted there rather than silently appearing.

Refs #362, #365
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant