Skip to content

Commit beb5d3f

Browse files
JAORMXclaude
andcommitted
Make forwarding tests' Legacy dependency explicit
The server->client forwarding integration tests (mid-call elicitation, sampling, progress, logging) exercise a surface that exists only on a Legacy (2025-11-25) session: the 2026-07-28 revision removed server-initiated requests, replacing them with client-polled multi-round retrieval (SEP-2322), and SEP-2577 deprecates sampling and logging outright. Today these tests land on Legacy only incidentally, because the Modern dispatch kill-switch (#5959) rejects the go-sdk client's Modern-first probe; once the switch is removed (#6033) the client negotiates Modern and all of them fail at connect. Pin the downstream test clients to Legacy explicitly via a transport-level RoundTripper that answers server/discover with the same -32022 rejection the kill-switch produces (mcpcompat cannot pin a protocol version, #5911), and pin the Modern-client contract for the same eliciting backend in a new integration test: an explicit JSON-RPC error naming the refused request, never a hang, a fabricated success, or an input_required envelope. Document the client-edge limitation and the intended future MRTR shape (#5743) in the vMCP architecture doc. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GvBj7nNWqDQ3rKYNeY6aam
1 parent 57e681f commit beb5d3f

3 files changed

Lines changed: 245 additions & 4 deletions

File tree

docs/arch/10-virtual-mcp-architecture.md

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,68 @@ multi-round tool retrieval (MRTR), so the fix is shaped MRTR-first rather than b
289289
extending the SSE standalone-stream model — see the epic (#5743) and the
290290
mid-call forwarding section below for the Legacy behaviour this contrasts with.
291291

292+
### Limitation: elicitation and sampling are unavailable to Modern clients
293+
294+
The client edge mirrors the backend edge. The Modern dispatcher
295+
(`pkg/vmcp/server`'s `dispatchModern`) is single-shot: every result it builds is
296+
`resultType: "complete"`, and it never emits `"input_required"` — MRTR
297+
(SEP-2322) is unimplemented on this edge too. When a backend tool issues a
298+
mid-call server-initiated request during a **Modern** client's `tools/call`,
299+
there is no client session to forward it to, so the call fails with an explicit
300+
`-32603` whose message names the refused request (pinned by
301+
`TestIntegration_Modern_RealBackend_ElicitingToolFailsCleanly`). This is a
302+
deliberate honest-unsupported error, not a gap left by accident:
303+
304+
- The 2026-07-28 revision **removed** server-initiated requests; go-sdk's
305+
`ServerSession.assertServerInitiatedRequestAllowed` refuses
306+
elicitation/sampling/roots purely by negotiated protocol version, so no
307+
capability negotiation can restore the Legacy forwarding model for Modern
308+
clients.
309+
- A server that never returns `input_required` is fully SEP-2575-conformant:
310+
the per-request `clientCapabilities` a client declares are an offer the
311+
server may use, not an obligation.
312+
- SEP-2577 deprecates sampling (and logging and roots) outright as of
313+
2026-07-28, with direct LLM-provider integration as the sanctioned
314+
replacement — so elicitation is the only durable consumer a future MRTR
315+
implementation would serve.
316+
317+
Legacy clients keep the full mid-call forwarding behaviour unchanged; the
318+
forwarding integration tests pin their downstream clients to Legacy explicitly
319+
(`legacyPinningRoundTripper` in `pkg/vmcp/server`) because that surface exists
320+
only on a Legacy session.
321+
322+
**Bridging was considered, costed, and rejected — this is a design position,
323+
not a backlog item.** Serving MRTR to Modern clients on top of a *Legacy*
324+
backend would require parking the live, mid-flight backend call server-side
325+
(the blocked goroutine and its open session cannot be serialized into the
326+
opaque `requestState` the SEP designed for handler re-invocation) and keying
327+
the resume on an unguessable token — per-round server state with TTL/eviction,
328+
identity binding on a token that becomes a capability to resume someone else's
329+
in-flight call, and replica affinity with no `Mcp-Session-Id` to route on. That
330+
is a session in disguise: exactly the construct the 2026-07-28 revision
331+
removed. The spec's own sanctioned path for genuinely stateful
332+
`input_required` work is the **Tasks** extension (SEP-1686: `tools/call`
333+
returns a `taskId`; the client polls `tasks/get`/`tasks/result` and answers via
334+
`tasks/input_response`) — if Modern-client elicitation over Legacy backends is
335+
ever truly demanded, that is the machinery to reach for, not parked
336+
`tools/call`.
337+
338+
The coherent future MRTR shape for a re-aggregating gateway is
339+
**Modern-client ↔ Modern-backend pass-through** — relay a Modern backend's
340+
`inputRequests`/`requestState` to the client and the client's
341+
`inputResponses` back, genuinely stateless at vMCP. It requires the egress
342+
half first (today a Modern backend's `input_required` surfaces as
343+
`errModernInputRequired`, the seam left in `pkg/vmcp/client`), and by the time
344+
Modern backends exist to relay from, SEP-2577's deprecations make elicitation
345+
its only durable consumer; see #5743.
346+
347+
Progress and log notifications toward Modern clients are a separate concern
348+
from MRTR: they remain spec-legal as request-scoped notifications on the
349+
POST-initiated SSE response stream (SEP-2260 requires messages on that stream
350+
to relate to the originating request; `progressToken` is unchanged), which the
351+
single-shot dispatcher does not produce today — a vMCP streaming-dispatch gap,
352+
not a spec absence.
353+
292354
## Served MCP Capabilities
293355

294356
Beyond tools, vMCP aggregates and serves the full complement of MCP capabilities. Every served capability flows through the domain **core** (`pkg/vmcp/core`), so the same admission decision that filters `tools/list` also gates reads, gets, and completions.

pkg/vmcp/server/forwarding_realbackend_integration_test.go

Lines changed: 134 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44
package server_test
55

66
import (
7+
"bytes"
78
"context"
9+
"encoding/json"
10+
"io"
811
"net/http"
912
"net/http/httptest"
1013
"sync/atomic"
@@ -18,12 +21,18 @@ import (
1821
"github.com/stacklok/toolhive-core/mcpcompat/client/transport"
1922
mcpmcp "github.com/stacklok/toolhive-core/mcpcompat/mcp"
2023
mcpserver "github.com/stacklok/toolhive-core/mcpcompat/server"
24+
mcpparser "github.com/stacklok/toolhive/pkg/mcp"
2125
)
2226

2327
// Forwarding integration fixtures. These exercise the server->client forwarding
2428
// cluster end-to-end: a real in-process backend, mid tools/call, drives
2529
// elicitation/sampling/progress/logging back at its caller; vMCP must relay that
26-
// traffic to the real downstream client on the same session.
30+
// traffic to the real downstream client on the same session. That session is
31+
// necessarily Legacy (2025-11-25): the Modern revision removed server-initiated
32+
// requests, so the downstream clients here are explicitly pinned to Legacy —
33+
// see legacyPinningRoundTripper. The Modern-client behavior for the same
34+
// eliciting backend is asserted separately in
35+
// TestIntegration_Modern_RealBackend_ElicitingToolFailsCleanly.
2736
const (
2837
fwdElicitTool = "elicit_tool"
2938
fwdSampleTool = "sample_tool"
@@ -130,9 +139,73 @@ func startForwardingBackend(t *testing.T) string {
130139
return ts.URL + "/mcp"
131140
}
132141

142+
// legacyPinningRoundTripper pins a downstream mcpcompat client to the Legacy
143+
// (2025-11-25) revision by answering its Modern-first server/discover probe
144+
// with the same HTTP 400 / JSON-RPC -32022 UnsupportedProtocolVersionError
145+
// body vMCP's kill-switch produces (reusing the production
146+
// mcpparser.ClassificationErrorResponse builder), which drives go-sdk's
147+
// Connect down its documented fall-back-to-Legacy-initialize path. Every
148+
// other request passes through to base untouched.
149+
//
150+
// WHY PIN: these fixtures assert the Legacy-only server-initiated surface —
151+
// mid-call elicitation/sampling requests and progress/logging notifications
152+
// relayed over a live session. The 2026-07-28 revision removes server-
153+
// initiated requests entirely (go-sdk's assertServerInitiatedRequestAllowed
154+
// refuses them by protocol version; the replacement is client-polled
155+
// multi-round retrieval, SEP-2322) and SEP-2577 deprecates sampling and
156+
// logging outright. While the Modern dispatch kill-switch (#5959) is on, vMCP
157+
// itself rejects the probe and these clients land on Legacy incidentally;
158+
// once it is removed (#6033) the go-sdk-based client would negotiate Modern
159+
// and this surface would vanish mid-test — failing at connect on
160+
// subscriptions/listen, not at the behavior under test. Pinning makes the
161+
// tests' Legacy dependency explicit instead of incidental.
162+
//
163+
// The pin lives in the transport because mcpcompat documents it cannot set a
164+
// protocol version (go-sdk's ClientSessionOptions.protocolVersion is
165+
// unexported — see the LIMITATION note in mcpcompat/client and #5911).
166+
// Replace this with a real client option if #5911 lands.
167+
//
168+
// LOAD-BEARING after #6033: once the kill-switch is gone, this RoundTripper
169+
// is the ONLY thing keeping these downstream clients on Legacy. It is not
170+
// leftover kill-switch scaffolding — deleting it silently flips every test
171+
// in this file to Modern and voids what they assert.
172+
type legacyPinningRoundTripper struct{ base http.RoundTripper }
173+
174+
func (rt *legacyPinningRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
175+
if req.Method != http.MethodPost || req.Body == nil {
176+
return rt.base.RoundTrip(req)
177+
}
178+
body, err := io.ReadAll(req.Body)
179+
_ = req.Body.Close()
180+
if err != nil {
181+
return nil, err
182+
}
183+
req.Body = io.NopCloser(bytes.NewReader(body))
184+
var probe struct {
185+
ID any `json:"id"`
186+
Method string `json:"method"`
187+
}
188+
if json.Unmarshal(body, &probe) == nil && probe.Method == "server/discover" {
189+
return mcpparser.ClassificationErrorResponse(req, probe.ID, &mcpparser.UnsupportedVersionError{
190+
Requested: mcpparser.MCPVersionModern,
191+
Supported: []string{mcpparser.MCPVersionLegacy},
192+
}), nil
193+
}
194+
return rt.base.RoundTrip(req)
195+
}
196+
197+
// legacyPinnedHTTPClient returns an *http.Client for
198+
// transport.WithHTTPBasicClient that pins the mcpcompat client to Legacy —
199+
// see legacyPinningRoundTripper.
200+
func legacyPinnedHTTPClient() *http.Client {
201+
return &http.Client{Transport: &legacyPinningRoundTripper{base: http.DefaultTransport}}
202+
}
203+
133204
// downstreamClient builds a real mcpcompat client against the vMCP endpoint,
134205
// wired for server->client traffic (elicitation/sampling handlers, continuous
135206
// listening) and collecting forwarded notifications on the returned channel.
207+
// It is pinned to the Legacy revision (see legacyPinningRoundTripper): the
208+
// forwarded traffic it collects exists only on a Legacy session.
136209
type downstreamClient struct {
137210
c *client.Client
138211
notifCh chan mcpmcp.JSONRPCNotification
@@ -185,7 +258,10 @@ func newDownstreamClient(ctx context.Context, t *testing.T, vmcpURL string, with
185258

186259
c, err := client.NewStreamableHttpClientWithOpts(
187260
vmcpURL,
188-
[]transport.StreamableHTTPCOption{transport.WithContinuousListening()},
261+
[]transport.StreamableHTTPCOption{
262+
transport.WithContinuousListening(),
263+
transport.WithHTTPBasicClient(legacyPinnedHTTPClient()),
264+
},
189265
clientOpts,
190266
)
191267
require.NoError(t, err)
@@ -235,6 +311,11 @@ func (dc *downstreamClient) waitNotification(ctx context.Context, t *testing.T,
235311
// TestForwarding_Elicitation_RealBackend verifies a backend's mid-call
236312
// elicitation/create is relayed to the downstream client and its response
237313
// carried back into the tool result.
314+
//
315+
// Legacy-pinned: a server-initiated elicitation/create toward the client does
316+
// not exist on Modern (2026-07-28) — SEP-2322 replaces it with client-polled
317+
// inputRequests, which vMCP deliberately does not serve (see the "unavailable
318+
// to Modern clients" limitation in docs/arch/10-virtual-mcp-architecture.md).
238319
func TestForwarding_Elicitation_RealBackend(t *testing.T) {
239320
t.Parallel()
240321
ctx, cancel := context.WithTimeout(t.Context(), forwardingRealBackendTimeout)
@@ -265,6 +346,11 @@ func TestForwarding_Elicitation_RealBackend(t *testing.T) {
265346
// TestForwarding_Sampling_RealBackend verifies a backend's mid-call
266347
// sampling/createMessage is relayed to the downstream client and the sampled
267348
// message carried back into the tool result.
349+
//
350+
// Legacy-pinned: like elicitation, server-initiated sampling does not exist on
351+
// Modern (SEP-2322) — and SEP-2577 additionally deprecates the sampling
352+
// feature itself as of 2026-07-28 (sanctioned replacement: direct LLM-provider
353+
// integration), so this surface is Legacy-only by spec, not by omission.
268354
func TestForwarding_Sampling_RealBackend(t *testing.T) {
269355
t.Parallel()
270356
ctx, cancel := context.WithTimeout(t.Context(), forwardingRealBackendTimeout)
@@ -288,6 +374,17 @@ func TestForwarding_Sampling_RealBackend(t *testing.T) {
288374
// TestForwarding_Progress_RealBackend verifies a backend's mid-call
289375
// notifications/progress is relayed to the downstream client, arriving before
290376
// the tool result is read.
377+
//
378+
// Legacy-pinned — but NOT because Modern lacks a progress channel. It does
379+
// not: on Modern, progress remains spec-legal as a request-scoped
380+
// notification riding the POST-initiated SSE response stream (SEP-2260
381+
// tightened exactly that channel; progressToken is unchanged in the draft
382+
// schema). What is missing is on vMCP's side: dispatchModern is single-shot —
383+
// writeModernResult emits one JSON body and cannot stream — so nothing riding
384+
// the per-request stream is deliverable to a Modern client today. That is a
385+
// vMCP "streaming Modern dispatch" gap (future work), distinct from both MRTR
386+
// and subscriptions/listen (whose subscribable set is fixed at four
387+
// list-changed/subscription types and excludes progress).
291388
func TestForwarding_Progress_RealBackend(t *testing.T) {
292389
t.Parallel()
293390
ctx, cancel := context.WithTimeout(t.Context(), forwardingRealBackendTimeout)
@@ -312,6 +409,22 @@ func TestForwarding_Progress_RealBackend(t *testing.T) {
312409
// TestForwarding_Logging_RealBackend verifies that vMCP requests debug logging
313410
// from the backend (so it emits) and relays the backend's notifications/message
314411
// to the downstream client, which has itself set a logging level.
412+
//
413+
// Legacy-pinned, for three stacked reasons — record them so this is not
414+
// re-litigated as a vMCP bug:
415+
// 1. Upstream go-sdk defect: SetLoggingLevel omits injectRequestMeta
416+
// (client.go:1303 in v1.7.0-pre.3; every other Modern-aware method
417+
// injects), so a Modern logging/setLevel carries the MCP-Protocol-Version
418+
// header but no _meta.protocolVersion. vMCP's ClassifyRevision CORRECTLY
419+
// rejects that with -32020; accepting the malformed request to make this
420+
// test pass would be worse than the failure. Filed upstream separately.
421+
// 2. Even with (1) fixed, Modern log delivery is gated on the per-request
422+
// logLevel _meta key (SEP-2575: absent means the server must not send log
423+
// notifications for that request) and rides the same per-request SSE
424+
// response stream the single-shot dispatcher cannot produce — the same
425+
// streaming gap as TestForwarding_Progress_RealBackend.
426+
// 3. SEP-2577 deprecates the logging feature itself as of 2026-07-28
427+
// (sanctioned replacement: stderr / OpenTelemetry).
315428
func TestForwarding_Logging_RealBackend(t *testing.T) {
316429
t.Parallel()
317430
ctx, cancel := context.WithTimeout(t.Context(), forwardingRealBackendTimeout)
@@ -340,6 +453,14 @@ func TestForwarding_Logging_RealBackend(t *testing.T) {
340453
// when the downstream client did not advertise sampling, the backend's
341454
// sampling request fails cleanly (a failed tools/call) rather than hanging until
342455
// the deadline.
456+
//
457+
// Legacy-pinned even though it happens to pass on Modern: capability-gated
458+
// failure on a live session only exists on Legacy. On Modern the same call
459+
// fails with the sessionless -32603 REGARDLESS of what the client advertises
460+
// (withHandlers makes no difference), so the lenient assertion here would be
461+
// satisfied for a reason unrelated to the gating this test exists to
462+
// exercise. The pin keeps it testing what its name says. (Verified against a
463+
// working subscriptions/listen build: unpinned, this test passes vacuously.)
343464
func TestForwarding_Sampling_NoDownstreamCapability(t *testing.T) {
344465
t.Parallel()
345466
ctx, cancel := context.WithTimeout(t.Context(), forwardingRealBackendTimeout)
@@ -365,6 +486,7 @@ func TestForwarding_Sampling_NoDownstreamCapability(t *testing.T) {
365486
// TestForwarding_Sampling_NoDownstreamCapability: when the downstream client did
366487
// not advertise elicitation, the backend's mid-call elicitation/create fails
367488
// cleanly (a failed tools/call) rather than hanging until the deadline.
489+
// Legacy-pinned for the same vacuous-pass reason as its sampling twin above.
368490
func TestForwarding_Elicitation_NoDownstreamCapability(t *testing.T) {
369491
t.Parallel()
370492
ctx, cancel := context.WithTimeout(t.Context(), forwardingRealBackendTimeout)
@@ -397,14 +519,18 @@ type samplingClient struct {
397519
// newSamplingClient connects a downstream client to vmcpURL whose sampling
398520
// handler always answers with the given summary and model (so the tool result
399521
// distinguishes which client's handler served the request) and increments a
400-
// per-client counter.
522+
// per-client counter. Pinned to Legacy like newDownstreamClient — see
523+
// legacyPinningRoundTripper.
401524
func newSamplingClient(ctx context.Context, t *testing.T, vmcpURL, summary, model string) *samplingClient {
402525
t.Helper()
403526

404527
sc := &samplingClient{}
405528
c, err := client.NewStreamableHttpClientWithOpts(
406529
vmcpURL,
407-
[]transport.StreamableHTTPCOption{transport.WithContinuousListening()},
530+
[]transport.StreamableHTTPCOption{
531+
transport.WithContinuousListening(),
532+
transport.WithHTTPBasicClient(legacyPinnedHTTPClient()),
533+
},
408534
[]client.ClientOption{
409535
client.WithSamplingHandler(client.SamplingHandlerFunc(
410536
func(_ context.Context, _ mcpmcp.CreateMessageRequest) (*mcpmcp.CreateMessageResult, error) {
@@ -448,6 +574,10 @@ func newSamplingClient(ctx context.Context, t *testing.T, vmcpURL, summary, mode
448574
// call's sampling request were relayed to B's session, A's tool result would show
449575
// "summary-B" (mismatching the "summary-A" assertion) and the handler counters
450576
// would be lopsided (A=0, B=2 instead of 1/1) — either check trips.
577+
//
578+
// Legacy-pinned for the same reasons as TestForwarding_Sampling_RealBackend:
579+
// per-session routing of a server-initiated request presupposes sessions,
580+
// which Modern removed along with the requests themselves.
451581
func TestForwarding_Sampling_RealBackend_SessionIsolation(t *testing.T) {
452582
t.Parallel()
453583
ctx, cancel := context.WithTimeout(t.Context(), forwardingRealBackendTimeout)

pkg/vmcp/server/modern_realbackend_integration_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,55 @@ func TestIntegration_Modern_RealBackend_UnknownMethod(t *testing.T) {
306306
assert.EqualValues(t, -32601, errObj["code"])
307307
}
308308

309+
// TestIntegration_Modern_RealBackend_ElicitingToolFailsCleanly pins the
310+
// Modern-client behavior for a backend tool that issues a mid-call
311+
// server-initiated request (elicitation/create, sampling/createMessage): the
312+
// call MUST resolve promptly to an explicit JSON-RPC error naming the refused
313+
// request — never a hang, never a fabricated success, and never a resultType
314+
// "input_required" envelope, which this dispatcher does not emit (client-polled
315+
// multi-round retrieval, SEP-2322, is unimplemented; modernResultTypeComplete
316+
// is the only resultType modern_envelope.go builds).
317+
//
318+
// This is the honest-unsupported contract for the surface the 2026-07-28
319+
// revision removed: server-initiated requests need a live session, Modern
320+
// requests have none by design, and SEP-2577 additionally deprecates sampling
321+
// outright. The Legacy-session behavior for the same backend tools is covered
322+
// by the TestForwarding_* fixtures, whose downstream clients pin Legacy
323+
// explicitly (see legacyPinningRoundTripper).
324+
func TestIntegration_Modern_RealBackend_ElicitingToolFailsCleanly(t *testing.T) {
325+
t.Parallel()
326+
327+
backendURL := startForwardingBackend(t)
328+
ts := newRealModernTestServer(t, backendURL)
329+
330+
tests := []struct {
331+
name string
332+
tool string
333+
want string // substring naming the refused server-initiated request
334+
}{
335+
{name: "elicitation", tool: fwdElicitTool, want: "elicitation/create"},
336+
{name: "sampling", tool: fwdSampleTool, want: "sampling/createMessage"},
337+
}
338+
for _, tc := range tests {
339+
t.Run(tc.name, func(t *testing.T) {
340+
t.Parallel()
341+
resp, decoded := postModern(t, ts.URL, "tools/call", map[string]any{"name": tc.tool}, 1, tc.tool)
342+
defer resp.Body.Close()
343+
344+
// A -32603 rides HTTP 200 per writeModernError's status mapping: the
345+
// request was accepted and processed; the failure is application-level.
346+
require.Equal(t, http.StatusOK, resp.StatusCode, "decoded: %+v", decoded)
347+
require.NotContains(t, decoded, "result",
348+
"must not fabricate a success or an input_required envelope: %+v", decoded)
349+
errObj, ok := decoded["error"].(map[string]any)
350+
require.True(t, ok, "decoded: %+v", decoded)
351+
assert.EqualValues(t, -32603, errObj["code"])
352+
assert.Contains(t, errObj["message"], tc.want,
353+
"the error must name the refused server-initiated request")
354+
})
355+
}
356+
}
357+
309358
// TestIntegration_Modern_RealBackend_MalformedArguments verifies a
310359
// syntactically valid tools/call whose "arguments" is present but not a JSON
311360
// object is rejected with 400 + JSON-RPC -32602, per hasNonObjectArguments'

0 commit comments

Comments
 (0)