From afad22cc7febca93acd43664774ff10e1cdad78f Mon Sep 17 00:00:00 2001 From: Brighton Date: Mon, 20 Jul 2026 22:33:55 +0800 Subject: [PATCH] fix(core): isolate invalid plugin tools during materialize (#35963) --- packages/core/src/tool/registry.ts | 27 ++++++++++++++---- .../test/session-runner-tool-registry.test.ts | 28 +++++++++++++++++++ 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/packages/core/src/tool/registry.ts b/packages/core/src/tool/registry.ts index ac003775f4ca..31e4552e5f97 100644 --- a/packages/core/src/tool/registry.ts +++ b/packages/core/src/tool/registry.ts @@ -265,7 +265,7 @@ const registryLayer = Layer.effect( registerBatch, materialize: Effect.fn("ToolRegistry.materialize")((permissions) => registrationLock.withPermit( - Effect.sync(() => { + Effect.gen(function* () { const direct = new Map() const codemode = new Map() const rules = permissions ?? [] @@ -278,11 +278,28 @@ 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 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: [ - ...Array.from(direct, ([name, registration]) => definition(name, registration.tool)), - ...(execute ? [definition("execute", execute)] : []), - ], + definitions, settle: (input: ExecuteInput) => { 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 19ba17d5e65d..09e6379ed205 100644 --- a/packages/core/test/session-runner-tool-registry.test.ts +++ b/packages/core/test/session-runner-tool-registry.test.ts @@ -505,4 +505,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" }) + }), + ) })