Skip to content

Commit 1313fc2

Browse files
committed
fix(mcp): use HTTP/1.1 for the streamable-HTTP transport (drop allowH2)
The MCP transport was the only place in the codebase opting undici into HTTP/2 (`allowH2: true`). undici's h2 support is experimental and has a documented cluster of stalls where response headers arrive but the body DATA frames never do, on POST bodies over reused/coalesced sessions (nodejs/undici #2311, #3433, #4143). Behind a shared egress IP fronted by a CDN, that is exactly what hung the streamable-HTTP `initialize` after OAuth: 200 + Mcp-Session-Id, then an empty body until the SDK's 30s timeout — reproducible only from the deployed egress, never from a fresh IP or in isolation. h2 buys the MCP transport nothing (one POST per JSON-RPC message plus a single long-lived SSE stream — no concurrency to multiplex), and both official MCP SDKs run the transport on HTTP/1.1: the TypeScript SDK's StreamableHTTPClientTransport calls global fetch (undici h1.1, no allowH2), and the Python SDK builds its httpx client with no http2=True. Dropping allowH2 aligns with them and steps off the h2 stall surface. CDN fronts serve h1.1 anyway; SSRF IP-pinning and Agent teardown are unchanged.
1 parent e02dbce commit 1313fc2

2 files changed

Lines changed: 47 additions & 14 deletions

File tree

apps/sim/lib/mcp/pinned-fetch.test.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,37 @@ vi.mock('@/lib/mcp/domain-check', () => ({
2323
validateMcpServerSsrf: mockValidateMcpServerSsrf,
2424
}))
2525

26-
import { createSsrfGuardedMcpFetch } from '@/lib/mcp/pinned-fetch'
26+
import { createPinnedMcpFetch, createSsrfGuardedMcpFetch } from '@/lib/mcp/pinned-fetch'
2727

2828
/** The per-request pinned Agent is always built with a DoS-backstop response cap. */
2929
const withResponseCap = expect.objectContaining({ maxResponseSize: expect.any(Number) })
3030

31+
describe('createPinnedMcpFetch', () => {
32+
beforeEach(() => {
33+
vi.clearAllMocks()
34+
mockDestroy.mockResolvedValue(undefined)
35+
mockCreatePinnedFetchWithDispatcher.mockReturnValue({
36+
fetch: sentinelFetch,
37+
dispatcher: { destroy: mockDestroy },
38+
})
39+
})
40+
41+
it('builds the transport on HTTP/1.1 — never opts into allowH2 (undici h2 stalls)', () => {
42+
const { close } = createPinnedMcpFetch('203.0.113.10')
43+
44+
// Called with the IP only: no `allowH2`, so the Agent stays on undici's h1.1 default.
45+
expect(mockCreatePinnedFetchWithDispatcher).toHaveBeenCalledWith('203.0.113.10')
46+
const options = mockCreatePinnedFetchWithDispatcher.mock.calls[0][1] as
47+
| { allowH2?: boolean }
48+
| undefined
49+
expect(options?.allowH2).toBeUndefined()
50+
51+
// close() tears down the pooled sockets (incl. the long-lived SSE) on disconnect.
52+
void close()
53+
expect(mockDestroy).toHaveBeenCalledTimes(1)
54+
})
55+
})
56+
3157
describe('createSsrfGuardedMcpFetch', () => {
3258
beforeEach(() => {
3359
vi.clearAllMocks()

apps/sim/lib/mcp/pinned-fetch.ts

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,32 @@ import { McpError } from '@/lib/mcp/types'
88
export interface PinnedMcpFetch {
99
/** Pinned fetch to hand to the MCP transport's `fetch` option. */
1010
fetch: typeof fetch
11-
/** Tears down the underlying HTTP/2 Agent; call when the MCP client disconnects. */
11+
/** Tears down the underlying Agent's pooled sockets; call when the MCP client disconnects. */
1212
close: () => Promise<void>
1313
}
1414

1515
/**
16-
* Pinned fetch for the long-lived MCP transport, which reuses one Agent across
17-
* a connection's requests. MCP servers are commonly behind HTTP/2 fronts (CDNs,
18-
* cloud LBs), and undici's Agent is h1.1-only unless opted into h2 via ALPN, so
19-
* the transport enables it. h2 is *not* used for one-shot flows (OAuth discovery,
20-
* auth-type probe), where a per-request Agent would leave idle h2 sessions with
21-
* no reuse benefit. Pinning is unaffected: the pinned lookup forces the socket to
22-
* `resolvedIP` regardless of negotiated protocol. The returned `close` binds the
23-
* Agent's teardown to the transport lifecycle so h2 sessions don't linger past
24-
* disconnect.
16+
* Pinned fetch for the long-lived MCP transport, which reuses one Agent across a
17+
* connection's requests.
18+
*
19+
* The transport speaks **HTTP/1.1** (undici's default — we do not opt into `allowH2`).
20+
* Every stock MCP client does the same: the official SDK's `StreamableHTTPClientTransport`
21+
* calls global `fetch` (undici on h1.1) and never touches HTTP/2. h2's only real win is
22+
* multiplexing many concurrent requests over one socket, which the MCP transport — one
23+
* POST per JSON-RPC message plus a single long-lived SSE stream — never does, so it buys
24+
* nothing here. undici's h2 support is still marked experimental and has a documented
25+
* cluster of "response headers arrive, body DATA frames never do" stalls on POST bodies
26+
* over reused/coalesced sessions (nodejs/undici #2311, #3433, #4143). Behind a shared
27+
* egress IP fronted by a CDN, that stall is exactly what hung the streamable-HTTP
28+
* `initialize` (200 + `Mcp-Session-Id`, then an empty body until the SDK's 30s timeout).
29+
* h1.1 sidesteps that whole surface, and CDN fronts serve h1.1 anyway.
30+
*
31+
* Pinning is unaffected: the pinned lookup forces the socket to `resolvedIP` regardless of
32+
* protocol. The returned `close` binds Agent teardown to the transport lifecycle so pooled
33+
* keep-alive sockets (including the SSE connection) don't linger past disconnect.
2534
*/
2635
export function createPinnedMcpFetch(resolvedIP: string): PinnedMcpFetch {
27-
const { fetch: pinnedFetch, dispatcher } = createPinnedFetchWithDispatcher(resolvedIP, {
28-
allowH2: true,
29-
})
36+
const { fetch: pinnedFetch, dispatcher } = createPinnedFetchWithDispatcher(resolvedIP)
3037
return { fetch: pinnedFetch, close: () => dispatcher.destroy() }
3138
}
3239

0 commit comments

Comments
 (0)