Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions packages/core/src/tool/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, Registration>()
const codemode = new Map<string, Registration>()
const rules = permissions ?? []
Expand All @@ -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)
Expand Down
28 changes: 28 additions & 0 deletions packages/core/test/session-runner-tool-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof Tool.make>[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" })
}),
)
})
Loading