fix(mcp): pin remote MCP transports to HTTP/1.1 for Cloudflare compatibility#1910
fix(mcp): pin remote MCP transports to HTTP/1.1 for Cloudflare compatibility#1910quinn-connor wants to merge 2 commits into
Conversation
…ibility Node's global fetch negotiates HTTP/2 (undici allowH2 defaults to true). The MCP SDK's streamable HTTP transport then multiplexes its standalone SSE GET stream and subsequent POSTs onto one H2 connection — and Cloudflare-hosted MCP servers never answer the POST while the GET stream is open, so connect() succeeds but listTools() hangs until startupTimeoutMs fires and the server is marked failed. A sweep of hosted MCP endpoints reproduced this with Cloudflare's own docs MCP server; endpoints on other platforms (Linear, Notion, Stripe, Vercel, Hugging Face, Context7, and others) are unaffected. Give HttpMcpClient/SseMcpClient a default fetch backed by an undici Agent with allowH2: false, so the stream and POSTs travel on separate connections. Proxy-configured environments keep the proxy dispatcher (CONNECT tunnels are already HTTP/1.1 to the origin); tests can still inject their own fetch or dispatcher. Adds a regression test with a local HTTP/2 server mimicking the stall: post-handshake POSTs are never answered over H2, while the pinned HTTP/1.1 fetch completes the handshake and lists tools. Verified live against a Cloudflare-hosted MCP endpoint.
🦋 Changeset detectedLatest commit: 9d4cf22 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96cd70eb81
ℹ️ 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".
|
|
||
| function getMcpDispatcher(): Dispatcher { | ||
| if (sharedMcpDispatcher === undefined) { | ||
| sharedMcpDispatcher = createProxyDispatcher() ?? new Agent({ allowH2: false }); |
There was a problem hiding this comment.
When any HTTP(S)_PROXY/ALL_PROXY env var is set, this branch returns createProxyDispatcher() instead of the new Agent({ allowH2: false }). I checked the repo's createProxyDispatcher path: it builds undici EnvHttpProxyAgent/ProxyAgent with only proxy URLs and noProxy, so HTTPS target connections can still negotiate HTTP/2 after the CONNECT tunnel; in proxy environments, Cloudflare-hosted MCP servers can therefore still hit the same tools/list H2 stall this patch is meant to fix. Please pass the HTTP/1.1 pin through the proxy/no-proxy agents as well.
Useful? React with 👍 / 👎.
| const port = (server.address() as AddressInfo).port; | ||
|
|
||
| return { | ||
| url: `https://localhost:${port}/mcp`, |
There was a problem hiding this comment.
On hosts where localhost resolves to ::1 first, this URL makes the new regression test connect to IPv6 while the TLS server only listens on 127.0.0.1, so the test fails with ECONNREFUSED before exercising the HTTP/1.1 pinning behavior. Return https://127.0.0.1:${port}/mcp instead (the test cert already includes the IPv4 SAN), or bind the server on IPv6 too; the same issue exists in the legacy copy of this helper.
Useful? React with 👍 / 👎.
| return config.transport === 'http' || config.transport === 'sse'; | ||
| } | ||
|
|
||
| // Node's global fetch negotiates HTTP/2 (undici `allowH2` defaults to true). |
There was a problem hiding this comment.
packages/agent-core-v2/AGENTS.md says comments in this package must live solely in the top-of-file /** */ block and never beside functions, methods, or statements. This new explanatory block violates that local rule (as do the new inline transport comments in the v2 MCP clients), so the v2 change should move the rationale into the module header or remove the comments.
Useful? React with 👍 / 👎.
Addresses review feedback: - The proxy-aware dispatcher path returned createProxyDispatcher() as-is, but undici's EnvHttpProxyAgent/ProxyAgent still negotiate H2 to the origin after the CONNECT tunnel, so proxied remote MCP traffic could hit the same stall. Verified empirically against a local CONNECT proxy. Pass allowH2: false through the HTTP-proxy agent factory (the SOCKS path is already HTTP/1.1 by construction). - Use the IPv4 literal in the H2 stall test URL so the test works on hosts where localhost resolves to ::1 first; the cert already carries the 127.0.0.1 SAN. - agent-core-v2 mandates header-only comments; move the rationale into the client-remote.ts file header and drop inline comments in the v2 sources and tests.
|
Addressed the Codex review findings in
All 331 MCP tests pass in both engines; |
1 participant
Related Issue
No linked issue in this repo — the problem is explained below. This is a clear, reproducible bug fix with a focused diff (per CONTRIBUTING). Reported upstream to Cloudflare at cloudflare/agents#1965, whose investigation confirmed the root cause is the known undici client bug nodejs/undici#5524, fixed upstream by nodejs/undici#5538 but not yet released in a Node version.
Problem
Remote (streamable HTTP) MCP servers hosted on Cloudflare fail to connect: the server is marked
failedafter the startup timeout (Timed out after 30000ms) even though the endpoint is healthy and other MCP clients connect to it fine.Root cause, reproduced at the wire level:
fetchnegotiates HTTP/2 (undiciallowH2defaults to true).StreamableHTTPClientTransportopens its standalone SSEGETstream afterinitializeand keeps it open (spec-compliant).POSTrequests (e.g.tools/list) behind the in-flight GET and never dispatches them (thebusy()guard inclient-h2.js).connect()succeeds, buttools/listhangs indefinitely untilstartupTimeoutMsfires — the server's tools never become available.I swept 14 popular hosted MCP endpoints with the SDK's exact startup sequence (
initialize→notifications/initialized→ standalone GET stream →tools/list) using Node's default H2-capable fetch:Every affected case completes in ~100ms when the same sequence runs over HTTP/1.1. The unaffected servers either reject the standalone GET stream (405), require auth up front (instant 401), or tolerate the multiplexing. (Auth-gated endpoints were only tested up to the 401 boundary.)
What changed
agent-core/agent-core-v2mcp/client-remote.ts: addedcreateMcpFetch(), afetchfor remote MCP transports backed by an undiciAgent({ allowH2: false }), pinning MCP traffic to HTTP/1.1 so the SSE stream and POSTs travel on separate connections. When proxy env vars are configured it uses the existingcreateProxyDispatcher()with the sameallowH2: falsepin passed through the HTTP-proxy agent factory — undici'sEnvHttpProxyAgent/ProxyAgentalso negotiate H2 to the origin after the CONNECT tunnel (verified against a local CONNECT proxy). The SOCKS path is HTTP/1.1 by construction.client-http.ts/client-sse.ts(both engines): the SDK transports now default to this fetch (options.fetch ?? createMcpFetch()). Injected fetches (tests) and the OAuth flow are unaffected.initialize, holds the GET stream open, never answers post-handshake POSTs over H2). An H2 agent stalls; the pinned H1.1 fetch connects and lists tools.Why this approach: the bug is in the HTTP client layer, but until the undici fix ships in the Node versions users run, the client-side pin is the only lever available — and HTTP/1.1 is valid for every spec-compliant MCP server. The change is confined to the MCP remote-transport fetch, so nothing else in the app is affected; MCP traffic (a few JSON-RPC POSTs plus one long-lived stream) gains nothing measurable from H2 multiplexing, and the fix unbreaks a whole class of Cloudflare-hosted servers.
Verification (all run locally):
agent-coreandagent-core-v2.tsc --noEmitandoxlintclean.HttpMcpClient: connect +tools/listsucceed in ~1s (previously timed out after 30s).Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.