Add stateless routing to streamable MCP proxy

[jhrozek]

Summary

The MCP 2026-07-28 (Modern) revision is stateless — requests carry protocol metadata per-request in _meta and there is no Mcp-Session-Id. The streamable proxy multiplexes every concurrent request onto a single stdio pipe and demuxes backend responses by compositeKey(sessID, idKey). A Modern request that carried a client-supplied session id would otherwise fall through to the session-lookup path and could collide with a concurrent request on the same key, cross-delivering one client's response to another — a confidentiality bug.

This wires per-request revision classification into the streamable proxy's routing:

• Classify each single POST via mcp.ClassifyRevision.
• Modern requests get a fresh per-request routing token, ignore any Mcp-Session-Id, and never touch the session manager.
• Classification errors are rejected with an HTTP 400 JSON-RPC error.
• Legacy behavior is unchanged.
• Adds mcp.ExtractMeta to pull _meta from raw params, deduped with the parser's existing extraction via a shared helper.

Message-shape enforcement (batches, client-sent responses, GET/DELETE → 405) is deferred — those paths remain Legacy-only by construction. Tracked as a follow-up.

Stacked on #5834 (the classifier/parser/authz foundation). Base will retarget to main once #5834 merges.

Part of #5830. Design: RFC THV-0081 + arch doc (#5808).

Type of change

• New feature (non-breaking change which adds functionality)

Test plan

• Unit tests

New tests:

• TestModernConcurrentRequestsAreNotMixed — the security case: two concurrent Modern POSTs sharing JSON-RPC id 1, one carrying a foreign Mcp-Session-Id, must not cross-deliver responses.
• TestModernRequestIgnoresClientSessionID — a Modern request with a bogus session id is served statelessly (200, no session header, session manager untouched) rather than 404'd.
• TestModernClassificationErrorsReturn400 — header/body mismatch → -32020, unsupported version → -32022, both HTTP 400.
• TestExtractMeta — tolerant _meta extraction across malformed/absent/wrong-typed inputs.

Special notes for reviewers

• The load-bearing invariant is that the Modern branch returns before any Mcp-Session-Id read or sessionManager access. Reviewed by an independent security pass which confirmed it holds and that routing tokens are crypto-random (no collision).
• task lint-fix has not been run locally; relying on CI lint.

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 89.36170% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.36%. Comparing base (38b03b6) to head (36d013a).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/transport/proxy/streamable/streamable_proxy.go	85.29%	3 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5839      +/-   ##
==========================================
+ Coverage   71.28%   71.36%   +0.08%     
==========================================
  Files         693      693              
  Lines       70572    70615      +43     
==========================================
+ Hits        50307    50395      +88     
+ Misses      16633    16568      -65     
- Partials     3632     3652      +20     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JAORMX]

Panel review — stateless routing for the streamable MCP proxy

Reviewed by a multi-lens panel (security & confidentiality · MCP spec conformance · Go idiom, duplication & reuse) against 6da2110, cross-checked with #5830 / #5755 and the draft Streamable-HTTP binding. This is the security-critical routing slice, so the confidentiality invariant got the hardest look.

Verdict: the confidentiality fix holds — no blockers. Tracing resolveSessionForRequest top-to-bottom against the worktree: the Modern branch mints a fresh crypto-random routing token and returns before any Mcp-Session-Id read (first at :805) and before any sessionManager access (:817/:837). So a Modern request can never route by a client-supplied session id or collide on a shared compositeKey, and the RNG-failure path is fail-closed. TestModernConcurrentRequestsAreNotMixed exercises exactly the regression (two id=1 Modern POSTs, one with a foreign Mcp-Session-Id, under a barrier backend) — a genuine guard. Findings below are all hardening / minor.

🟡 Minor

