From 9d85e8a3ea39461525893b5b2838a6472abaa09f Mon Sep 17 00:00:00 2001 From: Michael Crawford Date: Sun, 23 Aug 2026 18:12:34 -0400 Subject: [PATCH] fix(session): stop creating phantom "unknown" tool parts on re-emitted deltas When a provider re-sends `tool_calls[i].function.arguments` deltas for a call that already produced its result, the AI SDK emits tool-input-delta / tool-input-end for a call id the processor has already settled. Two things then go wrong: - the adapter deleted `state.toolNames[callID]` on tool-result, so the late chunks resolve to the literal name "unknown" - `ensureToolCall` finds no live call and creates a second pending part for the same call id, which `cleanup` then sweeps into `error: "Tool execution aborted"`, `metadata.interrupted: true` The result is a phantom part paired 1:1 with a real completed one. Both serialize into the next request, so the assistant turn carries two tool_use blocks sharing a toolCallId, one of them named "unknown". Track settled call ids on the processor context and refuse to mint a new part for them; keep tool names for the life of the stream so late chunks still resolve; and dedupe tool parts by call id when building model messages, which is what heals sessions already recorded with the duplicate. --- packages/opencode/src/session/llm/ai-sdk.ts | 5 +- packages/opencode/src/session/message-v2.ts | 8 ++ packages/opencode/src/session/processor.ts | 12 +++ .../opencode/test/session/message-v2.test.ts | 81 +++++++++++++++++++ .../test/session/processor-effect.test.ts | 68 ++++++++++++++++ 5 files changed, 172 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/session/llm/ai-sdk.ts b/packages/opencode/src/session/llm/ai-sdk.ts index 13d427aab62c..595826b0e1e9 100644 --- a/packages/opencode/src/session/llm/ai-sdk.ts +++ b/packages/opencode/src/session/llm/ai-sdk.ts @@ -236,8 +236,10 @@ export function toLLMEvents( case "tool-result": return Effect.sync(() => { + // Names are kept for the life of the stream rather than dropped here: + // providers may re-emit argument deltas for a call that already + // produced its result, and those chunks carry no name of their own. const name = state.toolNames[event.toolCallId] ?? "unknown" - delete state.toolNames[event.toolCallId] return [ LLMEvent.toolResult({ id: event.toolCallId, @@ -252,7 +254,6 @@ export function toLLMEvents( case "tool-error": return Effect.sync(() => { const name = state.toolNames[event.toolCallId] ?? ("toolName" in event ? event.toolName : "unknown") - delete state.toolNames[event.toolCallId] return [ LLMEvent.toolError({ id: event.toolCallId, diff --git a/packages/opencode/src/session/message-v2.ts b/packages/opencode/src/session/message-v2.ts index 9b3f2c46f405..78c598818083 100644 --- a/packages/opencode/src/session/message-v2.ts +++ b/packages/opencode/src/session/message-v2.ts @@ -274,7 +274,15 @@ export const toModelMessagesEffect = Effect.fnUntraced(function* ( if (part.type !== "reasoning") return false return part.metadata?.anthropic?.signature != null }) + // Sessions recorded before phantom tool parts were fixed at the source can + // hold two parts for one call ID. Replaying both emits duplicate tool_use + // blocks, which providers reject or answer with a confused retry loop. + const seenCallIDs = new Set() for (const part of msg.parts) { + if (part.type === "tool") { + if (seenCallIDs.has(part.callID)) continue + seenCallIDs.add(part.callID) + } if (part.type === "text") { const text = part.text === "" && hasSignedReasoning ? " " : part.text assistantMessage.parts.push({ diff --git a/packages/opencode/src/session/processor.ts b/packages/opencode/src/session/processor.ts index 9f8530929c15..b56afdfa7cbe 100644 --- a/packages/opencode/src/session/processor.ts +++ b/packages/opencode/src/session/processor.ts @@ -66,6 +66,11 @@ type ToolCall = { interface ProcessorContext extends Input { toolcalls: Record + // Call IDs whose part already reached a terminal state. Some providers + // re-emit argument deltas for a call that has already produced its result; + // without this, those late events would create a second part for the same + // call ID (see ensureToolCall). + settled: Set shouldBreak: boolean snapshot: string | undefined blocked: boolean @@ -105,6 +110,7 @@ const layer = Layer.effect( sessionID: input.sessionID, model: input.model, toolcalls: {}, + settled: new Set(), shouldBreak: false, snapshot: initialSnapshot, blocked: false, @@ -123,6 +129,7 @@ const layer = Layer.effect( const settleToolCall = Effect.fn("SessionProcessor.settleToolCall")(function* (toolCallID: string) { const done = ctx.toolcalls[toolCallID]?.done delete ctx.toolcalls[toolCallID] + ctx.settled.add(toolCallID) if (done) yield* Deferred.succeed(done, undefined).pipe(Effect.ignore) }) @@ -219,6 +226,10 @@ const layer = Layer.effect( providerExecuted?: boolean }) { const existing = yield* readToolCall(input.id) + // A settled call never gets a fresh part. Late events for it carry no + // tool name, so recreating one would persist a phantom `unknown` call + // that shares its call ID with the real, already-completed part. + if (!existing && ctx.settled.has(input.id)) return undefined if (existing) { if (!input.providerExecuted || existing.part.metadata?.providerExecuted) return existing const part = yield* session.updatePart({ @@ -606,6 +617,7 @@ const layer = Layer.effect( }) } ctx.toolcalls = {} + ctx.settled.clear() ctx.assistantMessage.time.completed = Date.now() yield* session.updateMessage(ctx.assistantMessage) }) diff --git a/packages/opencode/test/session/message-v2.test.ts b/packages/opencode/test/session/message-v2.test.ts index 734a30e42454..148367612b69 100644 --- a/packages/opencode/test/session/message-v2.test.ts +++ b/packages/opencode/test/session/message-v2.test.ts @@ -890,6 +890,87 @@ describe("session.message-v2.toModelMessage", () => { ]) }) + test("replays only the first part recorded for a duplicated tool call id", async () => { + const userID = "m-user" + const assistantID = "m-assistant" + + const input: SessionV1.WithParts[] = [ + { + info: userInfo(userID), + parts: [ + { + ...basePart(userID, "u1"), + type: "text", + text: "run tool", + }, + ] as SessionV1.Part[], + }, + { + info: assistantInfo(assistantID, userID), + parts: [ + { + ...basePart(assistantID, "a1"), + type: "tool", + callID: "call-1", + tool: "read", + state: { + status: "completed", + input: { filePath: "x.json" }, + output: "contents", + title: "read", + metadata: {}, + time: { start: 0, end: 1 }, + }, + }, + // Phantom part recorded by older builds: same call ID, no tool name. + { + ...basePart(assistantID, "a2"), + type: "tool", + callID: "call-1", + tool: "unknown", + state: { + status: "error", + input: {}, + error: "Tool execution aborted", + time: { start: 0, end: 1 }, + metadata: { interrupted: true }, + }, + }, + ] as SessionV1.Part[], + }, + ] + + expect(await MessageV2.toModelMessages(input, model)).toStrictEqual([ + { + role: "user", + content: [{ type: "text", text: "run tool" }], + }, + { + role: "assistant", + content: [ + { + type: "tool-call", + toolCallId: "call-1", + toolName: "read", + input: { filePath: "x.json" }, + providerExecuted: undefined, + }, + ], + }, + { + role: "tool", + content: [ + { + type: "tool-result", + toolCallId: "call-1", + toolName: "read", + output: { type: "text", value: "contents" }, + }, + ], + }, + ]) + }) + test("forwards partial bash output for aborted tool calls", async () => { const userID = "m-user" const assistantID = "m-assistant" diff --git a/packages/opencode/test/session/processor-effect.test.ts b/packages/opencode/test/session/processor-effect.test.ts index c67f82d9c71b..e54e27b18b7d 100644 --- a/packages/opencode/test/session/processor-effect.test.ts +++ b/packages/opencode/test/session/processor-effect.test.ts @@ -209,6 +209,34 @@ const providerErrorLLM = Layer.succeed( const providerErrorEnv = LayerNode.compile(root, [...replacements, [LLM.node, providerErrorLLM]]) const itProviderError = testEffect(providerErrorEnv) +// Regression: providers (Qwen via OpenRouter) can re-emit argument deltas for a +// tool call that already produced its result. Those late events carry no tool +// name, so the adapter falls back to "unknown". +const lateToolInputLLM = Layer.succeed( + LLM.Service, + LLM.Service.of({ + stream: () => + Stream.make( + LLMEvent.stepStart({ index: 0 }), + LLMEvent.toolInputStart({ id: "call-1", name: "lookup" }), + LLMEvent.toolInputEnd({ id: "call-1", name: "lookup" }), + LLMEvent.toolCall({ id: "call-1", name: "lookup", input: {}, providerExecuted: true }), + LLMEvent.toolResult({ + id: "call-1", + name: "lookup", + result: { type: "json", value: { output: "ok", title: "lookup", metadata: {} } }, + providerExecuted: true, + }), + LLMEvent.toolInputDelta({ id: "call-1", name: "unknown", text: "{}" }), + LLMEvent.toolInputEnd({ id: "call-1", name: "unknown" }), + LLMEvent.stepFinish({ index: 0, reason: "stop" }), + LLMEvent.finish({ reason: "stop" }), + ), + }), +) +const lateToolInputEnv = LayerNode.compile(root, [...replacements, [LLM.node, lateToolInputLLM]]) +const itLateToolInput = testEffect(lateToolInputEnv) + const fragmentFailureLLM = Layer.succeed( LLM.Service, LLM.Service.of({ @@ -1169,3 +1197,43 @@ itFragmentFailure.live("session.processor effect tests retain partial legacy par { config: cfg }, ), ) + +itLateToolInput.live("session.processor effect tests ignore tool input events after a call settles", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + const { processors, session, provider } = yield* boot() + + const chat = yield* session.create({}) + const parent = yield* user(chat.id, "late tool input") + const msg = yield* assistant(chat.id, parent.id, path.resolve(dir)) + const mdl = yield* provider.getModel(ref.providerID, ref.modelID) + const handle = yield* processors.create({ assistantMessage: msg, sessionID: chat.id, model: mdl }) + + yield* handle.process({ + user: { + id: parent.id, + sessionID: chat.id, + role: "user", + time: parent.time, + agent: parent.agent, + model: { providerID: ref.providerID, modelID: ref.modelID }, + } satisfies SessionV1.User, + sessionID: chat.id, + model: mdl, + agent: agent(), + system: [], + messages: [{ role: "user", content: "late tool input" }], + tools: {}, + }) + + const calls = (yield* MessageV2.parts(msg.id)).filter( + (part): part is SessionV1.ToolPart => part.type === "tool", + ) + expect(calls.map((part) => ({ tool: part.tool, status: part.state.status }))).toEqual([ + { tool: "lookup", status: "completed" }, + ]) + }), + { config: cfg }, + ), +)