diff --git a/packages/core/src/mcp/instructions.ts b/packages/core/src/mcp/instructions.ts index e5865de69f3b..6c0be1dc737f 100644 --- a/packages/core/src/mcp/instructions.ts +++ b/packages/core/src/mcp/instructions.ts @@ -17,7 +17,7 @@ type Summary = typeof Summary.Type const entries = (servers: ReadonlyArray) => servers.flatMap((server) => [ ` `, - ` Use tools from this server through \`execute\` under \`tools[${JSON.stringify(McpTool.group(server.server))}]\`.`, + ` Use tools from this server through \`execute\` under \`tools[${JSON.stringify(McpTool.namespace(server.server))}]\`.`, ...server.instructions.split("\n").map((line) => ` ${line}`), " ", ]) diff --git a/packages/core/src/tool/AGENTS.md b/packages/core/src/tool/AGENTS.md index 8e74bd5bb3af..beb838a567e6 100644 --- a/packages/core/src/tool/AGENTS.md +++ b/packages/core/src/tool/AGENTS.md @@ -29,7 +29,7 @@ Leaves own resolution, permission, and side-effect ordering. Translate only expe ## Registration Built-ins and plugin tools register through `Tools.Service.register({ [name]: tool })`. Registrations may provide a -group, which flattens direct model names to `_`, and default into CodeMode (`codemode` defaults true; +namespace, which flattens direct model names to `_`, and default into CodeMode (`codemode` defaults true; `codemode: false` keeps the tool on the provider's native tool list). Registrations are scoped: diff --git a/packages/core/src/tool/execute.ts b/packages/core/src/tool/execute.ts index 0a6222377090..e47adbba4caf 100644 --- a/packages/core/src/tool/execute.ts +++ b/packages/core/src/tool/execute.ts @@ -39,7 +39,7 @@ type CollectedFiles = { interface Registration { readonly tool: AnyTool readonly name: string - readonly group?: string + readonly namespace?: string } export const create = (registrations: ReadonlyMap) => { @@ -47,7 +47,7 @@ export const create = (registrations: ReadonlyMap) => { invoke: (name: string, registration: Registration, input: unknown) => Effect.Effect, hooks?: CodeMode.ToolCallHooks, ) => { - const tools: Record | Record>> = {} + const tools: Record> = {} for (const [name, registration] of registrations) { const child = definition(name, registration.tool) const value = Tool.make({ @@ -56,24 +56,8 @@ export const create = (registrations: ReadonlyMap) => { output: child.outputSchema, run: (input) => invoke(name, registration, input), }) - if (registration.group === undefined) { - const path = registration.name - if (Object.hasOwn(tools, path)) throw new TypeError(`CodeMode tool namespace conflict: ${path}`) - tools[path] = value - continue - } - const path = registration.name - const namespace = registration.group - const group = tools[namespace] - if (group && Tool.isDefinition(group)) throw new TypeError(`CodeMode tool namespace conflict: ${namespace}`) - if (group) { - if (Object.hasOwn(group, path)) throw new TypeError(`CodeMode tool namespace conflict: ${namespace}.${path}`) - group[path] = value - continue - } - const entries: Record> = {} - entries[path] = value - tools[namespace] = entries + const path = registration.namespace === undefined ? registration.name : `${registration.namespace}.${registration.name}` + tools[path] = value } return CodeMode.make({ tools, ...hooks }) } diff --git a/packages/core/src/tool/mcp.ts b/packages/core/src/tool/mcp.ts index a636671f6077..3263ee41f9cf 100644 --- a/packages/core/src/tool/mcp.ts +++ b/packages/core/src/tool/mcp.ts @@ -13,10 +13,11 @@ import { Tools } from "./tools" import { ToolRegistry } from "./registry" /** - * Registry group and permission action names for MCP tools. + * Registry namespace and permission action names for MCP tools. */ -export const group = (server: string) => server.replace(/[^a-zA-Z0-9_-]/g, "_") -export const name = (server: string, tool: string) => `${group(server)}_${tool.replace(/[^a-zA-Z0-9_-]/g, "_")}` +export const namespace = (server: string) => server.replace(/[^a-zA-Z0-9_-]/g, "_") +export const name = (server: string, tool: string) => + `${namespace(server)}_${tool.replace(/[^a-zA-Z0-9_-]/g, "_")}` export const layer = Layer.effectDiscard( Effect.gen(function* () { @@ -107,7 +108,7 @@ export const layer = Layer.effectDiscard( const next = yield* Scope.fork(scope) yield* Effect.forEach( groups, - ([group, record]) => tools.register(record, { group }), + ([server, record]) => tools.register(record, { namespace: namespace(server) }), { discard: true, }, diff --git a/packages/core/src/tool/registry.ts b/packages/core/src/tool/registry.ts index 8d95a95eafd5..844e4896db39 100644 --- a/packages/core/src/tool/registry.ts +++ b/packages/core/src/tool/registry.ts @@ -10,7 +10,15 @@ import { SessionSchema } from "../session/schema" import { ToolOutputStore } from "../tool-output-store" import { Wildcard } from "../util/wildcard" import { ExecuteTool } from "./execute" -import { definition, permission, registrationEntries, RegistrationError, settle, type AnyTool } from "./tool" +import { + definition, + permission, + registrationEntries, + RegistrationError, + settle, + validateNamespace, + type AnyTool, +} from "./tool" import { Tools } from "./tools" import { ToolHooks } from "./hooks" import { makeLocationNode } from "../effect/app-node" @@ -95,7 +103,7 @@ const registryLayer = Layer.effect( type Registration = { readonly tool: AnyTool readonly name: string - readonly group?: string + readonly namespace?: string readonly codemode: boolean } const local = new Map>() @@ -186,7 +194,8 @@ const registryLayer = Layer.effect( return Service.of({ register: Effect.fn("ToolRegistry.register")(function* (tools, options) { - const entries = registrationEntries(tools, options?.group) + 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") @@ -205,7 +214,7 @@ const registryLayer = Layer.effect( registration: { tool: entry.tool, name: entry.name, - group: entry.group, + namespace: entry.namespace, codemode, }, }, diff --git a/packages/core/test/mcp.test.ts b/packages/core/test/mcp.test.ts index fd6f99f53848..015bd0e541ff 100644 --- a/packages/core/test/mcp.test.ts +++ b/packages/core/test/mcp.test.ts @@ -268,6 +268,7 @@ describe("MCP errors", () => { }) test("MCP tool names match V1 sanitization", () => { + expect(McpTool.namespace("context 7")).toBe("context_7") 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 9ff717d66576..0d19e4e74b96 100644 --- a/packages/core/test/plugin.test.ts +++ b/packages/core/test/plugin.test.ts @@ -274,7 +274,7 @@ describe("PluginV2", () => { }), ) - it.effect("groups tool names and routes codemode registrations through execute", () => + it.effect("namespaces tool names and routes codemode registrations through execute", () => Effect.gen(function* () { const plugins = yield* PluginV2.Service const registry = yield* ToolRegistry.Service @@ -291,8 +291,8 @@ describe("PluginV2", () => { ctx.tool .transform((draft) => { draft.add("plain", tool("Plain"), { codemode: false }) - draft.add("look/up", tool("Lookup"), { group: "context 7", codemode: false }) - draft.add("search", tool("Search"), { group: "context 7" }) + draft.add("look/up", tool("Lookup"), { namespace: "context7", codemode: false }) + draft.add("search", tool("Search"), { namespace: "context7" }) }) .pipe(Effect.orDie), }) @@ -301,7 +301,7 @@ describe("PluginV2", () => { expect((yield* registry.materialize()).definitions.map((tool) => tool.name)).toEqual([ "plain", - "context_7_look_up", + "context7_look_up", "execute", ]) }), diff --git a/packages/core/test/session-runner-tool-registry.test.ts b/packages/core/test/session-runner-tool-registry.test.ts index 6cb012312b93..6f7b13f6e14a 100644 --- a/packages/core/test/session-runner-tool-registry.test.ts +++ b/packages/core/test/session-runner-tool-registry.test.ts @@ -84,6 +84,17 @@ const constant = (text: string) => }) describe("ToolRegistry", () => { + it.effect("rejects invalid dotted namespaces", () => + Effect.gen(function* () { + const service = yield* ToolRegistry.Service + const error = yield* service.register({ echo: make() }, { namespace: "slack..admin" }).pipe(Effect.flip) + + expect(error).toBeInstanceOf(Tool.RegistrationError) + expect(error.message).toBe('Invalid tool namespace: "slack..admin"') + expect((yield* service.materialize()).definitions).toEqual([]) + }), + ) + it.effect("filters disabled tools with edit aliases and ordered wildcard precedence", () => Effect.gen(function* () { const service = yield* ToolRegistry.Service diff --git a/packages/core/test/tool-execute.test.ts b/packages/core/test/tool-execute.test.ts index 3eedc6fbab1a..66254ec408db 100644 --- a/packages/core/test/tool-execute.test.ts +++ b/packages/core/test/tool-execute.test.ts @@ -46,3 +46,50 @@ test("execute preserves successful results with visible unhandled rejections", a }, ]) }) + +test("execute supports callable namespace tools", async () => { + const callable = Tool.make({ + description: "Administer Slack", + input: Schema.Struct({}), + output: Schema.String, + execute: () => Effect.succeed("admin"), + }) + const child = Tool.make({ + description: "Create a Slack resource", + input: Schema.Struct({}), + output: Schema.String, + execute: () => Effect.succeed("created"), + }) + const execute = ExecuteTool.create( + new Map([ + ["slack_admin", { tool: callable, name: "admin", namespace: "slack" }], + ["slack_admin_create", { tool: child, name: "create", namespace: "slack.admin" }], + ]), + ) + const result = await Effect.runPromise( + Tool.settle( + execute, + { + type: "tool-call", + id: "call_execute", + name: "execute", + input: { code: "return [await tools.slack.admin({}), await tools.slack.admin.create({})]" }, + }, + { + sessionID: Session.ID.make("ses_execute"), + agent: Agent.ID.make("build"), + messageID: SessionMessage.ID.make("msg_execute"), + callID: "call_execute", + progress: () => Effect.void, + }, + ), + ) + + expect(result.structured).toEqual({ + toolCalls: [ + { tool: "slack.admin", status: "completed" }, + { tool: "slack.admin.create", status: "completed" }, + ], + }) + expect(result.content).toEqual([{ type: "text", text: '[\n "admin",\n "created"\n]' }]) +}) diff --git a/packages/plugin/src/v2/effect/tool.ts b/packages/plugin/src/v2/effect/tool.ts index 59ef488433b3..879e5badef2e 100644 --- a/packages/plugin/src/v2/effect/tool.ts +++ b/packages/plugin/src/v2/effect/tool.ts @@ -150,18 +150,24 @@ export const validateName = (name: string) => ? Effect.void : Effect.fail(new RegistrationError({ name, message: `Invalid tool name: ${name}` })) -export const registrationEntries = (tools: Readonly>, group?: string) => +export const registrationEntries = (tools: Readonly>, namespace?: 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}`, + key: namespace === undefined ? normalized : `${namespace.replaceAll(".", "_")}_${normalized}`, name: normalized, - group: parent, + namespace, tool, } }) +export const validateNamespace = (namespace: string) => + namespace.split(".").every((segment) => /^[A-Za-z][A-Za-z0-9_-]{0,63}$/.test(segment)) + ? Effect.void + : Effect.fail( + new RegistrationError({ name: namespace, message: `Invalid tool namespace: ${JSON.stringify(namespace)}` }), + ) + export const withPermission = ( tool: T, permission: string, @@ -288,7 +294,7 @@ export interface ToolExecuteAfterEvent { } export interface RegisterOptions { - readonly group?: string + readonly namespace?: string /** Defaults to true. False exposes the tool directly to the provider. */ readonly codemode?: boolean }