1. The batch path doesn't apply Modern classification — "Legacy-only by construction" is a convention, not enforced — streamable_proxy.go:735-750
A client can send a JSON-array body carrying Modern _meta + a foreign Mcp-Session-Id; resolveSessionForBatch never calls ClassifyRevision, so with a header that resolves in sessionManager.Get it routes batch items onto compositeKey(victimSessID, idKey). Not exploitable today and not a regression — it requires knowing the victim's server-generated random session-id UUID (the same bearer credential the Legacy single-request path already trusts), and sessionless batches each get a fresh unique token so they can't collide. But it means the "ignore Mcp-Session-Id for Modern" invariant isn't yet uniform. Hardening (matches the deferred #5830 plan): classify on the batch path and reject a batch carrying a Modern signal (Modern has no batching).

2. writeClassificationError is the third JSON-RPC-error-to-ResponseWriter writer with the same shape — streamable_proxy.go:848-894
Not reusing session.WriteNotFound directly is correct — that helper is hard-wired to CodeSessionNotFound (-32001) / HTTP 404 / a fixed message, while this path needs variable code+message+data and HTTP 400. But it now shares the marshal-envelope-with-hand-crafted-fallback shape with session.NotFoundBody/WriteNotFound. Consider a generic session.WriteJSONRPCError(w, status, code, message, data, requestID) that both call. (writeHTTPError in utils.go:23 is plain-text http.Error — not the right target.) Non-blocking.

