diff --git a/docs/arch/10-virtual-mcp-architecture.md b/docs/arch/10-virtual-mcp-architecture.md index 70525f3c4d..766d138444 100644 --- a/docs/arch/10-virtual-mcp-architecture.md +++ b/docs/arch/10-virtual-mcp-architecture.md @@ -806,6 +806,32 @@ for what a Modern caller gets instead. **Known limitation (logging level)**: forwarded backend logging is not yet filtered to the downstream client's requested level. On Legacy, vMCP requests debug-level logging from the backend (`logging/setLevel`) so it emits `notifications/message`, and every such notification is forwarded — the downstream client's own `logging/setLevel` preference is not applied to the relayed stream. The same is true on Modern (2026-07-28), where the RPC is removed and the level rides per-request in `_meta["io.modelcontextprotocol/logLevel"]`: vMCP strips that reserved per-hop key from the downstream request and overlays its own (`debug`, when forwarding is bound) on the backend hop, so a Modern client's per-request level preference is likewise not honored — the relay runs at debug either way. +**Known limitation (advertised-but-no-stream elicitation fails fast)**: a client +that advertised the `elicitation` capability but holds **no open standalone SSE +stream** passes go-sdk's capability gate, yet the elicitation cannot be +delivered — under the shim's `JSONResponse` transport the go-sdk routes +server→client requests to the standalone stream, and a missing stream rejects +the write ("rejected by transport: stream not connected or already closed"). +The mid-call `tools/call` therefore fails fast with a tool error instead of +hanging to the deadline (pinned by +`TestForwarding_Elicitation_AdvertisedButNoStream_FastFails` in +`pkg/vmcp/server`). A cleaner pre-dispatch refusal awaits an upstream mcpcompat +accessor for stream presence (#5975). + +**Known limitation (cross-pod origination needs session affinity)**: a +server→client request can only be delivered by the replica currently holding +the client's standalone SSE stream. If the `tools/call` executes on replica A +but the client's GET stream is pinned to replica B, an elicitation or sampling +request originated from A cannot reach the client — the shim loads the go-sdk +session bound to *its* pod and has no cross-replica delivery channel for +request/response (only notifications rehydrate cross-replica). Multi-replica +deployments that rely on mid-call elicitation/sampling therefore need **session +affinity** at the load balancer pinning the standalone stream and the tool +calls to the same replica. The durable fix is the 2026-07-28 revision itself: +it replaces server-initiated requests with client-polled MRTR, which has no +stream-locality requirement — so this constraint is documented rather than +engineered around (#5975, #5743). + **Known limitation (resource-template authorization)**: a resource template is advertised on the template-string entity (e.g. `file:///logs/{date}.txt`), but a concrete read is admission-checked on the **expanded** URI (e.g. `file:///logs/2025-01-01.txt`). Operators should therefore author resource authorization policies against concrete URI patterns, not the template string. ## Two-Boundary Authentication diff --git a/pkg/vmcp/server/forwarding_realbackend_integration_test.go b/pkg/vmcp/server/forwarding_realbackend_integration_test.go index 3d7b2ec435..000caea625 100644 --- a/pkg/vmcp/server/forwarding_realbackend_integration_test.go +++ b/pkg/vmcp/server/forwarding_realbackend_integration_test.go @@ -286,6 +286,16 @@ type downstreamClient struct { // registers an OnNotification collector. func newDownstreamClient(ctx context.Context, t *testing.T, vmcpURL string, withHandlers bool) *downstreamClient { t.Helper() + return newDownstreamClientOpts(ctx, t, vmcpURL, withHandlers, true) +} + +// newDownstreamClientOpts is newDownstreamClient with explicit control over +// the standalone SSE stream (listen): pass listen=false to model a client that +// advertises elicitation/sampling but never opens the standalone stream. +func newDownstreamClientOpts( + ctx context.Context, t *testing.T, vmcpURL string, withHandlers, listen bool, +) *downstreamClient { + t.Helper() dc := &downstreamClient{ notifCh: make(chan mcpmcp.JSONRPCNotification, 8), @@ -325,12 +335,15 @@ func newDownstreamClient(ctx context.Context, t *testing.T, vmcpURL string, with } hc, pinRT := newLegacyPinnedHTTPClient() + transportOpts := []transport.StreamableHTTPCOption{ + transport.WithHTTPBasicClient(hc), + } + if listen { + transportOpts = append(transportOpts, transport.WithContinuousListening()) + } c, err := client.NewStreamableHttpClientWithOpts( vmcpURL, - []transport.StreamableHTTPCOption{ - transport.WithContinuousListening(), - transport.WithHTTPBasicClient(hc), - }, + transportOpts, clientOpts, ) require.NoError(t, err) @@ -344,6 +357,17 @@ func newDownstreamClient(ctx context.Context, t *testing.T, vmcpURL string, with }) require.NoError(t, c.Start(ctx)) + if !listen { + // Force-close this downstream client's idle keep-alive connections at + // teardown. Registration order matters: t.Cleanup runs + // last-added-first, and c.Close() sends the session-terminating DELETE + // over hc, opening a fresh connection that goes idle — so + // CloseIdleConnections must be registered FIRST to run AFTER c.Close(). + // Note the vMCP server holds its own backend-client connection, so the + // suite's ~30s httptest.Server.Close stall persists regardless; this + // only covers the connection this helper owns. + t.Cleanup(hc.CloseIdleConnections) + } t.Cleanup(func() { _ = c.Close() }) _, err = c.Initialize(ctx, mcpmcp.InitializeRequest{ @@ -572,6 +596,51 @@ func TestForwarding_Elicitation_NoDownstreamCapability(t *testing.T) { assert.True(t, res.IsError, "backend elicitation must fail when downstream lacks the capability") } +// TestForwarding_Elicitation_AdvertisedButNoStream_FastFails pins the runtime +// twin of TestForwarding_Elicitation_NoDownstreamCapability (#5975): a client +// that ADVERTISED the elicitation capability but holds NO open standalone SSE +// stream passes go-sdk's capability gate, yet the elicitation cannot be +// delivered — under JSONResponse the go-sdk routes server->client requests to +// the standalone stream, and a missing stream rejects the write +// ("rejected by transport: stream not connected or already closed"). +// +// Legacy-pinned like its siblings, for the same vacuous-pass reason: on Modern +// the call fails with the sessionless error regardless of stream state, so a +// Modern run would satisfy the assertions without exercising the delivery +// path this test exists for. +// +// The assertion is timing-structural, not string-matching: with a generous +// outer deadline, the call must fail as a tool error FAR below it (a hang-to- +// timeout regression blows the full deadline instead). This documents and pins +// the fail-fast until an upstream mcpcompat stream-presence accessor lets vMCP +// fail before dispatch with a cleaner error. +func TestForwarding_Elicitation_AdvertisedButNoStream_FastFails(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(t.Context(), forwardingRealBackendTimeout) + defer cancel() + + backendURL := startForwardingBackend(t) + vmcpTS := newRealTestServer(t, backendURL) + // withHandlers=true (capability advertised) but listen=false (no stream). + dc := newDownstreamClientOpts(ctx, t, vmcpTS.URL+"/mcp", true, false) + + start := time.Now() + res, err := dc.c.CallTool(ctx, mcpmcp.CallToolRequest{ + Params: mcpmcp.CallToolParams{Name: fwdElicitTool}, + }) + elapsed := time.Since(start) + + // Same structural shape as the capability-gate twin — see the rationale + // there — plus the timing assertion that distinguishes fail-fast from hang. + require.NoError(t, err, + "the call must round-trip on the live session, not die at transport level") + require.NotNil(t, res) + assert.True(t, res.IsError, + "backend elicitation must fail when the downstream holds no standalone stream") + assert.Less(t, elapsed, forwardingRealBackendTimeout/4, + "elicitation without a standalone stream must fail fast, not hang to the deadline") +} + // samplingClient is a downstream client whose sampling handler returns a // DISTINGUISHABLE summary+model and counts its own invocations, used to prove // per-session isolation of forwarded server->client sampling.