diff --git a/clients/web/src/test/integration/mcp/remote/server-extra-coverage.test.ts b/clients/web/src/test/integration/mcp/remote/server-extra-coverage.test.ts index 7d2fea1f..7314d514 100644 --- a/clients/web/src/test/integration/mcp/remote/server-extra-coverage.test.ts +++ b/clients/web/src/test/integration/mcp/remote/server-extra-coverage.test.ts @@ -353,6 +353,14 @@ describe("server.ts supplemental coverage", () => { }); const srv = createServer((req, res) => { upstreamHits.push(req.url ?? ""); + if (req.url === "/stream-hostile") { + // Headers, one frame, and a connection whose teardown the route must + // not wait on: `ReadableStream.cancel()` adopts the source's cancel + // promise, which may never settle. + res.writeHead(200, { "Content-Type": "text/event-stream" }); + res.write("data: hi\n\n"); + return; + } if (req.url === "/stream-open") { // Event-stream headers, one frame, and then nothing — the shape that // used to leave a socket open for good, because the route classifies @@ -568,6 +576,22 @@ describe("server.ts supplemental coverage", () => { expect(await settled).toBe("aborted by us"); }); + it("answers a discarded stream without waiting on its cancellation", async () => { + // ⚠️ The route starts the cancel and returns. Awaiting it would keep the + // handler pending on a source that never settles its cancel promise — + // and on an unbounded request there is no timer to release it either. + const res = await fetch(`${h.baseUrl}/api/fetch`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ url: `${targetUrl}/stream-hostile` }), + }); + + expect(res.status).toBe(200); + const payload = (await res.json()) as { status: number; body?: string }; + expect(payload.status).toBe(200); + expect(payload.body).toBeUndefined(); + }); + it("routes the outbound request through HTTP_PROXY (#2067)", async () => { // /api/fetch is the browser's ONLY way out to the network — the web // client's `environment.fetch` is `createRemoteFetch()`, which forwards diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts index ccdc7a8b..15179eb1 100644 --- a/core/mcp/remote/node/server.ts +++ b/core/mcp/remote/node/server.ts @@ -1323,7 +1323,14 @@ export function createRemoteApp( // its own in this configuration — so this is belt and braces rather // than a demonstrated leak, and it is kept because relying on that is // an implementation detail of the fetch beneath us, not a contract. - await res.body?.cancel().catch(() => {}); + // + // ⚠️ Started, not awaited. `ReadableStream.cancel()` adopts the + // underlying source's cancel promise, which is permitted never to + // settle — awaiting it would keep this handler pending forever, and on + // an unbounded request there is no timer to release it either + // (Copilot). The `void` is the documented case where the callee owns + // its failures, via the `catch` below, and the caller cannot await. + void res.body?.cancel().catch(() => {}); } return c.json({