Skip to content

fix: scope MCP replay to the requested stream - #1529

Open
jadch wants to merge 1 commit into
UsefulSoftwareCo:mainfrom
jadch:fix/mcp-stream-scoped-replay
Open

fix: scope MCP replay to the requested stream#1529
jadch wants to merge 1 commit into
UsefulSoftwareCo:mainfrom
jadch:fix/mcp-stream-scoped-replay

Conversation

@jadch

@jadch jadch commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Keep a cursor-bearing MCP recovery GET scoped to the stream identified by its Last-Event-ID, while preserving the existing cursorless fresh-GET fallback for completed POST responses that otherwise have no recovery cursor.

Problem

The Streamable HTTP spec says a server MUST NOT replay messages that would have been delivered on a different stream. The current patched transport first replays the cursor's stream, then appends responses from every other undelivered POST stream to the same recovery GET.

That can lose the response the client actually needs: the TypeScript SDK stops reconnecting after the first JSON-RPC result/error, so an unrelated completed response can make it close before the cursor's own result arrives.

Change

  • remove cross-stream replay from the Last-Event-ID branch
  • rename the all-undelivered helper to replayUndeliveredResponsesOnFreshGet and call it only when GET has no cursor
  • keep the existing delivery-ack behavior unchanged
  • add a real Cloud E2E regression with independent id=20 and id=21 POST streams
  • add a patch changeset

Regression proof

With the old cross-stream call deliberately restored, the new test fails immediately:

expected 21
received 20

With this fix restored, the full replay E2E file passes all three scenarios.

Validation

  • bun run check:patches
  • bun run --cwd packages/hosts/cloudflare test — 61 passed
  • bun run --cwd packages/hosts/cloudflare typecheck
  • bun run --cwd e2e typecheck
  • bunx vitest run --project cloud cloud/mcp-sse-replay.test.ts — 3 passed
  • hostile maintainer-style review of stream scoping, ACK safety, patch hygiene, and test realism found no blocking or high-value issues

Related context: #1345 and #1489.

@jadch

jadch commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

CI note: the only red check is the 16-minute self-host E2E suite, and it is the same deterministic pre-existing failure on this PR's exact base (59a6640e5):

Both fail mcp-oauth-reconnect-health.test.ts:171 because the test expects provider text Grant not found, while the runtime returns the intentionally normalized invalid_grant reconnect message. The test retried twice in both runs. All four cloud E2E shards, unit tests, typecheck, lint, format, and the targeted MCP replay regression pass, so I have not mixed that unrelated base fix into this PR.

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