From 58a27e75554d6e6e7d9883bdb0473ff03a28dfda Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Wed, 19 Aug 2026 18:30:02 -0400 Subject: [PATCH] fix(plugin): keep tool schema validation in the authoring effect instance Plugins load their own copy of effect, so their live Effect schemas cannot be interpreted by the host instance: parser sentinels and AST class identity are per-instance, making checks false-fail on valid input (bogus minLength errors) and branded IDs die as defects surfaced as bare 'Tool execution failed'. Plugin.define now converts tool input/output schemas to detached Standard Schema wrappers at the draft.add boundary, so validation and JSON Schema generation run as closures bound to the instance that created the schema. The core tool runtime detects still-foreign live schemas from older plugin packages and skips validation with a warning instead of misvalidating, and standard-schema validation errors now include the issue path. --- packages/core/src/tool/runtime.ts | 37 ++++- .../test/tool-runtime-foreign-schema.test.ts | 138 ++++++++++++++++++ packages/plugin/src/effect/plugin.ts | 22 ++- packages/plugin/src/effect/tool-schema.ts | 38 +++++ .../plugin/test/instance-safe-tool.test.ts | 71 +++++++++ 5 files changed, 300 insertions(+), 6 deletions(-) create mode 100644 packages/core/test/tool-runtime-foreign-schema.test.ts create mode 100644 packages/plugin/src/effect/tool-schema.ts create mode 100644 packages/plugin/test/instance-safe-tool.test.ts diff --git a/packages/core/src/tool/runtime.ts b/packages/core/src/tool/runtime.ts index f75dc543a42..d3d3a0cc465 100644 --- a/packages/core/src/tool/runtime.ts +++ b/packages/core/src/tool/runtime.ts @@ -1,7 +1,7 @@ import type { ToolDefinition } from "@opencode-ai/ai" import { Tool } from "@opencode-ai/schema/tool" import type { StandardJSONSchemaV1, StandardSchemaV1 } from "@standard-schema/spec" -import { Effect, JsonSchema, Schema } from "effect" +import { Effect, JsonSchema, Schema, SchemaAST } from "effect" export const definition = (tool: Tool.Info): ToolDefinition => ({ name: effectiveName(tool), @@ -32,22 +32,26 @@ export const execute = (tool: Tool.Info, input: unknown, context: Tool }) const decodeInput = (schema: Tool.ValueSchema, value: unknown) => { - if (Schema.isSchema(schema)) + if (Schema.isSchema(schema)) { + if (isForeignSchema(schema)) return foreignSchemaPassthrough(value) return Schema.decodeUnknownEffect(schema)(value).pipe( Effect.mapError((error) => new Tool.Error({ message: `Invalid tool input: ${error.message}` })), ) + } if (isStandardSchema(schema)) return validateStandard(schema, value, "Invalid tool input") return Effect.succeed(value) } const encodeOutput = (schema: Tool.ValueSchema, value: unknown) => { - if (Schema.isSchema(schema)) + if (Schema.isSchema(schema)) { + if (isForeignSchema(schema)) return foreignSchemaPassthrough(value) return Schema.encodeEffect(schema)(value).pipe( Effect.mapError( (error) => new Tool.Error({ message: `Tool returned an invalid value for its output schema: ${error.message}` }), ), ) + } if (isStandardSchema(schema)) return validateStandard(schema, value, "Tool returned an invalid value for its output schema") return Schema.decodeUnknownEffect(Schema.Json)(value).pipe( @@ -57,6 +61,23 @@ const encodeOutput = (schema: Tool.ValueSchema, value: unknown) => { ) } +// A schema created by a different copy of `effect` (for example one loaded from a +// plugin's own node_modules) still satisfies `Schema.isSchema` because the type +// identifier is a shared string, but it cannot be interpreted by this instance: +// schema parsing relies on per-instance sentinels and class identity, so checks +// false-fail on valid values and branded types die as defects. AST classes are plain +// classes, so an instanceof test against this instance's AST base distinguishes the +// two reliably. +const isForeignSchema = (schema: Schema.Top) => !(schema.ast instanceof SchemaAST.Base) + +// Current @opencode-ai/plugin versions convert plugin schemas to Standard Schema +// wrappers before registration, keeping validation in the authoring instance. For +// plugins built against older versions, skip validation rather than misvalidate. +const foreignSchemaPassthrough = (value: unknown) => + Effect.logWarning( + "Tool schema was created by a different `effect` module instance; skipping validation. Update the plugin's @opencode-ai/plugin dependency to restore validation.", + ).pipe(Effect.as(value)) + const isStandardSchema = ( schema: Tool.ValueSchema, ): schema is StandardSchemaV1 & StandardJSONSchemaV1 => @@ -78,11 +99,19 @@ const validateStandard = ( : pending if (result.issues) return yield* new Tool.Error({ - message: `${prefix}: ${result.issues.map((issue) => issue.message).join(", ")}`, + message: `${prefix}: ${result.issues.map(standardIssueText).join(", ")}`, }) return result.value }) +const standardIssueText = (issue: StandardSchemaV1.Issue) => { + if (issue.path === undefined || issue.path.length === 0) return issue.message + const segments = issue.path.map((segment) => + typeof segment === "object" && segment !== null && "key" in segment ? segment.key : segment, + ) + return `${issue.message} at ${JSON.stringify(segments)}` +} + const standardFailure = (prefix: string, error: unknown) => new Tool.Error({ message: `${prefix}: ${error instanceof Error ? error.message : String(error)}` }) diff --git a/packages/core/test/tool-runtime-foreign-schema.test.ts b/packages/core/test/tool-runtime-foreign-schema.test.ts new file mode 100644 index 00000000000..e72af07f07b --- /dev/null +++ b/packages/core/test/tool-runtime-foreign-schema.test.ts @@ -0,0 +1,138 @@ +import { beforeAll, expect, test } from "bun:test" +import { cp, mkdir, mkdtemp, readFile, symlink } from "node:fs/promises" +import { tmpdir } from "node:os" +import path from "node:path" +import { fileURLToPath, pathToFileURL } from "node:url" +import { Tool } from "@opencode-ai/core/tool" +import { definition, execute } from "@opencode-ai/core/tool/runtime" +import { Agent } from "@opencode-ai/schema/agent" +import { Session } from "@opencode-ai/schema/session" +import { SessionMessage } from "@opencode-ai/schema/session-message" +import type { Info } from "@opencode-ai/schema/tool" +import { Effect, Schema } from "effect" + +const context = { + sessionID: Session.ID.make("ses_foreign"), + agent: Agent.ID.make("build"), + messageID: SessionMessage.ID.make("msg_foreign"), + id: Tool.CallID.make("call_foreign"), + progress: () => Effect.void, +} + +// Plugins load `effect` from their own node_modules, so their schemas come from a +// different module instance than the host's. Simulate that by copying the effect +// package to a temporary directory and importing the copy: same version, distinct +// instance, exactly like a plugin installed in the config directory. +let foreign: typeof Schema + +beforeAll(async () => { + const source = path.dirname(fileURLToPath(import.meta.resolve("effect/package.json"))) + const base = await mkdtemp(path.join(tmpdir(), "opencode-foreign-effect-")) + const target = path.join(base, "node_modules", "effect") + await cp(source, target, { recursive: true }) + const dependencies = JSON.parse(await readFile(path.join(source, "package.json"), "utf8")).dependencies ?? {} + for (const name of Object.keys(dependencies)) { + const real = path.dirname(Bun.resolveSync(`${name}/package.json`, source)) + const link = path.join(base, "node_modules", name) + await mkdir(path.dirname(link), { recursive: true }) + await symlink(real, link, "dir") + } + const mod = (await import(pathToFileURL(path.join(target, "dist", "index.js")).href)) as { Schema: typeof Schema } + foreign = mod.Schema + expect(foreign).not.toBe(Schema) +}) + +test("foreign live schemas skip validation instead of misvalidating checks", async () => { + // Regression: a minLength check from a foreign instance used to fail on valid + // values ('Expected a value with a length of at least 1 at ["title"]') because the + // host parser hands the foreign filter an internal sentinel instead of the value. + const input = foreign.Struct({ + title: foreign.optional(foreign.String.check(foreign.isMinLength(1))), + prompt: foreign.optional(foreign.String), + }) + expect(Schema.isSchema(input)).toBe(true) + let received: unknown + const tool: Info = { + name: "create", + description: "Create", + input, + execute: (value) => { + received = value + return Effect.succeed({ content: "ok" }) + }, + } + const result = await Effect.runPromise(execute(tool, { title: "probe", prompt: "Say ready." }, context)) + expect(result.content).toEqual([{ type: "text", text: "ok" }]) + expect(received).toEqual({ title: "probe", prompt: "Say ready." }) +}) + +test("foreign branded schemas no longer die as defects", async () => { + // Regression: decoding a foreign branded ID (like Session.ID) threw "Sync adapter + // can only throw schema errors", surfacing as a bare "Tool execution failed". + const input = foreign.Struct({ + sessionID: foreign.String.check(foreign.isStartsWith("ses")).pipe(foreign.brand("SessionID")), + }) + const tool: Info = { + name: "notify", + description: "Notify", + input, + execute: (value) => Effect.succeed({ content: JSON.stringify(value) }), + } + const result = await Effect.runPromise(execute(tool, { sessionID: "ses_123" }, context)) + expect(result.content).toEqual([{ type: "text", text: '{"sessionID":"ses_123"}' }]) +}) + +test("foreign output schemas pass the produced value through", async () => { + const tool: Info = { + name: "get", + description: "Get", + input: foreign.Struct({}), + output: foreign.Struct({ sessionID: foreign.String }), + execute: () => Effect.succeed({ output: { sessionID: "ses_123" } }), + } + const result = await Effect.runPromise(execute(tool, {}, context)) + expect(result.output).toEqual({ sessionID: "ses_123" }) +}) + +// Mirrors the conversion current @opencode-ai/plugin versions perform in the +// authoring instance before registration (see packages/plugin/src/effect/tool-schema.ts). +const convert = (schema: unknown, direction: "input" | "output") => { + const anyForeign = foreign as any + const oriented = direction === "input" ? schema : anyForeign.flip(schema) + const augmented = anyForeign.toStandardJSONSchemaV1(anyForeign.toStandardSchemaV1(oriented)) + return { "~standard": augmented["~standard"] } as Info["input"] +} + +test("converted standard wrappers validate in the authoring instance", async () => { + const input = convert( + foreign.Struct({ + title: foreign.optional(foreign.String.check(foreign.isMinLength(1))), + }), + "input", + ) + expect(Schema.isSchema(input)).toBe(false) + let received: unknown + const tool: Info = { + name: "create", + description: "Create", + input, + output: convert(foreign.Struct({ sessionID: foreign.String }), "output"), + execute: (value) => { + received = value + return Effect.succeed({ output: { sessionID: "ses_123" }, content: "created" }) + }, + } + + const success = await Effect.runPromise(execute(tool, { title: "probe" }, context)) + expect(received).toEqual({ title: "probe" }) + expect(success.output).toEqual({ sessionID: "ses_123" }) + + const failure = await Effect.runPromiseExit(execute(tool, { title: "" }, context)) + expect(failure.toString()).toContain("Invalid tool input") + expect(failure.toString()).toContain("a value with a length of at least 1") + expect(failure.toString()).toContain('at ["title"]') + + const derived = definition(tool) + expect(derived.inputSchema).toMatchObject({ type: "object" }) + expect((derived.inputSchema as { properties?: Record }).properties).toHaveProperty("title") +}) diff --git a/packages/plugin/src/effect/plugin.ts b/packages/plugin/src/effect/plugin.ts index bd427034655..19703c7f5e1 100644 --- a/packages/plugin/src/effect/plugin.ts +++ b/packages/plugin/src/effect/plugin.ts @@ -1,5 +1,6 @@ import type { PluginApi } from "@opencode-ai/client/effect/api" import type { Effect, Scope } from "effect" +import { instanceSafeTool } from "./tool-schema.js" import type { PluginOptions } from "../options.js" import type { App } from "../app.js" import type { AgentDomain } from "./agent.js" @@ -41,6 +42,23 @@ export interface Plugin { readonly effect: (context: Context) => Effect.Effect } -export function define(plugin: Plugin) { - return plugin +export function define(plugin: Plugin): Plugin { + return { + ...plugin, + effect: (context) => plugin.effect(instanceSafeContext(context)), + } +} + +// Tool schemas cross from the plugin's module world into the host at `draft.add`; +// convert them while authoring-instance code is still on the stack so the host never +// interprets a foreign Effect schema. See `instanceSafeTool`. +function instanceSafeContext(context: Context): Context { + return { + ...context, + tool: { + ...context.tool, + transform: (callback) => + context.tool.transform((draft) => callback({ add: (tool) => draft.add(instanceSafeTool(tool)) })), + }, + } } diff --git a/packages/plugin/src/effect/tool-schema.ts b/packages/plugin/src/effect/tool-schema.ts new file mode 100644 index 00000000000..6d235d7b6c3 --- /dev/null +++ b/packages/plugin/src/effect/tool-schema.ts @@ -0,0 +1,38 @@ +import { Schema } from "effect" +import type { Tool } from "@opencode-ai/schema/tool" + +/** + * Converts a tool's Effect schemas into detached Standard Schema wrappers so they + * survive the crossing from the plugin's module world into the host. + * + * Plugins often load their own copy of `effect` (for example from the config + * directory's node_modules) while the host bundles a different instance. A live + * Effect schema cannot be interpreted across that boundary: schema parsing relies on + * per-instance sentinels and class identity, so the host misvalidates checks and + * turns branded-type failures into defects. A Standard Schema wrapper instead carries + * validation and JSON Schema generation as closures bound to the instance that + * created the schema, which the host invokes as-is. + */ +export function instanceSafeTool(tool: Tool.Info): Tool.Info { + const input = instanceSafeValueSchema(tool.input, "input") + const output = tool.output === undefined ? undefined : instanceSafeValueSchema(tool.output, "output") + if (input === tool.input && output === tool.output) return tool + return { ...tool, input, ...(output === undefined ? {} : { output }) } +} + +function instanceSafeValueSchema(schema: Tool.ValueSchema, direction: "input" | "output"): Tool.ValueSchema { + if (!Schema.isSchema(schema)) return schema + // Inputs are decoded (Encoded -> Type) but outputs are encoded (Type -> Encoded), + // so outputs use the flipped schema: its standard `validate` runs in the encode + // direction and its `jsonSchema.output` still describes the encoded shape. + const oriented = direction === "input" ? (schema as Schema.Top) : Schema.flip(schema as Schema.Top) + // Both converters augment the schema object in place and return it; the host must + // receive a plain wrapper instead, because the augmented object still satisfies + // `Schema.isSchema` and would route back into cross-instance interpretation. + const augmented = Schema.toStandardJSONSchemaV1( + Schema.toStandardSchemaV1(oriented as never) as never, + ) as unknown as StandardWrapper + return { "~standard": augmented["~standard"] } as Tool.ValueSchema +} + +type StandardWrapper = { readonly "~standard": Record } diff --git a/packages/plugin/test/instance-safe-tool.test.ts b/packages/plugin/test/instance-safe-tool.test.ts new file mode 100644 index 00000000000..e19d561a528 --- /dev/null +++ b/packages/plugin/test/instance-safe-tool.test.ts @@ -0,0 +1,71 @@ +import { expect, test } from "bun:test" +import type { StandardJSONSchemaV1, StandardSchemaV1 } from "@standard-schema/spec" +import { Effect, Schema } from "effect" +import { Plugin } from "../src/effect/index.js" +import type { Tool } from "@opencode-ai/schema/tool" + +// `define` must hand the host detached Standard Schema wrappers instead of live +// Effect schemas: hosts may run a different `effect` instance, which cannot +// interpret foreign schemas (checks false-fail and branded types die as defects). +const collectTool = async (tool: Tool.Info) => { + const added: Array> = [] + const context = { + tool: { + transform: (callback: (draft: { add: (tool: Tool.Info) => void }) => void) => { + callback({ add: (item) => added.push(item) }) + return Effect.succeed({ dispose: Effect.void }) + }, + }, + } as unknown as Plugin.Context + const plugin = Plugin.define({ + id: "test.instance-safe", + effect: (ctx) => ctx.tool.transform((draft) => draft.add(tool)).pipe(Effect.asVoid), + }) + await Effect.runPromise(Effect.scoped(plugin.effect(context))) + expect(added).toHaveLength(1) + return added[0] +} + +type StandardValue = StandardSchemaV1 & StandardJSONSchemaV1 + +test("define converts Effect schemas to detached standard wrappers", async () => { + const execute = (input: { title?: string }) => Effect.succeed({ output: { id: `ses_${input.title}` } }) + const registered = await collectTool({ + name: "create", + description: "Create", + input: Schema.Struct({ title: Schema.optional(Schema.String.check(Schema.isMinLength(1))) }), + output: Schema.Struct({ id: Schema.String }), + execute, + }) + + expect(registered.execute).toBe(execute) + expect(Schema.isSchema(registered.input)).toBe(false) + expect(Schema.isSchema(registered.output)).toBe(false) + + const input = registered.input as StandardValue + expect(await input["~standard"].validate({ title: "probe" })).toEqual({ value: { title: "probe" } }) + const invalid = await input["~standard"].validate({ title: "" }) + expect(invalid.issues?.[0]?.message).toContain("a value with a length of at least 1") + expect(input["~standard"].jsonSchema.input({ target: "draft-2020-12" })).toMatchObject({ type: "object" }) + + // Outputs validate in the encode direction (Type -> Encoded) and describe the + // encoded shape. + const output = registered.output as StandardValue + expect(await output["~standard"].validate({ id: "ses_x" })).toEqual({ value: { id: "ses_x" } }) + expect(output["~standard"].jsonSchema.output({ target: "draft-2020-12" })).toMatchObject({ + type: "object", + required: ["id"], + }) +}) + +test("define leaves non-Effect schemas untouched", async () => { + const input = { type: "object" as const } + const registered = await collectTool({ + name: "raw", + description: "Raw", + input, + execute: () => Effect.succeed({ content: "ok" }), + }) + expect(registered.input).toBe(input) + expect(registered.output).toBeUndefined() +})