[kun654]

What

subscriptions/listen no longer registers an abort listener on the caller's signal when the listen stream has already completed. One guard line in packages/server/src/server/listenRouter.ts, plus a test and a changeset.

Related: this is the defect reported in #3163, and the guard implemented here is the one suggested there. The issue's repro — abortListeners going 1 → 2 → 3 across three completed listens on one live signal, with openStreams and busListeners both at 0 and the count unchanged after closeAll() — matches the failing assertion quoted under Validation below. Credit to @JalenBuildsHub for the write-up; if they have a PR of their own in flight for it, this one can be closed in favour of theirs.

Why

serve() creates the SSE ReadableStream at listenRouter.ts:204, and ReadableStream runs start() synchronously. When the honored filter ends up empty — either because the client asked for {} or because the negotiated honoredSubset(filter, capabilities) narrowed it down to nothing — start() completes the subscription right there (listenRouter.ts:216-220):

// Nothing honored means nothing can ever be delivered: complete the subscription instead of holding the stream.
if (Object.keys(honored).length === 0) {
    teardown(true);
    return;
}

teardown() starts with if (closed) return; (listenRouter.ts:169-170) and sets closed = true at listenRouter.ts:188, so that call inside start() is the only one that ever runs to completion — and at that moment abortCleanup is still undefined, because the variable is only assigned further down (listenRouter.ts:247). By the time control returns to listenRouter.ts:241 the stream is dead, yet the block still runs:

if (signal !== undefined) {
    // ...
    const onAbort = () => teardown(false);
    signal.addEventListener('abort', onAbort, { once: true });
    abortCleanup = () => signal.removeEventListener('abort', onAbort);
}

That listener is now unreachable by both removal paths: teardown() returns at line 170 because closed is already set, so it never reaches abortCleanup?.() at line 195; and closeAll() (listenRouter.ts:264-266) only iterates the open set, which start() never added this subscription to (listenRouter.ts:231 is after the early return). So the handler stays registered — along with its closure over teardown, controller and the router's state — for as long as the signal itself lives.

How visible this is depends on whose signal it is:

• In the shipped HTTP path, createMcpHandler.ts:814 passes request.signal, so the signal is per-request and the retention dies with the request. Every other listen path (aborted, cancel(), closeAll(), a normal teardown) removes its listener; this is the one path that leaves one behind, and the listener can never fire anything but a no-op.
• For a caller that hands serve() a signal outliving one request, it accumulates one stranded listener plus closure per completed listen, and past Node's default limit that signal also starts emitting MaxListenersExceededWarning. createListenRouter is currently internal (only listenRouter.ts and createMcpHandler.ts reference it), so that accumulation is reached today by importing the module from inside the package — as the repro in subscriptions/listen retains abort listeners after immediately completed streams #3163 does — rather than through the published API. The guard is therefore both a dead-listener fix for the HTTP path and a correctness fix for the router's own invariant.

The change is one guard line: it removes wiring that cannot do anything, and makes the router's "no listener outlives its subscription" invariant hold for the empty-honored case too.

How

Guard the wiring on the stream still being open. A stream that completed inside start() has nothing left to tear down, so there is nothing to listen for:

