Merge pull request #21 from code-rabi/benr/feat-safe-meta-tools-registration

Benr/feat safe meta tools registration

Author: imbenrabi

package.json

@@ -1,6 +1,6 @@
 {
   "name": "toolception",
-  "version": "0.6.0",
+  "version": "0.6.1",
   "private": false,
   "type": "module",
   "main": "dist/index.js",

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)
 - Expecting disable to unregister tools from MCP (it can't)
 - Throwing on notification failures (they're expected in SSE disconnect)
+- Using `_meta` as a toolset key in the catalog (reserved for meta-tools, rejected at startup)
 
 ## Enable Toolset Flow
 

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

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<Mode, "ALL"> }
 ): 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)",

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<string, ModuleLoader>;
@@ -15,6 +21,14 @@ export class ModuleResolver {
   private readonly moduleLoaders: Record<string, ModuleLoader>;
 
   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,

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 = (

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,17 +66,23 @@ 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");
 
       expect(result).toHaveProperty("tools");
       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);
+    });
   });
 });

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);
+    });
+  });
 });