diff --git a/packages/server/src/server/createMcpHandler.ts b/packages/server/src/server/createMcpHandler.ts index 9adbe54fb8..99979fb774 100644 --- a/packages/server/src/server/createMcpHandler.ts +++ b/packages/server/src/server/createMcpHandler.ts @@ -686,6 +686,14 @@ export function createMcpHandler(factory: McpServerFactory, options: CreateMcpHa /** Modern per-request instances with an exchange still in flight (close() tears these down). */ const inflight = new Set(); + /** + * Instances whose `onclose` has already been wrapped for inflight tracking below. A factory + * that returns the same `Server`/`McpServer` for every session would otherwise have this + * wrapping installed once per request, chaining a new closure onto the previous `onclose` each + * time — a chain that grows without bound and eventually overflows the stack when it finally + * runs (#2607). Installing at most once per instance keeps the chain length at 1. + */ + const oncloseWrapped = new WeakSet(); let closed = false; const reportError = (error: Error) => { @@ -844,12 +852,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 = () => { - inflight.delete(server); - previousOnClose?.(); - }; + if (!oncloseWrapped.has(server)) { + oncloseWrapped.add(server); + const previousOnClose = server.onclose; + server.onclose = () => { + inflight.delete(server); + previousOnClose?.(); + }; + } 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..77343ec4d2 100644 --- a/packages/server/test/server/createMcpHandler.test.ts +++ b/packages/server/test/server/createMcpHandler.test.ts @@ -303,6 +303,38 @@ describe('createMcpHandler — modern path', () => { expect(closeCalls).toBe(1); }); + it('does not grow an unbounded onclose chain when the factory reuses one instance across requests (#2607)', async () => { + const shared = new McpServer({ name: 'reused-server', version: '1.0.0' }); + shared.registerTool('echo', { inputSchema: z.object({ text: z.string() }) }, async ({ text }) => ({ + content: [{ type: 'text', text }] + })); + + // Intercept assignments to the underlying Server's `onclose` field to count how many + // times createMcpHandler installs its inflight-tracking wrapper on this one instance. + let oncloseSetCount = 0; + let currentOnClose: (() => void) | undefined; + Object.defineProperty(shared.server, 'onclose', { + configurable: true, + get: () => currentOnClose, + set: (fn: (() => void) | undefined) => { + oncloseSetCount += 1; + currentOnClose = fn; + } + }); + + const handler = createMcpHandler(() => shared); + + for (let i = 0; i < 5; i++) { + const response = await handler.fetch(postRequest(modernToolsCall('echo', { text: 'x' }))); + expect(response.status).toBe(200); + } + + // Wrapped exactly once, no matter how many requests reused this instance: a growing + // chain here is what eventually overflows the stack when onclose finally runs. + expect(oncloseSetCount).toBe(1); + expect(() => currentOnClose?.()).not.toThrow(); + }); + it('rejects a malformed envelope behind a present claim with invalid params naming the offending key', async () => { const { factory, state } = testFactory(); const handler = createMcpHandler(factory);