3. Magic -32602 literal duplicated across the package boundary — streamable_proxy.go:853 & :878
Both hard-code mcp.jsonRPCCodeInvalidParams (revision.go:65, unexported); the inline comment acknowledges it. The :853 default is currently unreachable (every ClassifyRevision error implements mcp.CodedError, so errors.As always overwrites it), so drift risk is low. If you want it kept in sync, export mcp.CodeInvalidParams and reference it at :853 (the :878 fallback must stay a literal — it's the marshal-failure escape hatch). This ties into the #5834 finding that MissingModernMetadataError uses -32602 at all — worth resolving there first.

4. Non-object _meta with no matching header silently downgrades to Legacy — revision.go:242 (ExtractMeta) + streamable_proxy.go:783
ExtractMeta returns nil when _meta is present but not a JSON object, so hasModernSignal can then only see Modern via the header — the reserved io.modelcontextprotocol/* keys inside the non-object _meta become invisible, and the request is accepted on the stateful path. This can't happen for a conformant Modern client (object _meta with reserved keys, or the header); only a malformed request hits it, where defaulting to Legacy is defensible. Optionally note in the ExtractMeta doc that a non-object _meta erases the reserved-key signal.

⚪ Nits

• Notification/client-response path uses the raw Mcp-Session-Id for TTL refresh (:917-928, called before classification). Confirmed no cross-delivery — no waiter/compositeKey is registered for these — so it's safe; residual is only that a client knowing a session id could nudge someone else's session TTL (keep-alive, not confidentiality). Fold into the same deferred Modern-shape enforcement.
• Pre-existing: an invalid MCP-Protocol-Version header alone (no _meta signal) still isn't 400'd — isSupportedMCPVersion (utils.go:65) is a no-op returning true. Unchanged by this PR and out of scope; flagging for completeness.
• //nolint:gosec // G104 at :883 matches the established convention (session.WriteNotFound uses the identical suppression) — correct, no change.

✅ Strengths

• The invariant is enforced by control flow (Modern returns before the session read), not by remembering to null out a variable — the right structural fix, and the :763-772 doc comment captures exactly why.
• uuid.NewRandom() (v4, crypto-random) for routing tokens, with fail-closed RNG handling at every mint site (batch :738, Modern :796, Legacy sessionless :828).
• writeClassificationError marshals the body before writing status/headers (mirrors session.WriteNotFound), guaranteeing a valid single response; no double-write — the sole caller returns immediately on error (verified). No response-splitting: the id comes from req.ID.Raw() and error Data() only echoes client-supplied version strings / the static supported-version list — no secret leakage.
• Classification runs at the correct decode point (only after req.ID.IsValid()), so the echoed id is always valid; HTTP 400 + JSON-RPC body is a spec-conformant mapping.
• _meta extraction dedup is clean and complete — paramsMap["_meta"] is now accessed in exactly one shared helper (revision.go:257), with both parser.go and ExtractMeta routing through it.

_{🤖 AI-assisted panel review via Claude Code (security · MCP-spec · Go/reuse lenses). Line numbers against 6da2110. Note this PR is stacked on #5834 — the -32602 item is best resolved there.}

[JAORMX]

Inline notes, including a confirmation on the load-bearing confidentiality line (full rationale + strengths are in the panel summary review above). 🤖 AI-assisted panel review via Claude Code.

[JAORMX]

🔴 Regression: tools-call-sampling conformance times out on the Legacy path

Heads-up before merge — the E2E Tests / MCP Conformance failure here is not the (now-fixed) codespell nit. It's a real, reproducible regression in the tools-call-sampling scenario, and I've isolated it to the "Add stateless routing to streamable MCP proxy" commit:

Build	vs base	tools-call-sampling
#5834 (this PR's base, incl. the go-sdk migration)	—	✅ pass
#5839 6da2110 (earlier head)		✅ pass
#5839 e07a547 (base + routing commit)	run 1	🔴 60.0s timeout
#5839 22fe115 (= e07a547 + the comment-only codespell fix)	run 2	🔴 60.0s timeout

Two CI runs of runtime-identical code both fail with a clean 60-second timeout (server→client sampling/createMessage round-trip never completes), while the base passes — so it's deterministic, not a flake.

Why it matters: conformance exercises the Legacy (2025-11-25) path, so this appears to contradict this PR's stated "Legacy behavior is unchanged" invariant. Notably tools-call-elicitation — same server→client-request-then-client-response shape — still passes, so it looks specific to the sampling round-trip / client-response routing rather than a blanket break.

I isolated it to the commit but haven't pinned the exact line; the likely area is how the reordered classification path in resolveSessionForRequest / the client-response handling interacts with a server-initiated request's stream. Worth confirming before merge.

_{🤖 AI-assisted review via Claude Code — CI regression analysis (isolated across builds #5834 / 6da2110 / e07a547 / 22fe115).}

[JAORMX]

Correction / lower confidence on the above. I dug into the actual diff and want to walk back the "deterministic regression" framing a bit before anyone chases it too hard:

• For a Legacy request, the session-resolution code in resolveSessionForRequest is byte-identical after this commit — it only inserts a ClassifyRevision call ahead of the unchanged logic, which returns Legacy for a tools/call and falls straight through. writeClassificationError only fires on a classification error.
• The parser.go _meta refactor is behavior-preserving (metaFromParamsMap matches the old inline extraction).

So there's no obvious code mechanism here for a Legacy sampling round-trip to break. The build correlation is still real (2 pre-commit runs pass, 2 post-commit runs time out at exactly 60s), but with no mechanism it could just as well be a flake in the ~3-day-old conformance suite that lined up with the commit boundary, rather than a code regression.

Recommend a couple of re-runs of E2E Tests / MCP Conformance on this head to settle it: if it ever goes green, flake; if tools-call-sampling keeps timing out, it's a real interaction worth a deeper look (likely in server→client request / client-response dispatch, not the routing change itself). Apologies if the first note read as more definitive than the evidence supports.

_{🤖 AI-assisted review via Claude Code.}

[JAORMX]

Update — the re-run settles it: this is real, not a flake.

After #5834 merged and this PR retargeted to main, CI re-ran MCP Conformance a third time (fresh run, clean main base) and tools-call-sampling timed out again at exactly 60s. Tally across builds:

• ✅ Add MCP 2026-07-28 revision classification and method vocabulary #5834 base — pass
• ✅ 6da2110 (pre-routing-commit head) — pass
• 🔴 e07a547 — 60s timeout
• 🔴 22fe115 (stacked base) — 60s timeout
• 🔴 22fe115 (retargeted to main) — 60s timeout

3/3 with the routing commit fail; 2/2 without it pass, including against a clean main base — so flake is effectively ruled out. Walking back my earlier "probably a flake" note: it's a real, deterministic regression introduced by this PR.

The puzzle remains that the Legacy resolveSessionForRequest code is byte-identical in the diff (the added ClassifyRevision call is a no-op for a tools/call), so the cause likely isn't in the routing branch itself but in how the change interacts with server→client request / client-response dispatch for the sampling round-trip (note tools-call-elicitation, same shape, still passes — so it's specific to sampling). Worth a proper look before merge — happy to help dig into the dispatch path if useful. (Separately, the retarget left the PR with a merge conflict against main.)

_{🤖 AI-assisted review via Claude Code — this supersedes my two earlier notes on this failure.}

[JAORMX]

Final correction — this is a flake, not a deterministic regression. My earlier "confirmed real" note was wrong; sorry for the churn.

Definitive evidence: the same commit 874c588 produced two conformance runs, and they disagree:

• run 29581975456 — tools-call-sampling 🔴 60s timeout
• run 29581974997 — tools-call-sampling ✅ passed in ~22 ms (40 passed, 0 failed)

Same code, both outcomes → intermittent race, not a hard break. That also reconciles with the static read that the Legacy resolveSessionForRequest path is byte-identical in this PR (there was never a code mechanism for a deterministic Legacy regression).

Caveats, honestly: it flaked at a high rate across these builds (~4 fails : 1 pass observed), and the 60s-vs-22ms split points at a timing/race in the sampling round-trip or the conformance harness. I can't fully rule out that this PR's added per-request work slightly widens that window — but it should not be treated as a merge-blocking deterministic bug. A re-run will go green, and the sampling scenario looks worth deflaking on its own (separate from this PR). Apologies again for the back-and-forth — this is the definitive read and supersedes my two prior notes on this failure.

_{🤖 AI-assisted review via Claude Code.}

[JAORMX]

LGTM on the merits. ✅ The stateless-routing change is well-structured: the confidentiality invariant is enforced by control flow (the Modern branch returns before any Mcp-Session-Id read / sessionManager access), routing tokens are crypto-random with fail-closed RNG handling, and TestModernConcurrentRequestsAreNotMixed is a real guard for the cross-delivery case. The review findings are addressed (the -32602 literal now references mcp.CodeInvalidParams, codespell fixed) and the batch-path hardening is reasonably deferred to #5830.

On the red CI: as documented in the thread, E2E Tests / MCP Conformance (tools-call-sampling) and E2E Test Lifecycle (v1.33.7) are flakes — the same commit 874c588 produced both a passing (~22 ms) and a failing (60 s timeout) conformance run, so it's an intermittent race, not a Legacy regression. Approving the code.

Two non-blocking asks before merge:

1. Get a green conformance re-run (merge is gated on it anyway).
2. Consider deflaking tools-call-sampling separately — it failed ~4:1 across these builds, and I couldn't fully rule out that the added per-request work slightly widens the round-trip race window; worth a glance even though it's not a hard regression here.

_{🤖 AI-assisted review via Claude Code (security · MCP-spec · Go/reuse panel). Approval covers the code; CI gating still applies.}

[jhrozek]

Heads-up: the red MCP Conformance check is a pre-existing flake, not this PR

Re-running CI to land a green pass. For reviewers, the failing tools-call-sampling conformance scenario is not caused by this change:

• This PR only touches the stdio streamable proxy (pkg/transport/proxy/streamable) and pkg/mcp. The conformance reference server runs as a streamable-http container, which is fronted by the transparent proxy — a different code path this PR never exercises (the revision-classifier this PR adds doesn't fire once during a full conformance run).
• The flake is pre-existing and PR-agnostic: it fails on ~2/3 of recent main runs and even on an unrelated Renovate dependency-bump PR (Update module github.com/stacklok/toolhive-catalog to v0.20260717.0 #5858).

What it actually is (deep-dive tracked in #5867): the reference server's test_sampling tool must send a server→client sampling/createMessage, but its handler races the client's SSE-stream setup and never emits it, so the client times out after 60s (-32001). thv run fronts non-stdio workloads with a Squid ingress + Go transparent proxy; whether that stack causes or merely aggravates the client-ordering race is being pinned down with a direct-vs-through-the-stack comparison in #5867. Either way it's independent of this PR's routing change.