MCP tool rows in chat are dead ends: no args, no result, no completion — and pickforge-lanes should show lane cards

[ElbertePlinio]

Goal

Make MCP tool calls in agent chat legible: an expandable row carrying arguments, result/error, and a real terminal status — and for pickforge-lanes calls, render the pi-kit lane cards inline instead of a bare row.

Why

An MCP call currently renders as one dead grey line — MCP pickforge-lanes / lanes_wait — with no arguments, no result, no completion, and a disabled chevron. When an agent dispatches lanes and waits, the chat shows nothing about which lanes exist or how they are doing, while the Settings pi-kit panel has that exact information a screen away.

Verified against main at 9b852be. Not a stale build.

Where it breaks (three independent layers)

1. Rust parser drops args and results (Claude). crates/pickforge-core/src/agents/claude_stream.rs:273-281 special-cases the mcp__server__tool shape (claude_stream.rs:979-981) and emits AgentEvent::McpToolCall { status: InProgress, detail: None } — the non-MCP branch right below it (claude_stream.rs:283-287) does attach compact_input_summary(&input), so the MCP branch deliberately drops the summary its sibling keeps. And map_tool_result (claude_stream.rs:333-336) returns None unless the tool was Bash, so no MCP result is ever parsed and no completion event is ever emitted — the row never resolves. Codex app-server is the only provider that captures a result today (codex_app.rs:1059-1069, detail: string_field(item, &["error", "result"])); codex_exec.rs:373-379 also sends detail: None.

2. Frontend model drops what does arrive. The wire event carries status and detail (src/lib/agentChat.ts:73-79), but the timeline item type does not — src/stores/agentChat.ts:87 is { type: "mcpToolCall"; seq; itemId; server; tool }, no status, no detail, unlike the sibling toolUse on line 86. The reducer (agentChat.ts:976-986) appends and discards both, and has no itemId match/update path (contrast reduceToolUse, agentChat.ts:794-818) — so a completion event would append a duplicate row rather than update the existing one. Fixing layer 1 without this produces double rows.

3. The card is starved, not missing. McpCard.tsx:6-49 already implements the chevron/expand/<pre> body and accepts detail?: string | null. ChatTimeline.tsx:181-182 never passes it:

case "mcpToolCall":
  return <McpCard server={item.server} tool={item.tool} />;

With detail undefined, canExpand() is false → spacer instead of chevron, disabled button, no body. Exactly the observed symptom.

Note plans/002-agent-chat-gui.md:43 always intended McpToolCall to map to tool_use mcp__* + result for the Claude column. That half was never implemented.

Scope

A. Generic MCP row (all servers)

• Emit an arg summary and a completion event with status + result/error from claude_stream.rs; make codex_exec match.
• Carry status and detail on the timeline item; add the itemId update path so completion mutates the row instead of appending.
• Pass detail through ChatTimeline.tsx, hoist expansion state into RowExpansion (same contract as CommandCard.tsx:22-27 / ThinkingBubble.tsx:5-38) so it survives virtualization.
• Update the virtualizer height estimate — chatTimelineVirtual.ts:90-92 gives mcpToolCall a flat 68 while toolUse measures its detail.
• Truncation policy for large MCP payloads (they can be far bigger than a Bash output_tail) — cap in Rust, not in the view, since these rows are persisted through manager.rs:2676-2688.

B. pickforge-lanes renders lane cards

For server === "pickforge-lanes" (lanes_spawn / lanes_wait / lanes_status), the expanded body should show the run's lanes the way Settings does — the PiKitLanesPanel idiom (src/components/pikit/PiKitLanesPanel.tsx, display helpers in pikitLaneDisplay.ts, store src/stores/pikitLanes.ts reading <run>.status.json).

Yes, this is possible: the panel is already a self-contained run/lane card set with its own store and polling, and it was itself modelled on SwarmRunCard from chat. The work is extraction + correlation, not new rendering:

• extract the run/lane card out of PiKitLanesPanel so both Settings and chat render the same component (avoid a second divergent lane card);
• correlate the MCP call to a run id — from the lanes_spawn result, or the run argument on lanes_wait/lanes_status;
• decide polling ownership: startPiKitLanesPolling is currently panel-scoped (onMount/onCleanup); a chat row needs it live while the call is in flight and static afterwards.

Open question for A/B split: whether the chat body shows live lanes (polls, matches Settings) or a snapshot of the lane state at result time. Live is what makes lanes_wait useful; snapshot is honest for replayed history. Likely: live while in-progress, frozen snapshot once the call completes.

Flag

Behind a flag — this changes a persisted timeline row shape and adds polling into chat. Proposed name: mcpToolDetail. Nearest analogue is changesReview gating the richer card at ChatTimeline.tsx:108-110.

• merged behind flag mcpToolDetail
• tested on main (flag on)
• enabled in vX.Y.Z
• flag removed

PR plan

• PR 1 — Rust: MCP arg summary + completion event with result/error, with a payload cap
  • Depends on: none
  • Touches: claude_stream.rs, codex_exec.rs, event.rs if the payload shape grows
  • Validation: Rust unit tests over recorded stream-json fixtures (in-progress → completed → failed)
  • Link: pending
