fix: eight protocol/transport defects in MCP 2.0 negotiation, replay, and bridging - #259
Conversation
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix: eight protocol/transport defects in MCP 2.0 negotiation, replay, and bridging This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c4d44b981
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return { | ||
| context: await connectSseFallbackTransport( | ||
| client, | ||
| clientFactory.createLegacy(activeDefinition), |
There was a problem hiding this comment.
Don't fall back to legacy when modern is pinned
When a server is configured with protocolVersion: '2026-07-28', any primary Streamable HTTP error that is not tagged EraNegotiationFailed still reaches this fallback and creates a forced-legacy SSE client. For pinned-modern definitions, a 404/405 transport mismatch or similar primary failure can therefore silently connect and run calls in the 2025-era protocol even though the config says 2026-07-28 is required; skip the SSE fallback for pinned definitions rather than only for one error code.
Useful? React with 👍 / 👎.
Fixes eight protocol/transport defects found by an adversarial review of the MCP 2.0 series (#254–#258) and by testing against 30+ public MCP servers in the wild. Every fix ships with a regression test that fails before and passes after.
Negotiation
server/discoveron the SSE attempt (breaking legacy SSE-only servers), and pin mode masked the SDK's accurate error behind a meaninglessSSE error: Non-200 status code (405). Verified in the wild: pinning2026-07-28at Context7 now reports "the server did not offer pinned protocol version 2026-07-28 via server/discover (no fallback in pin mode)". The fallback site now builds a legacy client via a smallHttpClientFactoryseam rather than threading flags.McporterStdioTransportsubclasses the SDK transport, the probe runs in-place on the only process; a legacy server that exits on any pre-initializerequest left nothing to fall back to. Auto mode now retries once with a fresh legacy process. Modern servers keep the single-spawn fast path.MCPORTER_STDIO_PROBE_TIMEOUT_MS. The old 3s budget included process cold-start and misclassified slow modern servers as legacy.Record/replay
RecordTransportnow mirrors the wrapped transport's full capability surface. It was droppinghasPerRequestStream, so the v2 client never attachedrequestSignaland on timeout emitted the removednotifications/cancelledwhile the HTTP request kept running.Bridge & pagination
projectCallToolResult. PreviouslyoutputSchema: {type:"number"}was dropped on both eras and a v1 client's call failed-32602 expected record, received number.Proof:
pnpm checkclean; 955 passed / 3 skipped; autoreview clean (0.96). Re-verified against live servers across all four eras (2025-03-26 → 2026-07-28) with no regressions.🤖 Generated with Claude Code