-        if (signal !== undefined) {
+        // A stream honored down to nothing is already torn down inside `start()`, and
+        // `teardown()` short-circuits once `closed` is set — wiring `onAbort` to such a
+        // stream would strand it on the caller's signal, past `closeAll()`.
+        if (!closed && signal !== undefined) {

The already-aborted branch is unaffected: with !closed false the stream is complete, so calling teardown(false) there would be a no-op anyway.

Validation

Monorepo checked out at main = 1e608caeb5130b2ad0fa067c84b26a038c1c6e36, Node 24.21.0, Windows. pnpm install only — I did not run pnpm build:all, which is why the two dist-dependent test files below fail (they fail identically on unmodified main).

Before — the new test against unmodified listenRouter.ts (pnpm --filter @modelcontextprotocol/server exec vitest run test/server/listenRouterAbortCleanup.test.ts):

 ❯ test/server/listenRouterAbortCleanup.test.ts (3 tests | 2 failed) 37ms
     × attaches no abort listener when capabilities narrow the filter to nothing 35ms
     × attaches no abort listener when the requested filter is empty 1ms
     ✓ still wires and releases the abort listener for a stream that stays open 1ms

 FAIL  test/server/listenRouterAbortCleanup.test.ts > createListenRouter — completed listens do not retain abort listeners > attaches no abort listener when the requested filter is empty
AssertionError: expected [ [Function onAbort] ] to have a length of +0 but got 1

- 0
+ 1

 Test Files  1 failed (1)
      Tests  2 failed | 1 passed (3)

(The third case is the control: a stream that stays open must still get exactly one listener and release it on abort. It passes before and after, so the fix does not remove the wiring that is actually needed.)

After — same command with the fix:

 ✓ test/server/listenRouterAbortCleanup.test.ts (3 tests) 31ms

 Test Files  1 passed (1)
      Tests  3 passed (3)

packages/server full suite, pristine vs fixed. pnpm --filter @modelcontextprotocol/server test:

	pristine main	with this change
summary	Test Files 2 failed | 53 passed (55) / Tests 654 passed | 20 skipped (674)	Test Files 2 failed | 54 passed (56) / Tests 657 passed | 20 skipped (677)

Both runs fail the same two pre-existing files, and the extracted FAIL lines (ANSI stripped, sorted) are byte-identical between the two runs — 3 lines, non-empty:

 FAIL  test/server/barrelClean.test.ts > @modelcontextprotocol/server root and ext/skills are runtime-neutral
 FAIL  test/server/barrelClean.test.ts > @modelcontextprotocol/server root entry is browser-safe
 FAIL  test/server/workerdSchemaPreload.test.ts > workerd schema warm-up (built dist)

Both need a built dist/ resolved through the package's published export map, which my checkout does not have (pnpm build:all was not run); they fail identically on unmodified main, and both pass in CI on this PR, which is consistent with that explanation. Passed count goes 654 → 657, exactly the three tests added here.

Lint and typecheck, pnpm --filter @modelcontextprotocol/server lint / typecheck:

> eslint src/ && prettier --ignore-path ../../.prettierignore --check .
Checking formatting...
All matched files use Prettier code style!

> tsgo -p tsconfig.json --noEmit

(both exit 0; typecheck only prints Node's process.execve is an experimental feature warning)

CI at submission. All of the required checks (build, client-conformance, server-conformance, test (20/22/24), test-e2e (20/22/24), test-runtimes) plus examples (build + e2e) pass on commit 45f2c96c. The new file is picked up by the repo's own runner on all three matrices, e.g. test (22):

packages/server test:  ✓ test/server/listenRouterAbortCleanup.test.ts (3 tests) 124ms
packages/server test:  ✓ test/server/barrelClean.test.ts (13 tests) 10063ms
packages/server test:  ✓ test/server/workerdSchemaPreload.test.ts (4 tests) 12ms

which also confirms the two locally-failing files are only dist-dependent, not broken.

The one red check is pkg-publish ("Publish Any Commit"), which is not among the required checks: its Publish preview packages step exits 1 about a second after starting, on a 404 from pkg-pr-new's own endpoint —

Check failed (404): {"url":"/check","statusCode":404,"statusMessage":"Not Found","message":"There is no workflow defined for W09KeKOH2s","stack":""}

— before pnpm has resolved or packed anything, so nothing in this diff is reached. I have not re-run it; flagging it here rather than claiming a clean board.

Docs / changeset

A patch changeset for @modelcontextprotocol/server is included, describing the user-visible fix rather than the internal guard.

The regression test is packages/server/test/server/listenRouterAbortCleanup.test.ts — under test/, per the repo note that a test file placed next to its source never runs.

AI assistance

This PR was authored by an AI coding agent (Qoder) running as the account owner's standing daily contribution automation. The agent read listenRouter.ts, wrote the failing test, applied the guard, and executed every command whose output is quoted above; the outputs are copied from those runs, not reconstructed. The account owner reviews merged/open PRs rather than pre-approving each patch, so please treat the reasoning above as the agent's own analysis and file any follow-up questions on this thread — the automation revisits PRs daily and will respond to review comments and CI.

[changeset-bot]

🦋 Changeset detected

Latest commit: 66f1f66

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@modelcontextprotocol/server	Patch
@modelcontextprotocol/client	Patch
@modelcontextprotocol/codemod	Patch
@modelcontextprotocol/core	Patch
@modelcontextprotocol/server-legacy	Patch
@modelcontextprotocol/core-internal	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[koriyoshi2041]

Verified the exact head 45f2c96c locally. The focused regression passes 3/3; reverting only the !closed guard makes both completed-stream cases fail while the active-stream abort cleanup control still passes. Server typecheck, lint/format, and diff hygiene also pass.

The guard matches the synchronous ReadableStream.start() ordering and leaves the already-aborted and active-stream paths unchanged. The hosted build, conformance, runtime, and Node 20/22/24 matrices are green; the preview-publish 404 occurs before this diff is exercised.

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@3164

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@3164

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@3164

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@3164

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@3164

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@3164

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@3164

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@3164

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@3164

commit: 66f1f66

[koriyoshi2041]

Rechecked the merge-updated head 66f1f66. The two PR commits are byte-identical in the range-diff, and the completed-listen guard/tests are unchanged. Diff hygiene passes, and the refreshed build, conformance, Node 20/22/24, E2E, Bun, Deno, and package-publish checks are all green. I did not find a merge-introduced regression in this scope.