Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/tui-opentui/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,8 @@ export type ToolCallRowInput = {
readonly name: string
/** Raw JSON arguments as streamed by the model; may be absent or partial. */
readonly arguments?: string
/** Runtime call id, when the source (live bridge, saved history) carried one. */
readonly callId?: string
}

/**
Expand Down Expand Up @@ -492,6 +494,7 @@ export function toolCallRow(input: ToolCallRowInput): StreamRow {
meta,
pending: true,
callKey,
...(input.callId !== undefined ? { callId: input.callId } : {}),
...(diff !== null ? { diff } : {}),
...(verb !== undefined ? { verb } : {}),
// A summarised call may deliberately have no subject — its verb already
Expand Down
17 changes: 17 additions & 0 deletions src/tui-opentui/history-hydrate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,23 @@ describe("hydrateHistoryRows", () => {
])
})

// CL-5562: a resumed transcript with three parallel `task` dispatches has
// three tool_call blocks that all share name "task" — the callId each
// block carries is what tells them apart on replay.
test("resolves parallel same-name tool_call/tool_result pairs by callId", () => {
const rows = hydrateHistoryRows([
{ type: "tool_call", name: "task", arguments: '{"description":"Fix CL-5559"}', callId: "c1" },
{ type: "tool_call", name: "task", arguments: '{"description":"Fix CL-5560"}', callId: "c2" },
{ type: "tool_call", name: "task", arguments: '{"description":"Fix CL-5561"}', callId: "c3" },
{ type: "tool_result", name: "task", content: "done c2", callId: "c2" },
{ type: "tool_result", name: "task", content: "done c1", callId: "c1" },
{ type: "tool_result", name: "task", content: "done c3", callId: "c3" },
])
expect(rows.length).toBe(3)
expect(rows.every((r) => r.pending !== true)).toBe(true)
expect(rows.map((r) => r.text)).toEqual(["done c1", "done c2", "done c3"])
})

test("non-array returns empty", () => {
expect(hydrateHistoryRows(undefined)).toEqual([])
expect(hydrateHistoryRows(null)).toEqual([])
Expand Down
13 changes: 13 additions & 0 deletions src/tui-opentui/history-hydrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ export type HistoryBlock = {
readonly isError?: boolean
/** tool_call argument payload when `content` is absent (ContentBlockData). */
readonly arguments?: string
/**
* Call id carried by a `tool_call` / `tool_result` block. Two saved calls to
* the same tool are indistinguishable by name alone — a resumed transcript
* with parallel sub-agent dispatches needs this to pair each result with
* its own call rather than the newest pending call of that name.
*/
readonly callId?: string
/** view block payload — validated before it reaches the layout pass. */
readonly node?: unknown
/** plan block payload. */
Expand Down Expand Up @@ -51,6 +58,7 @@ function asHistoryBlock(raw: unknown): HistoryBlock | null {
message?: string
isError?: boolean
arguments?: string
callId?: string
node?: unknown
steps?: unknown
tasks?: unknown
Expand All @@ -60,6 +68,7 @@ function asHistoryBlock(raw: unknown): HistoryBlock | null {
if (typeof o.message === "string") out.message = o.message
if (typeof o.isError === "boolean") out.isError = o.isError
if (typeof o.arguments === "string") out.arguments = o.arguments
if (typeof o.callId === "string") out.callId = o.callId
if (o.node !== undefined) out.node = o.node
if (o.steps !== undefined) out.steps = o.steps
if (o.tasks !== undefined) out.tasks = o.tasks
Expand Down Expand Up @@ -135,13 +144,15 @@ export function rowFromHistoryBlock(block: HistoryBlock): StreamRow | null {
return toolCallRow({
name: block.name ?? "tool",
...(args !== undefined ? { arguments: args } : {}),
...(block.callId !== undefined ? { callId: block.callId } : {}),
})
}
case "tool_result":
return toolResultRow({
name: block.name ?? "tool",
content: block.content ?? (block.isError ? "error" : "ok"),
isError: block.isError === true,
...(block.callId !== undefined ? { callId: block.callId } : {}),
})
case "view": {
const text = viewText(block.node) || block.content?.trim() || ""
Expand Down Expand Up @@ -211,6 +222,7 @@ function pushHistoryBlock(rows: StreamRow[], block: HistoryBlock): void {
pushToolCall(rows, {
name: block.name ?? "tool",
...(args !== undefined ? { arguments: args } : {}),
...(block.callId !== undefined ? { callId: block.callId } : {}),
})
return
}
Expand All @@ -219,6 +231,7 @@ function pushHistoryBlock(rows: StreamRow[], block: HistoryBlock): void {
name: block.name ?? "tool",
content: block.content ?? (block.isError ? "error" : "ok"),
isError: block.isError === true,
...(block.callId !== undefined ? { callId: block.callId } : {}),
})
return
}
Expand Down
3 changes: 3 additions & 0 deletions src/tui-opentui/mcp-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ export type ToolResultRowInput = {
readonly name: string
readonly content: string
readonly isError?: boolean
/** Runtime call id this result answers, when the source carried one. */
readonly callId?: string
}

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

Expand Down
4 changes: 4 additions & 0 deletions src/tui-opentui/observe-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ export function rowFromBridgeEvent(event: BridgeInboundEvent): StreamRow | null
return toolCallRow({
name: event.name,
...(event.detail !== undefined ? { arguments: event.detail } : {}),
...(event.callId !== undefined ? { callId: event.callId } : {}),
})
case "tool_result":
return toolResultRow({
name: event.name,
content: event.detail ?? (event.isError ? "error" : "ok"),
isError: event.isError === true,
...(event.callId !== undefined ? { callId: event.callId } : {}),
})
case "system":
return { role: "system", text: event.text }
Expand Down Expand Up @@ -94,6 +96,7 @@ function pushBridgeEvent(
pushToolCall(rows, {
name: event.name,
...(event.detail !== undefined ? { arguments: event.detail } : {}),
...(event.callId !== undefined ? { callId: event.callId } : {}),
})
return
}
Expand All @@ -102,6 +105,7 @@ function pushBridgeEvent(
name: event.name,
content: event.detail ?? (event.isError ? "error" : "ok"),
isError: event.isError === true,
...(event.callId !== undefined ? { callId: event.callId } : {}),
})
return
}
Expand Down
3 changes: 2 additions & 1 deletion src/tui-opentui/runner-host.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ describe("rowFromTranscriptEntry", () => {
verb: "Grep",
pending: true,
callKey: "grep Grep ",
callId: "c",
})
expect(
rowFromTranscriptEntry({
Expand All @@ -81,7 +82,7 @@ describe("rowFromTranscriptEntry", () => {
content: "boom",
isError: true,
}),
).toEqual({ role: "tool", text: "boom", meta: "grep", failed: true })
).toEqual({ role: "tool", text: "boom", meta: "grep", failed: true, callId: "c" })
expect(rowFromTranscriptEntry({ kind: "report", content: "done" })).toEqual({
role: "assistant",
text: "done",
Expand Down
6 changes: 4 additions & 2 deletions src/tui-opentui/runner-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,13 @@ export function rowFromTranscriptEntry(entry: SubAgentTranscriptEntry): StreamRo
case "thinking":
return { role: "system", text: entry.content, meta: "thinking" }
case "tool":
return toolCallRow({ name: entry.name, arguments: entry.arguments })
return toolCallRow({ name: entry.name, arguments: entry.arguments, callId: entry.callId })
case "tool_result":
return toolResultRow({
name: entry.name,
content: entry.content,
isError: entry.isError,
callId: entry.callId,
})
case "report":
return { role: "assistant", text: entry.content, meta: "report" }
Expand All @@ -164,14 +165,15 @@ export function rowsFromTranscript(
const rows: StreamRow[] = []
for (const entry of entries) {
if (entry.kind === "tool") {
pushToolCall(rows, { name: entry.name, arguments: entry.arguments })
pushToolCall(rows, { name: entry.name, arguments: entry.arguments, callId: entry.callId })
continue
}
if (entry.kind === "tool_result") {
pushToolResult(rows, {
name: entry.name,
content: entry.content,
isError: entry.isError,
callId: entry.callId,
})
continue
}
Expand Down
58 changes: 58 additions & 0 deletions src/tui-opentui/runtime-bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -432,3 +432,61 @@ describe("committed inference retry", () => {
)
})
})

describe("parallel sub-agent dispatch on the live session bridge", () => {
// The live main-session path tracks a call's row by callId in its own map
// (applyToolCall/applyToolResult), independent of tool-rows.ts's name-based
// pendingCallIndex — this pins that down so a future change to either path
// cannot silently reintroduce CL-5562's misattribution on the parent
// transcript specifically (the observe overlay and resumed history are
// covered separately in tool-rows.test.ts / history-hydrate.test.ts).
test("three parallel task calls resolve to three rows, each with its own result", async () => {
await withTestRenderer(
async (h) => {
const shell = createAppShell(h.renderer, {
terminal: { columns: 80, rows: 24 },
wireKeys: false,
run: "idle",
})
const bridge = attachSessionBridge(shell, createRecordingPort())
try {
const events = [
{ type: "inference.start", data: {} },
{
type: "inference.tool_call.end",
data: { name: "task", callId: "c1", arguments: { description: "Fix CL-5559" } },
},
{
type: "inference.tool_call.end",
data: { name: "task", callId: "c2", arguments: { description: "Fix CL-5560" } },
},
{
type: "inference.tool_call.end",
data: { name: "task", callId: "c3", arguments: { description: "Fix CL-5561" } },
},
{ type: "inference.done", data: {} },
{ type: "tool.start", data: { call: { id: "c1", name: "task" } } },
{ type: "tool.start", data: { call: { id: "c2", name: "task" } } },
{ type: "tool.start", data: { call: { id: "c3", name: "task" } } },
// Completion order does not follow dispatch order.
{ type: "tool.done", data: { result: { callId: "c2", name: "task", content: "done c2" } } },
{ type: "tool.done", data: { result: { callId: "c1", name: "task", content: "done c1" } } },
{ type: "tool.done", data: { result: { callId: "c3", name: "task", content: "done c3" } } },
{ type: "reactor.done", data: {} },
] as const
for (const event of events) bridge.handle(event)

const toolRows = shell.streamLog.filter((r) => r.role === "tool")
expect(toolRows.length).toBe(3)
expect(toolRows.every((r) => r.pending !== true)).toBe(true)
expect(toolRows.every((r) => r.failed !== true)).toBe(true)
expect(toolRows.map((r) => r.text)).toEqual(["done c1", "done c2", "done c3"])
} finally {
bridge.dispose()
shell.dispose()
}
},
{ width: 80, height: 24 },
)
})
})
9 changes: 9 additions & 0 deletions src/tui-opentui/stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,15 @@ export type StreamRow = {
* onto a single row.
*/
readonly callKey?: string
/**
* Runtime id of the call this row answers, when the source carried one
* (a live reactor callId, a resumed transcript's saved id). A result finds
* the exact row it resolves by this id first — the tool name alone is
* ambiguous the moment two calls to the same tool are in flight at once,
* which parallel sub-agent dispatch does on every turn that fires more
* than one `task` call.
*/
readonly callId?: string
/**
* Row standing for a run of repeated calls. Its subject stays the call the
* run repeats (never a total across them, which would be a claim the
Expand Down
88 changes: 87 additions & 1 deletion src/tui-opentui/tool-rows.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ import { withTestRenderer } from "./harness"
import { attachSessionBridge, createRecordingPort } from "./runtime-bridge"
import { createAppShell } from "./shell"
import {
isCollapsibleRow,
paintStreamRow,
toolSentenceLines,
type RowLayout,
type StreamRow,
} from "./stream"
import { pushToolCall, pushToolResult } from "./tool-rows"
import { pendingCallIndex, pushToolCall, pushToolResult } from "./tool-rows"

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

Expand Down Expand Up @@ -124,6 +125,91 @@ describe("a run of identical calls", () => {
})
})

describe("parallel calls to the same tool", () => {
// CL-5562: three `task` calls dispatched in one turn all carry
// meta === "task" — name alone cannot tell them apart, so a result must
// find its own row by call id or it resolves whichever pending "task" row
// happens to be newest, leaving the others stranded pending forever and
// turning any later same-name result into an orphaned extra row.
test("each result resolves its own call by id, not the newest pending call of that name", () => {
const rows: StreamRow[] = []
pushToolCall(rows, {
name: "task",
arguments: JSON.stringify({ agent: "intern", description: "Fix CL-5559 heading shake" }),
callId: "c1",
})
pushToolCall(rows, {
name: "task",
arguments: JSON.stringify({ agent: "intern", description: "Fix CL-5560 approval UI" }),
callId: "c2",
})
pushToolCall(rows, {
name: "task",
arguments: JSON.stringify({ agent: "intern", description: "Fix CL-5561 scroll/history" }),
callId: "c3",
})
expect(rows.length).toBe(3)

// Results land out of dispatch order, as real sub-agent completion does.
pushToolResult(rows, { name: "task", content: "done c2", callId: "c2" })
pushToolResult(rows, { name: "task", content: "done c1", callId: "c1" })
pushToolResult(rows, { name: "task", content: "done c3", callId: "c3" })

expect(rows.length).toBe(3)
expect(rows.every((r) => r.pending !== true)).toBe(true)
expect(rows.every((r) => r.failed !== true)).toBe(true)
expect(rows[0]?.summary).toBe("Fix CL-5559 heading shake")
expect(rows[0]?.text).toBe("done c1")
expect(rows[1]?.summary).toBe("Fix CL-5560 approval UI")
expect(rows[1]?.text).toBe("done c2")
expect(rows[2]?.summary).toBe("Fix CL-5561 scroll/history")
expect(rows[2]?.text).toBe("done c3")
})

// A miss must not fall back to "the newest pending row of that name" — that
// fallback is exactly the LIFO misattribution this test file exists to rule
// out, and every live caller (the bridge's own call map, subagent session
// entries, resumed history with ids) always carries a real id, so a miss
// here means the id genuinely does not belong to anything on the log.
test("an id that matches nothing on the log answers nothing, not the newest pending call", () => {
const rows: StreamRow[] = [
{ role: "tool", text: "", meta: "task", pending: true, callId: "a1" },
{ role: "tool", text: "", meta: "task", pending: true, callId: "b1" },
]
expect(pendingCallIndex(rows, "task", "zzz-does-not-exist")).toBe(-1)

pushToolResult(rows, { name: "task", content: "orphan", callId: "zzz-does-not-exist" })
// Answers nothing on the log — appended as its own row rather than
// resolving (and thereby corrupting) an unrelated in-flight call.
expect(rows.length).toBe(3)
expect(rows[0]?.pending).toBe(true)
expect(rows[1]?.pending).toBe(true)
})

// Acceptance criterion: a failed sub-agent surfaces its error inline
// (expandable), not a bare mark with nothing behind it. `mergeToolRows` /
// `toolResultRow` already carry the failed result's own text into `detail`
// — untouched by this fix, but only reachable per-call once results resolve
// to the right row instead of a neighbour's.
test("a failed call keeps its error text behind the expand arrow", () => {
const rows: StreamRow[] = []
pushToolCall(rows, {
name: "task",
arguments: JSON.stringify({ agent: "intern", description: "Fix CL-5559 heading shake" }),
callId: "c1",
})
pushToolResult(rows, {
name: "task",
content: 'Error: sub-agent "Fix CL-5559 heading shake" failed: boom',
isError: true,
callId: "c1",
})
expect(rows[0]?.failed).toBe(true)
expect(isCollapsibleRow(rows[0]!)).toBe(true)
expect(rows[0]?.detail?.[0]?.[0]?.text).toContain("boom")
})
})

describe("a long subject", () => {
test("is cut to one line rather than wrapped", () => {
const row = toolCallRow({
Expand Down
Loading
Loading