fix(core): harden unified tool runtime (#31171)

This commit is contained in:
Kit Langton 2026-06-06 21:55:34 -04:00 committed by GitHub
commit eb9a683b40
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
35 changed files with 435 additions and 237 deletions

View file

@ -9,7 +9,7 @@ import { ToolRegistry } from "@opencode-ai/core/tool/registry"
import { executeTool, settleTool, toolDefinitions } from "./lib/tool"
import { ToolOutputStore } from "@opencode-ai/core/tool-output-store"
import { Tools } from "@opencode-ai/core/tool/tools"
import { Effect, Exit, Layer, Schema, Scope } from "effect"
import { Deferred, Effect, Exit, Fiber, Layer, Schema, Scope } from "effect"
import { testEffect } from "./lib/effect"
const permission = Layer.mock(PermissionV2.Service, {
@ -136,7 +136,7 @@ describe("ApplicationTools", () => {
],
},
output: {
structured: {},
structured: { answer: "HELLO" },
content: [
{ type: "text", text: "HELLO" },
{ type: "file", source: { type: "data", data: "aGVsbG8=" }, mime: "image/png", name: "result.png" },
@ -147,7 +147,7 @@ describe("ApplicationTools", () => {
}),
)
it.effect("removes an application tool when its attachment scope closes", () =>
it.effect("removes an application tool when its registration scope closes", () =>
Effect.gen(function* () {
const applications = yield* ApplicationTools.Service
const registry = yield* ToolRegistry.Service
@ -165,11 +165,11 @@ describe("ApplicationTools", () => {
Effect.gen(function* () {
const applications = yield* ApplicationTools.Service
const registry = yield* ToolRegistry.Service
const attachmentScope = yield* Scope.make()
yield* applications.register({ contextual: contextual([]) }).pipe(Scope.provide(attachmentScope))
const registrationScope = yield* Scope.make()
yield* applications.register({ contextual: contextual([]) }).pipe(Scope.provide(registrationScope))
expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["contextual"])
yield* Scope.close(attachmentScope, Exit.void)
yield* Scope.close(registrationScope, Exit.void)
expect(
yield* settleTool(registry, {
sessionID,
@ -181,7 +181,7 @@ describe("ApplicationTools", () => {
}),
)
it.effect("does not leak an attachment into an already closed scope", () =>
it.effect("does not leak a registration into an already closed scope", () =>
Effect.gen(function* () {
const applications = yield* ApplicationTools.Service
const registry = yield* ToolRegistry.Service
@ -194,13 +194,36 @@ describe("ApplicationTools", () => {
}),
)
it.effect("captures the attached record before later State rebuilds", () =>
it.effect("preserves an interrupted application registration until its scope closes", () =>
Effect.gen(function* () {
const applications = yield* ApplicationTools.Service
const registry = yield* ToolRegistry.Service
const attached = { stable: contextual([]) }
yield* applications.register(attached)
Object.assign(attached, { late: contextual([]) })
const scope = yield* Scope.make()
const registered = yield* Deferred.make<void>()
const fiber = yield* applications
.register({ interrupted: contextual([]) })
.pipe(
Effect.andThen(Deferred.succeed(registered, undefined)),
Effect.andThen(Effect.never),
Scope.provide(scope),
Effect.forkChild,
)
yield* Deferred.await(registered)
yield* Fiber.interrupt(fiber)
expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["interrupted"])
yield* Scope.close(scope, Exit.void)
expect(yield* toolDefinitions(registry)).toEqual([])
}),
)
it.effect("captures the registered record before later State rebuilds", () =>
Effect.gen(function* () {
const applications = yield* ApplicationTools.Service
const registry = yield* ToolRegistry.Service
const registered = { stable: contextual([]) }
yield* applications.register(registered)
Object.assign(registered, { late: contextual([]) })
yield* Effect.scoped(applications.register({ temporary: contextual([]) }))
@ -208,7 +231,7 @@ describe("ApplicationTools", () => {
}),
)
it.effect("settles with the current same-name application tool and restores earlier attachments", () =>
it.effect("settles with the current same-name application tool and restores earlier registrations", () =>
Effect.gen(function* () {
const applications = yield* ApplicationTools.Service
const registry = yield* ToolRegistry.Service

View file

@ -0,0 +1,13 @@
import { describe, expect, it } from "bun:test"
import { Tool } from "@opencode-ai/core/public"
import { Effect } from "effect"
describe("public Tool API", () => {
it("keeps the public registration capability narrow", () => {
const tools = {
register: () => Effect.void,
} satisfies Tool.Interface
expect(Object.keys(tools)).toEqual(["register"])
})
})

View file

@ -7,7 +7,7 @@ import { SessionMessage } from "@opencode-ai/core/session/message"
import { ToolOutputStore } from "@opencode-ai/core/tool-output-store"
import { ToolRegistry } from "@opencode-ai/core/tool/registry"
import { executeTool, settleTool, toolDefinitions } from "./lib/tool"
import { Cause, Deferred, Effect, Exit, Fiber, Layer, Option, Schema, Scope } from "effect"
import { Cause, Deferred, Effect, Exit, Fiber, Layer, Option, Schema, SchemaGetter, SchemaIssue, Scope } from "effect"
import { testEffect } from "./lib/effect"
const bounds: ToolOutputStore.BoundInput[] = []
@ -29,6 +29,7 @@ const outputStore = Layer.mock(ToolOutputStore.Service, {
})
const registry = ToolRegistry.layer.pipe(Layer.provide(ApplicationTools.layer), Layer.provide(outputStore))
const it = testEffect(registry)
const integrated = testEffect(Layer.mergeAll(ApplicationTools.layer, registry))
const identity = {
agent: AgentV2.ID.make("build"),
assistantMessageID: SessionMessage.ID.make("msg_registry"),
@ -125,6 +126,28 @@ describe("ToolRegistry", () => {
}),
)
it.effect("preserves an interrupted registration until its scope closes", () =>
Effect.gen(function* () {
const service = yield* ToolRegistry.Service
const scope = yield* Scope.make()
const registered = yield* Deferred.make<void>()
const fiber = yield* service
.register({ echo: make() })
.pipe(
Effect.andThen(Deferred.succeed(registered, undefined)),
Effect.andThen(Effect.never),
Scope.provide(scope),
Effect.forkChild,
)
yield* Deferred.await(registered)
yield* Fiber.interrupt(fiber)
expect((yield* toolDefinitions(service)).map((tool) => tool.name)).toEqual(["echo"])
yield* Scope.close(scope, Exit.void)
expect(yield* toolDefinitions(service)).toEqual([])
}),
)
it.effect("returns model errors without swallowing interruption or defects", () =>
Effect.gen(function* () {
const service = yield* ToolRegistry.Service
@ -237,6 +260,72 @@ describe("ToolRegistry", () => {
}),
)
it.effect("enforces transformed codecs at execution and projection boundaries", () =>
Effect.gen(function* () {
const service = yield* ToolRegistry.Service
const executed: string[] = []
const Transformed = Schema.Boolean.pipe(
Schema.decodeTo(Schema.String, {
decode: SchemaGetter.transform((value) => (value ? "yes" : "no")),
encode: SchemaGetter.transform((value) => value === "yes"),
}),
)
yield* service.register({
transformed: Tool.make({
description: "Transform values",
input: Schema.Struct({ value: Transformed }),
output: Schema.Struct({ value: Transformed }),
execute: ({ value }) => Effect.sync(() => executed.push(value)).pipe(Effect.as({ value })),
toModelOutput: ({ output }) => [{ type: "text", text: String(output.value) }],
}),
})
expect(
yield* executeTool(service, {
sessionID,
...identity,
call: { type: "tool-call", id: "transformed", name: "transformed", input: { value: true } },
}),
).toEqual({ type: "text", value: "true" })
expect(executed).toEqual(["yes"])
expect(
yield* executeTool(service, {
sessionID,
...identity,
call: { type: "tool-call", id: "invalid-input", name: "transformed", input: { value: "yes" } },
}),
).toMatchObject({ type: "error", value: expect.stringContaining("Invalid tool input") })
expect(executed).toEqual(["yes"])
yield* service.register({
invalid_output: Tool.make({
description: "Return invalid output",
input: Schema.Struct({}),
output: Schema.Struct({
value: Schema.Boolean.pipe(
Schema.decodeTo(Schema.String, {
decode: SchemaGetter.transform((value) => String(value)),
encode: SchemaGetter.transformOrFail((value) =>
value === "valid"
? Effect.succeed(true)
: Effect.fail(new SchemaIssue.InvalidValue(Option.some(value), { message: "invalid output" })),
),
}),
),
}),
execute: () => Effect.succeed({ value: "invalid" }),
}),
})
expect(
yield* executeTool(service, {
sessionID,
...identity,
call: { type: "tool-call", id: "invalid-output", name: "invalid_output", input: {} },
}),
).toMatchObject({ type: "error", value: expect.stringContaining("invalid value for its output schema") })
}),
)
it.effect("executes the unchanged registration advertised for a provider turn", () =>
Effect.gen(function* () {
const service = yield* ToolRegistry.Service
@ -293,6 +382,38 @@ describe("ToolRegistry", () => {
}),
)
integrated.effect("rejects an application call after a Location override is registered", () =>
Effect.gen(function* () {
const applications = yield* ApplicationTools.Service
const service = yield* ToolRegistry.Service
yield* applications.register({ echo: make() })
const materialized = yield* service.materialize()
yield* service.register({ echo: make() })
expect((yield* materialized.settle(call("echo"))).result).toEqual({
type: "error",
value: "Stale tool call: echo",
})
}),
)
integrated.effect("rejects a Location call after removal reveals an application registration", () =>
Effect.gen(function* () {
const applications = yield* ApplicationTools.Service
const service = yield* ToolRegistry.Service
yield* applications.register({ echo: make() })
const scope = yield* Scope.make()
yield* service.register({ echo: make() }).pipe(Scope.provide(scope))
const materialized = yield* service.materialize()
yield* Scope.close(scope, Exit.void)
expect((yield* materialized.settle(call("echo"))).result).toEqual({
type: "error",
value: "Stale tool call: echo",
})
}),
)
it.effect("keeps captured execution running after registration mutation", () =>
Effect.gen(function* () {
const service = yield* ToolRegistry.Service

View file

@ -2955,6 +2955,22 @@ describe("SessionRunnerLLM", () => {
expect(yield* session.resume(sessionID).pipe(Effect.catchDefect(Effect.succeed))).toBe("unexpected tool defect")
expect(requests).toHaveLength(1)
expect(yield* session.context(sessionID)).toMatchObject([
{ type: "user", text: "Call defect" },
{
type: "assistant",
content: [
{
type: "tool",
id: "call-defect",
state: {
status: "error",
error: { type: "unknown", message: "Tool execution failed: unexpected tool defect" },
},
},
],
},
])
}),
)

View file

@ -0,0 +1,34 @@
import { describe, expect } from "bun:test"
import { State } from "@opencode-ai/core/state"
import { Deferred, Effect, Exit, Fiber, Layer, Scope } from "effect"
import { testEffect } from "./lib/effect"
const it = testEffect(Layer.empty)
describe("State", () => {
it.effect("commits a transform atomically when its updater is interrupted", () =>
Effect.gen(function* () {
const rebuilding = yield* Deferred.make<void>()
const release = yield* Deferred.make<void>()
let block = true
const state = State.create({
initial: () => ({ values: [] as string[] }),
editor: (draft) => ({ add: (value: string) => draft.values.push(value) }),
finalize: () =>
block ? Deferred.succeed(rebuilding, undefined).pipe(Effect.andThen(Deferred.await(release))) : Effect.void,
})
const scope = yield* Scope.make()
const update = yield* state.transform().pipe(Scope.provide(scope))
const fiber = yield* update((editor) => editor.add("registered")).pipe(Effect.forkChild)
yield* Deferred.await(rebuilding)
const interruption = yield* Fiber.interrupt(fiber).pipe(Effect.forkChild)
block = false
yield* Deferred.succeed(release, undefined)
yield* Fiber.join(interruption)
expect(state.get().values).toEqual(["registered"])
yield* Scope.close(scope, Exit.void)
expect(state.get().values).toEqual([])
}),
)
})

View file

@ -115,7 +115,7 @@ const withTool = <A, E, R>(
}).pipe(Effect.provide(Layer.mergeAll(registry, bash)))
}
const call = (input: typeof BashTool.Parameters.Type, id = "call-bash") => ({
const call = (input: typeof BashTool.Input.Type, id = "call-bash") => ({
sessionID,
...toolIdentity,
call: { type: "tool-call" as const, id, name: "bash", input },

View file

@ -93,7 +93,7 @@ const withTool = <A, E, R>(directory: string, body: (registry: ToolRegistry.Inte
}).pipe(Effect.provide(Layer.mergeAll(registry, resolution, mutation, edit)))
}
const call = (input: typeof EditTool.Parameters.Type, id = "call-edit") => ({
const call = (input: typeof EditTool.Input.Type, id = "call-edit") => ({
sessionID,
...toolIdentity,
call: { type: "tool-call" as const, id, name: "edit", input },

View file

@ -91,7 +91,7 @@ const reset = () => {
result = new LocationSearch.FilesResult({ items: [], truncated: false, partial: false })
}
const call = (input: typeof GlobTool.Parameters.Type, id = "call-glob") => ({
const call = (input: typeof GlobTool.Input.Type, id = "call-glob") => ({
sessionID,
...toolIdentity,
call: { type: "tool-call" as const, id, name: "glob", input },

View file

@ -151,7 +151,7 @@ function provideLive(directory: string, projectReferences = references({})) {
}
describe("GrepTool", () => {
it.effect("registers the grep contribution", () =>
it.effect("registers grep", () =>
Effect.gen(function* () {
reset()
expect(yield* toolDefinitions(yield* ToolRegistry.Service)).toMatchObject([{ name: "grep" }])

View file

@ -43,7 +43,7 @@ const withStore = <A, E, R>(
const it = testEffect(Layer.empty)
describe("ToolOutputStore", () => {
it.live("bounds aggregate text and structured output with one managed file", () =>
it.live("bounds the provider-facing text channel with one managed file", () =>
withStore(({ store, fs }) =>
Effect.gen(function* () {
const first = "HEAD-" + "x".repeat(30_000)
@ -59,15 +59,9 @@ describe("ToolOutputStore", () => {
],
},
})
expect(result.output.structured).toEqual({})
expect(result.output.structured).toEqual({ kind: "report" })
expect(result.outputPaths).toHaveLength(1)
expect(JSON.parse(yield* fs.readFileString(result.outputPaths[0]))).toEqual({
structured: { kind: "report" },
content: [
{ type: "text", text: first },
{ type: "text", text: second },
],
})
expect(yield* fs.readFileString(result.outputPaths[0])).toBe(first + second)
if (result.output.content[0]?.type !== "text") throw new Error("expected text preview")
expect(Buffer.byteLength(result.output.content[0].text)).toBeLessThanOrEqual(ToolOutputStore.MAX_BYTES)
}),
@ -79,18 +73,18 @@ describe("ToolOutputStore", () => {
Effect.gen(function* () {
const structured = { text: "x".repeat(ToolOutputStore.MAX_BYTES) }
const result = yield* store.bound({ sessionID, toolCallID: "call-json", output: { structured, content: [] } })
expect(result.output.structured).toEqual({})
expect(result.output.structured).toEqual(structured)
expect(result.outputPaths).toHaveLength(1)
expect(JSON.parse(yield* fs.readFileString(result.outputPaths[0]))).toEqual({ structured, content: [] })
expect(JSON.parse(yield* fs.readFileString(result.outputPaths[0]))).toEqual(structured)
expect(result.output.content).toHaveLength(1)
}),
),
)
it.live("preserves oversized inline media without duplicating its data", () =>
it.live("preserves native media and structured metadata without applying a settlement media limit", () =>
withStore(({ store }) =>
Effect.gen(function* () {
const data = "a".repeat(ToolOutputStore.MAX_BYTES)
const data = "a".repeat(6 * 1024 * 1024)
const result = yield* store.bound({
sessionID,
toolCallID: "call-file",
@ -100,7 +94,7 @@ describe("ToolOutputStore", () => {
},
})
expect(result.outputPaths).toEqual([])
expect(result.output.structured).toEqual({})
expect(result.output.structured).toEqual({ caption: "pixel" })
expect(result.output.content).toHaveLength(1)
expect(result.output.content[0]).toEqual({
type: "file",
@ -112,51 +106,38 @@ describe("ToolOutputStore", () => {
),
)
it.live("rejects inline media beyond the settlement media limit", () =>
withStore(({ store }) =>
it.live("preserves structured metadata and native media when bounding text", () =>
withStore(({ store, fs }) =>
Effect.gen(function* () {
const exit = yield* store
.bound({
sessionID,
toolCallID: "call-file-too-large",
output: {
structured: {},
content: [
{
type: "file",
source: { type: "data", data: "a".repeat(ToolOutputStore.MAX_INLINE_MEDIA_BYTES + 1) },
mime: "image/png",
},
],
},
})
.pipe(Effect.exit)
expect(Exit.isFailure(exit)).toBe(true)
if (Exit.isFailure(exit))
expect(Option.getOrUndefined(Cause.findErrorOption(exit.cause))?._tag).toBe("ToolOutputStore.MediaLimitError")
const text = "x".repeat(ToolOutputStore.MAX_BYTES + 1)
const media = {
type: "file" as const,
source: { type: "data" as const, data: "aGVsbG8=" },
mime: "image/png",
name: "pixel.png",
}
const result = yield* store.bound({
sessionID,
toolCallID: "call-text-and-media",
output: { structured: { caption: "pixel" }, content: [{ type: "text", text }, media] },
})
expect(result.output.structured).toEqual({ caption: "pixel" })
expect(result.output.content[1]).toEqual(media)
expect(yield* fs.readFileString(result.outputPaths[0])).toBe(text)
}),
),
)
it.live("rejects inline media whose aggregate size exceeds the settlement limit", () =>
it.live("does not double-count structured data duplicated in projected text", () =>
withStore(({ store }) =>
Effect.gen(function* () {
const exit = yield* store
.bound({
sessionID,
toolCallID: "call-files-too-large",
output: {
structured: {},
content: [
{ type: "file", source: { type: "data", data: "a".repeat(3 * 1024 * 1024) }, mime: "image/png" },
{ type: "file", source: { type: "data", data: "b".repeat(3 * 1024 * 1024) }, mime: "image/png" },
],
},
})
.pipe(Effect.exit)
expect(Exit.isFailure(exit)).toBe(true)
if (Exit.isFailure(exit))
expect(Option.getOrUndefined(Cause.findErrorOption(exit.cause))?._tag).toBe("ToolOutputStore.MediaLimitError")
const text = "x".repeat(30_000)
const output = { structured: { output: text }, content: [{ type: "text" as const, text }] }
expect(yield* store.bound({ sessionID, toolCallID: "call-duplicated", output })).toEqual({
output,
outputPaths: [],
})
}),
),
)
@ -179,22 +160,14 @@ describe("ToolOutputStore", () => {
),
)
it.live("fails operationally when output cannot be encoded for bounding", () =>
it.live("does not encode ignored structured metadata when projected content exists", () =>
withStore(({ store }) =>
Effect.gen(function* () {
const exit = yield* store
.bound({
sessionID,
toolCallID: "call-unencodable",
output: {
structured: { value: 1n },
content: [{ type: "text", text: "readable text" }],
},
})
.pipe(Effect.exit)
expect(Exit.isFailure(exit)).toBe(true)
if (Exit.isFailure(exit))
expect(Option.getOrUndefined(Cause.findErrorOption(exit.cause))?._tag).toBe("ToolOutputStore.StorageError")
const output = { structured: { value: 1n }, content: [{ type: "text" as const, text: "readable text" }] }
expect(yield* store.bound({ sessionID, toolCallID: "call-unencodable", output })).toEqual({
output,
outputPaths: [],
})
}),
),
)

View file

@ -163,7 +163,7 @@ describe("ReadTool", () => {
...toolIdentity,
call: { type: "tool-call", id: "call-image-settle", name: "read", input: { path: "pixel.png" } },
})
expect(settled.output?.structured).toEqual({})
expect(settled.output?.structured).toMatchObject({ type: "binary", mime: "image/png", encoding: "base64" })
expect(settled.output?.content).toMatchObject([
{ type: "text", text: "Image read successfully" },
{ type: "file", mime: "image/png", source: { type: "data", data: png } },
@ -194,7 +194,7 @@ describe("ReadTool", () => {
})
expect(settled.outputPaths).toBeUndefined()
expect(settled.output?.structured).toEqual({})
expect(settled.output?.structured).toMatchObject({ type: "binary", mime: "image/png", encoding: "base64" })
expect(settled.result).toEqual({
type: "content",
value: [

View file

@ -51,7 +51,7 @@ const reset = () => {
respond = () => Effect.succeed(new Response("hello", { headers: { "content-type": "text/plain" } }))
}
const call = (input: typeof WebFetchTool.Parameters.Type, id = "call-webfetch") => ({
const call = (input: typeof WebFetchTool.Input.Type, id = "call-webfetch") => ({
sessionID,
...toolIdentity,
call: { type: "tool-call" as const, id, name: "webfetch", input },
@ -59,7 +59,7 @@ const call = (input: typeof WebFetchTool.Parameters.Type, id = "call-webfetch")
describe("WebFetchTool helpers", () => {
test("defaults format and rejects invalid timeout controls", () => {
const decode = Schema.decodeUnknownSync(WebFetchTool.Parameters)
const decode = Schema.decodeUnknownSync(WebFetchTool.Input)
expect(decode({ url: "https://example.com" })).toEqual({ url: "https://example.com", format: "markdown" })
expect(() => decode({ url: "https://example.com", timeout: 0 })).toThrow()
expect(() => decode({ url: "https://example.com", timeout: WebFetchTool.MAX_TIMEOUT_SECONDS + 1 })).toThrow()
@ -72,7 +72,7 @@ describe("WebFetchTool helpers", () => {
})
})
describe("WebFetchTool contribution", () => {
describe("WebFetchTool registration", () => {
it.effect("registers and fetches an ordinary hostname HTTP URL without rewriting it", () =>
Effect.gen(function* () {
reset()

View file

@ -18,7 +18,7 @@ const payload = (text: string) =>
describe("WebSearchTool provider selection", () => {
test("rejects out-of-range numeric controls", () => {
const decode = Schema.decodeUnknownSync(WebSearchTool.Parameters)
const decode = Schema.decodeUnknownSync(WebSearchTool.Input)
expect(() => decode({ query: "x", numResults: 0 })).toThrow()
expect(() => decode({ query: "x", numResults: WebSearchTool.MAX_NUM_RESULTS + 1 })).toThrow()
expect(() => decode({ query: "x", contextMaxCharacters: WebSearchTool.MAX_CONTEXT_CHARACTERS + 1 })).toThrow()
@ -122,7 +122,7 @@ const websearch = WebSearchTool.layer.pipe(
)
const it = testEffect(Layer.mergeAll(registry, permission, http, websearchConfig, websearch))
describe("WebSearchTool contribution", () => {
describe("WebSearchTool registration", () => {
it.effect("registers websearch, asserts query permission, and calls Exa", () =>
Effect.gen(function* () {
requests.length = 0

View file

@ -76,7 +76,7 @@ const withTool = <A, E, R>(directory: string, body: (registry: ToolRegistry.Inte
}).pipe(Effect.provide(Layer.mergeAll(registry, resolution, mutation, write)))
}
const call = (input: typeof WriteTool.Parameters.Type, id = "call-write") => ({
const call = (input: typeof WriteTool.Input.Type, id = "call-write") => ({
sessionID,
...toolIdentity,
call: { type: "tool-call" as const, id, name: "write", input },