diff --git a/src/server/mcp.test.ts b/src/server/mcp.test.ts index 10e550df4..b4146f029 100644 --- a/src/server/mcp.test.ts +++ b/src/server/mcp.test.ts @@ -4290,4 +4290,484 @@ describe("elicitInput()", () => { text: "No booking made. Original date not available." }]); }); + + /*** + * Test: Bulk Tool Registration - Memory Leak Prevention + */ + test("should handle multiple tool registrations without memory leak", async () => { + const mcpServer = new McpServer({ + name: "test server", + version: "1.0", + }); + + // Register first tool before connection (should work fine) + mcpServer.tool("tool1", async () => ({ + content: [{ type: "text", text: "Tool 1" }] + })); + + const notifications: Notification[] = [] + const client = new Client({ + name: "test client", + version: "1.0", + }); + client.fallbackNotificationHandler = async (notification) => { + notifications.push(notification) + } + + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + + await Promise.all([ + client.connect(clientTransport), + mcpServer.connect(serverTransport), + ]); + + // Clear any initial notifications + notifications.length = 0; + + // This should work since capabilities were already registered during first tool + mcpServer.tool("tool2", async () => ({ + content: [{ type: "text", text: "Tool 2" }] + })); + + // Yield event loop to let notifications process + await new Promise(process.nextTick); + + // Should have sent exactly one notification for the second tool + expect(notifications).toHaveLength(1); + expect(notifications[0]).toMatchObject({ + method: "notifications/tools/list_changed", + }); + + // Verify both tools are registered + const toolsResult = await client.request( + { method: "tools/list" }, + ListToolsResultSchema, + ); + expect(toolsResult.tools).toHaveLength(2); + expect(toolsResult.tools.map(t => t.name).sort()).toEqual(["tool1", "tool2"]); + }); + + /*** + * Test: registerTool() should work after connection (fixed behavior) + */ + test("registerTool() should work after connection", async () => { + const mcpServer = new McpServer({ + name: "test server", + version: "1.0", + }); + + // Register first tool before connection to establish capabilities + mcpServer.tool("tool1", async () => ({ + content: [{ type: "text", text: "Tool 1" }] + })); + + const notifications: Notification[] = [] + const client = new Client({ + name: "test client", + version: "1.0", + }); + client.fallbackNotificationHandler = async (notification) => { + notifications.push(notification) + } + + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + + await Promise.all([ + client.connect(clientTransport), + mcpServer.connect(serverTransport), + ]); + + // Clear any initial notifications + notifications.length = 0; + + // This should now work after the fix + mcpServer.registerTool("test2", { + description: "Test tool 2" + }, async () => ({ + content: [{ type: "text", text: "Test 2" }] + })); + + // Yield event loop to let notifications process + await new Promise(process.nextTick); + + // Should have sent exactly one notification + expect(notifications).toHaveLength(1); + expect(notifications[0]).toMatchObject({ + method: "notifications/tools/list_changed", + }); + + // Verify both tools are registered + const toolsResult = await client.request( + { method: "tools/list" }, + ListToolsResultSchema, + ); + expect(toolsResult.tools).toHaveLength(2); + expect(toolsResult.tools.map(t => t.name).sort()).toEqual(["test2", "tool1"]); + }); + + /*** + * Test: registerTools() bulk method + */ + test("should register multiple tools with single notification using registerTools()", async () => { + const mcpServer = new McpServer({ + name: "test server", + version: "1.0", + }); + + // Register first tool to establish capabilities + mcpServer.tool("initial", async () => ({ + content: [{ type: "text", text: "Initial" }] + })); + + const notifications: Notification[] = [] + const client = new Client({ + name: "test client", + version: "1.0", + }); + client.fallbackNotificationHandler = async (notification) => { + notifications.push(notification) + } + + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + + await Promise.all([ + client.connect(clientTransport), + mcpServer.connect(serverTransport), + ]); + + // Clear initial notifications + notifications.length = 0; + + // Register multiple tools at once using the bulk method + const tools = mcpServer.registerTools([ + { + name: "bulk1", + config: { + title: "Bulk Tool 1", + description: "First bulk tool" + }, + callback: async () => ({ + content: [{ type: "text" as const, text: "Bulk 1" }] + }) + }, + { + name: "bulk2", + config: { + title: "Bulk Tool 2", + description: "Second bulk tool" + }, + callback: async () => ({ + content: [{ type: "text" as const, text: "Bulk 2" }] + }) + }, + { + name: "bulk3", + config: { + description: "Third bulk tool" + }, + callback: async () => ({ + content: [{ type: "text" as const, text: "Bulk 3" }] + }) + } + ]); + + // Yield event loop to let notifications process + await new Promise(process.nextTick); + + // Should have sent exactly ONE notification for all three tools + expect(notifications).toHaveLength(1); + expect(notifications[0]).toMatchObject({ + method: "notifications/tools/list_changed", + }); + + // Should return array of registered tools + expect(tools).toHaveLength(3); + expect(tools[0].title).toBe("Bulk Tool 1"); + expect(tools[1].title).toBe("Bulk Tool 2"); + expect(tools[2].title).toBeUndefined(); // No title provided + + // Verify all tools are registered and functional + const toolsResult = await client.request( + { method: "tools/list" }, + ListToolsResultSchema, + ); + expect(toolsResult.tools).toHaveLength(4); // initial + 3 bulk tools + + const toolNames = toolsResult.tools.map(t => t.name).sort(); + expect(toolNames).toEqual(["bulk1", "bulk2", "bulk3", "initial"]); + + // Test that the tools actually work + const callResult = await client.request( + { + method: "tools/call", + params: { + name: "bulk1", + arguments: {} + } + }, + CallToolResultSchema, + ); + expect(callResult.content[0]).toMatchObject({ + type: "text", + text: "Bulk 1" + }); + }); + + /*** + * Test: registerResources() bulk method + */ + test("should register multiple resources with single notification using registerResources()", async () => { + const mcpServer = new McpServer({ + name: "test server", + version: "1.0", + }); + + // Register first resource to establish capabilities + mcpServer.resource("initial", "initial://resource", async () => ({ + contents: [{ + uri: "initial://resource", + text: "Initial resource" + }] + })); + + const notifications: Notification[] = [] + const client = new Client({ + name: "test client", + version: "1.0", + }); + client.fallbackNotificationHandler = async (notification) => { + notifications.push(notification) + } + + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + + await Promise.all([ + client.connect(clientTransport), + mcpServer.connect(serverTransport), + ]); + + // Clear initial notifications + notifications.length = 0; + + // Register multiple resources at once using the bulk method + const resources = mcpServer.registerResources([ + { + name: "bulk1", + uriOrTemplate: "bulk://resource1", + config: { + title: "Bulk Resource 1", + description: "First bulk resource" + }, + callback: async () => ({ + contents: [{ + uri: "bulk://resource1", + text: "Bulk resource 1 content" + }] + }) + }, + { + name: "bulk2", + uriOrTemplate: "bulk://resource2", + config: { + title: "Bulk Resource 2", + description: "Second bulk resource" + }, + callback: async () => ({ + contents: [{ + uri: "bulk://resource2", + text: "Bulk resource 2 content" + }] + }) + } + ]); + + // Yield event loop to let notifications process + await new Promise(process.nextTick); + + // Should have sent exactly ONE notification for all resources + expect(notifications).toHaveLength(1); + expect(notifications[0]).toMatchObject({ + method: "notifications/resources/list_changed", + }); + + // Should return array of registered resources + expect(resources).toHaveLength(2); + expect(resources[0].title).toBe("Bulk Resource 1"); + expect(resources[1].title).toBe("Bulk Resource 2"); + + // Verify all resources are registered and functional + const resourcesResult = await client.request( + { method: "resources/list" }, + ListResourcesResultSchema, + ); + expect(resourcesResult.resources).toHaveLength(3); // initial + 2 bulk resources + + const resourceNames = resourcesResult.resources.map(r => r.name).sort(); + expect(resourceNames).toEqual(["bulk1", "bulk2", "initial"]); + + // Test that a resource actually works + const readResult = await client.request( + { + method: "resources/read", + params: { + uri: "bulk://resource1" + } + }, + ReadResourceResultSchema, + ); + expect(readResult.contents[0]).toMatchObject({ + uri: "bulk://resource1", + text: "Bulk resource 1 content" + }); + }); + + test("registerTools() should handle empty array gracefully", () => { + const mcpServer = new McpServer({ + name: "test server", + version: "1.0", + }); + + const result = mcpServer.registerTools([]); + expect(result).toEqual([]); + }); + + test("registerResources() should handle empty array gracefully", () => { + const mcpServer = new McpServer({ + name: "test server", + version: "1.0", + }); + + const result = mcpServer.registerResources([]); + expect(result).toEqual([]); + }); + + test("registerPrompts() should handle empty array gracefully", () => { + const mcpServer = new McpServer({ + name: "test server", + version: "1.0", + }); + + const result = mcpServer.registerPrompts([]); + expect(result).toEqual([]); + }); + + test("registerPrompts() should register multiple prompts with single notification", async () => { + const mcpServer = new McpServer({ + name: "test server", + version: "1.0", + }); + const notifications: Notification[] = [] + const client = new Client({ + name: "test client", + version: "1.0", + }); + client.fallbackNotificationHandler = async (notification) => { + notifications.push(notification) + } + + // Register a single prompt first to test notification behavior + mcpServer.registerPrompt( + "initial", + { + title: "Initial Prompt", + description: "An initial prompt", + argsSchema: { input: z.string() } + }, + ({ input }) => ({ + messages: [{ + role: "user" as const, + content: { type: "text" as const, text: `Initial: ${input}` } + }] + }) + ); + + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + + await Promise.all([ + client.connect(clientTransport), + mcpServer.connect(serverTransport), + ]); + + // Clear any initial notifications + notifications.length = 0; + + // Register multiple prompts in bulk + const registeredPrompts = mcpServer.registerPrompts([ + { + name: "bulk1", + config: { + title: "Bulk Prompt One", + description: "First bulk prompt", + argsSchema: { input: z.string() } + }, + callback: (args, _extra) => ({ + messages: [{ + role: "user" as const, + content: { type: "text" as const, text: `Bulk 1: ${args.input}` } + }] + }) + }, + { + name: "bulk2", + config: { + title: "Bulk Prompt Two", + description: "Second bulk prompt", + argsSchema: { value: z.string() } + }, + callback: (args, _extra) => ({ + messages: [{ + role: "assistant" as const, + content: { type: "text" as const, text: `Bulk 2: ${args.value}` } + }] + }) + } + ]); + + // Yield event loop to let notifications process + await new Promise(process.nextTick); + + // Should return array of registered prompts + expect(registeredPrompts).toHaveLength(2); + expect(registeredPrompts[0].title).toBe("Bulk Prompt One"); + expect(registeredPrompts[1].title).toBe("Bulk Prompt Two"); + + // Should have sent exactly ONE notification for all prompts + expect(notifications).toHaveLength(1); + expect(notifications[0]).toMatchObject({ + method: "notifications/prompts/list_changed", + }); + + // Test list prompts shows all prompts + const promptsResult = await client.request( + { + method: "prompts/list", + params: {} + }, + ListPromptsResultSchema, + ); + + const promptNames = promptsResult.prompts.map(p => p.name).sort(); + expect(promptNames).toEqual(["bulk1", "bulk2", "initial"]); + + // Test that a prompt actually works + const getResult = await client.request( + { + method: "prompts/get", + params: { + name: "bulk1", + arguments: { input: "test" } + } + }, + GetPromptResultSchema, + ); + expect(getResult.messages[0]).toMatchObject({ + role: "user", + content: { type: "text", text: "Bulk 1: test" } + }); + }); }); diff --git a/src/server/mcp.ts b/src/server/mcp.ts index 791facef1..1964080d7 100644 --- a/src/server/mcp.ts +++ b/src/server/mcp.ts @@ -47,6 +47,24 @@ import { UriTemplate, Variables } from "../shared/uriTemplate.js"; import { RequestHandlerExtra } from "../shared/protocol.js"; import { Transport } from "../shared/transport.js"; +/** + * Simple interface for tool registration that avoids TypeScript "Type instantiation is excessively deep" + * errors when registering large numbers of tools with complex schemas. + * + * Uses unknown types to prevent TypeScript from attempting deep type inference on complex schemas. + */ +export interface ToolRegistration { + name: string; + config: { + title?: string; + description?: string; + inputSchema?: unknown; + outputSchema?: unknown; + annotations?: ToolAnnotations; + }; + callback: (args: unknown, extra: RequestHandlerExtra) => CallToolResult | Promise; +} + /** * High-level MCP server that provides a simpler API for working with resources, tools, and prompts. * For advanced usage (like sending notifications or setting custom request handlers), use the underlying @@ -666,6 +684,113 @@ export class McpServer { } } + /** + * Registers multiple resources at once with a single notification. + * This is more efficient than calling registerResource() multiple times, + * especially when registering many resources, as it sends only one list_changed notification. + */ + registerResources(resources: Array<{ + name: string; + uriOrTemplate: string | ResourceTemplate; + config: ResourceMetadata; + callback: ReadResourceCallback | ReadResourceTemplateCallback; + }>): (RegisteredResource | RegisteredResourceTemplate)[] { + if (resources.length === 0) { + return []; + } + + const results: (RegisteredResource | RegisteredResourceTemplate)[] = []; + + // First, validate that none of the resources are already registered + for (const { name, uriOrTemplate } of resources) { + if (typeof uriOrTemplate === "string") { + if (this._registeredResources[uriOrTemplate]) { + throw new Error(`Resource ${uriOrTemplate} is already registered`); + } + } else { + if (this._registeredResourceTemplates[name]) { + throw new Error(`Resource template ${name} is already registered`); + } + } + } + + // Register all resources without sending notifications + for (const { name, uriOrTemplate, config, callback } of resources) { + if (typeof uriOrTemplate === "string") { + const result = this._createRegisteredResource( + name, + (config as BaseMetadata).title, + uriOrTemplate, + config, + callback as ReadResourceCallback + ); + results.push(result); + } else { + const result = this._createRegisteredResourceTemplate( + name, + (config as BaseMetadata).title, + uriOrTemplate, + config, + callback as ReadResourceTemplateCallback + ); + results.push(result); + } + } + + // Set up handlers and send single notification at the end + this.setResourceRequestHandlers(); + this.sendResourceListChanged(); + + return results; + } + + /** + * Register multiple prompts in a single operation with a single notification. + * This is more efficient than calling registerPrompt() multiple times when registering many prompts, + * especially when registering many prompts, as it sends only one list_changed notification. + */ + registerPrompts(prompts: Array<{ + name: string; + config: { + title?: string; + description?: string; + argsSchema?: PromptArgsRawShape; + }; + callback: PromptCallback; + }>): RegisteredPrompt[] { + if (prompts.length === 0) { + return []; + } + + const results: RegisteredPrompt[] = []; + + // First, validate that none of the prompts are already registered + for (const { name } of prompts) { + if (this._registeredPrompts[name]) { + throw new Error(`Prompt ${name} is already registered`); + } + } + + // Register all prompts without sending notifications + for (const { name, config, callback } of prompts) { + const { title, description, argsSchema } = config; + const result = this._createRegisteredPrompt( + name, + title, + description, + argsSchema, + callback + ); + results.push(result); + } + + // Set up handlers and send single notification at the end + this.setPromptRequestHandlers(); + this.sendPromptListChanged(); + + return results; + } + private _createRegisteredResource( name: string, title: string | undefined, @@ -803,9 +928,6 @@ export class McpServer { }; this._registeredTools[name] = registeredTool; - this.setToolRequestHandlers(); - this.sendToolListChanged() - return registeredTool } @@ -914,7 +1036,12 @@ export class McpServer { } const callback = rest[0] as ToolCallback; - return this._createRegisteredTool(name, undefined, description, inputSchema, outputSchema, annotations, callback) + const result = this._createRegisteredTool(name, undefined, description, inputSchema, outputSchema, annotations, callback); + + this.setToolRequestHandlers(); + this.sendToolListChanged(); + + return result; } /** @@ -937,7 +1064,7 @@ export class McpServer { const { title, description, inputSchema, outputSchema, annotations } = config; - return this._createRegisteredTool( + const result = this._createRegisteredTool( name, title, description, @@ -946,6 +1073,56 @@ export class McpServer { annotations, cb as ToolCallback ); + + this.setToolRequestHandlers(); + this.sendToolListChanged(); + + return result; + } + + /** + * Registers multiple tools at once with a single notification. + * This is more efficient than calling registerTool() multiple times, + * especially when registering many tools, as it sends only one list_changed notification. + * + * Uses simplified types to avoid TypeScript compilation issues with large numbers of complex tools. + */ + registerTools(tools: ToolRegistration[]): RegisteredTool[] { + if (tools.length === 0) { + return []; + } + + const results: RegisteredTool[] = []; + + // First, validate that none of the tools are already registered + for (const { name } of tools) { + if (this._registeredTools[name]) { + throw new Error(`Tool ${name} is already registered`); + } + } + + // Register all tools without sending notifications + for (const { name, config, callback } of tools) { + const { title, description, inputSchema, outputSchema, annotations } = config; + + const result = this._createRegisteredTool( + name, + title, + description, + inputSchema as ZodRawShape | undefined, + outputSchema as ZodRawShape | undefined, + annotations, + callback as ToolCallback + ); + + results.push(result); + } + + // Set up handlers and send single notification at the end + this.setToolRequestHandlers(); + this.sendToolListChanged(); + + return results; } /**