Skip to content

fix(server): settle in-flight JSON-mode requests on transport close (v1.x) - #2588

Draft
axits-lab wants to merge 2 commits into
modelcontextprotocol:v1.xfrom
axits-lab:fix/v1x-json-mode-close-hang
Draft

fix(server): settle in-flight JSON-mode requests on transport close (v1.x)#2588
axits-lab wants to merge 2 commits into
modelcontextprotocol:v1.xfrom
axits-lab:fix/v1x-json-mode-close-hang

Conversation

@axits-lab

Copy link
Copy Markdown

v1.x backport of #2584. Refs #2559, which notes the item applies to both branches — I offered this on the issue and confirmed the gap is present here before writing anything.

Draft: non-write-access users are capped at one open non-draft PR on this repo and #2582 holds it. Ready for review as-is.

The gap, on this branch

JSON response mode's cleanup in WebStandardStreamableHTTPServerTransport is byte-identical to main's pre-fix code:

cleanup: () => {
    this._streamMapping.delete(streamId);
}

close() runs every stream mapping's cleanup, so a JSON-mode POST whose handler is still running has its map entry dropped while the Promise<Response> returned by handleRequest() is never settled — the HTTP request hangs until the client gives up.

It now resolves with a 503 JSON-RPC error. The success path is untouched: send() resolves before it calls cleanup(), and re-resolving a settled promise is a no-op, so this only fires when nothing was sent.

Scope note

Only the hang is backported. The companion half of #2559 — the _streamMapping leak on completed JSON-mode POSTs — does not reproduce on main (fixed by #2286) and I did not find it here either; send() calls stream.cleanup() on both branches. I left it alone rather than write a no-op change.

Test

Added a JSON response mode lifecycle block driving handleRequest() directly, alongside the existing WebStandardStreamableHTTPServerTransport describes. The handler signals that it has parked via a promise the test awaits, so close() is known to land mid-flight without depending on runner speed, and the hang-detector timer is cleared on the passing path.

Verified it fails without the src change:

× settles an in-flight request when the transport closes mid-handler
  AssertionError: expected true to be false

Verification

  • 1640/1640 tests pass across 52 files
  • npm run lint and npm run typecheck clean

Changeset included.

Backport of the main-branch fix (modelcontextprotocol#2584) to v1.x, where the same gap exists
in `WebStandardStreamableHTTPServerTransport`.

`close()` runs every stream mapping's `cleanup`. JSON response mode's
`cleanup` only deleted the map entry, so the `Promise<Response>` returned
by `handleRequest()` was never settled and a POST whose handler was still
running hung until the client gave up.

Resolve it with a 503 JSON-RPC error instead. The success path is
unaffected: `send()` resolves the response before calling `cleanup()`, and
re-resolving an already-settled promise is a no-op, so this only fires when
nothing was sent.

The regression test drives `handleRequest()` directly and waits for the
handler to signal that it has parked before closing, so the assertion does
not depend on runner speed.

Refs modelcontextprotocol#2559

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8e61b9a

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

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

@pkg-pr-new

pkg-pr-new Bot commented Jul 30, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@modelcontextprotocol/sdk@2588

commit: 8e61b9a

`prettier --check .` covers .changeset/*.md on this branch; the file was
written by hand and never formatted, which failed the build job.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant