diff --git a/.changeset/curvy-rivers-restore.md b/.changeset/curvy-rivers-restore.md new file mode 100644 index 0000000000..1c15f14631 --- /dev/null +++ b/.changeset/curvy-rivers-restore.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/server': patch +--- + +Fix a stack overflow in `createMcpHandler` when the factory returns the same server instance for more than one request. Returning a fresh instance per request is still required. diff --git a/packages/server/src/server/createMcpHandler.ts b/packages/server/src/server/createMcpHandler.ts index 82cfa061bc..52ac41fd20 100644 --- a/packages/server/src/server/createMcpHandler.ts +++ b/packages/server/src/server/createMcpHandler.ts @@ -867,10 +867,15 @@ export function createMcpHandler(factory: McpServerFactory, options: CreateMcpHa // Track the instance until its exchange tears down so close() can abort it. const previousOnClose = server.onclose; inflight.add(server); - server.onclose = () => { + const onExchangeClose = () => { inflight.delete(server); + // Restore by identity so a handler installed during the exchange is kept. + if (server.onclose === onExchangeClose) { + server.onclose = previousOnClose; + } previousOnClose?.(); }; + server.onclose = onExchangeClose; try { const response = await invoke(product, route.message, { diff --git a/packages/server/test/server/createMcpHandler.test.ts b/packages/server/test/server/createMcpHandler.test.ts index 4dae5c0ad7..8de6672f49 100644 --- a/packages/server/test/server/createMcpHandler.test.ts +++ b/packages/server/test/server/createMcpHandler.test.ts @@ -274,6 +274,53 @@ describe('createMcpHandler — modern path', () => { expect(onerror).toHaveBeenCalledWith(expect.objectContaining({ message: 'factory exploded' })); }); + it('restores a reused server onclose handler after each modern exchange', async () => { + const reused = new McpServer({ name: 'entry-test-server', version: '1.0.0' }); + reused.registerTool('echo', { inputSchema: z.object({ text: z.string() }) }, async ({ text }) => ({ + content: [{ type: 'text', text }] + })); + + const originalOnClose = vi.fn(); + reused.server.onclose = originalOnClose; + + const handler = createMcpHandler(() => reused); + + for (let i = 0; i < 3; i++) { + const response = await handler.fetch(postRequest(modernToolsCall('echo', { text: `hello-${i}` }))); + expect(response.status).toBe(200); + await response.text(); + expect(reused.server.onclose).toBe(originalOnClose); + } + + expect(originalOnClose).toHaveBeenCalledTimes(3); + }); + + it('keeps an onclose handler that was installed during the exchange', async () => { + const reused = new McpServer({ name: 'entry-test-server', version: '1.0.0' }); + const installedDuringExchange = vi.fn(); + let chained: (() => void) | undefined; + reused.registerTool('echo', { inputSchema: z.object({ text: z.string() }) }, async ({ text }) => { + if (chained === undefined) { + const previous = reused.server.onclose; + chained = () => { + installedDuringExchange(); + previous?.(); + }; + reused.server.onclose = chained; + } + return { content: [{ type: 'text', text }] }; + }); + + const handler = createMcpHandler(() => reused); + + const response = await handler.fetch(postRequest(modernToolsCall('echo', { text: 'hello' }))); + expect(response.status).toBe(200); + await response.text(); + + expect(reused.server.onclose).toBe(chained); + expect(installedDuringExchange).toHaveBeenCalledTimes(1); + }); + it('closes and releases the per-request instance when a modern exchange fails internally', async () => { const { factory, state } = testFactory(); const onerror = vi.fn();