Skip to content

Commit b5489c6

Browse files
committed
Resolve tool result rows by call id instead of tool name
Matching a result to its call row by tool name alone breaks the moment two calls to the same tool are in flight together, which parallel sub-agent dispatch does on every turn that fires more than one task call. The newest pending row of that name absorbed whichever result landed first, stranding the others pending and turning later results into orphan rows. An id that matches nothing on the log now answers nothing rather than falling back to the newest same-name row, since every current caller carries a real id. The name-based fallback survives only for callId undefined, which saved history from before ids were threaded through this path still produces.
1 parent f78c51b commit b5489c6

11 files changed

Lines changed: 224 additions & 8 deletions

‎src/tui-opentui/diff.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,8 @@ export type ToolCallRowInput = {
445445
readonly name: string
446446
/** Raw JSON arguments as streamed by the model; may be absent or partial. */
447447
readonly arguments?: string
448+
/** Runtime call id, when the source (live bridge, saved history) carried one. */
449+
readonly callId?: string
448450
}
449451

450452
/**
@@ -492,6 +494,7 @@ export function toolCallRow(input: ToolCallRowInput): StreamRow {
492494
meta,
493495
pending: true,
494496
callKey,
497+
...(input.callId !== undefined ? { callId: input.callId } : {}),
495498
...(diff !== null ? { diff } : {}),
496499
...(verb !== undefined ? { verb } : {}),
497500
// A summarised call may deliberately have no subject — its verb already

‎src/tui-opentui/history-hydrate.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,23 @@ describe("hydrateHistoryRows", () => {
208208
])
209209
})
210210

211+
// CL-5562: a resumed transcript with three parallel `task` dispatches has
212+
// three tool_call blocks that all share name "task" — the callId each
213+
// block carries is what tells them apart on replay.
214+
test("resolves parallel same-name tool_call/tool_result pairs by callId", () => {
215+
const rows = hydrateHistoryRows([
216+
{ type: "tool_call", name: "task", arguments: '{"description":"Fix CL-5559"}', callId: "c1" },
217+
{ type: "tool_call", name: "task", arguments: '{"description":"Fix CL-5560"}', callId: "c2" },
218+
{ type: "tool_call", name: "task", arguments: '{"description":"Fix CL-5561"}', callId: "c3" },
219+
{ type: "tool_result", name: "task", content: "done c2", callId: "c2" },
220+
{ type: "tool_result", name: "task", content: "done c1", callId: "c1" },
221+
{ type: "tool_result", name: "task", content: "done c3", callId: "c3" },
222+
])
223+
expect(rows.length).toBe(3)
224+
expect(rows.every((r) => r.pending !== true)).toBe(true)
225+
expect(rows.map((r) => r.text)).toEqual(["done c1", "done c2", "done c3"])
226+
})
227+
211228
test("non-array returns empty", () => {
212229
expect(hydrateHistoryRows(undefined)).toEqual([])
213230
expect(hydrateHistoryRows(null)).toEqual([])

‎src/tui-opentui/history-hydrate.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ export type HistoryBlock = {
2424
readonly isError?: boolean
2525
/** tool_call argument payload when `content` is absent (ContentBlockData). */
2626
readonly arguments?: string
27+
/**
28+
* Call id carried by a `tool_call` / `tool_result` block. Two saved calls to
29+
* the same tool are indistinguishable by name alone — a resumed transcript
30+
* with parallel sub-agent dispatches needs this to pair each result with
31+
* its own call rather than the newest pending call of that name.
32+
*/
33+
readonly callId?: string
2734
/** view block payload — validated before it reaches the layout pass. */
2835
readonly node?: unknown
2936
/** plan block payload. */
@@ -51,6 +58,7 @@ function asHistoryBlock(raw: unknown): HistoryBlock | null {
5158
message?: string
5259
isError?: boolean
5360
arguments?: string
61+
callId?: string
5462
node?: unknown
5563
steps?: unknown
5664
tasks?: unknown
@@ -60,6 +68,7 @@ function asHistoryBlock(raw: unknown): HistoryBlock | null {
6068
if (typeof o.message === "string") out.message = o.message
6169
if (typeof o.isError === "boolean") out.isError = o.isError
6270
if (typeof o.arguments === "string") out.arguments = o.arguments
71+
if (typeof o.callId === "string") out.callId = o.callId
6372
if (o.node !== undefined) out.node = o.node
6473
if (o.steps !== undefined) out.steps = o.steps
6574
if (o.tasks !== undefined) out.tasks = o.tasks
@@ -135,13 +144,15 @@ export function rowFromHistoryBlock(block: HistoryBlock): StreamRow | null {
135144
return toolCallRow({
136145
name: block.name ?? "tool",
137146
...(args !== undefined ? { arguments: args } : {}),
147+
...(block.callId !== undefined ? { callId: block.callId } : {}),
138148
})
139149
}
140150
case "tool_result":
141151
return toolResultRow({
142152
name: block.name ?? "tool",
143153
content: block.content ?? (block.isError ? "error" : "ok"),
144154
isError: block.isError === true,
155+
...(block.callId !== undefined ? { callId: block.callId } : {}),
145156
})
146157
case "view": {
147158
const text = viewText(block.node) || block.content?.trim() || ""
@@ -211,6 +222,7 @@ function pushHistoryBlock(rows: StreamRow[], block: HistoryBlock): void {
211222
pushToolCall(rows, {
212223
name: block.name ?? "tool",
213224
...(args !== undefined ? { arguments: args } : {}),
225+
...(block.callId !== undefined ? { callId: block.callId } : {}),
214226
})
215227
return
216228
}
@@ -219,6 +231,7 @@ function pushHistoryBlock(rows: StreamRow[], block: HistoryBlock): void {
219231
name: block.name ?? "tool",
220232
content: block.content ?? (block.isError ? "error" : "ok"),
221233
isError: block.isError === true,
234+
...(block.callId !== undefined ? { callId: block.callId } : {}),
222235
})
223236
return
224237
}

‎src/tui-opentui/mcp-view.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,8 @@ export type ToolResultRowInput = {
264264
readonly name: string
265265
readonly content: string
266266
readonly isError?: boolean
267+
/** Runtime call id this result answers, when the source carried one. */
268+
readonly callId?: string
267269
}
268270

269271
/** Bodies at or under this many lines read faster than a sentence about them. */
@@ -487,6 +489,7 @@ export function toolResultRow(input: ToolResultRowInput): StreamRow {
487489
role: "tool" as const,
488490
text: input.content,
489491
meta: input.name,
492+
...(input.callId !== undefined ? { callId: input.callId } : {}),
490493
}
491494
if (failed) return { ...base, failed: true }
492495

‎src/tui-opentui/observe-map.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,14 @@ export function rowFromBridgeEvent(event: BridgeInboundEvent): StreamRow | null
3333
return toolCallRow({
3434
name: event.name,
3535
...(event.detail !== undefined ? { arguments: event.detail } : {}),
36+
...(event.callId !== undefined ? { callId: event.callId } : {}),
3637
})
3738
case "tool_result":
3839
return toolResultRow({
3940
name: event.name,
4041
content: event.detail ?? (event.isError ? "error" : "ok"),
4142
isError: event.isError === true,
43+
...(event.callId !== undefined ? { callId: event.callId } : {}),
4244
})
4345
case "system":
4446
return { role: "system", text: event.text }
@@ -94,6 +96,7 @@ function pushBridgeEvent(
9496
pushToolCall(rows, {
9597
name: event.name,
9698
...(event.detail !== undefined ? { arguments: event.detail } : {}),
99+
...(event.callId !== undefined ? { callId: event.callId } : {}),
97100
})
98101
return
99102
}
@@ -102,6 +105,7 @@ function pushBridgeEvent(
102105
name: event.name,
103106
content: event.detail ?? (event.isError ? "error" : "ok"),
104107
isError: event.isError === true,
108+
...(event.callId !== undefined ? { callId: event.callId } : {}),
105109
})
106110
return
107111
}

