Support MCP 2026-07-28 spec (go-sdk v1.7 via toolhive-core) - #5993
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5993 +/- ##
==========================================
- Coverage 72.10% 72.10% -0.01%
==========================================
Files 718 719 +1
Lines 74465 74526 +61
==========================================
+ Hits 53695 53735 +40
- Misses 16921 16962 +41
+ Partials 3849 3829 -20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Context vs the #5754 gate (proceeding on pre.3 — deliberate maintainer decision)Flagging this explicitly for reviewers since it touches the go/no-go analysis on #5754:
Reviewed by a 3-way Opus panel (protocol-correctness/dual-revision headline) with every load-bearing claim verified against the real go-sdk v1.7.0-pre.3 source. cc @jhrozek |
|
Went through this one carefully. The core change is right: keying on I've opened #5997 stacked on this branch with the follow-ups rather than dumping them on you as review comments, since a couple turned into real code changes. Happy for you to take it, cherry-pick from it, or tell me to drop it. Two things in there are actual bugs, both cases where the same "is this peer Modern?" question still gets answered by the rule this PR replaces:
The other is that The rest is smaller. Five comments still justify the hand-rolled Modern shim with "go-sdk v1.6.1 can't express this", which is the premise this PR invalidates, and the One thing I deliberately left alone: the stale-Legacy cache. Your reclassify test correctly pins the new behavior, I just amended the comment, because the staleness isn't purely internal. Two upstream things also fell out of this that aren't yours to fix. mcpcompat's SSE server advertises 2026-07-28 because that transport has no |
* Fix nil-registry panic in ToolSchemaFidelity test main is red: TestRegression_ToolSchemaFidelity_PreservesCompositors panics with a nil-pointer dereference, which crashes the whole pkg/vmcp/client test binary and fails every test in it. The test builds httpBackendClient as a raw struct literal that never sets registry; since the per-backend revision handling landed, ListCapabilities now runs probeRevision -> buildModernHTTPClient -> resolveAuthStrategy, which calls h.registry.GetStrategy on the nil registry. The real constructor NewHTTPBackendClient rejects a nil registry, so this is purely a test-setup gap, not a missing production guard. Build the probe client through the existing newProbeClient helper, which wires a real registry with an UnauthenticatedStrategy. Test-only change. Carved out of #5993 (go-sdk v1.7 migration), which contains the same fix bundled with unrelated changes, so main and dependent PRs can go green without waiting on that larger PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Regenerate CRD reference docs for the mcpRevision field The per-backend MCP revision change added an MCPRevision field to the backend target status but did not regenerate docs/operator/crd-api.md, so the "Generate CRD Docs" job (crd-ref-docs + git diff --exit-code) fails on main and on every PR. Regenerate the reference doc to add the missing mcpRevision entry. Generated-docs only. Same root merge as the nil-registry panic fixed in this PR: pulling both into one change so a single run clears main's CI reds instead of each fix inheriting the other's red. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Move ToolHive onto toolhive-core v0.0.34, which embeds go-sdk v1.7.0-pre.3 — the SDK that implements the new MCP 2026-07-28 "stateless" revision. This lets vMCP negotiate and speak both revisions per backend: Legacy 2025-11-25 and Modern 2026-07-28. The build is unchanged (the mcpcompat shim absorbs the go-sdk API change); this fixes the behavioral gaps that only surface once the real v1.7 SDK is imported, without regressing 2025-11-25 (dual-revision is preserved). Supersedes the prior "hand-rolled Modern envelope, do not import go-sdk v1.7 into the root module" approach: importing the SDK is what makes the new spec reachable. CC @jhrozek (probe/classification and #5981 author). - Revision classification (the crux): decide a backend's revision by whether its server/discover result advertises 2026-07-28 in supportedVersions — the SEP-2575 negotiation signal, and exactly what go-sdk's own reference client keys on. The old "any clean discover is Modern" heuristic mis-classified stateful 2025-11-25 backends, which under v1.7 answer discover too (negotiating down). New sentinel errModernNegotiatedDown wires the negotiate-down case through the existing reclassify machinery. - Strip reserved io.modelcontextprotocol/* _meta at the Legacy backend tool-call egresses (httpBackendClient and the session client): these per-hop protocol-control keys must not cross onto the vMCP->backend hop (a stateful backend 400s them under v1.7). No-op for Legacy traffic. - Test adaptations for the real v1.7 client (server/discover-first over SSE; SSE-forwarding fake answers discover with -32601), a nil-registry test-construction fix, and new real-backend pin tests that assert stateful->Legacy / stateless->Modern so a future SDK discover-semantics change fails loudly here. Follow-up: #5992 (revision-cache TTL re-probe). Part of #5743 (#5754). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WG3mSjVGWNc8nfgbkdd79
ca1198a to
574d3f8
Compare
|
Rebased onto current What conflicted: #6001 ("Fall back to Legacy on an inconclusive vMCP revision probe") changed Also dropped: the CRD-docs regen commit ( Verified locally: 🤖 Rebased with Claude Code |
Bumping the shared YardstickServerImage to 1.2.0 (go-sdk v1.7) broke the
sibling vMCP/virtualmcp e2e suites, which were written for a Legacy
backend. A v1.7 server in session mode (how those tests deploy it)
answers server/discover with a Modern 200, so the vMCP classifies it
Modern and drives the Modern data-plane -- which that same server then
rejects ("2026-07-28 is only supported on stateless HTTP servers"). The
vMCP has no Legacy fallback after a conclusive Modern probe, so those
backends never aggregate (surfacing as "MCP server connection timed
out" / backend-not-ready). vMCP support for Modern backends is #5993.
Keep the shared YardstickServerImage on Legacy 1.1.1 for the operator
and vMCP suites; add YardstickServerImageDualEra (1.2.0) used only by
the dual-era transport-proxy tests, which handle Modern correctly. Wire
the workflows to pull/load both images (vmcp bucket -> 1.1.1, proxy
bucket -> 1.2.0; lifecycle loads both into kind).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
One design question I hit while reviewing, which I didn't want to change unilaterally in the stacked PR.
That's a trap for whoever adds the next key, and there already is one. Want to split it into two lists, so "strip this key" and "this key means Modern" are separate decisions? I left |
* Classify -32022 and SSE targets as Legacy Two backend-revision signals disagreed with the supportedVersions rule adopted for go-sdk v1.7, each able to strand a backend on the wrong path. A -32022 (CodeUnsupportedProtocolVersion) was treated as proof of Modern. It means the opposite: the peer does not support the version we asked for. A backend answering it was cached Modern permanently and enumerated zero capabilities, since isRevisionMismatch never reclassified it. Decode the error's data.supported list instead and keep it Modern-positive only when 2026-07-28 is still advertised, mirroring go-sdk's own reference client (mcp/client.go), whose test for this case is named "unsupported protocol version falls back to initialize". -32020/-32021 stay unconditionally Modern: both require the peer to have parsed Modern _meta. probeRevision was also transport-blind. TransportType "sse" names the deprecated 2024-11-05 two-endpoint transport, whose BaseURL is the GET-only /sse path, and modernCall is a single-endpoint POST, so no Modern endpoint is reachable there. Classify it Legacy without a probe, which also avoids POSTing into a stream and hanging to the client timeout (errModernTransient is uncached, so that cost recurred on every call). Retrying under the corrected Legacy classification stays safe for tools/call: go-sdk rejects an unsupported per-request protocol version before dispatching to any method handler, so the backend never executed the request. Also record that revision-cache self-healing is asymmetric under v1.7 and that the stale value reaches the mcpRevision CRD field (#5992). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Sync Modern shim comments with go-sdk v1.7 Five comments justified the hand-rolled Modern shim on the grounds that go-sdk v1.6.1 cannot express the Modern wire shape. The repo is now on v1.7.0-pre.3, which implements the revision, so that premise reads as false to anyone who checks it. The shim is still required, but for a narrower reason: mcpcompat's public Client API has no no-initialize primitive, only the private Legacy-shaped resumeCall. Say that instead. The parser integration test explained its SSE behavior by claiming the harness cannot capture an initialize carried over the SSE message channel. That was self-contradictory, since tools/call and resources/read traverse the same channel and the same middleware and are captured. The real mechanism: mcpcompat's SSE server uses go-sdk's NewSSEHandler, whose transport implements no ProtocolVersionSupporter gate, so it advertises 2026-07-28, discover succeeds, and initialize is never sent on that arm. Assert that directly rather than inferring it, and drop the claim that the streamable arm sends no discover -- it does, and is negotiated down. Note also that HTTP+SSE is deprecated but NOT removed by 2026-07-28; what that revision removed is Streamable HTTP's GET stream and sessions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Cover revision gaps in vMCP client tests The errModernNegotiatedDown arm of isRevisionMismatch had no truth-table row despite being the only surviving self-healing direction under go-sdk v1.7, so add both the Modern (mismatch) and Legacy (not a mismatch) cases. The reserved-_meta strip has two call sites but only the httpBackendClient one was tested; add the persistent-session counterpart, asserting reserved keys are stripped, non-reserved caller keys survive, and the caller's map is not mutated. Its fixture uses scalars because maps.Clone is shallow and would not have caught a nested mutation. Two robustness fixes: the SSE fake dropped a queued response after a five second timeout and returned an empty body, turning a failure into a hang against a deadline-free context, so fail loudly instead (both send sites, not just one). And the schema-fidelity test reached its Legacy path only because the fake's catch-all happens to answer discover with a body that lacks resultType; pin the revision so adding a field to that arm cannot silently route the test through the Modern shim and strand the injected clientFactory it exists to exercise. metaFor loses its method parameter (every caller passed tools/call, and a second calling function tripped unparam) and becomes toolCallMeta. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Self-heal a stale Legacy revision from initialize The revision cache healed in only one direction. A backend cached Modern that had negotiated down to Legacy recovered via reclassify, but one cached Legacy that had become Modern never did: go-sdk v1.7 negotiates Modern transparently on the Legacy dispatch path, so the call succeeds and the reclassify-on-error trigger never fires. The stale value is not internal bookkeeping — CachedRevision feeds health status and is republished into the mcpRevision CRD field on every tick, so a backend redeployed stateful to stateless reported 2025-11-25 indefinitely. Read the negotiated version off the initialize result instead of waiting for an error that never comes. initializeClient already received it and discarded it, and every Legacy dispatch builds a fresh client, so the mis-routed call itself re-negotiates and the flip costs no extra round trip. The value is genuine on both SDK paths: Connect() stores the discover-negotiated version, and the fallback stores the real initialize response. A backend without server/discover can never yield 2026-07-28, so this cannot promote a genuinely Legacy peer. Closes the Legacy-to-Modern half of #5992; the TTL re-probe now only has to cover whatever remains. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Encode Mcp-Name per spec and reuse the strip helper Two small conformance and duplication fixes on the Modern egress. The 2026-07-28 spec requires a client to Base64 sentinel-encode an Mcp-Name value that cannot be safely represented as a plain ASCII header, and to encode any plain-ASCII value that already matches the sentinel pattern so a server does not wrongly decode it. We sent the value raw, with only a comment acknowledging the gap. A non-ASCII tool name reaches a strict peer as unspecified behavior, and a URI containing CR or LF makes Go's transport reject the request outright. decodeSentinelName already existed with no counterpart, so add its exact mirror and use it. mergeModernMeta also hand-rolled the same maps.Clone plus delete loop that StripReservedModernMeta now performs. Two copies of one rule will drift as soon as a fourth reserved key is added, which is a live prospect -- io.modelcontextprotocol/logLevel is a real per-request reserved key we are deliberately deferring, because the key list currently does double duty as the ingress Modern-signal set and enrolling it there would change request classification. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* Add raw MCP HTTP client for e2e proxy tests The dual-era stateless proxy e2e tests must emit traffic a conforming MCP client never would: spoofed Mcp-Session-Id, colliding JSON-RPC ids, malformed _meta, and oversized/truncated/batch bodies. A hand-rolled, stdlib-only client gives the byte-level control those tests need without importing go-sdk v1.7 into the module (which would force an MVS bump). Adds a raw client that builds Modern (2026-07-28) and Legacy requests, allows arbitrary header/id/session/_meta overrides and raw bodies, and reuses one concurrency-safe http.Client for the crown-jewel fan-out. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add golden Modern-request oracle for raw client The raw client's Modern _meta encoding was written and asserted by the same author, a self-review blind spot: a wrong reserved key, wrong nesting, or wrong value type could pass its own tests. This adds an independent oracle — a 2026-07-28 tools/list request emitted by a real go-sdk v1.7.0-pre.3 client — and asserts NewModernRequest reproduces its _meta reserved-key set, nesting, and value kinds. A typo'd key or a non-object clientCapabilities now fails the test. The fixture is a committed static artifact; it is regenerated by pointing a go-sdk v1.7 client at yardstick-server --stateless (>= v1.2.0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add dual-era cross-delivery e2e test The streamable proxy's Modern (2026-07-28) routing must give every request a fresh routing token and never fall through to a client-supplied Mcp-Session-Id -- otherwise, when concurrent requests share the same (session, id) pair, one client's response can be delivered to another: a confidentiality bug. Unit tests can't observe this; it only surfaces when real traffic races through a running proxy. Drives yardstick-server v1.2.0 in barrier mode (buffers N requests, then releases together) behind the streamable proxy, firing N concurrent Modern requests that share one Mcp-Session-Id and one JSON-RPC id and differ only by a per-request nonce. Asserts conservation: exactly N distinct nonces, each response carrying the nonce of the request that sent it, zero timeouts, zero non-200. A one-shot positive control fires N-1 and asserts they are withheld until the barrier's safety timeout, so a barrier that silently degraded to a no-op fails the test rather than passing while forcing zero collisions. Bumps yardstick 1.1.1 -> 1.2.0 (adds barrier mode) and pre-pulls it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Guard Modern path from client session-id reuse The streamable proxy's Modern (2026-07-28) branch mints a fresh routing token and ignores any client-supplied Mcp-Session-Id. Its safety rests on an unenforced assumption: the client session id has no downstream authority here (stateless stdio backend), unlike the transparent proxy, which maps a session id to a backend and validates it. The existing Modern tests all use unregistered session ids, so none would catch a regression that reused a *known* client session id as the routing token (fall-back-on-lookup-hit) -- silently reopening the validation-bypass class the transparent proxy already hardened against. Adds TestModernNeverReusesClientSessionIDAsRoutingToken, which registers a real session and asserts the Modern branch still mints fresh UUID tokens distinct from it (mutation-verified: fails if the branch reuses a known session id, while the pre-existing tests stay green). Strengthens the resolveSessionForRequest doc comment to tie the safety to the no-downstream-authority assumption and warn against reintroducing a lookup on this path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add dual-era hostile-input e2e test The transparent proxy classifies each request's MCP revision and must reject malformed or hostile Modern (2026-07-28) inputs at the edge, before contacting the backend. This drives that boundary with a raw client through a session-aware transparent proxy fronting an in-process mock. Covers, each asserting the exact JSON-RPC error code, error.data, echoed id, and that the backend was not contacted: header/body version mismatch (-32020), unsupported version (-32022), missing clientCapabilities (-32021), and reserved _meta with no valid version (-32602). Also asserts a well-formed Modern request with a foreign Mcp-Session-Id is rejected 404 -32001 (correct-by-design: the session guard keys on header presence, not the client-forgeable revision) and with none is forwarded once (200); an oversized body is edge-rejected 413 while a truncated body is forwarded as Legacy passthrough (the proxy does not validate JSON syntax); and the proxy still serves a well-formed request afterward. Documents the deferred/known-gap cases (Mcp-Method/Mcp-Name, batch, GET/DELETE 405). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add dual-era concurrent mixing e2e test Legacy (session-based) and Modern (stateless) clients must be able to share one streamable-proxy instance concurrently without cross-delivering responses or losing either era's semantics. This fires several Legacy sessions and stateless Modern requests together each round (released via a shared start channel so both eras are genuinely in flight, asserted by a per-round timestamp-overlap check) and correlates every response by a unique _meta nonce -- a wrong nonce is a cross-era or cross-client leak. Era-distinctness is asserted where it is observable: a negative probe confirms a bogus Mcp-Session-Id is rejected (404 -32001), proving Legacy enforces its session rather than merely minting one, while Modern responses must carry no Mcp-Session-Id. Legacy initialize assigning a session id is the standing Legacy-behavior gate. Scope: with an echo backend this proves response-body delivery correctness, not session-metadata isolation; forced-collision is the crown jewel's job. Accept: text/event-stream is intentionally not set -- it flips this proxy to an SSE response body the raw client does not parse; the plain-JSON path is what this tier exercises. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add dual-era Tier-1 functional e2e tests Tier 1 exercises the proxies with a real, independent MCP SDK client (yardstick-client, go-sdk v1.7.0-pre.3) so it catches spec misreadings a hand-rolled client would share with the proxy code. Modern fidelity runs yardstick-client against the streamable proxy and confirms via -action info that it genuinely negotiated 2026-07-28 with an empty session id (server/discover, no initialize) -- not a masked downgrade. Legacy runs it against a non-stateless backend whose discover omits 2026-07-28, forcing the SDK's fall-back to a 2025-11-25 initialize handshake that the transparent proxy must faithfully pass through (session id preserved). A single raw-client check covers the transparent proxy's own Modern no-Mcp-Session-Id path. Also guards server/discover under authz: with a Cedar policy enabled it must 403 (default-deny, backend not contacted) -- a regression alarm against allow-listing discover without response-filtering (it would bypass ResponseFilteringWriter) -- while a permitted echo call still succeeds. Notes the non-conformant denial envelope (#5950). yardstick-client is go-installed into a scratch GOBIN (separate module, does not touch this repo's go.mod), version derived from the yardstick image tag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add dual-era observability e2e test The existing telemetry e2e tests only log-grep for the word span; none assert span attribute values. This adds a real span-attribute assertion for the Modern (2026-07-28) path: a Modern request's server span must carry mcp.protocol.version=2026-07-28 and mcp.client.name (sourced from _meta.clientInfo, the only client-attribution source in Modern since there is no initialize handshake). Stands up an in-process OTLP/HTTP receiver (httptest.Server decoding real ExportTraceServiceRequest protobuf, race-free span store) and points thv run --otel-endpoint at it, fires one Modern tools/call carrying _meta.clientInfo, and Eventually asserts a span with both attributes (polling since the BatchSpanProcessor flushes on a timer). No new dependencies: the OTLP proto types and protobuf runtime were already in the module graph (promoted from indirect to direct). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Test Modern load leaves no accumulated state The stateless Modern (2026-07-28) path must not leak state under load (the unbounded-growth concern behind toolhive-core#169). This white-box integration test drives a burst of concurrent Modern requests through the streamable proxy's real dispatch lifecycle and asserts, after they all complete, that no per-request state is left behind: the sessionManager holds zero sessions (Modern registers none), the waiters and idRestore maps are empty (per-request routing tokens are cleaned up -- cleanup is deferred in doRequest and runs before any response byte, so the check is deterministic), and the goroutine count is flat. Concurrency is capped with a semaphore (well under the proxy's fixed message-channel buffer) to sidestep an unrelated backpressure gap (#5952) without weakening the invariant, which is about state left after N requests, not N being simultaneous. Mutation-verified: blanking the waiter cleanup makes it fail with the maps holding every request. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add dual-era resilience e2e tests Tier-3 resilience: backend faults must degrade gracefully. Uses yardstick's hang and crash fault modes behind the streamable proxy. The hang spec proves request isolation: with one request wedged at the backend (bounded by the proxy request timeout, returning 504), a second concurrent request still completes promptly -- one slow backend call does not block unrelated traffic -- and a fresh Legacy initialize + session-keyed call afterward proves the backend process and Legacy session semantics survive the hang. The crash spec proves fail-fast: a backend that exits mid-request surfaces a clean 5xx well before the recovery window, not a hang. Redis-unavailable-to-503 is deferred to the k8s tier, where Redis session storage is actually wired via the operator (no thv run CLI flag exposes it); full crash-recovery latency is out of scope (documented). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add dual-era k8s multi-replica e2e tests The operator-deployed dual-era proxy over a shared session store is the production shape the design targets, so it needs its own tier. Deploys a multi-replica yardstick 1.2.0 MCPServer (Replicas + BackendReplicas, streamable-http, STATELESS/BACKEND_MODE echo) with Redis SessionStorage and SessionAffinity: None, in a dedicated namespace so its destructive Redis ops don't collide with sibling suites under parallel Ginkgo. Asserts: the proxy Service actually carries SessionAffinity: None (a regression to the ClientIP default is caught by a direct field read) and sessionless Modern requests succeed across the multi-replica deployment; mixed Legacy+Modern over the shared Redis stays isolated; a backend pod eviction heals back to full replicas with Modern traffic continuing; Redis-unavailable yields 503 (storage error, not 404), and after restore a fresh session works while the pre-outage session correctly 404s. The distinct-backend-pod distribution proof was intentionally NOT included: kube-proxy is L4/per-connection and both proxy hops pool connections, so pod distribution is not observable or controllable from the test -- a count could read 1 under perfectly correct behavior. The in-file comment documents this so it isn't reintroduced as a flaky/unsound check. Authored to run in test-e2e-lifecycle.yml. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Adapt server/discover authz test to #5953 Rebasing onto origin/main picked up #5953 (Serve MCP 2026-07-28 Modern stateless requests through vMCP), which intentionally flipped server/discover in pkg/authz/middleware.go from not-allow-listed (default-deny, 403) to always-allowed. The dual-era authz guard, which asserted a 403, correctly failed against the new behavior. Update it to the new contract: server/discover is allowed under authz (200), which is a positive result -- a real Modern client's discover no longer gets 403'd into a Legacy downgrade, so Modern-through-authz works. The policy-permitted echo call is still asserted. Per #5953's own comment, discover is not covered by the response-filter and passes through unfiltered; that is a documented, deliberate tradeoff in that PR, not something this test enforces. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Bump operator e2e yardstick image to 1.2.0 The operator e2e workflow docker-pulls and kind-loads YARDSTICK_IMAGE into the cluster. The operator acceptance tests (the dual-era k8s spec and ratelimit) now reference yardstick 1.2.0 via images.YardstickServerImage, so the preloaded image must match -- otherwise 1.2.0 is never loaded into kind and those specs can't find their backend image. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix dual-era e2e CI: headers, lint, spelling The E2E Lifecycle (k8s) tier drove Modern tools/call requests at a real go-sdk v1.7 streamable-HTTP backend, which rejects any Modern request missing the Mcp-Method header (and Mcp-Name for name-bearing methods) with -32020. NewModernRequest omitted both, so every k8s Modern request got HTTP 400. - NewModernRequest now emits the full conformant go-sdk v1.7 wire shape: Mcp-Method on every Modern request and Mcp-Name (plain target name) for tools/call / resources/read / prompts/get. The ToolHive single-server proxy never reads these headers, so the transparent/streamable tiers (hostile-input, crown-jewel, mixing) are unaffected; only a real Modern backend requires them. - Mark TestModernLoadDoesNotAccumulateState //nolint:paralleltest: it reads process-wide runtime.NumGoroutine() and starts an HTTP proxy, so it must run serially. Matches the package's existing body_limit_test.go. - "unparseable" -> "unparsable" (codespell). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Send required Accept header on raw MCP requests The k8s multi-replica tier drives Modern requests at a real go-sdk v1.7 streamable-HTTP backend (yardstick), which rejects any POST whose Accept header does not list both application/json and text/event-stream with HTTP 400 -- before any classification. The raw client sent no Accept header, so every k8s request got 400 on request 0. This was masked in local reproduction because curl auto-sends "Accept: */*", which go-sdk's streamableAccepts treats as satisfying both media types. Verified on the wire: with Accept absent the backend returns 400 "Accept must contain both ...", with it present, 200. The other tiers never hit a real go-sdk HTTP backend -- the transparent proxy's in-process mock and the streamable proxy's stdio backend both ignore Accept -- which is why they passed and hid this. thv itself is correct: it transparently forwards the client's headers; a real MCP client always sends Accept, so this deployment works in production. The gap is in the test harness, not the proxy. Default Accept in SendRaw alongside Content-Type (a transport-level requirement for both eras), overridable by a caller that sets its own. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Drop Legacy-session k8s specs; single-server can't bridge eras The k8s tier deploys one STATELESS=true backend (required so Modern 2026-07-28 per-request traffic works) but two specs then asserted Legacy session semantics against it: "serves mixed Legacy and Modern traffic" expected a Legacy initialize to mint an Mcp-Session-Id, and the Redis-503 spec built on the same session. A stateless go-sdk backend issues no sessions, and a single backend cannot be both stateless (for Modern clients) and session-issuing (for Legacy clients) at once. Verified on the wire: Legacy initialize against a stateless yardstick returns 200 but no Mcp-Session-Id, exactly as CI showed. Serving both eras over one backend is cross-generation bridging, which the epic (#5743) scopes to vMCP -- the single-server proxy does per-request version discrimination, not generation bridging. That bridge is design-only today (#5756, unimplemented). So these two specs tested an unsupported scenario against the wrong deployment type; this is a test defect, not a proxy bug. Remove both specs (and the now-orphaned scaleRedis helper / appsv1 import). The tier keeps the three Modern specs -- pods-ready + Redis wiring, Modern multi-replica routing, pod-eviction self-heal -- which is the correct single-server coverage. A file-level note records that when the vMCP cross-generation bridge (#5743/#5756) is implemented, the mixed-era and session-store-outage coverage should be re-added as a vMCP e2e test, not here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Revert global Accept default; opt in only for real backends The near-final "Send required Accept header" commit made SendRaw set Accept: application/json, text/event-stream on every request. The ToolHive streamable proxy switches to its SSE response path for any Accept containing text/event-stream, emitting `data: {...}` frames, but the raw client's populateEnvelope only parses a plain JSON body -- so on the streamable-proxy specs resp.Result stayed nil and nonce extraction returned "". That silently broke the four specs that parse response bodies (cross-delivery crown jewel, concurrent mixing, resilience hang/crash), and directly contradicted dual_era_mixing_test.go's own comment, which had already documented why Accept must NOT be set here. The resilience specs also assert HTTP 504 / >=500, which the SSE path never returns (it answers 200 with an in-band error frame) -- so SSE unwrapping alone would not have fixed them. Revert the global default so proxy requests get a plain JSON body again. The k8s tier legitimately needs the header (its real go-sdk backend 400s without it and only checks status), so add an explicit opt-in -- RawRequest.WithStreamableAccept() -- and call it on the two k8s Modern requests. This restores the documented, verified plain-JSON behavior for every proxy spec while keeping the k8s tier correct. Verified locally: LABEL_FILTER=dual-era task test-e2e -> 11/11 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Isolate yardstick 1.2.0 to the dual-era tests Bumping the shared YardstickServerImage to 1.2.0 (go-sdk v1.7) broke the sibling vMCP/virtualmcp e2e suites, which were written for a Legacy backend. A v1.7 server in session mode (how those tests deploy it) answers server/discover with a Modern 200, so the vMCP classifies it Modern and drives the Modern data-plane -- which that same server then rejects ("2026-07-28 is only supported on stateless HTTP servers"). The vMCP has no Legacy fallback after a conclusive Modern probe, so those backends never aggregate (surfacing as "MCP server connection timed out" / backend-not-ready). vMCP support for Modern backends is #5993. Keep the shared YardstickServerImage on Legacy 1.1.1 for the operator and vMCP suites; add YardstickServerImageDualEra (1.2.0) used only by the dual-era transport-proxy tests, which handle Modern correctly. Wire the workflows to pull/load both images (vmcp bucket -> 1.1.1, proxy bucket -> 1.2.0; lifecycle loads both into kind). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
ReservedModernMetaKeys did double duty: the egress strip set (keys to remove before a Legacy backend hop) and the ingress Modern-detection set (presence => the request claims Modern). Those are different questions, and the next reserved per-request key exposes the conflict. io.modelcontextprotocol/logLevel (draft RequestMetaObject; go-sdk protocol.go) must be stripped on egress, but its presence must NOT mark a request Modern: go-sdk's validateRequestMeta gates Modern-ness purely on protocolVersion, and SEP-2577 deprecates logLevel. Under the single list, adding logLevel would start rejecting logLevel-carrying requests that work today and that go-sdk accepts. Split into ReservedModernMetaKeys (strip set, now including logLevel) and modernSignalMetaKeys (detection set, unchanged three keys, read by hasModernSignal). Classification behavior is otherwise identical. Add a ClassifyRevision row pinning logLevel-alone => Legacy as a tripwire, and exercise logLevel stripping in the strip unit test and the real-backend egress test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WG3mSjVGWNc8nfgbkdd79
Yes — good catch, done in this PR (commit Split into two:
I kept Added a 🤖 via Claude Code |
jhrozek
left a comment
There was a problem hiding this comment.
LGTM. The strip/signal split is exactly right, and the ClassifyRevision row pinning logLevel-alone => Legacy is the part I'd have asked for — it's the tripwire that catches someone re-merging the two lists later.
I re-reviewed the full branch after the #5997 merge and ran ./pkg/mcp/... and ./pkg/vmcp/... locally against ab6d5f2d1; all green. Spot-checked that hasModernSignal is the only consumer that needed repointing at the new signal set, and that the remaining ReservedModernMetaKeys readers all genuinely want the strip semantics.
One nit, not worth holding the PR for, so I've filed it as #6002 item 1: the strip set is applied on the Modern egress too, via mergeModernMeta, and ModernRequestMeta doesn't re-supply logLevel — so a caller's logLevel is now dropped on Modern hops as well, where before it passed through. I think that's the right behavior (vMCP is the backend's peer, and SEP-2577 deprecates logging), but ReservedModernMetaKeys' doc still says "before a Legacy (session-based, stateful) backend hop", which points the other way. A sentence and a mergeModernMeta test row would make it a stated decision rather than a side effect.
#5981 temporarily split the yardstick image -- Legacy 1.1.1 for the operator/vMCP suites, Modern 1.2.0 only for the dual-era tests -- because the vMCP classified a session-mode v1.7 backend as Modern (from a server/discover 200) and could not drive it. #5993 fixed that: the vMCP now keys classification on server/discover's supportedVersions, so a session-mode v1.7 backend (which omits 2026-07-28) is correctly classified Legacy and driven via the Legacy data plane. With that in main, the split is no longer needed. Collapse back to a single YardstickServerImage at 1.2.0 for every e2e suite, drop the YardstickServerImageDualEra constant, and unify the workflow pulls/loads. Verified: TestProbeRevision_RealBackends (a real session-mode go-sdk v1.7 backend classifies Legacy) passes on main, and the dual-era e2e suite passes locally on the unified 1.2.0 image. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Refs #5959. Config.ModernDispatchEnabled (env TOOLHIVE_VMCP_MODERN_STATELESS, default off) was a temporary safety lever gating whether well-formed MCP 2026-07-28 ("Modern") stateless requests were served by classifyingHandler -> dispatchModern or fell through to the SDK path. Both of the issue's gates are now met: the dual-era conformance harness landed in #5837 and go-sdk v1.7 was adopted via #5993. Serve Modern unconditionally and delete the switch, its ServerConfig/Config plumbing, its env var, and the test helpers and specs that existed only to toggle it. This also removes #6009's -32022 UnsupportedVersionError refusal and its server/discover exemption. That refusal answered exactly one condition -- "vMCP supports this revision but has chosen not to serve it" -- which cannot arise once the switch is gone. Every genuine version-grounds rejection still happens, earlier and unchanged, in mcpparser.ClassifyRevision: a _meta protocolVersion naming any other version yields the same conformant -32022, so ClassifyRevision is now the single owner of that decision. server/discover no longer needs an exemption because nothing refuses it on version grounds; dispatchModern answers it via dispatchModernDiscover. authz_integration_test.go's buildCedarAuthzServer had to *set* the switch for TestIntegration_CedarAuthzDenial_ModernPath_IsAudited to reach dispatchModern's call gate at all -- a silently-vacuous test class that disappears along with the switch. KNOWN BLOCKER, do not merge as-is: vMCP's Modern dispatcher does not implement subscriptions/listen. Once server/discover is dispatched rather than passed through, it advertises supportedVersions ["2026-07-28","2025-11-25"], so a go-sdk v1.7 client negotiates Modern and then opens the subscriptions/listen stream that mcpcompat's Initialize implies (it installs list-changed handlers unconditionally). dispatchModern answers -32601 and the client tears the connection down, breaking every client that wants server->client traffic. See pkg/vmcp/session/factory.go's initOneBackend, which already records "its push channel is subscriptions/listen, which vMCP does not implement" for the backend edge.
Refs #5959. Config.ModernDispatchEnabled (env TOOLHIVE_VMCP_MODERN_STATELESS, default off) was a temporary safety lever gating whether well-formed MCP 2026-07-28 ("Modern") stateless requests were served by classifyingHandler -> dispatchModern or fell through to the SDK path. Both of the issue's gates are now met: the dual-era conformance harness landed in #5837 and go-sdk v1.7 was adopted via #5993. Serve Modern unconditionally and delete the switch, its ServerConfig/Config plumbing, its env var, and the test helpers and specs that existed only to toggle it. This also removes #6009's -32022 UnsupportedVersionError refusal and its server/discover exemption. That refusal answered exactly one condition -- "vMCP supports this revision but has chosen not to serve it" -- which cannot arise once the switch is gone. Every genuine version-grounds rejection still happens, earlier and unchanged, in mcpparser.ClassifyRevision: a _meta protocolVersion naming any other version yields the same conformant -32022, so ClassifyRevision is now the single owner of that decision. server/discover no longer needs an exemption because nothing refuses it on version grounds; dispatchModern answers it via dispatchModernDiscover. authz_integration_test.go's buildCedarAuthzServer had to *set* the switch for TestIntegration_CedarAuthzDenial_ModernPath_IsAudited to reach dispatchModern's call gate at all -- a silently-vacuous test class that disappears along with the switch. KNOWN BLOCKER, do not merge as-is: vMCP's Modern dispatcher does not implement subscriptions/listen. Once server/discover is dispatched rather than passed through, it advertises supportedVersions ["2026-07-28","2025-11-25"], so a go-sdk v1.7 client negotiates Modern and then opens the subscriptions/listen stream that mcpcompat's Initialize implies (it installs list-changed handlers unconditionally). dispatchModern answers -32601 and the client tears the connection down, breaking every client that wants server->client traffic. See pkg/vmcp/session/factory.go's initOneBackend, which already records "its push channel is subscriptions/listen, which vMCP does not implement" for the backend edge.
Refs #5959. Config.ModernDispatchEnabled (env TOOLHIVE_VMCP_MODERN_STATELESS, default off) was a temporary safety lever gating whether well-formed MCP 2026-07-28 ("Modern") stateless requests were served by classifyingHandler -> dispatchModern or fell through to the SDK path. Both of the issue's gates are now met: the dual-era conformance harness landed in #5837 and go-sdk v1.7 was adopted via #5993. Serve Modern unconditionally and delete the switch, its ServerConfig/Config plumbing, its env var, and the test helpers and specs that existed only to toggle it. This also removes #6009's -32022 UnsupportedVersionError refusal and its server/discover exemption. That refusal answered exactly one condition -- "vMCP supports this revision but has chosen not to serve it" -- which cannot arise once the switch is gone. Every genuine version-grounds rejection still happens, earlier and unchanged, in mcpparser.ClassifyRevision: a _meta protocolVersion naming any other version yields the same conformant -32022, so ClassifyRevision is now the single owner of that decision. server/discover no longer needs an exemption because nothing refuses it on version grounds; dispatchModern answers it via dispatchModernDiscover. authz_integration_test.go's buildCedarAuthzServer had to *set* the switch for TestIntegration_CedarAuthzDenial_ModernPath_IsAudited to reach dispatchModern's call gate at all -- a silently-vacuous test class that disappears along with the switch. KNOWN BLOCKER, do not merge as-is: vMCP's Modern dispatcher does not implement subscriptions/listen. Once server/discover is dispatched rather than passed through, it advertises supportedVersions ["2026-07-28","2025-11-25"], so a go-sdk v1.7 client negotiates Modern and then opens the subscriptions/listen stream that mcpcompat's Initialize implies (it installs list-changed handlers unconditionally). dispatchModern answers -32601 and the client tears the connection down, breaking every client that wants server->client traffic. See pkg/vmcp/session/factory.go's initOneBackend, which already records "its push channel is subscriptions/listen, which vMCP does not implement" for the backend edge.
Refs #5959. Config.ModernDispatchEnabled (env TOOLHIVE_VMCP_MODERN_STATELESS, default off) was a temporary safety lever gating whether well-formed MCP 2026-07-28 ("Modern") stateless requests were served by classifyingHandler -> dispatchModern or fell through to the SDK path. Both of the issue's gates are now met: the dual-era conformance harness landed in #5837 and go-sdk v1.7 was adopted via #5993. Serve Modern unconditionally and delete the switch, its ServerConfig/Config plumbing, its env var, and the test helpers and specs that existed only to toggle it. This also removes #6009's -32022 UnsupportedVersionError refusal and its server/discover exemption. That refusal answered exactly one condition -- "vMCP supports this revision but has chosen not to serve it" -- which cannot arise once the switch is gone. Every genuine version-grounds rejection still happens, earlier and unchanged, in mcpparser.ClassifyRevision: a _meta protocolVersion naming any other version yields the same conformant -32022, so ClassifyRevision is now the single owner of that decision. server/discover no longer needs an exemption because nothing refuses it on version grounds; dispatchModern answers it via dispatchModernDiscover. authz_integration_test.go's buildCedarAuthzServer had to *set* the switch for TestIntegration_CedarAuthzDenial_ModernPath_IsAudited to reach dispatchModern's call gate at all -- a silently-vacuous test class that disappears along with the switch. KNOWN BLOCKER, do not merge as-is: vMCP's Modern dispatcher does not implement subscriptions/listen. Once server/discover is dispatched rather than passed through, it advertises supportedVersions ["2026-07-28","2025-11-25"], so a go-sdk v1.7 client negotiates Modern and then opens the subscriptions/listen stream that mcpcompat's Initialize implies (it installs list-changed handlers unconditionally). dispatchModern answers -32601 and the client tears the connection down, breaking every client that wants server->client traffic. See pkg/vmcp/session/factory.go's initOneBackend, which already records "its push channel is subscriptions/listen, which vMCP does not implement" for the backend edge.
Refs #5959. Config.ModernDispatchEnabled (env TOOLHIVE_VMCP_MODERN_STATELESS, default off) was a temporary safety lever gating whether well-formed MCP 2026-07-28 ("Modern") stateless requests were served by classifyingHandler -> dispatchModern or fell through to the SDK path. Both of the issue's gates are now met: the dual-era conformance harness landed in #5837 and go-sdk v1.7 was adopted via #5993. Serve Modern unconditionally and delete the switch, its ServerConfig/Config plumbing, its env var, and the test helpers and specs that existed only to toggle it. This also removes #6009's -32022 UnsupportedVersionError refusal and its server/discover exemption. That refusal answered exactly one condition -- "vMCP supports this revision but has chosen not to serve it" -- which cannot arise once the switch is gone. Every genuine version-grounds rejection still happens, earlier and unchanged, in mcpparser.ClassifyRevision: a _meta protocolVersion naming any other version yields the same conformant -32022, so ClassifyRevision is now the single owner of that decision. server/discover no longer needs an exemption because nothing refuses it on version grounds; dispatchModern answers it via dispatchModernDiscover. authz_integration_test.go's buildCedarAuthzServer had to *set* the switch for TestIntegration_CedarAuthzDenial_ModernPath_IsAudited to reach dispatchModern's call gate at all -- a silently-vacuous test class that disappears along with the switch. KNOWN BLOCKER, do not merge as-is: vMCP's Modern dispatcher does not implement subscriptions/listen. Once server/discover is dispatched rather than passed through, it advertises supportedVersions ["2026-07-28","2025-11-25"], so a go-sdk v1.7 client negotiates Modern and then opens the subscriptions/listen stream that mcpcompat's Initialize implies (it installs list-changed handlers unconditionally). dispatchModern answers -32601 and the client tears the connection down, breaking every client that wants server->client traffic. See pkg/vmcp/session/factory.go's initOneBackend, which already records "its push channel is subscriptions/listen, which vMCP does not implement" for the backend edge.
Refs #5959. Config.ModernDispatchEnabled (env TOOLHIVE_VMCP_MODERN_STATELESS, default off) was a temporary safety lever gating whether well-formed MCP 2026-07-28 ("Modern") stateless requests were served by classifyingHandler -> dispatchModern or fell through to the SDK path. Both of the issue's gates are now met: the dual-era conformance harness landed in #5837 and go-sdk v1.7 was adopted via #5993. Serve Modern unconditionally and delete the switch, its ServerConfig/Config plumbing, its env var, and the test helpers and specs that existed only to toggle it. This also removes #6009's -32022 UnsupportedVersionError refusal and its server/discover exemption. That refusal answered exactly one condition -- "vMCP supports this revision but has chosen not to serve it" -- which cannot arise once the switch is gone. Every genuine version-grounds rejection still happens, earlier and unchanged, in mcpparser.ClassifyRevision: a _meta protocolVersion naming any other version yields the same conformant -32022, so ClassifyRevision is now the single owner of that decision. server/discover no longer needs an exemption because nothing refuses it on version grounds; dispatchModern answers it via dispatchModernDiscover. authz_integration_test.go's buildCedarAuthzServer had to *set* the switch for TestIntegration_CedarAuthzDenial_ModernPath_IsAudited to reach dispatchModern's call gate at all -- a silently-vacuous test class that disappears along with the switch. A blocker documented when this commit was first written -- dispatchModern had no subscriptions/listen case, so once server/discover advertised Modern, a go-sdk v1.7 client negotiated 2026-07-28 and tore its connection down when the listen stream it opens (mcpcompat's Initialize installs list-changed handlers unconditionally) was answered -32601 -- was closed by #6050 (modern_subscriptions.go and Modern list pagination) before this landed: this commit sits on a base that already serves subscriptions/listen.
Summary
Part of #5743 (Workstream C). Advances #5754.
The goal is support for the new MCP spec revision, 2026-07-28 ("stateless") — not a Go SDK for its own sake. Bumping
toolhive-coreto v0.0.34 pulls in go-sdk v1.7.0-pre.3, the SDK that implements 2026-07-28, so vMCP can negotiate and speak both revisions per backend (Legacy 2025-11-25 + Modern 2026-07-28). The build is unchanged (themcpcompatshim absorbs the go-sdk API change); this PR fixes the behavioral gaps that only surface once the real v1.7 SDK is imported, without regressing 2025-11-25 (dual-revision is a hard requirement).What changed
server/discoverresult advertises2026-07-28insupportedVersions— the SEP-2575 negotiation signal, and exactly what go-sdk's own reference client keys on (mcp/client.go:428-444). The old "any cleanserver/discover⇒ Modern" heuristic mis-classified stateful 2025-11-25 backends, which under v1.7 answer discover too (negotiating down) → their Legacytools/listwas parsed as Modern → sessions died. New sentinelerrModernNegotiatedDownroutes the negotiate-down case through the existing reclassify-on-mismatch machinery (retry-safe: only read-only discover/list can produce it)._metaegress strip. Strip the per-hopio.modelcontextprotocol/*control keys at both Legacy backend tool-call egresses (httpBackendClient.CallTool,mcpSession.CallTool) — a stateful backend 400s them under v1.7. Exact keys only, copy-before-mutate, no-op for Legacy.server/discover-first over SSE (parser assertion made transport-aware); the SSE-forwarding fake now answers discover with-32601; a nil-registry test-construction fix; and new real-backend pin tests assertingstateful → Legacy/stateless → Modernso a future SDK discover-semantics change fails loudly here, not as mystery failures.ponytail:comments toNOTE:.Type of change
Test plan
task test)task lint-fix)The full suite goes from 137 failures to exactly the 3 pre-existing, sandbox-environment failures (no container runtime ×2, no keyring ×1 — none in touched packages). New/changed tests pass under
-race.Special notes for reviewers
SupportsProtocolVersionstateless gate atmcp/streamable.go:871, thesupportedVersionsJSON tag, the reference-client fallback). Verdict: SHIP / SHIP-WITH-NITS ×2, no MUST-FIX. All actionable nits addressed (added aStripReservedModernMetaunit test, a version-exact-match tripwire comment, and filed the cache-staleness follow-up).probeRevisiondeliberately builds its own discover client (rather than delegating) so a transport-misconfiguration hard-fails instead of being mis-cached as Legacy — see the doc comment.🤖 Generated with Claude Code