diff --git a/packages/core/src/plugin/host.ts b/packages/core/src/plugin/host.ts index d25555eb37..585c254a78 100644 --- a/packages/core/src/plugin/host.ts +++ b/packages/core/src/plugin/host.ts @@ -302,7 +302,7 @@ export const make = Effect.fn("PluginHost.make")(function* (plugin: PluginV2.Int }), }, tool: { - register: (input) => tools.register(input), + register: (input, options) => tools.register(input, options), execute: { before: (callback) => toolHooks.hook.before((event) => { diff --git a/packages/core/src/tool/AGENTS.md b/packages/core/src/tool/AGENTS.md index 3719322fa1..4b427f9835 100644 --- a/packages/core/src/tool/AGENTS.md +++ b/packages/core/src/tool/AGENTS.md @@ -28,7 +28,8 @@ Leaves own resolution, permission, and side-effect ordering. Translate only expe ## Registration -Built-ins and plugin tools register through `Tools.Service.register({ [name]: tool })`. +Built-ins and plugin tools register through `Tools.Service.register({ [name]: tool })`. Registrations may provide a +group, which flattens direct model names to `_`, and may be deferred from direct model exposure. Registrations are scoped: diff --git a/packages/core/src/tool/mcp.ts b/packages/core/src/tool/mcp.ts index 571b747bab..7867fc6571 100644 --- a/packages/core/src/tool/mcp.ts +++ b/packages/core/src/tool/mcp.ts @@ -1,6 +1,5 @@ export * as McpTool from "./mcp" -import { createHash } from "node:crypto" import { ToolFailure } from "@opencode-ai/llm" import { McpEvent } from "@opencode-ai/schema/mcp-event" import { Effect, Exit, type JsonSchema, Layer, Scope, Semaphore, Stream } from "effect" @@ -11,35 +10,11 @@ import { Tool } from "./tool" import { Tools } from "./tools" import { ToolRegistry } from "./registry" -const MAX_NAME_LENGTH = 64 -const HASH_LENGTH = 8 - -const sanitize = (value: string) => value.replace(/[^A-Za-z0-9_-]/g, "_") - -// Deterministic short suffix used to keep overlong or colliding names unique and stable across restarts. -const hashSuffix = (raw: string) => "_" + createHash("sha1").update(raw).digest("hex").slice(0, HASH_LENGTH) - -const fit = (base: string, raw: string) => base.slice(0, MAX_NAME_LENGTH - HASH_LENGTH - 1) + hashSuffix(raw) - /** - * Registry/permission action name for an MCP tool: V1-compatible `_` so existing deny - * rules keep working. Sanitized to a valid tool name, prefixed when it would not start with a letter, - * and hashed down when it would exceed the 64-char limit. + * Registry and permission action name for an MCP tool. */ -export const name = (server: string, tool: string) => { - const joined = sanitize(server) + "_" + sanitize(tool) - const base = /^[A-Za-z]/.test(joined) ? joined : "mcp_" + joined - return base.length > MAX_NAME_LENGTH ? fit(base, `${server}\u0000${tool}`) : base -} - -const toContent = (part: MCP.ToolResultContent): Tool.Content => - part.type === "text" ? { type: "text", text: part.text } : { type: "file", data: part.data, mime: part.mimeType } - -const errorText = (content: ReadonlyArray) => - content - .flatMap((part) => (part.type === "text" ? [part.text] : [])) - .join("\n") - .trim() +export const name = (server: string, tool: string) => + `${server.replace(/[^a-zA-Z0-9_-]/g, "_")}_${tool.replace(/[^a-zA-Z0-9_-]/g, "_")}` export const layer = Layer.effectDiscard( Effect.gen(function* () { @@ -50,47 +25,71 @@ export const layer = Layer.effectDiscard( const lock = Semaphore.makeUnsafe(1) let current: Scope.Closeable | undefined - const make = (server: MCP.ServerName, tool: MCP.Tool) => - Tool.make({ - description: tool.description ?? "", - jsonSchema: (tool.inputSchema as JsonSchema.JsonSchema | undefined) ?? { type: "object", properties: {} }, - execute: (input) => - Effect.gen(function* () { - const result = yield* mcp.callTool({ server, name: tool.name, args: (input ?? {}) as Record }).pipe( - Effect.catchTags({ - "MCP.NotFoundError": (error) => new ToolFailure({ message: `MCP server "${error.server}" is not available` }), - "MCP.ToolCallError": (error) => new ToolFailure({ message: error.message }), - }), - ) - if (result.isError) - return yield* new ToolFailure({ message: errorText(result.content) || "MCP tool returned an error" }) - return { structured: result.structured ?? {}, content: result.content.map(toContent) } - }), - }) - // Register the current tool set under a fresh child scope, then close the previous one so the // registry never has a gap where MCP tools disappear mid-swap. const reconcile = lock.withPermit( Effect.gen(function* () { - const used = new Set() - const record: Record = {} + const groups = new Map>() for (const tool of yield* mcp.tools()) { - const initial = name(tool.server, tool.name) - const key = used.has(initial) ? fit(initial, `${tool.server}\u0000${tool.name}`) : initial - used.add(key) - record[key] = make(tool.server, tool) + const group = groups.get(tool.server) ?? {} + const schema = (tool.inputSchema ?? {}) as JsonSchema.JsonSchema + group[tool.name] = Tool.make({ + description: tool.description ?? "", + jsonSchema: { + ...schema, + type: "object", + properties: schema.properties ?? {}, + additionalProperties: false, + }, + execute: (input) => + Effect.gen(function* () { + const result = yield* mcp + .callTool({ + server: tool.server, + name: tool.name, + args: (input ?? {}) as Record, + }) + .pipe( + Effect.catchTags({ + "MCP.NotFoundError": (error) => + new ToolFailure({ message: `MCP server "${error.server}" is not available` }), + "MCP.ToolCallError": (error) => new ToolFailure({ message: error.message }), + }), + ) + if (result.isError) + return yield* new ToolFailure({ + message: + result.content + .flatMap((part) => (part.type === "text" ? [part.text] : [])) + .join("\n") + .trim() || "MCP tool returned an error", + }) + return { + structured: result.structured ?? {}, + content: result.content.map((part) => + part.type === "text" + ? { type: "text" as const, text: part.text } + : { type: "file" as const, data: part.data, mime: part.mimeType }, + ), + } + }), + }) + groups.set(tool.server, group) } const next = yield* Scope.fork(scope) - yield* tools.register(record).pipe(Scope.provide(next), Effect.orDie) + yield* Effect.forEach(groups, ([group, record]) => tools.register(record, { group }), { + discard: true, + }).pipe(Scope.provide(next), Effect.orDie) if (current) yield* Scope.close(current, Exit.void) current = next }), ) yield* reconcile.pipe(Effect.forkScoped) - yield* events - .subscribe(McpEvent.ToolsChanged) - .pipe(Stream.runForEach(() => reconcile), Effect.forkScoped({ startImmediately: true })) + yield* events.subscribe(McpEvent.ToolsChanged).pipe( + Stream.runForEach(() => reconcile), + Effect.forkScoped({ startImmediately: true }), + ) }), ) diff --git a/packages/core/src/tool/registry.ts b/packages/core/src/tool/registry.ts index 1dc7478c11..dcdbb1c888 100644 --- a/packages/core/src/tool/registry.ts +++ b/packages/core/src/tool/registry.ts @@ -23,7 +23,10 @@ export type ExecuteInput = { export interface Interface { readonly materialize: (input: MaterializeInput) => Effect.Effect /** Internal registration capability exposed publicly only through Tools.Service. */ - readonly register: (tools: Readonly>) => Effect.Effect + readonly register: ( + tools: Readonly>, + options?: Tools.RegisterOptions, + ) => Effect.Effect } export interface MaterializeInput { @@ -49,7 +52,13 @@ const registryLayer = Layer.effect( Effect.gen(function* () { const resources = yield* ToolOutputStore.Service const toolHooks = yield* ToolHooks.Service - type Registration = { readonly identity: object; readonly tool: AnyTool } + type Registration = { + readonly identity: object + readonly tool: AnyTool + readonly name: string + readonly group?: string + readonly deferred: boolean + } const local = new Map>() const settleWith = Effect.fn("ToolRegistry.settle")(function* (input: ExecuteInput, advertised?: object) { @@ -73,12 +82,16 @@ const registryLayer = Layer.effect( input: input.call.input, } yield* toolHooks.runBefore(beforeEvent) - const pending = yield* settle(registration.tool, { ...input.call, input: beforeEvent.input }, { - sessionID: input.sessionID, - agent: input.agent, - assistantMessageID: input.assistantMessageID, - toolCallID: input.call.id, - }).pipe( + const pending = yield* settle( + registration.tool, + { ...input.call, input: beforeEvent.input }, + { + sessionID: input.sessionID, + agent: input.agent, + assistantMessageID: input.assistantMessageID, + toolCallID: input.call.id, + }, + ).pipe( Effect.map((output) => ({ output })), Effect.catchTag("LLM.ToolFailure", (failure) => Effect.succeed({ result: { type: "error" as const, value: failure.message } }), @@ -88,7 +101,11 @@ const registryLayer = Layer.effect( if ("result" in pending) { settlement = pending } else { - const bounded = yield* resources.bound({ sessionID: input.sessionID, toolCallID: input.call.id, output: pending.output }) + const bounded = yield* resources.bound({ + sessionID: input.sessionID, + toolCallID: input.call.id, + output: pending.output, + }) const result = ToolOutput.toResultValue(bounded.output) settlement = result.type === "error" @@ -119,20 +136,33 @@ const registryLayer = Layer.effect( }) return Service.of({ - register: Effect.fn("ToolRegistry.register")(function* (tools) { - const entries = registrationEntries(tools) + register: Effect.fn("ToolRegistry.register")(function* (tools, options) { + const entries = registrationEntries(tools, options?.group) if (entries.length === 0) return yield* Effect.uninterruptible( Effect.gen(function* () { const token = {} - for (const [name, tool] of entries) - local.set(name, [...(local.get(name) ?? []), { token, registration: { identity: {}, tool } }]) + for (const entry of entries) + local.set(entry.key, [ + ...(local.get(entry.key) ?? []), + { + token, + registration: { + identity: {}, + tool: entry.tool, + name: entry.name, + group: entry.group, + deferred: options?.deferred ?? false, + }, + }, + ]) yield* Effect.addFinalizer(() => Effect.sync(() => { - for (const [name] of entries) { - const registrations = local.get(name)?.filter((registration) => registration.token !== token) ?? [] - if (registrations.length > 0) local.set(name, registrations) - else local.delete(name) + 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) } }), ) @@ -149,7 +179,11 @@ const registryLayer = Layer.effect( const usePatch = input.model.provider.toLowerCase() === "openai" || input.model.id.toLowerCase().includes("gpt") for (const [name, registration] of registrations) { const wrongEditTool = name === "apply_patch" ? !usePatch : (name === "edit" || name === "write") && usePatch - if (wrongEditTool || whollyDisabled(permission(registration.tool, name), input.permissions ?? [])) + if ( + registration.deferred || + wrongEditTool || + whollyDisabled(permission(registration.tool, name), input.permissions ?? []) + ) registrations.delete(name) } return { diff --git a/packages/core/src/tool/tools.ts b/packages/core/src/tool/tools.ts index 0939ce957b..06b920a484 100644 --- a/packages/core/src/tool/tools.ts +++ b/packages/core/src/tool/tools.ts @@ -3,9 +3,12 @@ export * as Tools from "./tools" import { Context, Effect, Scope } from "effect" import { Tool } from "./tool" +export type RegisterOptions = Tool.RegisterOptions + export interface Interface { readonly register: ( tools: Readonly>, + options?: Tool.RegisterOptions, ) => Effect.Effect } diff --git a/packages/core/test/mcp.test.ts b/packages/core/test/mcp.test.ts index 15ff95a28f..7c62032784 100644 --- a/packages/core/test/mcp.test.ts +++ b/packages/core/test/mcp.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test" import { MCP } from "@opencode-ai/core/mcp/index" import { MCPClient } from "@opencode-ai/core/mcp/client" +import { McpTool } from "@opencode-ai/core/tool/mcp" describe("MCP errors", () => { test("expose useful messages", () => { @@ -12,3 +13,7 @@ describe("MCP errors", () => { expect(new MCPClient.ConnectError({ server: "demo", message: "offline" }).message).toBe("offline") }) }) + +test("MCP tool names match V1 sanitization", () => { + expect(McpTool.name("context 7", "resolve.library/id")).toBe("context_7_resolve_library_id") +}) diff --git a/packages/core/test/plugin.test.ts b/packages/core/test/plugin.test.ts index 42f2a768ec..1a60decb0b 100644 --- a/packages/core/test/plugin.test.ts +++ b/packages/core/test/plugin.test.ts @@ -126,6 +126,38 @@ describe("PluginV2", () => { }), ) + it.effect("groups tool names and defers registrations from direct exposure", () => + Effect.gen(function* () { + const plugins = yield* PluginV2.Service + const registry = yield* ToolRegistry.Service + const tool = (description: string) => + Tool.make({ + description, + input: Schema.Struct({}), + output: Schema.Struct({ ok: Schema.Boolean }), + execute: () => Effect.succeed({ ok: true }), + }) + const plugin = define({ + id: "grouped-tools", + effect: (ctx) => + Effect.gen(function* () { + yield* ctx.tool.register({ plain: tool("Plain") }).pipe(Effect.orDie) + yield* ctx.tool.register({ "look/up": tool("Lookup") }, { group: "context 7" }).pipe(Effect.orDie) + yield* ctx.tool + .register({ search: tool("Search") }, { group: "context 7", deferred: true }) + .pipe(Effect.orDie) + }), + }) + + yield* plugins.add(PluginV2.ID.make(plugin.id), plugin.effect) + + expect((yield* registry.materialize({ model: testModel })).definitions.map((tool) => tool.name)).toEqual([ + "plain", + "context_7_look_up", + ]) + }), + ) + it.effect("fires before/after tool hooks with mutable events around settlement", () => Effect.gen(function* () { const plugins = yield* PluginV2.Service diff --git a/packages/plugin/src/v2/effect/tool.ts b/packages/plugin/src/v2/effect/tool.ts index c25f79c58b..98efaa3ce2 100644 --- a/packages/plugin/src/v2/effect/tool.ts +++ b/packages/plugin/src/v2/effect/tool.ts @@ -186,8 +186,17 @@ export const validateName = (name: string) => ? Effect.void : Effect.fail(new RegistrationError({ name, message: `Invalid tool name: ${name}` })) -export const registrationEntries = (tools: Readonly>) => - Object.entries(tools).map(([name, tool]) => [name.replace(/[^a-zA-Z0-9_-]/g, "_"), tool] as const) +export const registrationEntries = (tools: Readonly>, group?: string) => + Object.entries(tools).map(([name, tool]) => { + const normalized = name.replace(/[^a-zA-Z0-9_-]/g, "_") + const parent = group?.replace(/[^a-zA-Z0-9_-]/g, "_") + return { + key: parent === undefined ? normalized : `${parent}_${normalized}`, + name: normalized, + group: parent, + tool, + } + }) export const withPermission = , Output extends SchemaType>( tool: Definition, @@ -235,7 +244,15 @@ export interface ToolExecuteAfterEvent { outputPaths?: ReadonlyArray } +export interface RegisterOptions { + readonly group?: string + readonly deferred?: boolean +} + export interface ToolDomain { - readonly register: (tools: Readonly>) => Effect.Effect + readonly register: ( + tools: Readonly>, + options?: RegisterOptions, + ) => Effect.Effect readonly execute: Hooks<{ before: ToolExecuteBeforeEvent; after: ToolExecuteAfterEvent }> }