Skip to content

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

Merged
RhysSullivan merged 2 commits into
UsefulSoftwareCo:mainfrom
jadch:fix/mcp-stream-scoped-replay
Aug 28, 2026
Merged

fix: scope MCP replay to the requested stream#1529
RhysSullivan merged 2 commits 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.

The agents@0.17.3 patch was rewritten on main by UsefulSoftwareCo#1754 (standalone
listeners attach alongside each other instead of superseding, plus the
_replayInFlight guard). Re-rolled this branch's change with bun patch on
top of main's patched dist rather than merging the patch file textually:
the Last-Event-ID branch no longer drains other streams, and the helper
is now replayUndeliveredResponsesOnFreshGet, called only from the
cursorless GET path. The fresh-GET fallback and its in-flight guard are
unchanged.
@RhysSullivan
RhysSullivan merged commit 0004ea3 into UsefulSoftwareCo:main Aug 28, 2026
40 checks passed
This was referenced Aug 28, 2026
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.

2 participants