From 8ac7cf0e762d09016ffc0ae90fa349ab67deb286 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Wed, 22 Jul 2026 08:59:32 -0400 Subject: [PATCH] fix: preserve partial tool failures --- packages/cli/src/acp/event.ts | 2 +- packages/cli/src/run/noninteractive.ts | 12 ++++++---- packages/cli/test/acp/event-behavior.test.ts | 2 +- packages/cli/test/run/noninteractive.test.ts | 6 ++--- .../client/src/promise/generated/types.ts | 4 ++-- packages/core/src/session/message-updater.ts | 2 +- packages/core/src/session/runner/llm.ts | 2 -- .../src/session/runner/publish-llm-event.ts | 20 +++++++++------- .../test/session-runner-tool-events.test.ts | 12 +++++++++- packages/core/test/session-runner.test.ts | 2 +- .../core/test/session-tool-progress.test.ts | 7 +++--- packages/schema/src/session-event.ts | 23 +++++++++---------- packages/tui/src/context/data.tsx | 2 +- packages/tui/src/mini/stream-v2.subagent.ts | 4 ++-- packages/tui/src/mini/stream-v2.transport.ts | 2 +- .../tui/test/mini/stream-v2.transport.test.ts | 7 +++--- 16 files changed, 60 insertions(+), 49 deletions(-) diff --git a/packages/cli/src/acp/event.ts b/packages/cli/src/acp/event.ts index 277f107a11..da8bfe168b 100644 --- a/packages/cli/src/acp/event.ts +++ b/packages/cli/src/acp/event.ts @@ -198,7 +198,7 @@ export async function streamTurn(input: { toolCallId: event.data.callID, toolName: current.name, input: current.input, - structured: event.data.structured ?? current.structured, + structured: event.data.metadata ?? current.structured, content: event.data.content ?? current.content, error: event.data.error.message, cwd: input.cwd, diff --git a/packages/cli/src/run/noninteractive.ts b/packages/cli/src/run/noninteractive.ts index 7a564626c7..41859d3b64 100644 --- a/packages/cli/src/run/noninteractive.ts +++ b/packages/cli/src/run/noninteractive.ts @@ -398,6 +398,8 @@ export async function runNonInteractivePrompt(input: Input) { const key = toolKey(event.data.assistantMessageID, event.data.callID) const current = tools.get(key) ?? fallbackTool(event) const error = event.data.error.message + const structured = event.data.metadata ?? current.structured + const content = event.data.content ?? current.content const tool: SessionMessageAssistantTool = { type: "tool", id: event.data.callID, @@ -408,8 +410,8 @@ export async function runNonInteractivePrompt(input: Input) { state: { status: "error", input: current.input, - structured: event.data.structured ?? current.structured, - content: event.data.content ?? current.content, + structured, + content, error: event.data.error, result: event.data.result, }, @@ -439,14 +441,14 @@ export async function runNonInteractivePrompt(input: Input) { renderedTools.add(key) if (input.compatibility === "v1" && (permissionRejected || formCancelled)) continue if (!emit("tool_use", time, { part })) { - if (toolOutputText(current.tool, current.content).trim()) + if (toolOutputText(current.tool, content).trim()) await input.renderTool({ ...tool, state: { status: "completed", input: current.input, - structured: current.structured, - content: current.content, + structured, + content, result: event.data.result, }, }) diff --git a/packages/cli/test/acp/event-behavior.test.ts b/packages/cli/test/acp/event-behavior.test.ts index 3c63f063a4..1f9935f5c8 100644 --- a/packages/cli/test/acp/event-behavior.test.ts +++ b/packages/cli/test/acp/event-behavior.test.ts @@ -265,7 +265,7 @@ describe("acp event behavior", () => { assistantMessageID: "msg_tools", callID: "call_fail", error: { type: "tool.error", message: "not found" }, - structured: { bytes: 0 }, + metadata: { bytes: 0 }, content: [{ type: "text", text: "opening" }], executed: true, }), diff --git a/packages/cli/test/run/noninteractive.test.ts b/packages/cli/test/run/noninteractive.test.ts index 1462c7da59..904ced7474 100644 --- a/packages/cli/test/run/noninteractive.test.ts +++ b/packages/cli/test/run/noninteractive.test.ts @@ -148,7 +148,7 @@ function failedTool(inputID: string): V2Event[] { assistantMessageID: "msg_failed_tool", callID: "call_failed_tool", error: { type: "unknown", message: "tool failed" }, - structured: { checkpoint: 1 }, + metadata: { checkpoint: 1 }, content: [{ type: "text", text: "partial output" }], executed: true, }, @@ -441,11 +441,11 @@ describe("runNonInteractivePrompt", () => { expect(output).toEqual({ stdout: "", stderr: "", exitCode: 0 }) }) - test("renders native failed tool output before the terminal error", async () => { + test("renders a native terminal failure snapshot when live progress was missed", async () => { const rendered: SessionMessageAssistantTool[] = [] const failed: SessionMessageAssistantTool[] = [] await capture({ - turn: failedTool, + turn: (inputID) => failedTool(inputID).filter((event) => event.type !== "session.tool.progress"), renderTool: (part) => { rendered.push(part) return Promise.resolve() diff --git a/packages/client/src/promise/generated/types.ts b/packages/client/src/promise/generated/types.ts index 9446220da5..b9c46e4010 100644 --- a/packages/client/src/promise/generated/types.ts +++ b/packages/client/src/promise/generated/types.ts @@ -1863,8 +1863,8 @@ export type SessionToolFailed = { assistantMessageID: string callID: string error: SessionStructuredError - structured?: { [x: string]: any } - content?: Array + content?: [LLMToolContent, ...Array] + metadata?: { [x: string]: any } result?: any executed: boolean resultState?: SessionMessageProviderState7 diff --git a/packages/core/src/session/message-updater.ts b/packages/core/src/session/message-updater.ts index fa7b86a773..ecc6bf6257 100644 --- a/packages/core/src/session/message-updater.ts +++ b/packages/core/src/session/message-updater.ts @@ -402,7 +402,7 @@ export function update(adapter: Adapter, event: SessionEvent.Event) { status: "error", error: event.data.error, input: typeof match.state.input === "string" ? {} : match.state.input, - structured: event.data.structured ?? (match.state.status === "running" ? match.state.structured : {}), + structured: event.data.metadata ?? (match.state.status === "running" ? match.state.structured : {}), content: event.data.content ?? (match.state.status === "running" ? match.state.content : []), result: event.data.result, }), diff --git a/packages/core/src/session/runner/llm.ts b/packages/core/src/session/runner/llm.ts index 8c38de47bb..ad792ce1c7 100644 --- a/packages/core/src/session/runner/llm.ts +++ b/packages/core/src/session/runner/llm.ts @@ -62,8 +62,6 @@ const layer = Layer.effect( assistantMessageID: message.id, callID: tool.id, error: { type: "aborted", message: `Tool execution interrupted: ${tool.name}` }, - structured: tool.state.status === "running" ? tool.state.structured : {}, - content: tool.state.status === "running" ? tool.state.content : [], executed: tool.executed === true, }) } diff --git a/packages/core/src/session/runner/publish-llm-event.ts b/packages/core/src/session/runner/publish-llm-event.ts index 0f9e0586dc..df3e950895 100644 --- a/packages/core/src/session/runner/publish-llm-event.ts +++ b/packages/core/src/session/runner/publish-llm-event.ts @@ -58,10 +58,14 @@ export const createLLMEventPublisher = (events: Pick() - const failureState = (tool: { readonly progress?: ToolRegistry.Progress }) => ({ - structured: tool.progress?.structured ?? {}, - content: tool.progress?.content ?? [], - }) + const failureSnapshot = (tool: { readonly progress?: ToolRegistry.Progress }) => { + if (!tool.progress) return {} + const first = tool.progress.content[0] + return { + ...(first === undefined ? {} : { content: [first, ...tool.progress.content.slice(1)] as const }), + metadata: tool.progress.structured, + } + } let assistantMessageID = input.assistantMessageID let stepStarted = false let stepFailed = false @@ -238,7 +242,7 @@ export const createLLMEventPublisher = (events: Pick event.type === "session.tool.failed.1")?.data).toMatchObject({ - structured: { phase: "visible" }, + metadata: { phase: "visible" }, content: [{ type: "text", text: "visible" }], }) }) +test("failure before progress omits partial output fields", async () => { + const { published, publisher } = capture() + await Effect.runPromise(publisher.publish(call)) + await Effect.runPromise(publisher.failUnsettledTools({ type: "aborted", message: "interrupted" })) + + const failed = published.find((event) => event.type === "session.tool.failed.1")?.data + expect(failed).not.toHaveProperty("content") + expect(failed).not.toHaveProperty("metadata") +}) + test("provider metadata is flattened using the route key", async () => { const { published, publisher } = capture() await Effect.runPromise( diff --git a/packages/core/test/session-runner.test.ts b/packages/core/test/session-runner.test.ts index 59bcd8a34b..6ce9cf0f20 100644 --- a/packages/core/test/session-runner.test.ts +++ b/packages/core/test/session-runner.test.ts @@ -892,7 +892,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("persists the final progress snapshot when a tool fails", () => + it.effect("persists the latest partial snapshot when a tool fails", () => Effect.gen(function* () { const session = yield* setup const registry = yield* ToolRegistry.Service diff --git a/packages/core/test/session-tool-progress.test.ts b/packages/core/test/session-tool-progress.test.ts index 9e11a7aef3..cc81820e39 100644 --- a/packages/core/test/session-tool-progress.test.ts +++ b/packages/core/test/session-tool-progress.test.ts @@ -22,10 +22,10 @@ const it = testEffect(LayerNode.compile(LayerNode.group([Database.node, EventV2. const timestamp = DateTime.makeUnsafe(1) const model = { id: ModelV2.ID.make("model"), providerID: ProviderV2.ID.make("provider") } -const content = (text: string) => [{ type: "text" as const, text }] +const content = (text: string) => [{ type: "text" as const, text }] as const describe("Tool.Progress", () => { - it.effect("keeps progress live-only and terminal settlements replayable", () => + it.effect("keeps progress live-only and terminal settlements durable", () => Effect.gen(function* () { const { db } = yield* Database.Service const service = yield* EventV2.Service @@ -123,7 +123,7 @@ describe("Tool.Progress", () => { assistantMessageID, callID: "call-failed", error: { type: "unknown", message: "boom" }, - structured: { phase: "checkpoint" }, + metadata: { phase: "checkpoint" }, content: content("before failure"), executed: false, }) @@ -149,7 +149,6 @@ describe("Tool.Progress", () => { expect(rows.map((row) => row.type)).not.toContain(EventV2.versionedType(SessionEvent.Tool.Progress.type, 1)) expect(rows.map((row) => row.type)).toContain(EventV2.versionedType(SessionEvent.Tool.Success.type, 1)) expect(rows.map((row) => row.type)).toContain(EventV2.versionedType(SessionEvent.Tool.Failed.type, 1)) - }), ) }) diff --git a/packages/schema/src/session-event.ts b/packages/schema/src/session-event.ts index 5c9c40a871..cc56a83eab 100644 --- a/packages/schema/src/session-event.ts +++ b/packages/schema/src/session-event.ts @@ -3,7 +3,7 @@ export * as SessionEvent from "./session-event.js" import { Schema } from "effect" import { optional } from "./schema.js" import { Event } from "./event.js" -import { FinishReason } from "./llm.js" +import { FinishReason, ToolContent } from "./llm.js" import { Model } from "./model.js" import { NonNegativeInt, PositiveInt, RelativePath } from "./schema.js" import { FileAttachment } from "./prompt.js" @@ -409,15 +409,14 @@ export namespace Tool { }) export type Called = typeof Called.Type - const ToolOutputFields = { - structured: SessionMessage.ToolStateRunning.fields.structured, - content: SessionMessage.ToolStateRunning.fields.content, - } - - /** Live replacement snapshot for a running tool. Terminal events own durable output. */ + /** Live replacement snapshot for a running tool. */ export const Progress = Event.ephemeral({ type: "session.tool.progress", - schema: { ...ToolBase, ...ToolOutputFields }, + schema: { + ...ToolBase, + structured: Schema.Record(Schema.String, Schema.Unknown), + content: Schema.Array(ToolContent), + }, }) export type Progress = typeof Progress.Type @@ -426,7 +425,8 @@ export namespace Tool { ...options, schema: { ...ToolBase, - ...ToolOutputFields, + structured: Schema.Record(Schema.String, Schema.Unknown), + content: Schema.Array(ToolContent), result: Schema.Unknown.pipe(optional), executed: Schema.Boolean, resultState: SessionMessage.ProviderState.pipe(optional), @@ -440,9 +440,8 @@ export namespace Tool { schema: { ...ToolBase, error: SessionError.Error, - // Optional only for compatibility with existing v1 failure rows. - structured: ToolOutputFields.structured.pipe(optional), - content: ToolOutputFields.content.pipe(optional), + content: Schema.NonEmptyArray(ToolContent).pipe(optional), + metadata: Schema.Record(Schema.String, Schema.Unknown).pipe(optional), result: Schema.Unknown.pipe(optional), executed: Schema.Boolean, resultState: SessionMessage.ProviderState.pipe(optional), diff --git a/packages/tui/src/context/data.tsx b/packages/tui/src/context/data.tsx index 8ea43a921a..ddf3823e75 100644 --- a/packages/tui/src/context/data.tsx +++ b/packages/tui/src/context/data.tsx @@ -643,7 +643,7 @@ export const { use: useData, provider: DataProvider } = createSimpleContext({ status: "error", error: event.data.error, input: typeof match.state.input === "string" ? {} : match.state.input, - structured: event.data.structured ?? (match.state.status === "running" ? match.state.structured : {}), + structured: event.data.metadata ?? (match.state.status === "running" ? match.state.structured : {}), content: event.data.content ?? (match.state.status === "running" ? match.state.content : []), result: event.data.result, } diff --git a/packages/tui/src/mini/stream-v2.subagent.ts b/packages/tui/src/mini/stream-v2.subagent.ts index 07f0059a0a..ea3b566af4 100644 --- a/packages/tui/src/mini/stream-v2.subagent.ts +++ b/packages/tui/src/mini/stream-v2.subagent.ts @@ -838,7 +838,7 @@ export function createSubagentTracker(input: SubagentTrackerInput): SubagentTrac status: "error", input: part && part.state.status !== "streaming" ? part.state.input : {}, structured: - event.data.structured ?? (part && part.state.status !== "streaming" ? part.state.structured : {}), + event.data.metadata ?? (part && part.state.status !== "streaming" ? part.state.structured : {}), content: event.data.content ?? (part && part.state.status !== "streaming" ? part.state.content : []), error: event.data.error, result: event.data.result, @@ -967,7 +967,7 @@ export function createSubagentTracker(input: SubagentTrackerInput): SubagentTrac const key = sourceKey(event.data.assistantMessageID, event.data.callID) const pending = pendingCalls.get(key) if (event.type !== "session.tool.progress") pendingCalls.delete(key) - const found = childSessionID(record(event.data.structured)) + const found = childSessionID(record(event.type === "session.tool.failed" ? event.data.metadata : event.data.structured)) if (!found) return const child = admitChild(found.sessionID) if (!child) return diff --git a/packages/tui/src/mini/stream-v2.transport.ts b/packages/tui/src/mini/stream-v2.transport.ts index a4f7c7cc74..496bda83b3 100644 --- a/packages/tui/src/mini/stream-v2.transport.ts +++ b/packages/tui/src/mini/stream-v2.transport.ts @@ -1085,7 +1085,7 @@ export async function createSessionTransport(input: StreamInput): Promise { assistantMessageID: "msg_progress", callID: "call_progress", error: { type: "unknown", message: "boom" }, - structured: { checkpoint: 1 }, + metadata: { checkpoint: 1 }, content: [{ type: "text", text: "partial" }], executed: true, }, @@ -2892,8 +2892,7 @@ describe("V2 mini transport", () => { assistantMessageID: "msg_failed_subagent", callID: "call_failed_subagent", error: { type: "unknown", message: "subagent failed" }, - structured: { sessionID: "ses_child_failed", status: "running" }, - content: [], + metadata: { sessionID: "ses_child_failed", status: "running" }, executed: true, }, }) @@ -3032,7 +3031,7 @@ describe("V2 mini transport", () => { assistantMessageID: "msg_child_tool", callID: "call_child_shell", error: { type: "unknown", message: "child boom" }, - structured: { checkpoint: "child" }, + metadata: { checkpoint: "child" }, content: [{ type: "text", text: "child partial" }], executed: true, },