Преглед на файлове

fix(ai): keep response ids provider-owned

Aiden Cline преди 3 дни
родител
ревизия
0f67def34d

+ 18 - 24
packages/core/src/session/runner/publish-llm-event.ts

@@ -116,11 +116,7 @@ export const createLLMEventPublisher = (bus: Pick<Bus.Interface, "publish">, 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<string, unknown>) => Effect.Effect<void>,
@@ -344,7 +340,7 @@ export const createLLMEventPublisher = (bus: Pick<Bus.Interface, "publish">, 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<Bus.Interface, "publish">, 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<Bus.Interface, "publish">, 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<Bus.Interface, "publish">, 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<Bus.Interface, "publish">, 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<Bus.Interface, "publish">, 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<Bus.Interface, "publish">, 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<Bus.Interface, "publish">, 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<Bus.Interface, "publish">, 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)

+ 8 - 46
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<string, unknown> | undefined,
 ): ProviderMetadata | undefined => (state === undefined ? undefined : { [provider]: state })
 
-const responseItemID = (state: Record<string, unknown> | undefined) =>
-  typeof state?.itemId === "string" ? state.itemId : undefined
-
-const portableProviderState = (state: Record<string, unknown> | 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({

+ 1 - 17
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,

+ 8 - 10
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<typeof definition>
         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)
 })

+ 9 - 36
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,