diff --git a/README.md b/README.md index f795f05..22868f1 100644 --- a/README.md +++ b/README.md @@ -98,14 +98,16 @@ const catalog = { ### Step 4: Define a tool +`inputSchema` should be a Zod raw shape (or a Zod object schema), the same form `McpServer.registerTool` takes. A plain JSON Schema object still works, but clients see an open object schema and arguments reach the handler unvalidated. + ```ts +import { z } from "zod"; + const quoteTool = { name: "price", description: "Return a fake price", inputSchema: { - type: "object", - properties: { symbol: { type: "string" } }, - required: ["symbol"], + symbol: z.string(), }, handler: async ({ symbol }: { symbol: string }) => ({ content: [{ type: "text", text: `${symbol}: 123.45` }], @@ -252,11 +254,7 @@ const adminTool = { name: "delete_user", description: "Delete a user account", inputSchema: { - type: "object", - properties: { - userId: { type: "string", description: "User ID to delete" }, - }, - required: ["userId"], + userId: z.string().describe("User ID to delete"), }, handler: async ({ userId }: { userId: string }) => ({ content: [{ type: "text", text: `User ${userId} deleted` }], @@ -267,11 +265,7 @@ const userTool = { name: "get_profile", description: "Get user profile information", inputSchema: { - type: "object", - properties: { - userId: { type: "string", description: "User ID" }, - }, - required: ["userId"], + userId: z.string().describe("User ID"), }, handler: async ({ userId }: { userId: string }) => ({ content: [{ type: "text", text: `Profile for ${userId}: {...}` }], @@ -847,9 +841,7 @@ const moduleLoaders = { name: "echo", description: "Echo back provided text", inputSchema: { - type: "object", - properties: { text: { type: "string" } }, - required: ["text"], + text: z.string(), }, handler: async ({ text }: { text: string }) => ({ content: [{ type: "text", text }], @@ -935,9 +927,7 @@ const moduleLoaders = { name: "echo", description: "Echo using a backing service", inputSchema: { - type: "object", - properties: { text: { type: "string" } }, - required: ["text"], + text: z.string(), }, handler: async ({ text }: { text: string }) => { const result = await ctx.apiClients.echoService.send(text); diff --git a/examples/custom-endpoints-demo.ts b/examples/custom-endpoints-demo.ts index b563b27..d582db1 100644 --- a/examples/custom-endpoints-demo.ts +++ b/examples/custom-endpoints-demo.ts @@ -63,11 +63,7 @@ async function runStandardServerExample() { name: "greet", description: "Greet a user", inputSchema: { - type: "object", - properties: { - name: { type: "string" }, - }, - required: ["name"], + name: z.string(), }, handler: async (args: any) => ({ content: [ @@ -426,7 +422,7 @@ async function runPermissionBasedServerExample() { { name: "admin-action", description: "Perform admin action", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [{ type: "text", text: "Admin action completed" }], }), @@ -440,7 +436,7 @@ async function runPermissionBasedServerExample() { { name: "user-action", description: "Perform user action", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [{ type: "text", text: "User action completed" }], }), diff --git a/package-lock.json b/package-lock.json index 7c905c5..250a3d9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,7 @@ "@fastify/cors": "^10.0.1", "@modelcontextprotocol/sdk": "^1.32.1", "fastify": "^5.12.5", - "zod": "^3.22.4" + "zod": "^3.25.76" }, "devDependencies": { "@types/connect": "^3.4.38", diff --git a/package.json b/package.json index a20cbd4..3fb8a4f 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "@fastify/cors": "^10.0.1", "@modelcontextprotocol/sdk": "^1.32.1", "fastify": "^5.12.5", - "zod": "^3.22.4" + "zod": "^3.25.76" }, "devDependencies": { "@types/connect": "^3.4.38", diff --git a/src/core/AGENTS.md b/src/core/AGENTS.md index bbb3bf3..f2d7d95 100644 --- a/src/core/AGENTS.md +++ b/src/core/AGENTS.md @@ -18,7 +18,7 @@ Central orchestration layer. Manages toolset lifecycle, tool registration, and s 4. **Namespacing applied BEFORE collision check** — `mapAndValidate()` generates safe names first, then checks for conflicts. 5. **Toolset added to activeToolsets AFTER registration** — All tools must register successfully before toolset is marked active. Partial failures leave the toolset inactive but orphaned tools remain registered (MCP limitation). 6. **`enableToolsets()` batches notifications** — Calls `enableToolset(name, skipNotification=true)` per toolset, sends one `tools/list_changed` notification at the end. -7. **JSON Schema `inputSchema` uses `registerTool` with a passthrough object** — SDK >= 1.31 rejects plain JSON Schema in `server.tool()`. Zod raw shapes still go through `server.tool()`. Clients see an open object schema for JSON Schema tools, not the declared one. +7. **JSON Schema `inputSchema` uses `registerTool` with a passthrough object** — The SDK rejects plain JSON Schema as a tool schema. Zod raw shapes and schemas are passed to `registerTool` as is. Clients see an open object schema for JSON Schema tools, not the declared one. ## Enable Toolset Flow @@ -31,6 +31,10 @@ enableToolset(name) → notifyToolsChanged() — unless skipNotification ``` +## SDK registration + +Tools are registered with `McpServer.registerTool` (the 5-arg `server.tool()` overloads are deprecated). Before 1.28 the SDK silently treated a JSON Schema `inputSchema` as annotations, so handlers received the request `extra` instead of arguments; newer SDKs throw. `isZodInputSchema()` routes Zod shapes straight through and everything else to the passthrough object. + ## Anti-patterns - Bypassing ToolRegistry for tool registration (causes collision issues) diff --git a/src/core/DynamicToolManager.ts b/src/core/DynamicToolManager.ts index 486ea1e..c1ed0d5 100644 --- a/src/core/DynamicToolManager.ts +++ b/src/core/DynamicToolManager.ts @@ -1,5 +1,6 @@ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; +import type { ZodRawShapeCompat } from "@modelcontextprotocol/sdk/server/zod-compat.js"; import type { ExposurePolicy, McpToolDefinition, @@ -250,42 +251,25 @@ export class DynamicToolManager { const hasAnnotations = tool.annotations && Object.keys(tool.annotations).length > 0; - if (!isZodRawShape(tool.inputSchema)) { - // SDK >= 1.31 rejects plain JSON Schema objects in server.tool(); older - // versions silently treated them as annotations and dropped the args. - // Register with a passthrough object so arguments reach the handler. - this.server.registerTool( - tool.name, - { - description: tool.description, - inputSchema: z.object({}).passthrough(), - ...(hasAnnotations ? { annotations: tool.annotations } : {}), - }, - async (args: Record) => { - return await tool.handler(args); - } - ); - } else if (hasAnnotations && tool.annotations) { - this.server.tool( - tool.name, - tool.description, - tool.inputSchema as Parameters[2], - tool.annotations, - async (args: Record) => { - return await tool.handler(args); - } - ); - } else { - // Legacy 4-parameter call when no annotations - this.server.tool( - tool.name, - tool.description, - tool.inputSchema as Parameters[2], - async (args: Record) => { - return await tool.handler(args); - } - ); - } + // SDK >= 1.28 rejects plain JSON Schema inputSchema; older versions silently + // treated it as annotations and dropped the args. Non-Zod schemas register + // with a passthrough object so arguments still reach the handler. + const inputSchema = isZodInputSchema(tool.inputSchema) + ? (tool.inputSchema as ZodRawShapeCompat) + : z.object({}).passthrough(); + + this.server.registerTool( + tool.name, + { + title: tool.annotations?.title, + description: tool.description, + inputSchema, + annotations: hasAnnotations ? tool.annotations : undefined, + }, + async (args: Record) => { + return await tool.handler(args); + } + ); this.toolRegistry.addForToolset(toolsetKey, tool.name); } @@ -410,20 +394,15 @@ export class DynamicToolManager { } /** - * Mirrors the SDK's raw-shape check: an empty object, or an object with at - * least one Zod schema value (e.g. `{ symbol: z.string() }`). + * True when the schema is a Zod schema instance (v3 or v4) or a raw shape whose + * values are all Zod schemas. An empty object counts as an empty raw shape. + * + * @param schema - The tool's inputSchema */ -function isZodRawShape(schema: unknown): boolean { +function isZodInputSchema(schema: unknown): boolean { + const isZodType = (v: unknown) => + typeof v === "object" && v !== null && ("_def" in v || "_zod" in v); if (typeof schema !== "object" || schema === null) return false; - const values = Object.values(schema); - return values.length === 0 || values.some(isZodTypeLike); -} - -function isZodTypeLike(value: unknown): boolean { - return ( - typeof value === "object" && - value !== null && - typeof (value as { parse?: unknown }).parse === "function" && - typeof (value as { safeParse?: unknown }).safeParse === "function" - ); + if (isZodType(schema)) return true; + return Object.values(schema).every(isZodType); } diff --git a/src/meta/registerMetaTools.ts b/src/meta/registerMetaTools.ts index 17fc5d0..60db1c1 100644 --- a/src/meta/registerMetaTools.ts +++ b/src/meta/registerMetaTools.ts @@ -39,13 +39,15 @@ export function registerMetaTools( // Dynamic-mode only tools: enable/disable toolsets at runtime if (mode === "DYNAMIC") { - // Register with ToolRegistry for collision detection before server.tool() + // Register with ToolRegistry for collision detection before server.registerTool() toolRegistry.addForToolset(META_TOOLSET_KEY, "enable_toolset"); - server.tool( + server.registerTool( "enable_toolset", - "Enable a toolset by name", - { name: z.string().describe("Toolset name") }, - { destructiveHint: true, idempotentHint: true }, + { + description: "Enable a toolset by name", + inputSchema: { name: z.string().describe("Toolset name") }, + annotations: { destructiveHint: true, idempotentHint: true }, + }, async (args: { name: string }) => { const result = await manager.enableToolset(args.name); return { @@ -55,11 +57,13 @@ export function registerMetaTools( ); toolRegistry.addForToolset(META_TOOLSET_KEY, "disable_toolset"); - server.tool( + server.registerTool( "disable_toolset", - "Disable a toolset by name (state only)", - { name: z.string().describe("Toolset name") }, - { destructiveHint: true, idempotentHint: true }, + { + description: "Disable a toolset by name (state only)", + inputSchema: { name: z.string().describe("Toolset name") }, + annotations: { destructiveHint: true, idempotentHint: true }, + }, async (args: { name: string }) => { const result = await manager.disableToolset(args.name); return { @@ -69,11 +73,13 @@ export function registerMetaTools( ); toolRegistry.addForToolset(META_TOOLSET_KEY, "list_toolsets"); - server.tool( + server.registerTool( "list_toolsets", - "List available toolsets with active status and definitions", - {}, - { readOnlyHint: true, idempotentHint: true }, + { + description: "List available toolsets with active status and definitions", + inputSchema: {}, + annotations: { readOnlyHint: true, idempotentHint: true }, + }, async () => { const available = manager.getAvailableToolsets(); const byToolset = manager.getStatus().toolsetToTools; @@ -102,11 +108,13 @@ export function registerMetaTools( ); toolRegistry.addForToolset(META_TOOLSET_KEY, "describe_toolset"); - server.tool( + server.registerTool( "describe_toolset", - "Describe a toolset with definition, active status and tools", - { name: z.string().describe("Toolset name") }, - { readOnlyHint: true, idempotentHint: true }, + { + description: "Describe a toolset with definition, active status and tools", + inputSchema: { name: z.string().describe("Toolset name") }, + annotations: { readOnlyHint: true, idempotentHint: true }, + }, async (args: { name: string }) => { const def = manager.getToolsetDefinition(args.name); const byToolset = manager.getStatus().toolsetToTools; @@ -140,11 +148,13 @@ export function registerMetaTools( // list_tools is available in both modes toolRegistry.addForToolset(META_TOOLSET_KEY, "list_tools"); - server.tool( + server.registerTool( "list_tools", - "List currently registered tool names (best effort)", - {}, - { readOnlyHint: true, idempotentHint: true }, + { + description: "List currently registered tool names (best effort)", + inputSchema: {}, + annotations: { readOnlyHint: true, idempotentHint: true }, + }, async () => { const status = manager.getStatus(); const payload = { diff --git a/src/types/AGENTS.md b/src/types/AGENTS.md index 6e05f84..2d182b6 100644 --- a/src/types/AGENTS.md +++ b/src/types/AGENTS.md @@ -23,6 +23,7 @@ Also: `src/errors/ToolingError.ts` (18 LOC) — thin error class wrapping Toolin 2. **ToolingErrorCode values are exhaustive** — All error codes defined here; do not add codes elsewhere 3. **PermissionConfig requires source field** — Must specify `"headers"` or `"config"` 4. **SessionContextConfig.allowedKeys is security-critical** — Always whitelist allowed keys; omitting allows arbitrary config injection +5. **`McpToolDefinition.inputSchema` should be Zod** — A Zod raw shape or Zod object schema. `McpServer.registerTool` does not accept JSON Schema, so `DynamicToolManager` registers JSON Schema tools with an open passthrough object (args unvalidated, properties not advertised) ## Anti-patterns diff --git a/src/types/index.ts b/src/types/index.ts index 4aa44be..34cb2f6 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -3,12 +3,17 @@ export type McpToolDefinition = { name: string; description: string; + /** + * Zod raw shape (e.g. `{ symbol: z.string() }`) or Zod object schema, as accepted + * by `McpServer.registerTool`. A plain JSON Schema object is registered as an + * open passthrough object: clients don't see its properties and args are not validated. + */ inputSchema: Record; handler: (args: any) => Promise | any; /** * Optional annotations providing hints about tool behavior. * All annotations are hints and not guaranteed. - * - title: Optional display title for the tool + * - title: Optional display title for the tool (also sent as the tool's top-level `title`) * - readOnlyHint: Tool does not modify its environment * - destructiveHint: Tool may perform destructive updates * - idempotentHint: Repeated calls with same arguments have no additional effect diff --git a/tests/createMcpServer.test.ts b/tests/createMcpServer.test.ts index 0e8957b..2596165 100644 --- a/tests/createMcpServer.test.ts +++ b/tests/createMcpServer.test.ts @@ -50,9 +50,6 @@ const catalog = { function makeFakeServer() { const calls: string[] = []; const server = { - tool: (name: string) => { - calls.push(name); - }, registerTool: (name: string) => { calls.push(name); }, @@ -213,7 +210,7 @@ describe("createMcpServer", () => { { name: "test_tool", description: "A test tool", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [{ type: "text", text: "test" }], }), diff --git a/tests/createPermissionBasedMcpServer.test.ts b/tests/createPermissionBasedMcpServer.test.ts index ee85e9d..758b3ac 100644 --- a/tests/createPermissionBasedMcpServer.test.ts +++ b/tests/createPermissionBasedMcpServer.test.ts @@ -19,7 +19,7 @@ describe("createPermissionBasedMcpServer", () => { { name: "tool-a", description: "Test tool", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [] }), }, ], @@ -400,7 +400,7 @@ describe("createPermissionBasedMcpServer", () => { { name: "module-tool", description: "Tool from module", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [] }), }, ], diff --git a/tests/customEndpoints.integration.test.ts b/tests/customEndpoints.integration.test.ts index e648350..1889f7e 100644 --- a/tests/customEndpoints.integration.test.ts +++ b/tests/customEndpoints.integration.test.ts @@ -25,7 +25,7 @@ describe("Custom Endpoints Integration", () => { { name: "ping", description: "Test tool", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [{ type: "text", text: "pong" }], }), @@ -285,7 +285,7 @@ describe("Custom Endpoints Integration", () => { { name: "admin-ping", description: "Admin test tool", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [{ type: "text", text: "admin-pong" }], }), @@ -299,7 +299,7 @@ describe("Custom Endpoints Integration", () => { { name: "user-ping", description: "User test tool", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [{ type: "text", text: "user-pong" }], }), diff --git a/tests/dynamicToolManager.test.ts b/tests/dynamicToolManager.test.ts index 3c0da35..3c126f6 100644 --- a/tests/dynamicToolManager.test.ts +++ b/tests/dynamicToolManager.test.ts @@ -1,4 +1,8 @@ import { describe, it, expect, vi } from "vitest"; +import { z } from "zod"; +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; import { DynamicToolManager } from "../src/core/DynamicToolManager.js"; import { ModuleResolver } from "../src/mode/ModuleResolver.js"; import { ToolRegistry } from "../src/core/ToolRegistry.js"; @@ -276,3 +280,63 @@ describe("DynamicToolManager", () => { }); }); }); + +describe("DynamicToolManager with a real McpServer", () => { + async function connect(tools: any[]) { + const server = new McpServer({ name: "t", version: "1.0.0" }); + const resolver = new ModuleResolver({ + catalog: { core: { name: "Core", description: "", tools } } as any, + }); + const manager = new DynamicToolManager({ + server, + resolver, + toolRegistry: new ToolRegistry({ namespaceWithToolset: false }), + }); + const result = await manager.enableToolset("core"); + const [serverSide, clientSide] = InMemoryTransport.createLinkedPair(); + await server.connect(serverSide); + const client = new Client({ name: "c", version: "1.0.0" }); + await client.connect(clientSide); + return { result, client }; + } + + it("passes validated arguments from a Zod raw shape to the handler", async () => { + const { result, client } = await connect([ + { + name: "price", + description: "Price", + inputSchema: { symbol: z.string() }, + annotations: { title: "Price lookup", readOnlyHint: true }, + handler: async (args: { symbol: string }) => ({ + content: [{ type: "text", text: args.symbol }], + }), + }, + ]); + expect(result.success).toBe(true); + + const { tools } = await client.listTools(); + expect(tools[0].title).toBe("Price lookup"); + expect(tools[0].inputSchema.properties).toHaveProperty("symbol"); + expect(tools[0].annotations?.readOnlyHint).toBe(true); + + const res = await client.callTool({ name: "price", arguments: { symbol: "AAPL" } }); + expect((res.content as any)[0].text).toBe("AAPL"); + }); + + it("registers a plain JSON Schema inputSchema as a passthrough object", async () => { + const { result, client } = await connect([ + { + name: "price", + description: "Price", + inputSchema: { type: "object", properties: { symbol: { type: "string" } } }, + handler: async (args: { symbol: string }) => ({ + content: [{ type: "text", text: args.symbol }], + }), + }, + ]); + expect(result.success).toBe(true); + + const res = await client.callTool({ name: "price", arguments: { symbol: "MSFT" } }); + expect((res.content as any)[0].text).toBe("MSFT"); + }); +}); diff --git a/tests/e2e/helpers.ts b/tests/e2e/helpers.ts index 7e415a7..2a3ab06 100644 --- a/tests/e2e/helpers.ts +++ b/tests/e2e/helpers.ts @@ -32,7 +32,7 @@ export const testCatalog: ToolSetCatalog = { { name: "ping", description: "Returns pong", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [{ type: "text", text: "pong" }], }), @@ -46,7 +46,7 @@ export const testCatalog: ToolSetCatalog = { { name: "reset", description: "Reset system", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [{ type: "text", text: "reset done" }], }), @@ -96,7 +96,7 @@ export const sessionContextModuleLoaders = { { name: "get_context", description: "Returns the current context values", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [ { diff --git a/tests/helpers/fakes.ts b/tests/helpers/fakes.ts index ebee3c5..2281ff7 100644 --- a/tests/helpers/fakes.ts +++ b/tests/helpers/fakes.ts @@ -15,22 +15,9 @@ export type RegisteredTool = { export function createFakeMcpServer(options: { withNotifier?: boolean } = {}) { const tools: RegisteredTool[] = []; const server: any = { - tool( - name: string, - description: string, - schema: any, - annotationsOrHandler: any, - maybeHandler?: any - ) { - // Support both 4-param (legacy) and 5-param (with annotations) signatures - const isLegacy = typeof annotationsOrHandler === "function"; - const annotations = isLegacy ? undefined : annotationsOrHandler; - const handler = isLegacy ? annotationsOrHandler : maybeHandler; - tools.push({ name, description, schema, annotations, handler }); - }, registerTool( name: string, - config: { description: string; inputSchema?: any; annotations?: any }, + config: { description: string; inputSchema?: any; annotations?: RegisteredTool["annotations"] }, handler: (args: any) => Promise | any ) { tools.push({ diff --git a/tests/metaTools.test.ts b/tests/metaTools.test.ts index 2ab2615..00e269c 100644 --- a/tests/metaTools.test.ts +++ b/tests/metaTools.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect } from "vitest"; +import { z } from "zod"; import { registerMetaTools, META_TOOLSET_KEY } from "../src/meta/registerMetaTools.js"; import { DynamicToolManager } from "../src/core/DynamicToolManager.js"; import { ModuleResolver } from "../src/mode/ModuleResolver.js"; @@ -14,7 +15,7 @@ describe("Meta-tools return formats", () => { { name: "ping", description: "Returns pong", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [{ type: "text", text: "pong" }] }), }, ], @@ -32,7 +33,7 @@ describe("Meta-tools return formats", () => { { name: "echo", description: "Echoes text", - inputSchema: { type: "object", properties: { text: { type: "string" } } }, + inputSchema: { text: z.string() }, handler: async ({ text }: { text: string }) => ({ content: [{ type: "text", text }], }), @@ -304,7 +305,7 @@ describe("Meta-tools return formats", () => { { name: "enable_toolset", // Collides with meta-tool description: "A user tool", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [{ type: "text", text: "user" }] }), }, ], diff --git a/tests/smoke-e2e/server-demo.ts b/tests/smoke-e2e/server-demo.ts index 46dd744..af18206 100644 --- a/tests/smoke-e2e/server-demo.ts +++ b/tests/smoke-e2e/server-demo.ts @@ -12,7 +12,7 @@ const catalog: ToolSetCatalog = { { name: "ping", description: "Responds with pong", - inputSchema: { type: "object", properties: {} }, + inputSchema: {}, handler: async () => ({ content: [{ type: "text", text: "pong" }] }), }, ],