Skip to content

[v1.x] test(server): read SSE streams until expected events arrive instead of assuming one fetch chunk - #3174

Open
MuhammadFaizan-prog wants to merge 1 commit into
modelcontextprotocol:v1.xfrom
MuhammadFaizan-prog:fix/2661-v1x-sse-chunks
Open

MuhammadFaizan-prog wants to merge 1 commit into
modelcontextprotocol:v1.xfrom
MuhammadFaizan-prog:fix/2661-v1x-sse-chunks

Conversation

@MuhammadFaizan-prog

@MuhammadFaizan-prog MuhammadFaizan-prog commented Nov 11, 2026 •

Copy link
Copy Markdown

v1.x port of #2662. Refs #2661.

Problem

Two tests in test/server/streamableHttp.test.ts call reader.read() once and then assert on more than one SSE event:

  • should handle batch request messages with SSE stream for responses: expects both the req-1 and req-2 responses in the first chunk.
  • should store and replay MCP server tool notifications: expects the replayed notification in the first chunk after reconnecting with Last-Event-ID.

fetch gives no guarantee that separately written SSE events share a chunk. On Node 26.7.0 they arrive in separate reads, so on v1.x as it is:

Tests  4 failed | 183 passed (187)
AssertionError: expected 'event: message\ndata: {"result":{"tools"…"id":"req-1"}' to contain '"id":"req-2"'
AssertionError: expected 'event: message\nid: …\ndata: {}' to contain 'Second notification from MCP server'

The transport behaves correctly; only the tests assume coalescing. The same file passes on Node 22.

Change (test-only)

Adds the same readSSEUntil(response, predicate, timeoutMs = 2000) helper as #2662: it accumulates decoded chunks until the predicate holds, the stream ends, or the timeout cancels the reader, then releases the lock. It is used at the three single-read sites in the two tests. The replay test now cancels through sseResponse.body.cancel() because the helper has released the reader. No production code changes, so no changeset.

I kept this to the two failing tests. The extra cleanup in #2662's follow-up commit (folding the other inline read loops into the helper) is left out to keep the v1.x diff minimal.

Testing

  • Node 26.7.0: before, 4 failed / 183 passed; after, 187 / 187 (repeated runs).

  • Node 22: 187 / 187 before and after.

  • Sanity check: with a predicate that can never match, the test fails after the 2 s bound instead of hanging.

  • npm run typecheck, Prettier and ESLint are clean on the changed file.

  • Full npm test on Node 22 and Node 24: 1857 / 1858. The one failure, processCleanup.test.ts > should exit cleanly for a server that hangs, timed out at about 5.01 s under load. It is untouched by this PR and passes alone (4.6 to 4.8 s, close to its 5 s limit). npm run lint, npm run build and npm run typecheck pass on Node 22.

Credit

The failure report, repro and the read-until-predicate approach come from @jstar0 (#2661). The helper is the one from #2662. @Parithosh-Varma and @shi-felix also prepared ports for main.

AI disclosure

I used an AI assistant (Hark) to reproduce the failure on Node 26 and to draft this patch and description. I reviewed every line, ran the tests myself, and can explain the change.

  • Test-only; no public API or behaviour change
  • Fails before / passes after on Node 26.7.0
  • typecheck / prettier / eslint clean

…t of modelcontextprotocol#2662)

fetch may deliver separately written SSE events in separate chunks
(Node 26.7 does), so the batch and replay tests read until the expected
event text arrives instead of asserting on a single read().

Refs modelcontextprotocol#2661

Co-authored-by: jstar0 <54024410+jstar0@users.noreply.github.com>
@changeset-bot

changeset-bot Bot commented Nov 11, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: fc4b380

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

@pkg-pr-new

pkg-pr-new Bot commented Nov 11, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: fc4b380

@MuhammadFaizan-prog
MuhammadFaizan-prog marked this pull request as ready for review November 11, 2026 17:34
@MuhammadFaizan-prog
MuhammadFaizan-prog requested a review from a team as a code owner November 11, 2026 17:34
@claude claude Bot added the v1 Issues / PRs related to v1.x label Nov 12, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v1 Issues / PRs related to v1.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant