From 0f67def34df4d53bdea473b09f42e057835f342c Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Fri, 7 Aug 2026 17:15:00 -0500 Subject: [PATCH] fix(ai): keep response ids provider-owned --- .../src/session/runner/publish-llm-event.ts | 42 +++++++-------- .../core/src/session/runner/to-llm-message.ts | 54 +++---------------- .../core/test/session-runner-message.test.ts | 18 +------ .../test/session-runner-tool-events.test.ts | 18 +++---- packages/core/test/session-runner.test.ts | 45 ++++------------ 5 files changed, 44 insertions(+), 133 deletions(-) diff --git a/packages/core/src/session/runner/publish-llm-event.ts b/packages/core/src/session/runner/publish-llm-event.ts index db11154dc1..cafbefd67a 100644 --- a/packages/core/src/session/runner/publish-llm-event.ts +++ b/packages/core/src/session/runner/publish-llm-event.ts @@ -116,11 +116,7 @@ export const createLLMEventPublisher = (bus: Pick, inp }) const currentAssistantMessageID = () => stepStarted ? Effect.succeed(assistantMessageID) : Effect.die(new Error("Tool event before assistant step start")) - const providerState = (metadata: ProviderMetadata | undefined, itemId?: string) => { - const state = metadata?.[input.providerMetadataKey] - if (itemId === undefined) return state - return { ...(typeof state === "object" && state !== null && !Array.isArray(state) ? state : {}), itemId } - } + const providerState = (metadata: ProviderMetadata | undefined) => metadata?.[input.providerMetadataKey] const fragments = ( name: string, ended: (id: string, value: string, ordinal: number, state?: Record) => Effect.Effect, @@ -344,7 +340,7 @@ export const createLLMEventPublisher = (bus: Pick, inp return case "text-start": outputStarted = true - const startedTextOrdinal = yield* text.start(event.id, providerState(event.providerMetadata, event.itemId)) + const startedTextOrdinal = yield* text.start(event.id, providerState(event.providerMetadata)) yield* bus.publish(SessionEvent.Text.Started, { sessionID: input.sessionID, assistantMessageID: yield* startAssistant(), @@ -352,11 +348,7 @@ export const createLLMEventPublisher = (bus: Pick, inp }) return case "text-delta": - const deltaTextOrdinal = yield* text.append( - event.id, - event.text, - providerState(event.providerMetadata, event.itemId), - ) + const deltaTextOrdinal = yield* text.append(event.id, event.text, providerState(event.providerMetadata)) yield* bus.publish(SessionEvent.Text.Delta, { sessionID: input.sessionID, assistantMessageID: yield* currentAssistantMessageID(), @@ -365,26 +357,23 @@ export const createLLMEventPublisher = (bus: Pick, inp }) return case "text-end": - yield* text.end(event.id, providerState(event.providerMetadata, event.itemId)) + yield* text.end(event.id, providerState(event.providerMetadata)) return case "reasoning-start": outputStarted = true - const startedReasoningOrdinal = yield* reasoning.start( - event.id, - providerState(event.providerMetadata, event.itemId), - ) + const startedReasoningOrdinal = yield* reasoning.start(event.id, providerState(event.providerMetadata)) yield* bus.publish(SessionEvent.Reasoning.Started, { sessionID: input.sessionID, assistantMessageID: yield* startAssistant(), ordinal: startedReasoningOrdinal, - state: providerState(event.providerMetadata, event.itemId), + state: providerState(event.providerMetadata), }) return case "reasoning-delta": const deltaReasoningOrdinal = yield* reasoning.append( event.id, event.text, - providerState(event.providerMetadata, event.itemId), + providerState(event.providerMetadata), ) yield* bus.publish(SessionEvent.Reasoning.Delta, { sessionID: input.sessionID, @@ -394,7 +383,7 @@ export const createLLMEventPublisher = (bus: Pick, inp }) return case "reasoning-end": - yield* reasoning.end(event.id, providerState(event.providerMetadata, event.itemId)) + yield* reasoning.end(event.id, providerState(event.providerMetadata)) return case "tool-input-start": outputStarted = true @@ -438,7 +427,7 @@ export const createLLMEventPublisher = (bus: Pick, inp id: event.id, input: asRecord(event.input), executed: tool.providerExecuted, - state: providerState(event.providerMetadata, event.itemId), + state: providerState(event.providerMetadata), }) return } @@ -456,7 +445,7 @@ export const createLLMEventPublisher = (bus: Pick, inp } tool.settled = true const executed = event.providerExecuted === true || tool.providerExecuted - const resultState = providerState(event.providerMetadata, event.itemId) + const resultState = providerState(event.providerMetadata) if (event.result.type === "error") { yield* bus.publish(SessionEvent.Tool.Failed, { sessionID: input.sessionID, @@ -496,7 +485,7 @@ export const createLLMEventPublisher = (bus: Pick, inp : { type: "tool.execution", message: event.message }, ...failureSnapshot(tool), executed: tool.providerExecuted, - resultState: providerState(event.providerMetadata, event.itemId), + resultState: providerState(event.providerMetadata), }) return } @@ -521,7 +510,8 @@ export const createLLMEventPublisher = (bus: Pick, inp const progress = Effect.fnUntraced(function* (id: string, update: Tool.Metadata) { const tool = tools.get(id) - if (!tool?.called || tool.settled) return yield* Effect.die(new Error(`Tool progress outside running call: ${id}`)) + if (!tool?.called || tool.settled) + return yield* Effect.die(new Error(`Tool progress outside running call: ${id}`)) tool.progress = update yield* bus.publish(SessionEvent.Tool.Progress, { sessionID: input.sessionID, @@ -532,7 +522,11 @@ export const createLLMEventPublisher = (bus: Pick, inp }) /** Publishes one canonical terminal event for a locally executed tool call. */ - const toolExecution = Effect.fnUntraced(function* (id: string, name: string, result: Tool.Result) { + const toolExecution = Effect.fnUntraced(function* ( + id: string, + name: string, + result: Tool.Result, + ) { const tool = tools.get(id) if (!tool?.called) return yield* Effect.die(new Error(`Tool execution before call: ${id}`)) if (tool.name !== name) diff --git a/packages/core/src/session/runner/to-llm-message.ts b/packages/core/src/session/runner/to-llm-message.ts index 6fc2107da9..a58ced07be 100644 --- a/packages/core/src/session/runner/to-llm-message.ts +++ b/packages/core/src/session/runner/to-llm-message.ts @@ -1,10 +1,4 @@ -import { - Message, - ToolCallPart, - ToolResultPart, - type ContentPart, - type ProviderMetadata, -} from "@opencode-ai/ai" +import { Message, ToolCallPart, ToolResultPart, type ContentPart, type ProviderMetadata } from "@opencode-ai/ai" import { Option, Schema } from "effect" import type { Model } from "../../model" import { SessionMessage } from "../message" @@ -72,46 +66,27 @@ const providerMetadata = ( state: Record | undefined, ): ProviderMetadata | undefined => (state === undefined ? undefined : { [provider]: state }) -const responseItemID = (state: Record | undefined) => - typeof state?.itemId === "string" ? state.itemId : undefined - -const portableProviderState = (state: Record | undefined) => { - if (state === undefined || !("itemId" in state)) return state - const { itemId: _itemId, ...portable } = state - return portable -} - const toolInput = (tool: SessionMessage.AssistantTool) => tool.state.status === "streaming" ? Option.getOrElse(decodeToolInput(tool.state.input), () => tool.state.input) : tool.state.input -const toolCall = ( - tool: SessionMessage.AssistantTool, - itemId: string | undefined, - providerMetadata: ProviderMetadata | undefined, -): ContentPart => +const toolCall = (tool: SessionMessage.AssistantTool, providerMetadata: ProviderMetadata | undefined): ContentPart => ToolCallPart.make({ id: tool.id, - ...(itemId === undefined ? {} : { itemId }), name: tool.name, input: toolInput(tool), providerExecuted: tool.executed, providerMetadata, }) -const toolResult = ( - tool: SessionMessage.AssistantTool, - itemId: string | undefined, - providerMetadata: ProviderMetadata | undefined, -) => { +const toolResult = (tool: SessionMessage.AssistantTool, providerMetadata: ProviderMetadata | undefined) => { if (tool.state.status === "completed") { // TODO: Materialize remote and managed URIs before provider-history lowering. const content = tool.state.content const single = content.length === 1 ? content[0] : undefined return ToolResultPart.make({ id: tool.id, - ...(itemId === undefined ? {} : { itemId }), name: tool.name, result: single?.type === "text" @@ -124,7 +99,6 @@ const toolResult = ( if (tool.state.status === "error") { return ToolResultPart.make({ id: tool.id, - ...(itemId === undefined ? {} : { itemId }), name: tool.name, result: { error: tool.state.error, content: tool.state.content ?? [] }, resultType: "error", @@ -144,13 +118,7 @@ const assistant = (message: SessionMessage.Assistant, model: Model.Ref, provider { type: "text", text: item.text, - itemId: reuseProviderMetadata ? responseItemID(item.state) : undefined, - providerMetadata: sameProvider - ? providerMetadata( - providerMetadataKey, - reuseProviderMetadata ? item.state : portableProviderState(item.state), - ) - : undefined, + providerMetadata: sameProvider ? providerMetadata(providerMetadataKey, item.state) : undefined, }, ] if (item.type === "reasoning") @@ -159,7 +127,6 @@ const assistant = (message: SessionMessage.Assistant, model: Model.Ref, provider { type: "reasoning", text: item.text, - itemId: responseItemID(item.state), providerMetadata: providerMetadata(providerMetadataKey, item.state), }, ] @@ -171,7 +138,6 @@ const assistant = (message: SessionMessage.Assistant, model: Model.Ref, provider (sameModel && item.executed === true && (item.state.status === "completed" || item.state.status === "error")) const call = toolCall( item, - reuseToolProviderMetadata ? responseItemID(item.providerState) : undefined, reuseToolProviderMetadata ? providerMetadata(providerMetadataKey, item.providerState) : undefined, ) if (item.executed !== true) return [call] @@ -179,11 +145,6 @@ const assistant = (message: SessionMessage.Assistant, model: Model.Ref, provider // replay must survive a model switch within the same provider. const result = toolResult( item, - reuseToolProviderMetadata - ? responseItemID(item.providerResultState ?? (item.executed === true ? item.providerState : undefined)) - : sameProvider && item.executed === true - ? responseItemID(item.providerResultState) - : undefined, reuseToolProviderMetadata ? providerMetadata(providerMetadataKey, item.providerResultState ?? item.providerState) : sameProvider && item.executed === true && item.providerResultState !== undefined @@ -202,8 +163,9 @@ const assistant = (message: SessionMessage.Assistant, model: Model.Ref, provider .map((item) => toolResult( item, - responseItemID(item.providerResultState) ?? `fco_${item.id}`, - reuseProviderMetadata ? providerMetadata(providerMetadataKey, item.providerResultState) : undefined, + reuseProviderMetadata + ? providerMetadata(providerMetadataKey, item.providerResultState ?? item.providerState) + : undefined, ), ) .filter((message) => message !== undefined) @@ -242,7 +204,7 @@ function toLLMMessage(message: SessionMessage.Info, model: Model.Ref, providerMe case "skill": return [Message.make({ id: message.id, role: "user", content: message.text, metadata: message.metadata })] case "system": - return [Message.make({ id: message.id, role: "system", content: message.text })] + return [Message.system(message.text)] case "shell": return [ Message.make({ diff --git a/packages/core/test/session-runner-message.test.ts b/packages/core/test/session-runner-message.test.ts index 8387dbcf92..5e5b0e5ddb 100644 --- a/packages/core/test/session-runner-message.test.ts +++ b/packages/core/test/session-runner-message.test.ts @@ -110,13 +110,7 @@ describe("toLLMMessages", () => { ) expect(messages.map((message) => message.role)).toEqual(["system", "user", "user", "user", "user"]) - expect(messages[0]).toEqual( - Message.make({ - id: "msg_system", - role: "system", - content: [{ type: "text", text: "Updated context\n\nOther context" }], - }), - ) + expect(messages[0]).toEqual(Message.system("Updated context\n\nOther context")) expect(messages[1]).toEqual( Message.make({ id: id("user"), @@ -482,7 +476,6 @@ Recent work { type: "tool-result", id: "completed", - itemId: "fco_completed", name: "read", result: { type: "content", @@ -520,7 +513,6 @@ Recent work { type: "reasoning", text: "Think", - itemId: "rs_1", providerMetadata: { provider: { itemId: "rs_1", reasoningEncryptedContent: "encrypted-state" } }, }, ]) @@ -610,7 +602,6 @@ Recent work { type: "tool-call", id: "hosted-completed", - itemId: "call_completed", name: "web_search", input: { query: "Effect" }, providerExecuted: true, @@ -619,7 +610,6 @@ Recent work { type: "tool-result", id: "hosted-completed", - itemId: "result_completed", name: "web_search", result: { type: "text", value: '{"found":true}' }, providerExecuted: true, @@ -630,7 +620,6 @@ Recent work { type: "tool-call", id: "hosted-failed", - itemId: "call_failed", name: "web_search", input: { query: "Effect" }, providerExecuted: true, @@ -639,7 +628,6 @@ Recent work { type: "tool-result", id: "hosted-failed", - itemId: "result_failed", name: "web_search", result: { type: "error", @@ -710,7 +698,6 @@ Recent work { type: "tool-call", id: "hosted-old-model", - itemId: undefined, name: "web_search", input: { query: "Effect" }, providerExecuted: true, @@ -719,7 +706,6 @@ Recent work { type: "tool-result", id: "hosted-old-model", - itemId: "hosted-old-model", name: "web_search", result: { type: "text", value: '{"status":"completed"}' }, providerExecuted: true, @@ -732,7 +718,6 @@ Recent work { type: "tool-call", id: "local-old-model", - itemId: undefined, name: "read", input: { path: "README.md" }, providerExecuted: false, @@ -743,7 +728,6 @@ Recent work { type: "tool-result", id: "local-old-model", - itemId: "fco_local-old-model", name: "read", result: { type: "text", value: "Hello" }, providerExecuted: false, diff --git a/packages/core/test/session-runner-tool-events.test.ts b/packages/core/test/session-runner-tool-events.test.ts index 53c2544a63..b33aa91d54 100644 --- a/packages/core/test/session-runner-tool-events.test.ts +++ b/packages/core/test/session-runner-tool-events.test.ts @@ -24,7 +24,9 @@ const capture = (providerMetadataKey = "anthropic", options?: { readonly interru const publish = Effect.sync(() => { const event = { id: Event.ID.create(), type: definition.type, data } as Event.Payload published.push({ - type: definition.durable ? Bus.versionedType(definition.type, definition.durable.version) : definition.type, + type: definition.durable + ? Bus.versionedType(definition.type, definition.durable.version) + : definition.type, data, }) return event @@ -64,10 +66,9 @@ const hostedResult = LLMEvent.toolResult({ test("local tool success serializes media base64 once through canonical content", async () => { const { published, publisher } = capture() - const localCall = LLMEvent.toolCall({ ...call, itemId: "fc_call-image" }) - await Effect.runPromise(publisher.publish(localCall)) + await Effect.runPromise(publisher.publish(call)) await Effect.runPromise( - publisher.toolExecution(localCall.id, localCall.name, { + publisher.toolExecution(call.id, call.name, { output: { type: "media", mime: "image/png" }, content: [ { type: "text", text: "Image read successfully" }, @@ -83,11 +84,6 @@ test("local tool success serializes media base64 once through canonical content" expect(success?.data).not.toHaveProperty("result") expect(success?.data).not.toHaveProperty("output") - const called = published.find((event) => event.type === "session.tool.called.1")?.data - expect(called).toMatchObject({ state: { itemId: "fc_call-image" } }) - expect(success?.data).not.toHaveProperty("resultState") - expect(JSON.stringify(success?.data)).not.toContain('"itemId":"fc_call-image"') - expect(success?.data).toMatchObject({ content: [ { type: "text", text: "Image read successfully" }, @@ -230,7 +226,9 @@ test("provider-executed tool metadata is flattened using the route key", async ( test("binary failure emits no success event", async () => { const { published, publisher } = capture() await Effect.runPromise(publisher.publish(call)) - await Effect.runPromise(publisher.failTool(call.id, { type: "tool.execution", message: "Cannot read binary file" })) + await Effect.runPromise( + publisher.failTool(call.id, { type: "tool.execution", message: "Cannot read binary file" }), + ) expect(published.some((event) => event.type === "session.tool.success.2")).toBe(false) expect(published.some((event) => event.type === "session.tool.failed.2")).toBe(true) }) diff --git a/packages/core/test/session-runner.test.ts b/packages/core/test/session-runner.test.ts index 806656a8f8..72d15f16b1 100644 --- a/packages/core/test/session-runner.test.ts +++ b/packages/core/test/session-runner.test.ts @@ -558,15 +558,6 @@ const messageTexts = (request: LLMRequest, role: "user" | "system") => const userTexts = (request: LLMRequest) => messageTexts(request, "user") const systemTexts = (request: LLMRequest) => messageTexts(request, "system") const messageRoles = (request: LLMRequest | undefined) => request?.messages.map((message) => message.role) -const withoutItemIDs = (messages: LLMRequest["messages"]) => - messages.map((message) => ({ - role: message.role, - content: message.content.map((part) => { - if (!("itemId" in part)) return part - const { itemId: _itemId, ...content } = part - return content - }), - })) const recordedEventTypes = (id: Session.ID) => Effect.gen(function* () { @@ -865,8 +856,8 @@ describe("SessionRunnerLLM", () => { yield* Fiber.join(renamed) expect(requests).toHaveLength(5) - expect(withoutItemIDs(requests[2]!.messages)).toContainEqual(withoutItemIDs([Message.user("First prompt")])[0]) - expect(withoutItemIDs(requests[4]!.messages)).toContainEqual(withoutItemIDs([Message.user("First prompt")])[0]) + expect(requests[2]?.messages).toContainEqual(Message.user("First prompt")) + expect(requests[4]?.messages).toContainEqual(Message.user("First prompt")) expect((yield* session.get(sessionID)).title).toBe("Generated title") }), ) @@ -891,7 +882,7 @@ describe("SessionRunnerLLM", () => { // A hook-removed call fails independently and continues while step allowance remains. expect(requests).toHaveLength(2) expect(requests[0]?.system.map((part) => part.text)).toEqual(["Hooked system"]) - expect(withoutItemIDs(requests[0]!.messages)).toEqual(withoutItemIDs([Message.user("Hooked message")])) + expect(requests[0]?.messages).toEqual([Message.user("Hooked message")]) expect(requests[0]?.tools.map((tool) => tool.name)).not.toContain("echo") expect(requests[0]?.tools.map((tool) => tool.name)).not.toContain("unregistered") expect(executions).toEqual([]) @@ -1316,14 +1307,12 @@ describe("SessionRunnerLLM", () => { systemBaseline = "Changed context" yield* runPrompt(session, "Second") - const firstSnapshot = PromptCacheDiagnostics.snapshot(requests[0]!) - const secondSnapshot = PromptCacheDiagnostics.snapshot(requests[1]!) - expect(PromptCacheDiagnostics.compare(firstSnapshot, secondSnapshot)).toEqual({ - status: "append-only", - previousMessages: 1, - currentMessages: 3, - }) - expect(secondSnapshot.messages[0]).toEqual(firstSnapshot.messages[0]) + expect( + PromptCacheDiagnostics.compare( + PromptCacheDiagnostics.snapshot(requests[0]), + PromptCacheDiagnostics.snapshot(requests[1]), + ), + ).toEqual({ status: "append-only", previousMessages: 1, currentMessages: 3 }) expect(requests.map((request) => request.system.map((part) => part.text))).toEqual([ [defaultSystem, "Initial context"], [defaultSystem, "Initial context"], @@ -2543,24 +2532,9 @@ describe("SessionRunnerLLM", () => { { type: "reasoning", text: "Encrypted thought", - itemId: "rs_1", providerMetadata: { openai: { itemId: "rs_1", reasoningEncryptedContent: "encrypted-state" } }, }, ]) - - yield* admit(session, "Continue again") - yield* TestLLM.push([]) - yield* session.resume(sessionID) - - expect(requests[2]?.messages[1]?.content.map((part) => ("itemId" in part ? part.itemId : undefined))).toEqual( - requests[1]?.messages[1]?.content.map((part) => ("itemId" in part ? part.itemId : undefined)), - ) - expect( - PromptCacheDiagnostics.compare( - PromptCacheDiagnostics.snapshot(requests[1]!), - PromptCacheDiagnostics.snapshot(requests[2]!), - ), - ).toEqual({ status: "append-only", previousMessages: 3, currentMessages: 4 }) }), ) @@ -2639,7 +2613,6 @@ describe("SessionRunnerLLM", () => { { type: "tool-call", id: "hosted-search", - itemId: "hosted-search", name: "web_search", input: { query: "Effect" }, providerExecuted: true,