• PR 2 — Frontend: carry status/detail, itemId update path, pass detail to McpCard, hoist expansion, fix height estimate
  • Depends on: PR 1 (needs the events to consume, but can land dark behind the flag)
  • Touches: stores/agentChat.ts, lib/agentChat.ts, ChatTimeline.tsx, McpCard.tsx, chatTimelineVirtual.ts
  • Validation: bun run test:unit reducer tests (no duplicate row on completion); VRT of an expanded MCP row
  • Link: pending
• PR 3 — pickforge-lanes lane cards inline: extract the run/lane card, correlate run id, polling lifecycle
  • Depends on: PR 2; coordinate with pi-kit extension integration: managed Pi extensions for deep PickForge interop #274
  • Touches: components/pikit/*, ChatTimeline.tsx, stores/pikitLanes.ts
  • Validation: unit test for run-id correlation; VRT of a lanes_wait row with lanes; confirm Settings panel is unchanged
  • Link: pending

Traceability

Acceptance criterion	PR/slice	Validation	Status
An MCP row shows an inline arg summary	PR 1+2	Rust fixture test + VRT	pending
The row resolves (WORKING → done/failed), exactly one row per call	PR 2	reducer unit test	pending
Chevron expands to full args + result/error, survives scroll	PR 2	VRT + virtualization check	pending
Large MCP payloads are capped before persistence	PR 1	Rust unit test	pending
pickforge-lanes calls show lane cards matching Settings	PR 3	VRT + shared component assertion	pending
Settings pi-kit panel renders unchanged after extraction	PR 3	existing Settings VRT	pending

Open decisions

1. Live vs snapshot lanes in the chat body (see above) — blocks PR 3 design.
2. Payload cap size and whether truncation is marked in the UI.

Refs #274

[ElbertePlinio]

Split note: #365 now tracks the turn-level half — working-row identity, elapsed time from the dropped tool_progress messages, and the generic (non-MCP) tool row that starves on the four-key compact_input_summary allowlist.

Shared surface to serialize, not duplicate:

• claude_stream.rs:334 Bash-only guard in map_tool_result — this issue lifts it for MCP, A running turn is opaque: WORKING names nothing, no elapsed time, generic tool rows show no args and never complete #365 for generic ToolUse. Same function, coordinate PR 1 of each.
• RememberedTool (claude_stream.rs:431) discards the tool name, so neither completion can be routed until it retains it. Whichever PR 1 lands first should own that change.
• The timeline item status/detail fields (PR 2 here, PR 2 there) — first to land owns the shape, the other rebases.

No scope change to this issue.

[ElbertePlinio]

The shared parser slice is up as #387, covering the half both issues named as "serialize, not duplicate".

Landed there:

• RememberedTool retains identity (Bash / Mcp { server, tool } / Other(name)) instead of collapsing everything non-Bash to a nameless Other. This is the change both issues said whichever PR 1 landed first should own — without it, lifting the Bash guard could not route a completion anywhere.
• The Bash-only guard in map_tool_result is gone, so every tool reaches a terminal status instead of being emitted InProgress forever.
• The MCP branch now attaches the arg summary its non-MCP sibling always did.
• compact_input_summary no longer starves on the four-key allowlist — known keys still win, anything else falls back to a capped rendering of the whole input.
• Payloads capped in Rust (rows are persisted): output tails keep the 2000-char tail, arg summaries take the head at 300, since the meaningful part of an argument list is at its front.

Also in that PR, because the two are inseparable: reduceMcpToolCall gained the itemId match path. Without it the new completion events would append a second row — turning "the row never finishes" into "the row finishes twice".

8 Rust tests + 2 store tests, all verified load-bearing (restoring the Bash-only guard fails exactly the three completion tests).

Still open, deliberately not in that slice

• tool_progress → elapsed and system/status → compacting/requesting (A running turn is opaque: WORKING names nothing, no elapsed time, generic tool rows show no args and never complete #365 B1). These add a new event variant to event.rs and ripple into the frontend, so they were kept out to keep feat(chat): make MCP and tool rows legible end to end (parser + UI, flag-gated) #387 reviewable.
• The frontend half of both issues: status / detail / elapsedSeconds on the timeline items, McpCard receiving detail, hoisted expansion, ToolUseCard status, WorkingRow naming the running tool, and the virtualizer height estimates.
• MCP tool rows in chat are dead ends: no args, no result, no completion — and pickforge-lanes should show lane cards #362 PR 3 (pickforge-lanes rendering lane cards inline) — unchanged; note fix(pikit): keep expanded lane runs open across the 4s poll #375 already moved the pi-kit store to reconcile and feat(pikit): bound the runs list without hiding live work #388 gives it a bounded page + total, which the extraction should build on rather than re-derive.

Merge-order caveat on #387: it makes the backend emit completions while mcpToolDetail / turnActivity are not yet wired, so its behaviour is ungated. Worth holding it until the frontend PR exists, or gating the events.

Open decision still unresolved for PR 3: live vs snapshot lanes in the chat body.