‎src/tui-opentui/runner-host.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ describe("rowFromTranscriptEntry", () => {
7272
verb: "Grep",
7373
pending: true,
7474
callKey: "grep Grep ",
75+
callId: "c",
7576
})
7677
expect(
7778
rowFromTranscriptEntry({
@@ -81,7 +82,7 @@ describe("rowFromTranscriptEntry", () => {
8182
content: "boom",
8283
isError: true,
8384
}),
84-
).toEqual({ role: "tool", text: "boom", meta: "grep", failed: true })
85+
).toEqual({ role: "tool", text: "boom", meta: "grep", failed: true, callId: "c" })
8586
expect(rowFromTranscriptEntry({ kind: "report", content: "done" })).toEqual({
8687
role: "assistant",
8788
text: "done",

‎src/tui-opentui/runner-host.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,13 @@ export function rowFromTranscriptEntry(entry: SubAgentTranscriptEntry): StreamRo
141141
case "thinking":
142142
return { role: "system", text: entry.content, meta: "thinking" }
143143
case "tool":
144-
return toolCallRow({ name: entry.name, arguments: entry.arguments })
144+
return toolCallRow({ name: entry.name, arguments: entry.arguments, callId: entry.callId })
145145
case "tool_result":
146146
return toolResultRow({
147147
name: entry.name,
148148
content: entry.content,
149149
isError: entry.isError,
150+
callId: entry.callId,
150151
})
151152
case "report":
152153
return { role: "assistant", text: entry.content, meta: "report" }
@@ -164,14 +165,15 @@ export function rowsFromTranscript(
164165
const rows: StreamRow[] = []
165166
for (const entry of entries) {
166167
if (entry.kind === "tool") {
167-
pushToolCall(rows, { name: entry.name, arguments: entry.arguments })
168+
pushToolCall(rows, { name: entry.name, arguments: entry.arguments, callId: entry.callId })
168169
continue
169170
}
170171
if (entry.kind === "tool_result") {
171172
pushToolResult(rows, {
172173
name: entry.name,
173174
content: entry.content,
174175
isError: entry.isError,
176+
callId: entry.callId,
175177
})
176178
continue
177179
}

‎src/tui-opentui/runtime-bridge.test.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,3 +432,61 @@ describe("committed inference retry", () => {
432432
)
433433
})
434434
})
435+
436+
describe("parallel sub-agent dispatch on the live session bridge", () => {
437+
// The live main-session path tracks a call's row by callId in its own map
438+
// (applyToolCall/applyToolResult), independent of tool-rows.ts's name-based
439+
// pendingCallIndex — this pins that down so a future change to either path
440+
// cannot silently reintroduce CL-5562's misattribution on the parent
441+
// transcript specifically (the observe overlay and resumed history are
442+
// covered separately in tool-rows.test.ts / history-hydrate.test.ts).
443+
test("three parallel task calls resolve to three rows, each with its own result", async () => {
444+
await withTestRenderer(
445+
async (h) => {
446+
const shell = createAppShell(h.renderer, {
447+
terminal: { columns: 80, rows: 24 },
448+
wireKeys: false,
449+
run: "idle",
450+
})
451+
const bridge = attachSessionBridge(shell, createRecordingPort())
452+
try {
453+
const events = [
454+
{ type: "inference.start", data: {} },
455+
{
456+
type: "inference.tool_call.end",
457+
data: { name: "task", callId: "c1", arguments: { description: "Fix CL-5559" } },
458+
},
459+
{
460+
type: "inference.tool_call.end",
461+
data: { name: "task", callId: "c2", arguments: { description: "Fix CL-5560" } },
462+
},
463+
{
464+
type: "inference.tool_call.end",
465+
data: { name: "task", callId: "c3", arguments: { description: "Fix CL-5561" } },
466+
},
467+
{ type: "inference.done", data: {} },
468+
{ type: "tool.start", data: { call: { id: "c1", name: "task" } } },
469+
{ type: "tool.start", data: { call: { id: "c2", name: "task" } } },
470+
{ type: "tool.start", data: { call: { id: "c3", name: "task" } } },
471+
// Completion order does not follow dispatch order.
472+
{ type: "tool.done", data: { result: { callId: "c2", name: "task", content: "done c2" } } },
473+
{ type: "tool.done", data: { result: { callId: "c1", name: "task", content: "done c1" } } },
474+
{ type: "tool.done", data: { result: { callId: "c3", name: "task", content: "done c3" } } },
475+
{ type: "reactor.done", data: {} },
476+
] as const
477+
for (const event of events) bridge.handle(event)
478+
479+
const toolRows = shell.streamLog.filter((r) => r.role === "tool")
480+
expect(toolRows.length).toBe(3)
481+
expect(toolRows.every((r) => r.pending !== true)).toBe(true)
482+
expect(toolRows.every((r) => r.failed !== true)).toBe(true)
483+
expect(toolRows.map((r) => r.text)).toEqual(["done c1", "done c2", "done c3"])
484+
} finally {
485+
bridge.dispose()
486+
shell.dispose()
487+
}
488+
},
489+
{ width: 80, height: 24 },
490+
)
491+
})
492+
})

‎src/tui-opentui/stream.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,15 @@ export type StreamRow = {
7171
* onto a single row.
7272
*/
7373
readonly callKey?: string
74+
/**
75+
* Runtime id of the call this row answers, when the source carried one
76+
* (a live reactor callId, a resumed transcript's saved id). A result finds
77+
* the exact row it resolves by this id first — the tool name alone is
78+
* ambiguous the moment two calls to the same tool are in flight at once,
79+
* which parallel sub-agent dispatch does on every turn that fires more
80+
* than one `task` call.
81+
*/
82+
readonly callId?: string
7483
/**
7584
* Row standing for a run of repeated calls. Its subject stays the call the
7685
* run repeats (never a total across them, which would be a claim the

‎src/tui-opentui/tool-rows.test.ts‎

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ import { withTestRenderer } from "./harness"
99
import { attachSessionBridge, createRecordingPort } from "./runtime-bridge"
1010
import { createAppShell } from "./shell"
1111
import {
12+
isCollapsibleRow,
1213
paintStreamRow,
1314
toolSentenceLines,
1415
type RowLayout,
1516
type StreamRow,
1617
} from "./stream"
17-
import { pushToolCall, pushToolResult } from "./tool-rows"
18+
import { pendingCallIndex, pushToolCall, pushToolResult } from "./tool-rows"
1819

1920
const LAYOUT: RowLayout = { width: 72, multiAgent: false }
2021

@@ -124,6 +125,91 @@ describe("a run of identical calls", () => {
124125
})
125126
})
126127

128+
describe("parallel calls to the same tool", () => {
129+
// CL-5562: three `task` calls dispatched in one turn all carry
130+
// meta === "task" — name alone cannot tell them apart, so a result must
131+
// find its own row by call id or it resolves whichever pending "task" row
132+
// happens to be newest, leaving the others stranded pending forever and
133+
// turning any later same-name result into an orphaned extra row.
134+
test("each result resolves its own call by id, not the newest pending call of that name", () => {
135+
const rows: StreamRow[] = []
136+
pushToolCall(rows, {
137+
name: "task",
138+
arguments: JSON.stringify({ agent: "intern", description: "Fix CL-5559 heading shake" }),
139+
callId: "c1",
140+
})
141+
pushToolCall(rows, {
142+
name: "task",
143+
arguments: JSON.stringify({ agent: "intern", description: "Fix CL-5560 approval UI" }),
144+
callId: "c2",
145+
})
146+
pushToolCall(rows, {
147+
name: "task",
148+
arguments: JSON.stringify({ agent: "intern", description: "Fix CL-5561 scroll/history" }),
149+
callId: "c3",
150+
})
151+
expect(rows.length).toBe(3)
152+
153+
// Results land out of dispatch order, as real sub-agent completion does.
154+
pushToolResult(rows, { name: "task", content: "done c2", callId: "c2" })
155+
pushToolResult(rows, { name: "task", content: "done c1", callId: "c1" })
156+
pushToolResult(rows, { name: "task", content: "done c3", callId: "c3" })
157+
158+
expect(rows.length).toBe(3)
159+
expect(rows.every((r) => r.pending !== true)).toBe(true)
160+
expect(rows.every((r) => r.failed !== true)).toBe(true)
161+
expect(rows[0]?.summary).toBe("Fix CL-5559 heading shake")
162+
expect(rows[0]?.text).toBe("done c1")
163+
expect(rows[1]?.summary).toBe("Fix CL-5560 approval UI")
164+
expect(rows[1]?.text).toBe("done c2")
165+
expect(rows[2]?.summary).toBe("Fix CL-5561 scroll/history")
166+
expect(rows[2]?.text).toBe("done c3")
167+
})
168+
169+
// A miss must not fall back to "the newest pending row of that name" — that
170+
// fallback is exactly the LIFO misattribution this test file exists to rule
171+
// out, and every live caller (the bridge's own call map, subagent session
172+
// entries, resumed history with ids) always carries a real id, so a miss
173+
// here means the id genuinely does not belong to anything on the log.
174+
test("an id that matches nothing on the log answers nothing, not the newest pending call", () => {
175+
const rows: StreamRow[] = [
176+
{ role: "tool", text: "", meta: "task", pending: true, callId: "a1" },
177+
{ role: "tool", text: "", meta: "task", pending: true, callId: "b1" },
178+
]
179+
expect(pendingCallIndex(rows, "task", "zzz-does-not-exist")).toBe(-1)
180+
181+
pushToolResult(rows, { name: "task", content: "orphan", callId: "zzz-does-not-exist" })
182+
// Answers nothing on the log — appended as its own row rather than
183+
// resolving (and thereby corrupting) an unrelated in-flight call.
184+
expect(rows.length).toBe(3)
185+
expect(rows[0]?.pending).toBe(true)
186+
expect(rows[1]?.pending).toBe(true)
187+
})
188+
189+
// Acceptance criterion: a failed sub-agent surfaces its error inline
190+
// (expandable), not a bare mark with nothing behind it. `mergeToolRows` /
191+
// `toolResultRow` already carry the failed result's own text into `detail`
192+
// — untouched by this fix, but only reachable per-call once results resolve
193+
// to the right row instead of a neighbour's.
194+
test("a failed call keeps its error text behind the expand arrow", () => {
195+
const rows: StreamRow[] = []
196+
pushToolCall(rows, {
197+
name: "task",
198+
arguments: JSON.stringify({ agent: "intern", description: "Fix CL-5559 heading shake" }),
199+
callId: "c1",
200+
})
201+
pushToolResult(rows, {
202+
name: "task",
203+
content: 'Error: sub-agent "Fix CL-5559 heading shake" failed: boom',
204+
isError: true,
205+
callId: "c1",
206+
})
207+
expect(rows[0]?.failed).toBe(true)
208+
expect(isCollapsibleRow(rows[0]!)).toBe(true)
209+
expect(rows[0]?.detail?.[0]?.[0]?.text).toContain("boom")
210+
})
211+
})
212+
127213
describe("a long subject", () => {
128214
test("is cut to one line rather than wrapped", () => {
129215
const row = toolCallRow({

0 commit comments

Comments
 (0)