From e7ecee5df24ae16d81d01f42f4b6d71ebd70711e Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Thu, 23 Jul 2026 17:40:42 -0400 Subject: [PATCH] fix(core): isolate tool hook outcomes (#38571) --- packages/codemode/src/tool.ts | 2 +- packages/core/src/plugin/host.ts | 24 ++++++-------- packages/core/src/session/runner/llm.ts | 2 +- .../src/session/runner/publish-llm-event.ts | 31 +++++++++---------- packages/core/test/plugin.test.ts | 2 +- .../test/session-runner-tool-events.test.ts | 16 ++++++++-- 6 files changed, 40 insertions(+), 37 deletions(-) diff --git a/packages/codemode/src/tool.ts b/packages/codemode/src/tool.ts index d44e3d0f4e2..93b5a23598c 100644 --- a/packages/codemode/src/tool.ts +++ b/packages/codemode/src/tool.ts @@ -29,7 +29,7 @@ export type JsonSchema = { /** Either a validating Effect Schema or a render-only JSON Schema document. */ export type SchemaType = Schema.Decoder | JsonSchema -/** Executable tool tool exposed through CodeMode's `tools` object. */ +/** Executable tool exposed through CodeMode's `tools` object. */ export type Tool = { readonly _tag: "CodeModeTool" readonly description: string diff --git a/packages/core/src/plugin/host.ts b/packages/core/src/plugin/host.ts index 7761aa18235..640fdea1482 100644 --- a/packages/core/src/plugin/host.ts +++ b/packages/core/src/plugin/host.ts @@ -394,19 +394,15 @@ export const make = Effect.fn("PluginHost.make")(function* (plugin: PluginV2.Int }) } return toolHooks.hook.after((event) => { - // JS plugin boundary: marshal the canonical outcome out, copy mutations back. - const output: Record = { + // Decode first so plugin mutations cannot alias the canonical outcome. + const output = { tool: event.tool, sessionID: event.sessionID, agent: event.agent, messageID: event.messageID, callID: event.callID, input: event.input, - status: event.status, - content: event.content, - metadata: event.metadata, - outputPaths: event.outputPaths, - ...(event.status === "error" ? { error: event.error } : {}), + ...Schema.decodeUnknownSync(Tool.ExecuteAfterOutcome)(event), } return Reflect.apply(callback, undefined, [output]).pipe( Effect.tap(() => { @@ -417,16 +413,16 @@ export const make = Effect.fn("PluginHost.make")(function* (plugin: PluginV2.Int return Effect.logWarning("ignoring execute.after tool status change", { tool: event.tool }) return Effect.sync(() => { if (event.status === "completed" && decoded.value.status === "completed") { - if (output.content !== event.content) event.content = decoded.value.content - if (output.metadata !== event.metadata) event.metadata = decoded.value.metadata - if (output.outputPaths !== event.outputPaths) event.outputPaths = decoded.value.outputPaths + event.content = decoded.value.content + event.metadata = decoded.value.metadata + event.outputPaths = decoded.value.outputPaths return } if (event.status === "error" && decoded.value.status === "error") { - if (output.error !== event.error) event.error = decoded.value.error - if (output.content !== event.content) event.content = decoded.value.content - if (output.metadata !== event.metadata) event.metadata = decoded.value.metadata - if (output.outputPaths !== event.outputPaths) event.outputPaths = decoded.value.outputPaths + event.error = decoded.value.error + event.content = decoded.value.content + event.metadata = decoded.value.metadata + event.outputPaths = decoded.value.outputPaths } }) }), diff --git a/packages/core/src/session/runner/llm.ts b/packages/core/src/session/runner/llm.ts index 3d92a5c8134..ff00a8337bb 100644 --- a/packages/core/src/session/runner/llm.ts +++ b/packages/core/src/session/runner/llm.ts @@ -130,7 +130,7 @@ const layer = Layer.effect( // Durable publishes are serialized so tool fibers and step settlement never interleave // mid-event. const serialized = (effect: Effect.Effect) => publication.withPermit(effect) - const publish = (event: LLMEvent, error?: SessionError.Error) => serialized(publisher.publish(event, error)) + const publish = (event: LLMEvent) => serialized(publisher.publish(event)) let overflowFailure: ProviderErrorEvent | undefined const providerStream = llm.stream(prepared.request).pipe( Stream.runForEach((event) => diff --git a/packages/core/src/session/runner/publish-llm-event.ts b/packages/core/src/session/runner/publish-llm-event.ts index f84253c39d2..d3d21fbfc82 100644 --- a/packages/core/src/session/runner/publish-llm-event.ts +++ b/packages/core/src/session/runner/publish-llm-event.ts @@ -1,5 +1,5 @@ -import { type LLMEvent, type ProviderMetadata, type ToolContent, type ToolResultValue } from "@opencode-ai/ai" -import { Effect, Schema } from "effect" +import { type LLMEvent, type ProviderMetadata, type ToolResultValue } from "@opencode-ai/ai" +import { Effect } from "effect" import { EventV2 } from "../../event" import { ModelV2 } from "../../model" import { SessionEvent } from "../event" @@ -12,7 +12,6 @@ import { Snapshot } from "../../snapshot" import { RelativePath } from "../../schema" import { SessionUsage } from "../usage" import { Tool } from "../../tool/tool" -import { MAX_BYTES } from "../../tool-output-store" import type { ToolRegistry } from "../../tool/registry" type Input = { @@ -28,9 +27,11 @@ const record = (value: unknown): Record => typeof value === "object" && value !== null && !Array.isArray(value) ? (value as Record) : { value } /** Derives canonical model content from a provider-hosted tool result. */ -const hostedContent = (result: ToolResultValue): readonly [ToolContent, ...ToolContent[]] => { - if (result.type === "content" && result.value.length > 0) - return result.value as unknown as readonly [ToolContent, ...ToolContent[]] +const hostedContent = (result: ToolResultValue): Tool.NonEmptyContent => { + if (result.type === "content") { + const content = Tool.nonEmpty(result.value) + if (content !== undefined) return content + } return [{ type: "text", text: Tool.stringify(result.value) }] } @@ -47,11 +48,8 @@ export const createLLMEventPublisher = (events: Pick() - const failureSnapshot = (tool: { readonly progress?: ToolRegistry.Progress }) => { - if (!tool.progress) return {} - const metadata = Tool.jsonMetadata(tool.progress, MAX_BYTES) - return metadata === undefined ? {} : { metadata } - } + const failureSnapshot = (tool: { readonly progress?: ToolRegistry.Progress }) => + tool.progress === undefined ? {} : { metadata: tool.progress } let assistantMessageID = input.assistantMessageID let stepStarted = false let stepFailed = false @@ -292,7 +290,7 @@ export const createLLMEventPublisher = (events: Pick { yield* ctx.tool .hook("execute.after", (event) => Effect.sync(() => { - if (event.status === "completed") event.content = [] as never + if (event.status === "completed") (event.content as unknown as unknown[]).splice(0) }), ) .pipe(Effect.asVoid) diff --git a/packages/core/test/session-runner-tool-events.test.ts b/packages/core/test/session-runner-tool-events.test.ts index 2ba096b306a..702999719c3 100644 --- a/packages/core/test/session-runner-tool-events.test.ts +++ b/packages/core/test/session-runner-tool-events.test.ts @@ -118,9 +118,7 @@ test("provider-executed success derives content and retains provider result stat test("interrupted progress metadata remains in the terminal failure snapshot", async () => { const { published, publisher } = capture("anthropic", { interruptProgress: true }) await Effect.runPromise(publisher.publish(call)) - const exit = await Effect.runPromiseExit( - publisher.progress(call.id, { phase: "visible" }), - ) + const exit = await Effect.runPromiseExit(publisher.progress(call.id, { phase: "visible" })) expect(Exit.isFailure(exit) && Cause.hasInterruptsOnly(exit.cause)).toBe(true) await Effect.runPromise(publisher.failUnsettledTools({ type: "aborted", message: "interrupted" })) @@ -129,6 +127,18 @@ test("interrupted progress metadata remains in the terminal failure snapshot", a }) }) +test("failure snapshot retains canonical progress above the default byte limit", async () => { + const { published, publisher } = capture("anthropic", { interruptProgress: true }) + await Effect.runPromise(publisher.publish(call)) + const detail = "x".repeat(60 * 1024) + await Effect.runPromiseExit(publisher.progress(call.id, { detail })) + await Effect.runPromise(publisher.failUnsettledTools({ type: "aborted", message: "interrupted" })) + + expect(published.find((event) => event.type === "session.tool.failed.2")?.data).toMatchObject({ + metadata: { detail }, + }) +}) + test("failure before progress omits partial output fields", async () => { const { published, publisher } = capture() await Effect.runPromise(publisher.publish(call))