From c79a9634d37dd5e7a0d7aedba45b150d04e25d0c Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Tue, 19 May 2026 11:07:28 -0400 Subject: [PATCH] fix(tool): tolerate plugin tool defs with missing args (#28357) --- packages/opencode/src/tool/registry.ts | 7 +- packages/opencode/test/tool/registry.test.ts | 92 ++++++++++++++++++-- 2 files changed, 92 insertions(+), 7 deletions(-) diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index 2547daee6e..6ef6d39a65 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -145,9 +145,12 @@ export const layer: Layer.Layer< function fromPlugin(id: string, def: ToolDefinition): Tool.Def { // Plugin tools still expose Zod args publicly; keep that compatibility // boxed at the registry boundary and give the LLM the original JSON Schema. - const entries = Object.entries(def.args) + // Normalize missing args to `{}` once — pre-1.14.49 the code was + // `z.object(def.args)` and Zod silently tolerated undefined (#27451, #27630). + const args = def.args ?? {} + const entries = Object.entries(args) const allZod = entries.every((entry) => isZodType(entry[1])) - const zodParams = allZod ? z.object(def.args) : undefined + const zodParams = allZod ? z.object(args) : undefined const jsonSchema = zodParams ? zodJsonSchema(zodParams) : legacyJsonSchema(entries) const parameters = zodParams ? Schema.declare((u): u is unknown => zodParams.safeParse(u).success) diff --git a/packages/opencode/test/tool/registry.test.ts b/packages/opencode/test/tool/registry.test.ts index 261c469372..321ab12444 100644 --- a/packages/opencode/test/tool/registry.test.ts +++ b/packages/opencode/test/tool/registry.test.ts @@ -40,11 +40,16 @@ const configLayer = TestConfig.layer({ directories: () => InstanceState.directory.pipe(Effect.map((dir) => [path.join(dir, ".opencode")])), }) -const registryLayer = (flags: Partial = {}) => +type RegistryLayerOptions = { + flags?: Partial + plugin?: Layer.Layer +} + +const registryLayer = (opts: RegistryLayerOptions = {}) => ToolRegistry.layer .pipe( Layer.provide(configLayer), - Layer.provide(Plugin.defaultLayer), + Layer.provide(opts.plugin ?? Plugin.defaultLayer), Layer.provide(Question.defaultLayer), Layer.provide(Todo.defaultLayer), Layer.provide(Skill.defaultLayer), @@ -64,12 +69,38 @@ const registryLayer = (flags: Partial = {}) => Layer.provide(Ripgrep.defaultLayer), Layer.provide(Truncate.defaultLayer), ) - .pipe(Layer.provide(RuntimeFlags.layer(flags))) + .pipe(Layer.provide(RuntimeFlags.layer(opts.flags ?? {}))) + +// Fake Plugin.Service that returns a single plugin whose `tool` map contains +// one definition with `args: undefined`. Used to exercise the plugin entry +// point of `fromPlugin` for the #27451 / #27630 regression. +const brokenPluginLayer = Layer.succeed( + Plugin.Service, + Plugin.Service.of({ + init: () => Effect.void, + trigger: ((_name: unknown, _input: unknown, output: unknown) => Effect.succeed(output)) as Plugin.Interface["trigger"], + list: () => + Effect.succeed([ + { + tool: { + broken_plugin_tool: { + description: "plugin tool with missing args", + args: undefined as unknown as Record, + execute: async () => "ok", + }, + }, + }, + ]), + }), +) const it = testEffect(Layer.mergeAll(registryLayer(), node, Agent.defaultLayer)) -const scout = testEffect(Layer.mergeAll(registryLayer({ experimentalScout: true }), node, Agent.defaultLayer)) +const scout = testEffect(Layer.mergeAll(registryLayer({ flags: { experimentalScout: true } }), node, Agent.defaultLayer)) const background = testEffect( - Layer.mergeAll(registryLayer({ experimentalBackgroundSubagents: true }), node, Agent.defaultLayer), + Layer.mergeAll(registryLayer({ flags: { experimentalBackgroundSubagents: true } }), node, Agent.defaultLayer), +) +const withBrokenPlugin = testEffect( + Layer.mergeAll(registryLayer({ plugin: brokenPluginLayer }), node, Agent.defaultLayer), ) afterEach(async () => { @@ -186,6 +217,57 @@ describe("tool.registry", () => { }), ) + // Regression for #27451 / #27630: a custom tool that omits `args` must not + // crash registry initialization with + // `Object.entries requires that input parameter not be null or undefined`. + // Pre-1.14.49 the code path was `z.object(def.args)`, and `z.object(undefined)` + // silently produced an empty schema — so the tool registered as no-args. + // Preserve that tolerance. + it.instance("tolerates a custom tool exporting null/undefined args (no-args fallback)", () => + Effect.gen(function* () { + const test = yield* TestInstance + const tool = path.join(test.directory, ".opencode", "tool") + yield* Effect.promise(() => fs.mkdir(tool, { recursive: true })) + yield* Effect.promise(() => + Bun.write( + path.join(tool, "noargs.ts"), + [ + "export default {", + " description: 'tool with no args',", + " args: undefined,", + " execute: async () => 'ok',", + "}", + "", + ].join("\n"), + ), + ) + + const registry = yield* ToolRegistry.Service + const ids = yield* registry.ids() + // Built-in tools must still load — a single malformed custom tool must + // not poison the whole registry. + expect(ids).toContain("read") + const loaded = (yield* registry.all()).find((t) => t.id === "noargs") + if (!loaded) throw new Error("noargs tool was not loaded") + expect(loaded.jsonSchema).toMatchObject({ type: "object", properties: {} }) + }), + ) + + // Same regression, plugin entry point. The original reports (#27451, #27630) + // came in through `plugin.list()` — `oh-my-opencode` was registering a tool + // with `args: undefined` and crashing every message submit. The file-scan + // and plugin-list loops both funnel through `fromPlugin`, but covering both + // entry points means a future refactor that splits them won't silently lose + // protection. + withBrokenPlugin.instance("tolerates a plugin tool registered with null/undefined args", () => + Effect.gen(function* () { + const registry = yield* ToolRegistry.Service + const ids = yield* registry.ids() + expect(ids).toContain("read") + expect(ids).toContain("broken_plugin_tool") + }), + ) + it.instance("loads tools from .opencode/tools (plural)", () => Effect.gen(function* () { const test = yield* TestInstance