From 868e057238b116a888d103beae3877badd073bb7 Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Sun, 1 Feb 2026 11:11:59 +0200 Subject: [PATCH 1/3] feat: safe meta tools registration --- src/core/CLAUDE.md | 7 ++- src/core/ServerOrchestrator.ts | 2 +- src/meta/registerMetaTools.ts | 19 +++++++ tests/metaTools.test.ts | 99 ++++++++++++++++++++++++++++++---- 4 files changed, 115 insertions(+), 12 deletions(-) diff --git a/src/core/CLAUDE.md b/src/core/CLAUDE.md index ecfdb29..45714a7 100644 --- a/src/core/CLAUDE.md +++ b/src/core/CLAUDE.md @@ -38,18 +38,21 @@ Central orchestration layer that wires together all components. Manages toolset Located in `src/meta/registerMetaTools.ts` (called by ServerOrchestrator): +Meta-tools are registered with `ToolRegistry` under the reserved `_meta` toolset key (`META_TOOLSET_KEY` constant). This ensures collision detection with user-defined tools and makes meta-tools visible in `toolRegistry.list()` and `toolRegistry.listByToolset()`. + **DYNAMIC mode only:** - `enable_toolset` / `disable_toolset` - Runtime toolset management - `list_toolsets` / `describe_toolset` - Discovery **Both modes:** -- `list_tools` - List registered tool names +- `list_tools` - List registered tool names (includes meta-tools) ## Anti-patterns -- Bypassing ToolRegistry for tool registration (causes collision issues) +- Bypassing ToolRegistry for tool registration (causes collision issues with meta-tools and user tools) - Expecting disable to unregister tools from MCP (it can't) - Throwing on notification failures (they're expected in SSE disconnect) +- Defining user tools with meta-tool names (`enable_toolset`, `disable_toolset`, `list_toolsets`, `describe_toolset`, `list_tools`) - will cause E_TOOL_NAME_CONFLICT ## Enable Toolset Flow diff --git a/src/core/ServerOrchestrator.ts b/src/core/ServerOrchestrator.ts index 174691e..2e386e3 100644 --- a/src/core/ServerOrchestrator.ts +++ b/src/core/ServerOrchestrator.ts @@ -54,7 +54,7 @@ export class ServerOrchestrator { // Register meta-tools only if requested (default true) if (options.registerMetaTools !== false) { - registerMetaTools(options.server, this.manager, { mode: this.mode }); + registerMetaTools(options.server, this.manager, toolRegistry, { mode: this.mode }); } // Startup behavior - store promise for async initialization diff --git a/src/meta/registerMetaTools.ts b/src/meta/registerMetaTools.ts index d70e5f0..17fc5d0 100644 --- a/src/meta/registerMetaTools.ts +++ b/src/meta/registerMetaTools.ts @@ -2,6 +2,14 @@ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { Mode } from "../types/index.js"; import { z } from "zod"; import { DynamicToolManager } from "../core/DynamicToolManager.js"; +import { ToolRegistry } from "../core/ToolRegistry.js"; + +/** + * Reserved toolset key for meta-tools. + * Meta-tools are registered under this key to enable collision detection + * and tracking via the ToolRegistry. + */ +export const META_TOOLSET_KEY = "_meta"; /** * Registers meta-tools on the MCP server for toolset management. @@ -13,19 +21,26 @@ import { DynamicToolManager } from "../core/DynamicToolManager.js"; * * In STATIC mode, only list_tools is registered since toolsets are fixed at startup. * + * Meta-tools are registered with the ToolRegistry under the reserved "_meta" toolset key + * to enable collision detection with user-defined tools. + * * @param server - The MCP server to register tools on * @param manager - The DynamicToolManager instance + * @param toolRegistry - The ToolRegistry for collision detection * @param options - Configuration options including the mode */ export function registerMetaTools( server: McpServer, manager: DynamicToolManager, + toolRegistry: ToolRegistry, options?: { mode?: Exclude } ): void { const mode = options?.mode ?? "DYNAMIC"; // Dynamic-mode only tools: enable/disable toolsets at runtime if (mode === "DYNAMIC") { + // Register with ToolRegistry for collision detection before server.tool() + toolRegistry.addForToolset(META_TOOLSET_KEY, "enable_toolset"); server.tool( "enable_toolset", "Enable a toolset by name", @@ -39,6 +54,7 @@ export function registerMetaTools( } ); + toolRegistry.addForToolset(META_TOOLSET_KEY, "disable_toolset"); server.tool( "disable_toolset", "Disable a toolset by name (state only)", @@ -52,6 +68,7 @@ export function registerMetaTools( } ); + toolRegistry.addForToolset(META_TOOLSET_KEY, "list_toolsets"); server.tool( "list_toolsets", "List available toolsets with active status and definitions", @@ -84,6 +101,7 @@ export function registerMetaTools( } ); + toolRegistry.addForToolset(META_TOOLSET_KEY, "describe_toolset"); server.tool( "describe_toolset", "Describe a toolset with definition, active status and tools", @@ -121,6 +139,7 @@ export function registerMetaTools( } // list_tools is available in both modes + toolRegistry.addForToolset(META_TOOLSET_KEY, "list_tools"); server.tool( "list_tools", "List currently registered tool names (best effort)", diff --git a/tests/metaTools.test.ts b/tests/metaTools.test.ts index b8bc2cc..2ab2615 100644 --- a/tests/metaTools.test.ts +++ b/tests/metaTools.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { registerMetaTools } from "../src/meta/registerMetaTools.js"; +import { registerMetaTools, META_TOOLSET_KEY } from "../src/meta/registerMetaTools.js"; import { DynamicToolManager } from "../src/core/DynamicToolManager.js"; import { ModuleResolver } from "../src/mode/ModuleResolver.js"; import { ToolRegistry } from "../src/core/ToolRegistry.js"; @@ -49,8 +49,8 @@ describe("Meta-tools return formats", () => { resolver, toolRegistry, }); - registerMetaTools(server, manager, { mode: "DYNAMIC" }); - return { server, tools, manager }; + registerMetaTools(server, manager, toolRegistry, { mode: "DYNAMIC" }); + return { server, tools, manager, toolRegistry }; } function findTool(tools: RegisteredTool[], name: string): RegisteredTool | undefined { @@ -66,7 +66,7 @@ describe("Meta-tools return formats", () => { } describe("list_tools", () => { - it("returns { tools: [], toolsetToTools: {} } when no toolsets enabled", async () => { + it("returns meta-tools when no user toolsets enabled", async () => { const { tools } = createTestSetup(); const result = await callTool(tools, "list_tools"); @@ -74,9 +74,15 @@ describe("Meta-tools return formats", () => { expect(result).toHaveProperty("toolsetToTools"); expect(Array.isArray(result.tools)).toBe(true); expect(typeof result.toolsetToTools).toBe("object"); - // No toolsets enabled, so no user tools (only meta-tools not tracked) - expect(result.tools).toEqual([]); - expect(result.toolsetToTools).toEqual({}); + // Meta-tools are now tracked in the registry + expect(result.tools).toContain("enable_toolset"); + expect(result.tools).toContain("disable_toolset"); + expect(result.tools).toContain("list_toolsets"); + expect(result.tools).toContain("describe_toolset"); + expect(result.tools).toContain("list_tools"); + // Meta-tools appear under _meta key + expect(result.toolsetToTools[META_TOOLSET_KEY]).toBeDefined(); + expect(result.toolsetToTools[META_TOOLSET_KEY]).toContain("list_tools"); }); it("returns correct structure after enabling toolsets", async () => { @@ -236,9 +242,10 @@ describe("Meta-tools return formats", () => { it("only registers list_tools in STATIC mode", () => { const { server, tools } = createFakeMcpServer(); const resolver = new ModuleResolver({ catalog }); - const manager = new DynamicToolManager({ server, resolver }); + const toolRegistry = new ToolRegistry({ namespaceWithToolset: true }); + const manager = new DynamicToolManager({ server, resolver, toolRegistry }); - registerMetaTools(server, manager, { mode: "STATIC" }); + registerMetaTools(server, manager, toolRegistry, { mode: "STATIC" }); const toolNames = tools.map((t) => t.name); expect(toolNames).toContain("list_tools"); @@ -247,5 +254,79 @@ describe("Meta-tools return formats", () => { expect(toolNames).not.toContain("list_toolsets"); expect(toolNames).not.toContain("describe_toolset"); }); + + it("registers list_tools in ToolRegistry in STATIC mode", () => { + const { server } = createFakeMcpServer(); + const resolver = new ModuleResolver({ catalog }); + const toolRegistry = new ToolRegistry({ namespaceWithToolset: true }); + const manager = new DynamicToolManager({ server, resolver, toolRegistry }); + + registerMetaTools(server, manager, toolRegistry, { mode: "STATIC" }); + + expect(toolRegistry.has("list_tools")).toBe(true); + expect(toolRegistry.listByToolset()[META_TOOLSET_KEY]).toContain("list_tools"); + }); + }); + + describe("ToolRegistry integration", () => { + it("meta-tools appear in toolRegistry.list()", () => { + const { toolRegistry } = createTestSetup(); + + const registeredTools = toolRegistry.list(); + expect(registeredTools).toContain("enable_toolset"); + expect(registeredTools).toContain("disable_toolset"); + expect(registeredTools).toContain("list_toolsets"); + expect(registeredTools).toContain("describe_toolset"); + expect(registeredTools).toContain("list_tools"); + }); + + it("meta-tools appear in toolRegistry.listByToolset() under _meta key", () => { + const { toolRegistry } = createTestSetup(); + + const byToolset = toolRegistry.listByToolset(); + expect(byToolset[META_TOOLSET_KEY]).toBeDefined(); + expect(byToolset[META_TOOLSET_KEY]).toContain("enable_toolset"); + expect(byToolset[META_TOOLSET_KEY]).toContain("disable_toolset"); + expect(byToolset[META_TOOLSET_KEY]).toContain("list_toolsets"); + expect(byToolset[META_TOOLSET_KEY]).toContain("describe_toolset"); + expect(byToolset[META_TOOLSET_KEY]).toContain("list_tools"); + }); + + it("returns collision error when user tool collides with meta-tool", async () => { + const { server } = createFakeMcpServer(); + const toolRegistry = new ToolRegistry({ namespaceWithToolset: false }); // No namespacing to force collision + const resolver = new ModuleResolver({ + catalog: { + conflict: { + name: "Conflict", + description: "Toolset with conflicting tool name", + tools: [ + { + name: "enable_toolset", // Collides with meta-tool + description: "A user tool", + inputSchema: { type: "object", properties: {} }, + handler: async () => ({ content: [{ type: "text", text: "user" }] }), + }, + ], + }, + }, + }); + const manager = new DynamicToolManager({ server, resolver, toolRegistry }); + + // Register meta-tools first + registerMetaTools(server, manager, toolRegistry, { mode: "DYNAMIC" }); + + // Enabling a toolset with a conflicting tool name returns failure with collision message + const result = await manager.enableToolset("conflict"); + expect(result.success).toBe(false); + expect(result.message).toMatch(/collision/i); + }); + + it("cannot register tool named enable_toolset after meta-tools registered", () => { + const { toolRegistry } = createTestSetup(); + + // Attempting to add a tool with the same name should throw + expect(() => toolRegistry.add("enable_toolset")).toThrow(/collision/i); + }); }); }); From 3701f8c8dfa30028ca528df763f5ace859d862cb Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Sun, 1 Feb 2026 11:12:48 +0200 Subject: [PATCH 2/3] chore: bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index acd7e28..4053e97 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "toolception", - "version": "0.6.0", + "version": "0.6.1", "private": false, "type": "module", "main": "dist/index.js", From 4755f8330351070b38f770bfe74ab75ed672dd33 Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Sun, 1 Feb 2026 14:02:55 +0200 Subject: [PATCH 3/3] chore: cr comments --- src/core/CLAUDE.md | 4 ++-- src/mode/ModuleResolver.ts | 21 +++++++++++++++++++++ src/types/index.ts | 1 - tests/moduleResolver.test.ts | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/core/CLAUDE.md b/src/core/CLAUDE.md index 45714a7..d557c97 100644 --- a/src/core/CLAUDE.md +++ b/src/core/CLAUDE.md @@ -49,10 +49,10 @@ Meta-tools are registered with `ToolRegistry` under the reserved `_meta` toolset ## Anti-patterns -- Bypassing ToolRegistry for tool registration (causes collision issues with meta-tools and user tools) +- Bypassing ToolRegistry for tool registration (causes collision issues) - Expecting disable to unregister tools from MCP (it can't) - Throwing on notification failures (they're expected in SSE disconnect) -- Defining user tools with meta-tool names (`enable_toolset`, `disable_toolset`, `list_toolsets`, `describe_toolset`, `list_tools`) - will cause E_TOOL_NAME_CONFLICT +- Using `_meta` as a toolset key in the catalog (reserved for meta-tools, rejected at startup) ## Enable Toolset Flow diff --git a/src/mode/ModuleResolver.ts b/src/mode/ModuleResolver.ts index 7167491..b41c69e 100644 --- a/src/mode/ModuleResolver.ts +++ b/src/mode/ModuleResolver.ts @@ -5,6 +5,12 @@ import type { ModuleLoader, } from "../types/index.js"; +/** + * Reserved toolset keys that cannot be used in user catalogs. + * Must match META_TOOLSET_KEY in src/meta/registerMetaTools.ts + */ +const RESERVED_TOOLSET_KEYS = ["_meta"]; + export interface ModuleResolverOptions { catalog: ToolSetCatalog; moduleLoaders?: Record; @@ -15,6 +21,14 @@ export class ModuleResolver { private readonly moduleLoaders: Record; constructor(options: ModuleResolverOptions) { + // Validate catalog doesn't use reserved keys + for (const key of RESERVED_TOOLSET_KEYS) { + if (key in options.catalog) { + throw new Error( + `Toolset key '${key}' is reserved for internal use and cannot be used in the catalog` + ); + } + } this.catalog = options.catalog; this.moduleLoaders = options.moduleLoaders ?? {}; } @@ -49,6 +63,13 @@ export class ModuleResolver { )}`, }; } + // Check for reserved keys (defense in depth) + if (RESERVED_TOOLSET_KEYS.includes(sanitized)) { + return { + isValid: false, + error: `Toolset key '${sanitized}' is reserved for internal use`, + }; + } if (!this.catalog[sanitized]) { return { isValid: false, diff --git a/src/types/index.ts b/src/types/index.ts index 9322112..f2fb827 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -53,7 +53,6 @@ export type ToolingErrorCode = | "E_NOTIFY_FAILED" | "E_INTERNAL"; -// Module loader API: returns tools contributed by a module // Module loader API: returns tools contributed by a module. // Loaders may ignore the context argument if not needed. export type ModuleLoader = ( diff --git a/tests/moduleResolver.test.ts b/tests/moduleResolver.test.ts index 8f8affe..18d7774 100644 --- a/tests/moduleResolver.test.ts +++ b/tests/moduleResolver.test.ts @@ -158,4 +158,36 @@ describe("ModuleResolver", () => { expect(tools.map((t) => t.name)).toContain("sync_tool"); }); }); + + describe("reserved toolset keys", () => { + it("throws when catalog contains _meta key", () => { + expect(() => new ModuleResolver({ + catalog: { + _meta: { name: "Meta", description: "User-defined meta", tools: [] }, + } as any, + })).toThrow(/reserved for internal use/); + }); + + it("validateToolsetName rejects _meta as reserved", () => { + const r = new ModuleResolver({ + catalog: { core: { name: "Core", description: "", tools: [] } } as any, + }); + + const result = r.validateToolsetName("_meta"); + expect(result.isValid).toBe(false); + expect(result.error).toMatch(/reserved for internal use/); + }); + + it("allows normal toolset keys", () => { + const r = new ModuleResolver({ + catalog: { + core: { name: "Core", description: "", tools: [] }, + "my-toolset": { name: "My Toolset", description: "", tools: [] }, + } as any, + }); + + expect(r.validateToolsetName("core").isValid).toBe(true); + expect(r.validateToolsetName("my-toolset").isValid).toBe(true); + }); + }); });