fix(core): isolate tool hook outcomes (#38571)

This commit is contained in:
Kit Langton 2026-07-23 17:40:42 -04:00 committed by GitHub
commit e7ecee5df2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 40 additions and 37 deletions

View file

@ -29,7 +29,7 @@ export type JsonSchema = {
/** Either a validating Effect Schema or a render-only JSON Schema document. */ /** Either a validating Effect Schema or a render-only JSON Schema document. */
export type SchemaType = Schema.Decoder<unknown> | JsonSchema export type SchemaType = Schema.Decoder<unknown> | JsonSchema
/** Executable tool tool exposed through CodeMode's `tools` object. */ /** Executable tool exposed through CodeMode's `tools` object. */
export type Tool<R = never> = { export type Tool<R = never> = {
readonly _tag: "CodeModeTool" readonly _tag: "CodeModeTool"
readonly description: string readonly description: string

View file

@ -394,19 +394,15 @@ export const make = Effect.fn("PluginHost.make")(function* (plugin: PluginV2.Int
}) })
} }
return toolHooks.hook.after((event) => { return toolHooks.hook.after((event) => {
// JS plugin boundary: marshal the canonical outcome out, copy mutations back. // Decode first so plugin mutations cannot alias the canonical outcome.
const output: Record<string, unknown> = { const output = {
tool: event.tool, tool: event.tool,
sessionID: event.sessionID, sessionID: event.sessionID,
agent: event.agent, agent: event.agent,
messageID: event.messageID, messageID: event.messageID,
callID: event.callID, callID: event.callID,
input: event.input, input: event.input,
status: event.status, ...Schema.decodeUnknownSync(Tool.ExecuteAfterOutcome)(event),
content: event.content,
metadata: event.metadata,
outputPaths: event.outputPaths,
...(event.status === "error" ? { error: event.error } : {}),
} }
return Reflect.apply(callback, undefined, [output]).pipe( return Reflect.apply(callback, undefined, [output]).pipe(
Effect.tap(() => { 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.logWarning("ignoring execute.after tool status change", { tool: event.tool })
return Effect.sync(() => { return Effect.sync(() => {
if (event.status === "completed" && decoded.value.status === "completed") { if (event.status === "completed" && decoded.value.status === "completed") {
if (output.content !== event.content) event.content = decoded.value.content event.content = decoded.value.content
if (output.metadata !== event.metadata) event.metadata = decoded.value.metadata event.metadata = decoded.value.metadata
if (output.outputPaths !== event.outputPaths) event.outputPaths = decoded.value.outputPaths event.outputPaths = decoded.value.outputPaths
return return
} }
if (event.status === "error" && decoded.value.status === "error") { if (event.status === "error" && decoded.value.status === "error") {
if (output.error !== event.error) event.error = decoded.value.error event.error = decoded.value.error
if (output.content !== event.content) event.content = decoded.value.content event.content = decoded.value.content
if (output.metadata !== event.metadata) event.metadata = decoded.value.metadata event.metadata = decoded.value.metadata
if (output.outputPaths !== event.outputPaths) event.outputPaths = decoded.value.outputPaths event.outputPaths = decoded.value.outputPaths
} }
}) })
}), }),

View file

@ -130,7 +130,7 @@ const layer = Layer.effect(
// Durable publishes are serialized so tool fibers and step settlement never interleave // Durable publishes are serialized so tool fibers and step settlement never interleave
// mid-event. // mid-event.
const serialized = <A, E, R>(effect: Effect.Effect<A, E, R>) => publication.withPermit(effect) const serialized = <A, E, R>(effect: Effect.Effect<A, E, R>) => 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 let overflowFailure: ProviderErrorEvent | undefined
const providerStream = llm.stream(prepared.request).pipe( const providerStream = llm.stream(prepared.request).pipe(
Stream.runForEach((event) => Stream.runForEach((event) =>

View file

@ -1,5 +1,5 @@
import { type LLMEvent, type ProviderMetadata, type ToolContent, type ToolResultValue } from "@opencode-ai/ai" import { type LLMEvent, type ProviderMetadata, type ToolResultValue } from "@opencode-ai/ai"
import { Effect, Schema } from "effect" import { Effect } from "effect"
import { EventV2 } from "../../event" import { EventV2 } from "../../event"
import { ModelV2 } from "../../model" import { ModelV2 } from "../../model"
import { SessionEvent } from "../event" import { SessionEvent } from "../event"
@ -12,7 +12,6 @@ import { Snapshot } from "../../snapshot"
import { RelativePath } from "../../schema" import { RelativePath } from "../../schema"
import { SessionUsage } from "../usage" import { SessionUsage } from "../usage"
import { Tool } from "../../tool/tool" import { Tool } from "../../tool/tool"
import { MAX_BYTES } from "../../tool-output-store"
import type { ToolRegistry } from "../../tool/registry" import type { ToolRegistry } from "../../tool/registry"
type Input = { type Input = {
@ -28,9 +27,11 @@ const record = (value: unknown): Record<string, unknown> =>
typeof value === "object" && value !== null && !Array.isArray(value) ? (value as Record<string, unknown>) : { value } typeof value === "object" && value !== null && !Array.isArray(value) ? (value as Record<string, unknown>) : { value }
/** Derives canonical model content from a provider-hosted tool result. */ /** Derives canonical model content from a provider-hosted tool result. */
const hostedContent = (result: ToolResultValue): readonly [ToolContent, ...ToolContent[]] => { const hostedContent = (result: ToolResultValue): Tool.NonEmptyContent => {
if (result.type === "content" && result.value.length > 0) if (result.type === "content") {
return result.value as unknown as readonly [ToolContent, ...ToolContent[]] const content = Tool.nonEmpty(result.value)
if (content !== undefined) return content
}
return [{ type: "text", text: Tool.stringify(result.value) }] return [{ type: "text", text: Tool.stringify(result.value) }]
} }
@ -47,11 +48,8 @@ export const createLLMEventPublisher = (events: Pick<EventV2.Interface, "publish
progress?: ToolRegistry.Progress progress?: ToolRegistry.Progress
} }
>() >()
const failureSnapshot = (tool: { readonly progress?: ToolRegistry.Progress }) => { const failureSnapshot = (tool: { readonly progress?: ToolRegistry.Progress }) =>
if (!tool.progress) return {} tool.progress === undefined ? {} : { metadata: tool.progress }
const metadata = Tool.jsonMetadata(tool.progress, MAX_BYTES)
return metadata === undefined ? {} : { metadata }
}
let assistantMessageID = input.assistantMessageID let assistantMessageID = input.assistantMessageID
let stepStarted = false let stepStarted = false
let stepFailed = false let stepFailed = false
@ -292,7 +290,7 @@ export const createLLMEventPublisher = (events: Pick<EventV2.Interface, "publish
return tool ? Effect.succeed(tool.assistantMessageID) : Effect.die(new Error(`Unknown tool call: ${callID}`)) return tool ? Effect.succeed(tool.assistantMessageID) : Effect.die(new Error(`Unknown tool call: ${callID}`))
} }
const publish = Effect.fn("SessionRunner.publishLLMEvent")(function* (event: LLMEvent, error?: SessionError.Error) { const publish = Effect.fn("SessionRunner.publishLLMEvent")(function* (event: LLMEvent) {
switch (event.type) { switch (event.type) {
case "step-start": case "step-start":
yield* startAssistant() yield* startAssistant()
@ -405,12 +403,12 @@ export const createLLMEventPublisher = (events: Pick<EventV2.Interface, "publish
tool.settled = true tool.settled = true
const executed = event.providerExecuted === true || tool.providerExecuted const executed = event.providerExecuted === true || tool.providerExecuted
const resultState = providerState(event.providerMetadata) const resultState = providerState(event.providerMetadata)
if (error !== undefined || event.result.type === "error") { if (event.result.type === "error") {
yield* events.publish(SessionEvent.Tool.Failed, { yield* events.publish(SessionEvent.Tool.Failed, {
sessionID: input.sessionID, sessionID: input.sessionID,
assistantMessageID: tool.assistantMessageID, assistantMessageID: tool.assistantMessageID,
callID: event.id, callID: event.id,
error: error ?? { type: "tool.execution", message: Tool.stringify(event.result.value) }, error: { type: "tool.execution", message: Tool.stringify(event.result.value) },
...failureSnapshot(tool), ...failureSnapshot(tool),
executed, executed,
resultState, resultState,
@ -471,13 +469,12 @@ export const createLLMEventPublisher = (events: Pick<EventV2.Interface, "publish
const tool = tools.get(callID) const tool = tools.get(callID)
if (!tool?.called || tool.settled) if (!tool?.called || tool.settled)
return yield* Effect.die(new Error(`Tool progress outside running call: ${callID}`)) return yield* Effect.die(new Error(`Tool progress outside running call: ${callID}`))
const current = { ...update } tool.progress = update
tool.progress = current
yield* events.publish(SessionEvent.Tool.Progress, { yield* events.publish(SessionEvent.Tool.Progress, {
sessionID: input.sessionID, sessionID: input.sessionID,
assistantMessageID: tool.assistantMessageID, assistantMessageID: tool.assistantMessageID,
callID, callID,
metadata: current, metadata: update,
}) })
}) })

View file

@ -365,7 +365,7 @@ describe("PluginV2", () => {
yield* ctx.tool yield* ctx.tool
.hook("execute.after", (event) => .hook("execute.after", (event) =>
Effect.sync(() => { 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) .pipe(Effect.asVoid)

View file

@ -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 () => { test("interrupted progress metadata remains in the terminal failure snapshot", async () => {
const { published, publisher } = capture("anthropic", { interruptProgress: true }) const { published, publisher } = capture("anthropic", { interruptProgress: true })
await Effect.runPromise(publisher.publish(call)) await Effect.runPromise(publisher.publish(call))
const exit = await Effect.runPromiseExit( const exit = await Effect.runPromiseExit(publisher.progress(call.id, { phase: "visible" }))
publisher.progress(call.id, { phase: "visible" }),
)
expect(Exit.isFailure(exit) && Cause.hasInterruptsOnly(exit.cause)).toBe(true) expect(Exit.isFailure(exit) && Cause.hasInterruptsOnly(exit.cause)).toBe(true)
await Effect.runPromise(publisher.failUnsettledTools({ type: "aborted", message: "interrupted" })) 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 () => { test("failure before progress omits partial output fields", async () => {
const { published, publisher } = capture() const { published, publisher } = capture()
await Effect.runPromise(publisher.publish(call)) await Effect.runPromise(publisher.publish(call))