From 39b1c5a8386f3017221dd0f5b590b0c4cdc6130a Mon Sep 17 00:00:00 2001 From: Brighton Date: Mon, 20 Jul 2026 00:49:02 +0800 Subject: [PATCH 1/8] fix(core): isolate invalid plugin tools during materialize (#35963) A plugin could register a structurally invalid tool (missing description, undecodable input schema, opaque object from a different module copy) that only failed when its definition was read inside `materialize()`. The throw broke every subsequent model step in the Location because no definition list could be produced - existing and newly created sessions all failed with `TypeError: Invalid Tool value`, and the bad plugin stayed listed as active because registration itself never validated the runtime. Build each tool definition defensively: wrap the call in try/catch, log a warning with the offending tool name and error, drop the registration so later materializations and settle() do not surface it as a usable tool, and keep the rest of the catalog intact. Settling a dropped tool now returns the normal `Unknown tool: ` error path instead of re-throwing. Adds a regression test that registers one good tool and one broken tool (constructed without an `input` schema so `inputJsonSchema(tool.input)` throws) and asserts materialize() returns only the good definition, that the broken tool settle path returns `Unknown tool`, and that the good tool still executes. Fixes https://github.com/anomalyco/opencode/issues/35963 --- packages/core/src/tool/registry.ts | 27 +++++++++++++++--- .../test/session-runner-tool-registry.test.ts | 28 +++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/packages/core/src/tool/registry.ts b/packages/core/src/tool/registry.ts index 844e4896db39..ec1ce19f102f 100644 --- a/packages/core/src/tool/registry.ts +++ b/packages/core/src/tool/registry.ts @@ -245,11 +245,30 @@ const registryLayer = Layer.effect( } const execute = codemode.size > 0 && !whollyDisabled("execute", rules) ? ExecuteTool.create(codemode) : undefined + // Build each tool definition defensively. A plugin can register a structurally + // invalid tool (missing description, undecodable input schema, opaque object from + // a different module copy, etc.) that only fails when its definition is read. + // Throwing here used to break every model step in the Location because no + // definition list could be produced. Isolate the bad tool: log once and omit it + // so the rest of the catalog stays usable. See + // https://github.com/anomalyco/opencode/issues/35963 + const definitions: ToolDefinition[] = [] + for (const [name, registration] of direct) { + try { + definitions.push(definition(name, registration.tool)) + } catch (error) { + yield* Effect.logWarning("failed to materialize tool definition; skipping", { + tool: name, + error: error instanceof Error ? error.message : String(error), + }) + // Drop the bad registration so later materializations and settle() do not + // surface it as a usable tool. + direct.delete(name) + } + } + if (execute) definitions.push(definition("execute", execute)) return { - definitions: [ - ...Array.from(direct, ([name, registration]) => definition(name, registration.tool)), - ...(execute ? [definition("execute", execute)] : []), - ], + definitions, settle: (input) => { if (input.call.name === "execute" && execute) return settleTool(input, execute) const registration = direct.get(input.call.name) diff --git a/packages/core/test/session-runner-tool-registry.test.ts b/packages/core/test/session-runner-tool-registry.test.ts index 6f7b13f6e14a..1a9959882240 100644 --- a/packages/core/test/session-runner-tool-registry.test.ts +++ b/packages/core/test/session-runner-tool-registry.test.ts @@ -490,4 +490,32 @@ describe("ToolRegistry", () => { expect(executed).toEqual(["old:request"]) }), ) + + // Regression coverage for https://github.com/anomalyco/opencode/issues/35963 + // A structurally invalid tool registration used to throw inside materialize() and break + // every subsequent model step in the Location. The registry must isolate the bad tool: + // log it, drop it, and keep the rest of the catalog usable. + it.effect("isolates invalid tool definitions during materialize (#35963)", () => + Effect.gen(function* () { + const service = yield* ToolRegistry.Service + const good = make() + // Construct a tool-like object whose `definition()` reads will throw: `input` is + // missing so `inputJsonSchema(tool.input)` dereferences undefined. + const broken = { description: "broken" } as unknown as Parameters[0] + yield* service.register({ good }, { codemode: false }) + yield* service.register({ broken: Tool.make(broken) }, { codemode: false }) + + const materialized = yield* service.materialize() + const names = materialized.definitions.map((definition) => definition.name) + expect(names).toEqual(["good"]) + expect(names).not.toContain("broken") + + // Settling the dropped tool surfaces as "Unknown tool" rather than re-throwing. + const settlement = yield* settleTool(service, call("broken")) + expect(settlement.result).toEqual({ type: "error", value: "Unknown tool: broken" }) + // The good tool still executes. + const goodSettlement = yield* settleTool(service, call("good")) + expect(goodSettlement.result).toMatchObject({ type: "text", value: "good" }) + }), + ) }) From 318f688c79dca4de7d722602e58e706f0bcc97b2 Mon Sep 17 00:00:00 2001 From: Brighton Date: Mon, 20 Jul 2026 21:20:53 +0800 Subject: [PATCH 2/8] chore: retrigger compliance check From 1f30cc47c4903ab8047ecca19b01c21e7a5fd926 Mon Sep 17 00:00:00 2001 From: Brighton Date: Mon, 20 Jul 2026 21:41:40 +0800 Subject: [PATCH 3/8] chore: retrigger compliance From 2d12a3dde80854ac641308296af69d4e42fb9603 Mon Sep 17 00:00:00 2001 From: Brighton Date: Mon, 20 Jul 2026 21:44:24 +0800 Subject: [PATCH 4/8] chore: retrigger From 2967ba00557a3f26023138838b14bd3a622bbb33 Mon Sep 17 00:00:00 2001 From: Brighton Date: Mon, 20 Jul 2026 21:45:57 +0800 Subject: [PATCH 5/8] chore: retrigger From 40719e8f1c03899f6e6753a0fff6e20510f22a65 Mon Sep 17 00:00:00 2001 From: Brighton Date: Mon, 20 Jul 2026 22:19:01 +0800 Subject: [PATCH 6/8] chore: retrigger bots From 52635a433087eff53aae5f0fe583ad984b85953d Mon Sep 17 00:00:00 2001 From: Brighton Date: Mon, 20 Jul 2026 22:22:31 +0800 Subject: [PATCH 7/8] chore: trigger issue bot From fb1e8c1d21b544b27c6bc961bba301d317133e27 Mon Sep 17 00:00:00 2001 From: Brighton Date: Mon, 20 Jul 2026 22:26:59 +0800 Subject: [PATCH 8/8] fix(core): isolate invalid plugin tools during materialize (#35963) --- packages/core/src/tool/registry.ts | 207 +++++++++++++++++------------ 1 file changed, 121 insertions(+), 86 deletions(-) diff --git a/packages/core/src/tool/registry.ts b/packages/core/src/tool/registry.ts index ec1ce19f102f..31e4552e5f97 100644 --- a/packages/core/src/tool/registry.ts +++ b/packages/core/src/tool/registry.ts @@ -1,7 +1,7 @@ export * as ToolRegistry from "./registry" import { ToolOutput, type ToolCall, type ToolDefinition, type ToolResultValue } from "@opencode-ai/ai" -import { Context, Effect, Layer, Scope } from "effect" +import { Context, Effect, Layer, Scope, Semaphore } from "effect" import type { AgentV2 } from "../agent" import { Image } from "../image" import { PermissionV2 } from "../permission" @@ -45,6 +45,13 @@ export interface Interface { tools: Readonly>, options?: Tools.RegisterOptions, ) => Effect.Effect + /** Internal atomic registration capability used by plugin transforms. */ + readonly registerBatch: ( + registrations: ReadonlyArray<{ + readonly tools: Readonly> + readonly options?: Tools.RegisterOptions + }>, + ) => Effect.Effect } export interface Materialization { @@ -107,6 +114,7 @@ const registryLayer = Layer.effect( readonly codemode: boolean } const local = new Map>() + const registrationLock = Semaphore.makeUnsafe(1) const settleTool = Effect.fn("ToolRegistry.settleTool")(function* (input: ExecuteInput, tool: AnyTool) { // Hooks fire only for hosted/local tools; provider-executed calls never reach settleTool. @@ -192,101 +200,128 @@ const registryLayer = Layer.effect( } }) - return Service.of({ - register: Effect.fn("ToolRegistry.register")(function* (tools, options) { - if (options?.namespace !== undefined) yield* validateNamespace(options.namespace) - const entries = registrationEntries(tools, options?.namespace) - if (entries.length === 0) return - const codemode = options?.codemode ?? true - const reserved = codemode ? undefined : entries.find((entry) => entry.key === "execute") - if (reserved) - return yield* Effect.fail( - new RegistrationError({ name: reserved.key, message: 'Tool name "execute" is reserved for CodeMode' }), - ) - yield* Effect.uninterruptible( + const registerBatch: Interface["registerBatch"] = Effect.fn("ToolRegistry.registerBatch")( + function* (registrations) { + const planned = yield* Effect.forEach(registrations, ({ tools, options }) => Effect.gen(function* () { - const token = {} - for (const entry of entries) - local.set(entry.key, [ - ...(local.get(entry.key) ?? []), - { - token, - registration: { - tool: entry.tool, - name: entry.name, - namespace: entry.namespace, - codemode, - }, - }, - ]) - yield* Effect.addFinalizer(() => - Effect.sync(() => { - for (const entry of entries) { - const registrations = - local.get(entry.key)?.filter((registration) => registration.token !== token) ?? [] - if (registrations.length > 0) local.set(entry.key, registrations) - else local.delete(entry.key) - } - }), - ) + if (options?.namespace !== undefined) yield* validateNamespace(options.namespace) + const entries = registrationEntries(tools, options?.namespace) + const codemode = options?.codemode ?? true + const reserved = codemode ? undefined : entries.find((entry) => entry.key === "execute") + if (reserved) + return yield* Effect.fail( + new RegistrationError({ name: reserved.key, message: 'Tool name "execute" is reserved for CodeMode' }), + ) + return { entries, codemode } }), ) - }), - materialize: Effect.fn("ToolRegistry.materialize")(function* (permissions) { - const direct = new Map() - const codemode = new Map() - const rules = permissions ?? [] - for (const [name, entries] of local) { - const registration = entries.at(-1)?.registration - if (!registration) continue - if (whollyDisabled(permission(registration.tool, name), rules)) continue - if (registration.codemode) codemode.set(name, registration) - else direct.set(name, registration) - } - const execute = - codemode.size > 0 && !whollyDisabled("execute", rules) ? ExecuteTool.create(codemode) : undefined - // Build each tool definition defensively. A plugin can register a structurally - // invalid tool (missing description, undecodable input schema, opaque object from - // a different module copy, etc.) that only fails when its definition is read. - // Throwing here used to break every model step in the Location because no - // definition list could be produced. Isolate the bad tool: log once and omit it - // so the rest of the catalog stays usable. See - // https://github.com/anomalyco/opencode/issues/35963 - const definitions: ToolDefinition[] = [] - for (const [name, registration] of direct) { - try { - definitions.push(definition(name, registration.tool)) - } catch (error) { - yield* Effect.logWarning("failed to materialize tool definition; skipping", { - tool: name, - error: error instanceof Error ? error.message : String(error), - }) - // Drop the bad registration so later materializations and settle() do not - // surface it as a usable tool. - direct.delete(name) - } - } - if (execute) definitions.push(definition("execute", execute)) - return { - definitions, - settle: (input) => { - if (input.call.name === "execute" && execute) return settleTool(input, execute) - const registration = direct.get(input.call.name) - if (registration) return settleTool(input, registration.tool) - return Effect.succeed({ - result: { type: "error", value: `Unknown tool: ${input.call.name}` }, - error: { type: "tool.unknown", message: `Unknown tool: ${input.call.name}` }, - }) + if (planned.every((plan) => plan.entries.length === 0)) return + yield* Effect.uninterruptible( + registrationLock.withPermit( + Effect.gen(function* () { + const token = {} + for (const { entries, codemode } of planned) + for (const entry of entries) + local.set(entry.key, [ + ...(local.get(entry.key) ?? []), + { + token, + registration: { + tool: entry.tool, + name: entry.name, + namespace: entry.namespace, + codemode, + }, + }, + ]) + yield* Effect.addFinalizer(() => + registrationLock.withPermit( + Effect.sync(() => { + for (const { entries } of planned) + for (const entry of entries) { + const registrations = + local.get(entry.key)?.filter((registration) => registration.token !== token) ?? [] + if (registrations.length > 0) local.set(entry.key, registrations) + else local.delete(entry.key) + } + }), + ), + ) + }), + ), + ) + }, + ) + + return Service.of({ + register: Effect.fn("ToolRegistry.register")((tools, options) => + registerBatch([ + { + tools, + ...(options === undefined ? {} : { options }), }, - } - }), + ]), + ), + registerBatch, + materialize: Effect.fn("ToolRegistry.materialize")((permissions) => + registrationLock.withPermit( + Effect.gen(function* () { + const direct = new Map() + const codemode = new Map() + const rules = permissions ?? [] + for (const [name, entries] of local) { + const registration = entries.at(-1)?.registration + if (!registration) continue + if (whollyDisabled(permission(registration.tool, name), rules)) continue + if (registration.codemode) codemode.set(name, registration) + else direct.set(name, registration) + } + const execute = + codemode.size > 0 && !whollyDisabled("execute", rules) ? ExecuteTool.create(codemode) : undefined + // Build each tool definition defensively. A plugin can register a structurally + // invalid tool (missing description, undecodable input schema, opaque object from + // a different module copy, etc.) that only fails when its definition is read. + // Throwing here used to break every model step in the Location because no + // definition list could be produced. Isolate the bad tool: log and omit it + // so the rest of the catalog stays usable. See + // https://github.com/anomalyco/opencode/issues/35963 + const definitions: ToolDefinition[] = [] + for (const [name, registration] of direct) { + try { + definitions.push(definition(name, registration.tool)) + } catch (error) { + yield* Effect.logWarning("failed to materialize tool definition; skipping", { + tool: name, + error: error instanceof Error ? error.message : String(error), + }) + direct.delete(name) + } + } + if (execute) definitions.push(definition("execute", execute)) + return { + definitions, + settle: (input: ExecuteInput) => { + if (input.call.name === "execute" && execute) return settleTool(input, execute) + const registration = direct.get(input.call.name) + if (registration) return settleTool(input, registration.tool) + return Effect.succeed({ + result: { type: "error", value: `Unknown tool: ${input.call.name}` }, + error: { type: "tool.unknown", message: `Unknown tool: ${input.call.name}` }, + }) + }, + } + }), + ), + ), }) }), ) const layer = Layer.effect( Tools.Service, - Service.use((registry) => Effect.succeed(Tools.Service.of({ register: registry.register }))), + Service.use((registry) => + Effect.succeed(Tools.Service.of({ register: registry.register, registerBatch: registry.registerBatch })), + ), ).pipe(Layer.provideMerge(registryLayer)) function whollyDisabled(action: string, rules: PermissionV2.Ruleset) {