Skip to content

Commit cd21f54

Browse files
Merge call-id keyed tool result rows
2 parents 95b4267 + b1869f5 commit cd21f54

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)