From 1358c221c6c8c59fcc4f15c647111a7828e82e21 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Mon, 27 Jul 2026 20:56:19 +0000 Subject: [PATCH 01/11] Design MRTR for the vMCP Modern path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2026-07-28 revision removes server-initiated requests outright and replaces them with client-polled Multi Round-Trip Requests (SEP-2322). vMCP has no MRTR implementation: elicitation and sampling forward only over Legacy sessions, and the Modern egress shim declares empty clientCapabilities, so a compliant Modern backend that needs input answers -32021 and the call surfaces as an opaque protocol error. With the Modern dispatch kill-switch being removed (#5959/#6033), that gap becomes the served behavior. Add the design the closed-as-not-planned #5759 asked for, as its own document so it composes with the in-flight edits to doc 10 (#6050, #6051). Decisions, grounded in the spec text read one day before the revision's finalization and in RFC-0083: - Modern-client/Modern-backend rounds are a stateless pass-through: per-request capability mirroring, verbatim inputRequests/requestState relay, deterministic re-routing of the retry. vMCP adds no state, so no durable store is triggered. - Legacy-client/Modern-backend rounds bridge in-request: vMCP fulfills the backend's inputRequests through the existing ElicitationRequester/SamplingRequester seams and retries the backend call itself, bounded, on one pod — the same bridge go-sdk's own serverMultiRoundTripMiddleware performs for its handlers. - Modern-client/Legacy-backend stays deliberately unbridged, per the costed rejection recorded in doc 10; the Tasks extension is the sanctioned stateful path. - Only composite-workflow suspend/resume needs durable state, and it takes RFC-0083 D5 verbatim: an opaque >=128-bit handle as the client-visible requestState, WorkflowStatus.Owner bound to plaintext (iss, sub) via session/binding.Format, owner-checked resume, TTL, audit redaction, Redis store behind a core.Config seam. - Sampling gets no feature work: SEP-2577 deprecates it, and the relay is type-agnostic, so it transits for existing counterparts only. Roots additionally has no existing vMCP counterpart and is refused on the bridge cell. The gap list names what the pinned go-sdk/mcpcompat cannot express (mcpcompat drops inputRequests/requestState in both directions; the SDK's MRTR internals are unexported), phrased for toolhive-core. Refs #5743, #5759, #6018 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n --- docs/arch/16-vmcp-mrtr.md | 352 ++++++++++++++++++++++++++++++++++++++ docs/arch/README.md | 7 + 2 files changed, 359 insertions(+) create mode 100644 docs/arch/16-vmcp-mrtr.md diff --git a/docs/arch/16-vmcp-mrtr.md b/docs/arch/16-vmcp-mrtr.md new file mode 100644 index 0000000000..55d06e4efc --- /dev/null +++ b/docs/arch/16-vmcp-mrtr.md @@ -0,0 +1,352 @@ +# vMCP Multi Round-Trip Requests (MRTR) + +Design for serving MCP 2026-07-28 Multi Round-Trip Requests (SEP-2322) through +the Virtual MCP Server — the Modern replacement for the server-initiated +elicitation/sampling forwarding that exists only on Legacy (2025-11-25) +sessions. This is the design issue #5759 asked for and the "coherent future +MRTR shape" the client-edge limitation section of +[10-virtual-mcp-architecture.md](10-virtual-mcp-architecture.md) names. + +Decision framing follows RFC-0083 (`stacklok/toolhive-rfcs`, +`rfcs/THV-0083-stateless-vmcp.md`), which defers MRTR and fixes the +workflow-handle design in its D5; this document sequences D5's enforcement +machinery and specifies the MRTR flows that trigger it. + +**Spec sources.** Written against the 2026-07-28 revision as published in +`modelcontextprotocol/modelcontextprotocol@main` on 2026-07-27, one day before +the revision's finalization date, when it still lived at `schema/draft` +(`LATEST_PROTOCOL_VERSION = "2026-07-28"`): `schema/draft/schema.ts`, +`/specification/draft/basic/patterns/mrtr`, SEP-2322 (Status: Final), SEP-2577 +(Status: Final), and the draft changelog. Anything cited from those sources +should be re-verified against the final text once cut; the MRTR SEP and schema +types were already Final at time of writing. + +## Protocol recap (what MRTR actually is) + +The 2026-07-28 revision removes server-initiated requests: "Servers **MUST** +send server-to-client requests (such as `roots/list`, `sampling/createMessage`, +or `elicitation/create`) using the MRTR pattern. The previous pattern of +server-initiated requests is no longer supported." (spec, `basic/patterns/mrtr`). + +Instead, a server answers a request it cannot complete with a `Result` whose +required `resultType` is `"input_required"`: + +```typescript +// schema/draft/schema.ts +export type InputRequest = CreateMessageRequest | ListRootsRequest | ElicitRequest; +export interface InputRequests { [key: string]: InputRequest; } +export interface InputRequiredResult extends Result { + inputRequests?: InputRequests; + requestState?: string; // opaque to the client +} +export interface InputResponseRequestParams extends RequestParams { + inputResponses?: InputResponses; + requestState?: string; +} +``` + +The client fulfills the `inputRequests`, then **retries the original request** +(new JSON-RPC id — "The JSON-RPC `id` MUST be different between the initial +request and the retry") carrying `inputResponses` and the echoed +`requestState`. The rounds are fully independent requests; any server replica +can serve any round. Only `tools/call`, `resources/read`, and `prompts/get` +may return `input_required` ("Servers **MUST NOT** send `InputRequiredResult` +responses on any other client requests"). + +Two requirements shape everything below: + +- **Capability gating** (server requirement 7): "Servers **MUST NOT** send an + `inputRequests` that the client has not declared support for in its + capabilities." Capabilities are declared per request in + `_meta.io.modelcontextprotocol/clientCapabilities`. +- **`requestState` is attacker-controlled** (server requirements 4–5): the + minting server "MUST treat `requestState` as an attacker-controlled input", + must protect its integrity (HMAC/AEAD) when it influences authorization or + business logic, and SHOULD bind the authenticated principal, a TTL, and an + originating-request identifier into the protected payload. + +## The four bridge cells + +vMCP terminates the client connection and re-originates backend calls, so MRTR +has a different shape in each client×backend era combination. The non-MRTR +halves of these cells were built in #6006; this table is the MRTR overlay. + +| Downstream client | Backend | MRTR shape | +|---|---|---| +| Modern | Modern | **Stateless pass-through** (the core design, below) | +| Modern | Legacy | **No MRTR bridge — deliberate.** Parking a live Legacy backend call server-side to synthesize `input_required` rounds is per-round server state with token-capability and replica-affinity costs: the session the revision removed, in different clothes. Costed and rejected in the client-edge limitation section of doc 10; the sanctioned stateful path is the Tasks extension (SEP-2663). The call fails with the honest explicit error that section documents. | +| Legacy | Modern | **In-request bridge**: vMCP fulfills the backend's `inputRequests` through the existing Legacy forwarding seams and retries the backend call itself (below) | +| Legacy | Legacy | Today's server-initiated forwarding, unchanged | + +### Cell 1 — Modern client ↔ Modern backend: stateless pass-through + +For a plain (non-composite) `tools/call` / `resources/read` / `prompts/get`, +vMCP adds **no state of its own**: + +1. **Capability mirroring (egress).** On the backend call, vMCP declares in + `_meta.io.modelcontextprotocol/clientCapabilities` exactly the relayable + capabilities the downstream client declared on this request — never more + (vMCP must not solicit `inputRequests` it cannot deliver downstream), never + less (an empty declaration makes a compliant backend that needs input + return `-32021 MissingRequiredClientCapability`, which is today's behavior: + `mcpparser.ModernRequestMeta` hardcodes `clientCapabilities: {}`). +2. **`input_required` relay (ingress ← egress).** When the backend returns + `resultType:"input_required"`, vMCP relays `inputRequests` (values opaque, + verbatim) and `requestState` (opaque, verbatim) to the client in its own + `input_required` envelope, under the client's original request id. +3. **Retry relay.** The client's retry arrives as a fresh request carrying + `inputResponses` + `requestState`. vMCP routes it exactly like the first + round (routing is deterministic by capability name through the routing + table) and forwards both fields verbatim to the backend. + +Round-trip state lives **only** in the backend's `requestState`, whose +integrity/user-binding obligations sit with the backend that minted it, per +the spec. The end-to-end principal binding survives because vMCP's outgoing +auth (OBO / header injection / token exchange) presents the calling +principal's identity to the backend on every round — the backend binds its +state to the same principal each time. vMCP MUST NOT interpret, rewrite, or +append to the relayed `requestState`; the moment a design change requires +vMCP-owned data inside it, that data moves to the workflow-handle machinery +below (D5 protections), not into ad-hoc fields. + +Failure modes, all client-recoverable per the spec's error-handling guidance +(a server that got unusable `inputResponses` "SHOULD respond with a new +`InputRequiredResult` requesting the missing information again"): + +- Backend set changed between rounds and the tool re-routed: the new backend + fails `requestState` validation and re-elicits or errors — no vMCP handling. +- Backend sends an `inputRequest` whose capability the client (and therefore + vMCP's mirrored declaration) did not declare: a backend protocol violation. + vMCP fails the call with an explicit error naming the violation; it never + forwards a request the downstream client cannot fulfill (server + requirement 7 applies to vMCP as a server in its own right). +- Unbounded re-elicitation: vMCP relays rounds without counting (the client + owns its retry budget; go-sdk's client middleware caps at 10 rounds, 3 + consecutive load-shedding rounds). vMCP's per-request deadline bounds each + individual round. + +### Cell 3 — Legacy client ↔ Modern backend: in-request bridge + +The inverse direction bridges cleanly because the blocking machinery already +exists. When a Modern backend returns `input_required` during a **Legacy** +client's call, vMCP — inside the same in-flight downstream request, on one +pod — fulfills each `inputRequest` through the existing server-initiated +forwarding seams (`vmcp.ElicitationRequester`, `vmcp.SamplingRequester`, both +capability-gated on what the Legacy client advertised at `initialize`), then +retries the backend call with the collected `inputResponses` and echoed +`requestState`. Precedent: go-sdk v1.7's own `serverMultiRoundTripMiddleware` +performs exactly this bridge for its handlers ("When a handler returns +InputRequests and the client does not support multi-round-trip, the middleware +fulfills the requests by calling the client directly and reinvokes the +handler"). Round count is bounded (mirror go-sdk's 10/3 caps) and the loop is +single-request-scoped: no durable state, no handle, no replica concern — +the downstream Legacy session vMCP already holds provides the delivery +channel. + +`ListRootsRequest` `inputRequests` are refused explicitly on this cell: vMCP +has never had a roots forwarding seam, and roots is SEP-2577-deprecated — an +explicit error beats building a new seam for a feature in its removal window. + +## Composite (workflow) tools: where durable state actually starts + +The pass-through above is stateless because vMCP is a relay. Composite tools +invert that: **vMCP is the server** executing a multi-step workflow +(`pkg/vmcp/composer`), so when a round trip interrupts a workflow, the +suspended state is vMCP's own. Two triggers: + +- a workflow step's Modern backend returns `input_required`, or +- the workflow itself needs user input (the composer's elicitation handler, + today implemented only over a Legacy session's `ElicitationRequester`). + +For a **Legacy** downstream client, neither suspends anything: the existing +blocking seams answer mid-call on the live session, and composites keep +executing synchronously inside one `tools/call` (RFC-0083 Alternative 3 — +the pod-local `InMemoryWorkflowStateStore` remains correct). + +For a **Modern** downstream client there is no session to block on, so the +workflow must **suspend**: vMCP returns `input_required` to the client with +the workflow's outstanding `inputRequests`, and the retry — a fresh request, +possibly on another pod — must **resume** it. This, precisely, is the trigger +RFC-0083 D5 sequences the durable machinery on: + +- **Round state**: the suspended `WorkflowStatus` (accumulated step outputs, + position, pending input keys) persists in a `WorkflowStateStore` backed by + Redis, injected via a `core.Config` seam (per RFC-0083's API-changes list). + It is too large and too sensitive to round-trip through the client, so the + client-visible `requestState` is a **handle**, not serialized state. +- **Handle format** (RFC-0083 D5, followed verbatim): a server-minted, + ≥128-bit opaque random token; an owner field on `WorkflowStatus` holding + the authenticated `(iss, sub)` pair encoded exactly as the existing + identity binding does — `pkg/vmcp/session/binding`'s + `Format(iss, sub)` → `iss + "\x00" + sub`, plaintext, not hashed — so the + two owner-binding mechanisms stay consistent. +- **Owner check on resume**: the retry's authenticated identity must satisfy + `binding.Format(iss, sub) == status.Owner` before any state is loaded; + a mismatch is an audit-visible security signal (owner-mismatch resume), + fails closed, and reveals nothing about the handle's existence. +- **TTL at write** (bounding the replay window per MRTR server requirement 5) + and **redaction**: the handle never appears in audit or telemetry output. +- This satisfies the spec's requirement to "cryptographically bind the data + to the original user": the state never leaves the server, and the handle is + an unguessable capability whose owner is checked server-side — stronger + than client-side AEAD, with no key-management surface. + +**The distinction that sizes the infrastructure**: plain-tool pass-through +rounds and the Legacy-client bridge need *no* durable store; only +cross-request workflow suspend/resume does. The store, its injection seam, +the owner field, and redaction all land with the composite slice — nothing +earlier. + +## What replaces `ElicitationRequester` + +Nothing replaces it one-for-one; the model inverts. `ElicitationRequester` / +`SamplingRequester` are *push* seams — block an in-flight call while the +server asks the client. They remain the implementation of the two Legacy +cells (unchanged) and become the fulfillment mechanism inside the +Legacy-client bridge (cell 3). On Modern paths, the seam is a *value*, not a +call: `input_required` results flow back through the ordinary return path — + +- domain: `vmcp.InputRequiredResult` (`inputRequests` as opaque raw values + keyed by string, plus `requestState`), carried by a typed error from + `BackendClient` so every existing complete-result path is untouched; +- core: `core.CallTool`/`ReadResource`/`GetPrompt` propagate it (suspending a + workflow first when the caller is the composer); +- server: `dispatchModern` renders it as the `resultType:"input_required"` + envelope, and accepts `inputResponses`/`requestState` params on the retry. + +## Sampling: deprecated pass-through, no feature work + +Explicit answer to "does sampling deserve an MRTR path": **vMCP builds no +sampling feature; sampling flows through the generic relay only.** + +- The MRTR union still carries it: `InputRequest = CreateMessageRequest | + ListRootsRequest | ElicitRequest` (schema/draft, above) — deprecated types + stay in the unions during the window ("The following union types reference + deprecated types but MUST NOT be modified during the deprecation period", + SEP-2577). +- SEP-2577 (Final) deprecates sampling as of 2026-07-28: "New implementations + SHOULD NOT add support for deprecated features unless needed for backward + compatibility with existing counterparts", with "integrate directly with + LLM provider APIs" as the sanctioned replacement. +- vMCP's position under that carve-out: the pass-through relay is + **type-agnostic** — vMCP forwards whatever `inputRequests` the backend sent + and the client declared capability for, without interpreting them, so + sampling transits for the existing counterparts that still declare it; that + is compatibility plumbing, not added support. The one sampling-specific + code path retained is the cell-3 bridge fulfilling a sampling + `inputRequest` through the *existing* `SamplingRequester` — again existing + counterparts, no new capability. No new sampling machinery of any kind is + built, and when the deprecation window closes, sampling disappears from + vMCP by clients/backends dropping it, with zero vMCP code to remove beyond + the bridge's fulfillment case. + +Roots: same union, same deprecation, but unlike sampling there is no existing +vMCP counterpart (no roots forwarding seam has ever existed) — so roots gets +relay-only transit on cell 1 and an explicit refusal on cell 3. + +## Gaps in the pinned SDK surface (upstream candidates for toolhive-core) + +Verified against go-sdk `v1.7.0-pre.3` (the transitive pin via +`toolhive-core v0.0.34`) and `toolhive-core/mcpcompat`: + +1. **mcpcompat drops MRTR fields in both directions.** + `mcpcompat/mcp.CallToolResult` carries only an unexported `resultType` + (populated by `UnmarshalJSON`, feeds `NeedsInput()`); it has **no** + `InputRequests`/`RequestState` fields, so a decoded `input_required` + result loses its payload. `CallToolParams` (`Name`, `Arguments`, `Meta`, + `Task`) cannot carry `inputResponses`/`requestState` on a retry. Upstream + ask: add both field pairs (go-sdk's own `mcp.CallToolResult` already + exports `InputRequests InputRequestMap` and `RequestState string`). +2. **No no-initialize client primitive** in mcpcompat's public API (tracked + as #6018) — the reason `pkg/vmcp/client`'s hand-rolled `modernCall` exists + at all, and therefore where MRTR egress decode/retry params must live + until #6018's upstream work lands. +3. **go-sdk's MRTR internals are unexported where vMCP needs them.** The + client retry loop exists (`clientMultiRoundTripMiddleware`, + `MultiRoundTripOptions.Disabled` for manual handling) but its pieces + (`fulfillInputRequests`, `setMultiRoundTripRetryParams`) are unexported; + server-side, `resultType` is settable only via unexported + `setResultType`/`setCompleteResultType` inside `ServerSession` dispatch — + exactly the dispatch the stateless Modern path bypasses — so the + hand-rolled `modern_envelope.go` must emit `input_required` itself. + Upstream ask (shared with #6018's step 1): exported stateless + client/server MRTR primitives. +4. **`ServerSession.assertServerInitiatedRequestAllowed` gates by negotiated + protocol version only** (`v1.7.0-pre.3` `mcp/server.go:1544-1553`: + `iparams.ProtocolVersion >= protocolVersion20260728`), never consulting + capability declarations. Harmless for this design — the seams it guards + are only used on Legacy sessions — but it forecloses any notion of + re-enabling server-initiated requests for a capability-declaring Modern + client. Accounted for, no action. +5. **`-32021` at HTTP 400 is unshippable** with current go-sdk clients (their + transient set is 500/502/503/504/429; any other 4xx is permanent session + death), which is why the undeclared-capability error lands at HTTP 200 as + a documented deviation — analysis in doc 10's client-edge limitation + section; MRTR does not change it, it *removes the case* for capabilities + the client does declare. + +## Sequencing (collision-aware) + +In-flight at time of writing: #6050 (modern pagination/subscriptions — +rewrites `modern_dispatch.go`/`modern_envelope.go`), #6033 (kill-switch +removal — `classification.go`, `server.go`, `serve.go` and test helpers), +#6051 (Legacy-pins the forwarding tests, adds doc 10's client-edge limitation +section). Slices are ordered so each lands without touching those files until +its predecessors merge: + +1. **Egress MRTR surface (safe now; implemented alongside this doc).** Domain + type `vmcp.InputRequiredResult`; `pkg/vmcp/client`'s Modern shim decodes an + `input_required` envelope into a typed, `errors.Is`-compatible error + carrying the payload instead of the opaque + `errModernInputRequired`. No behavior change: capabilities are still + declared empty, so a compliant backend cannot yet send `input_required`; + the seam #6051 calls "the egress half" simply becomes load-bearing. + Files: `pkg/vmcp/mrtr.go`, `pkg/vmcp/client/modern.go` (one hook), + `pkg/vmcp/client/modern_mrtr.go` — none touched by the in-flight PRs. +2. **Ingress envelope + retry params (after #6050/#6033).** `dispatchModern` + accepts `inputResponses`/`requestState`, threads them through + `core.CallTool`→`BackendClient`, and renders `input_required` envelopes; + parser vocabulary for the two params. +3. **Capability mirroring (activates cell 1).** `ModernRequestMeta` gains a + capabilities argument; dispatch threads the downstream declaration to the + egress call; the relay goes live end-to-end. The `-32021`-shaped + undeclared-capability error contract from doc 10's follow-up lands with or + before this. +4. **Cell-3 bridge.** The in-request fulfillment loop over + `ElicitationRequester`/`SamplingRequester` with go-sdk-mirrored round caps. +5. **Composite suspend/resume.** Durable `WorkflowStateStore` (Redis) behind + the `core.Config` seam, D5 handle + owner check + TTL + redaction, resume + dispatch, owner-mismatch audit signal. + +Slices 2–5 each update doc 10's client-edge limitation section (whose current +text truthfully says MRTR is unserved) and the forwarding-test dispositions +from #6051 as their claims become false. + +## Testing strategy + +- Slice 1: envelope decode (payload extraction, `errors.Is` back-compat with + the sentinel, malformed-payload tolerance), no-behavior-change pins. +- Slice 2/3: dispatcher round-trip — `input_required` envelope shape + (resultType, echoed id, relayed keys), retry threading, capability-gate + refusals (undeclared → no forward; backend violation → explicit error). +- Cell 1 end-to-end: a Modern fake backend demanding one elicitation round; + assert both rounds' independence (distinct ids, verbatim + `requestState` echo, no vMCP-side state between them — serve round 2 from + a *fresh server instance* to prove it). +- Cell 3: Legacy downstream client + `input_required` Modern backend; assert + fulfillment via the existing seams, round caps, and the roots refusal. +- Slice 5: owner-binding (B cannot resume A's workflow), TTL expiry, + Redis-unavailable fail-closed — the RFC-0083 test list, verbatim. + +## References + +- SEP-2322 (Final), SEP-2577 (Final), SEP-2663 (tasks redesign); + `schema/draft/schema.ts` and `/specification/draft/basic/patterns/mrtr` as + of 2026-07-27. +- RFC-0083 (`stacklok/toolhive-rfcs`) — D5, Alternative 3, implementation + plan's deferred list. +- [10-virtual-mcp-architecture.md](10-virtual-mcp-architecture.md) — the + client-edge limitation section (added by #6051) this design supersedes + step by step; forwarding seams; #6006 bridge cells. +- Issues: #5743 (epic), #5759 (this design), #6018 (shim retirement), + #5959/#6033 (kill-switch), #6050 (pagination/subscriptions). diff --git a/docs/arch/README.md b/docs/arch/README.md index a39a67a97c..898ce7bc77 100644 --- a/docs/arch/README.md +++ b/docs/arch/README.md @@ -127,6 +127,13 @@ Welcome to the ToolHive architecture documentation. This directory contains comp - Dynamic forward proxy with per-request DNS and native HTTPS CONNECT tunnelling - Squid-vs-Envoy comparison and known limitations (`AllowPort` gap, V4_ONLY DNS, deny-all on empty profile) +16. **[vMCP Multi Round-Trip Requests (MRTR)](16-vmcp-mrtr.md)** + - MRTR (SEP-2322) design for the Modern (2026-07-28) elicitation/sampling path through vMCP + - The four client×backend bridge cells: stateless Modern↔Modern pass-through, in-request Legacy-client bridge + - Per-request capability mirroring and verbatim `requestState` relay + - Composite-workflow suspend/resume as the sole durable-state trigger (RFC-0083 D5 handles, owner binding, TTL) + - Sampling's deprecated-pass-through-only disposition (SEP-2577) and the go-sdk/mcpcompat gap list + ### Existing Documentation For middleware architecture, see: **[docs/middleware.md](../middleware.md)** From bbfe1e223a6e1ce6e821a2f851698e4dbb6b7438 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Mon, 27 Jul 2026 20:56:19 +0000 Subject: [PATCH 02/11] Surface Modern input_required rounds as typed values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An MCP 2026-07-28 backend that needs more input answers with a resultType:"input_required" envelope (SEP-2322). The Modern egress shim collapsed that into the opaque errModernInputRequired sentinel, discarding the inputRequests and requestState payload — so no upper layer could ever relay or fulfill a round, and the MRTR work designed in docs/arch/16-vmcp-mrtr.md had no seam to build on. Decode the envelope into a domain vmcp.InputRequiredResult carried by a typed error. The values stay opaque json.RawMessage (the pass-through relay must forward them verbatim, never reinterpret them) with a Methods() probe for the capability gate; the error unwraps to errModernInputRequired with a byte-identical message, so revision classification (probeRevision) and every client-visible behavior are unchanged — capabilities are still declared empty, so a compliant backend cannot yet send a round at all. InputRequiredFromError is the single branch point the ingress relay and the Legacy-client bridge (slices 2-4 of the design) will consume. Confined to pkg/vmcp and pkg/vmcp/client, which none of the in-flight Modern-path PRs (#6050, #6033, #6051) touch. Refs #5743, #5759 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n --- pkg/vmcp/client/modern.go | 6 +- pkg/vmcp/client/modern_mrtr.go | 78 +++++++++++++++ pkg/vmcp/client/modern_mrtr_test.go | 144 ++++++++++++++++++++++++++++ pkg/vmcp/mrtr.go | 54 +++++++++++ pkg/vmcp/mrtr_test.go | 53 ++++++++++ 5 files changed, 334 insertions(+), 1 deletion(-) create mode 100644 pkg/vmcp/client/modern_mrtr.go create mode 100644 pkg/vmcp/client/modern_mrtr_test.go create mode 100644 pkg/vmcp/mrtr.go create mode 100644 pkg/vmcp/mrtr_test.go diff --git a/pkg/vmcp/client/modern.go b/pkg/vmcp/client/modern.go index 493fce748c..7e463fd91c 100644 --- a/pkg/vmcp/client/modern.go +++ b/pkg/vmcp/client/modern.go @@ -267,7 +267,11 @@ func interpretModernResult(result json.RawMessage, rpcErr *modernRPCError, metho // request. Distinct from errWrongEra so the caller does not auto-retry. return errLegacyResponseBody default: - return fmt.Errorf("%w: resultType=%q", errModernInputRequired, envelope.ResultType) + // Typed so an "input_required" round's SEP-2322 payload (inputRequests, + // requestState) survives for MRTR consumers (InputRequiredFromError); + // unwraps to errModernInputRequired with an identical message, so + // classification and behavior are unchanged for everyone else. + return newInputRequiredError(envelope.ResultType, result) } if out != nil { diff --git a/pkg/vmcp/client/modern_mrtr.go b/pkg/vmcp/client/modern_mrtr.go new file mode 100644 index 0000000000..80244e045c --- /dev/null +++ b/pkg/vmcp/client/modern_mrtr.go @@ -0,0 +1,78 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package client + +import ( + "encoding/json" + "errors" + "fmt" + + "github.com/stacklok/toolhive/pkg/vmcp" +) + +// inputRequiredError is the typed form of errModernInputRequired: a Modern +// envelope decoded with a non-"complete" resultType. For +// resultType:"input_required" it carries the decoded SEP-2322 payload so +// upper layers can drive a Multi Round-Trip Request round (relay to a Modern +// downstream client, or fulfill in-request for a Legacy one — see +// docs/arch/16-vmcp-mrtr.md); for any other unrecognized resultType the +// payload is nil and only the sentinel classification remains. +// +// It unwraps to errModernInputRequired so every existing errors.Is check — +// notably probeRevision's Modern-positive classification (client.go) — keeps +// working unchanged, and Error() renders byte-identically to the previous +// fmt.Errorf wrapping so no client-visible message shifts. +type inputRequiredError struct { + resultType string + result *vmcp.InputRequiredResult +} + +func (e *inputRequiredError) Error() string { + return fmt.Sprintf("%s: resultType=%q", errModernInputRequired, e.resultType) +} + +func (*inputRequiredError) Unwrap() error { return errModernInputRequired } + +// newInputRequiredError builds the typed error for a non-"complete" Modern +// result envelope. Only resultType:"input_required" carries a payload: the +// spec says clients "SHOULD treat unrecognized values as invalid protocol +// responses", so other resultTypes keep pure sentinel semantics. The payload +// decode is tolerant — a malformed inputRequests/requestState still yields +// the typed error with whatever decoded (the round is undrivable either way, +// and the caller's error handling must not depend on backend well-formedness). +func newInputRequiredError(resultType string, result json.RawMessage) error { + e := &inputRequiredError{resultType: resultType} + if resultType != "input_required" { + return e + } + var payload struct { + InputRequests map[string]json.RawMessage `json:"inputRequests"` + RequestState string `json:"requestState"` + } + // Tolerant decode: see the doc comment. + _ = json.Unmarshal(result, &payload) + e.result = &vmcp.InputRequiredResult{ + InputRequests: payload.InputRequests, + RequestState: payload.RequestState, + } + return e +} + +// InputRequiredFromError extracts the SEP-2322 input_required payload from an +// error chain, however deeply the backend client wrapped it. It reports false +// for errors that are not an input_required round — including the +// unrecognized-resultType variant of the same sentinel — so callers can use +// it as the single MRTR branch point: +// +// if round, ok := client.InputRequiredFromError(err); ok { ... relay round ... } +// +// This is the seam the ingress half (dispatchModern's input_required envelope +// and the Legacy-client bridge; docs/arch/16-vmcp-mrtr.md slices 2-4) consumes. +func InputRequiredFromError(err error) (*vmcp.InputRequiredResult, bool) { + var ire *inputRequiredError + if errors.As(err, &ire) && ire.result != nil { + return ire.result, true + } + return nil, false +} diff --git a/pkg/vmcp/client/modern_mrtr_test.go b/pkg/vmcp/client/modern_mrtr_test.go new file mode 100644 index 0000000000..4231b5464f --- /dev/null +++ b/pkg/vmcp/client/modern_mrtr_test.go @@ -0,0 +1,144 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package client + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestInterpretModernResultInputRequired pins the egress MRTR seam +// (docs/arch/16-vmcp-mrtr.md, slice 1): an input_required envelope must +// surface as an error that (a) still satisfies every existing +// errors.Is(err, errModernInputRequired) classification, (b) renders the +// exact message the previous fmt.Errorf wrapping produced (no client-visible +// drift), and (c) now carries the decoded SEP-2322 payload, extractable +// through arbitrary wrapping via InputRequiredFromError. +func TestInterpretModernResultInputRequired(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + result string + wantPayload bool + wantKeys map[string]string // request key -> method, per Methods() + wantState string + }{ + { + name: "input_required with elicitation and state", + result: `{ + "resultType": "input_required", + "inputRequests": { + "github_login": {"method": "elicitation/create", "params": {"message": "user?"}}, + "summary": {"method": "sampling/createMessage", "params": {"maxTokens": 10}} + }, + "requestState": "opaque-blob" + }`, + wantPayload: true, + wantKeys: map[string]string{ + "github_login": "elicitation/create", + "summary": "sampling/createMessage", + }, + wantState: "opaque-blob", + }, + { + // SEP-2322's load-shedding shape: requestState only, no inputRequests. + // The client may retry immediately; the payload must still decode. + name: "load-shedding round (requestState only)", + result: `{"resultType": "input_required", "requestState": "resume-here"}`, + wantPayload: true, + wantKeys: nil, + wantState: "resume-here", + }, + { + // Malformed payload: the round is undrivable, but the typed error and + // sentinel classification must survive a backend that violates the + // schema (tolerant decode). + name: "malformed inputRequests still yields the typed error", + result: `{"resultType": "input_required", "inputRequests": "not-a-map"}`, + wantPayload: true, + wantKeys: nil, + wantState: "", + }, + { + // An unrecognized resultType is an invalid protocol response per the + // spec; it keeps pure sentinel semantics with NO extractable round. + name: "unrecognized resultType carries no payload", + result: `{"resultType": "task"}`, + wantPayload: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := interpretModernResult(json.RawMessage(tt.result), nil, "tools/call", nil) + require.Error(t, err) + + // (a) classification is unchanged. + require.ErrorIs(t, err, errModernInputRequired) + + // (b) message is byte-identical to the previous wrapping. + var envelope struct { + ResultType string `json:"resultType"` + } + require.NoError(t, json.Unmarshal([]byte(tt.result), &envelope)) + assert.Equal(t, + fmt.Errorf("%w: resultType=%q", errModernInputRequired, envelope.ResultType).Error(), + err.Error()) + + // (c) payload extraction, through an extra wrapping layer to mirror + // how modernCallTool wraps backend errors before they reach dispatch. + wrapped := fmt.Errorf("tool call failed on backend %s: %w", "b1", err) + round, ok := InputRequiredFromError(wrapped) + require.Equal(t, tt.wantPayload, ok) + if !tt.wantPayload { + assert.Nil(t, round) + return + } + require.NotNil(t, round) + assert.Equal(t, tt.wantState, round.RequestState) + assert.Equal(t, tt.wantKeys, round.Methods()) + }) + } +} + +// TestInputRequiredFromErrorRejectsForeignErrors pins that the extractor is a +// safe single branch point: unrelated errors — including the OTHER Modern +// sentinels — report ok=false. +func TestInputRequiredFromErrorRejectsForeignErrors(t *testing.T) { + t.Parallel() + + for _, err := range []error{ + nil, + errWrongEra, + errLegacyResponseBody, + fmt.Errorf("wrapped: %w", errModernProtocolError), + } { + round, ok := InputRequiredFromError(err) + assert.False(t, ok, "error %v must not extract as input_required", err) + assert.Nil(t, round) + } +} + +// TestInputRequiredRequestsAreRelayedVerbatim pins the pass-through +// invariant: the raw bytes of each input request survive the decode +// untouched, so the relay (slice 2) can forward them without reinterpretation. +func TestInputRequiredRequestsAreRelayedVerbatim(t *testing.T) { + t.Parallel() + + const rawReq = `{"method":"elicitation/create","params":{"mode":"form","message":"x","extra":[1,2,3]}}` + result := `{"resultType":"input_required","inputRequests":{"k1":` + rawReq + `}}` + + err := interpretModernResult(json.RawMessage(result), nil, "tools/call", nil) + round, ok := InputRequiredFromError(err) + require.True(t, ok) + require.Contains(t, round.InputRequests, "k1") + assert.JSONEq(t, rawReq, string(round.InputRequests["k1"])) +} diff --git a/pkg/vmcp/mrtr.go b/pkg/vmcp/mrtr.go new file mode 100644 index 0000000000..aa169d3c86 --- /dev/null +++ b/pkg/vmcp/mrtr.go @@ -0,0 +1,54 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package vmcp + +import "encoding/json" + +// InputRequiredResult carries a Modern (2026-07-28) backend's +// resultType:"input_required" envelope — a Multi Round-Trip Request round +// (SEP-2322): the backend cannot complete the call until the caller fulfills +// InputRequests and retries the original request echoing RequestState. +// +// This is the domain-typed egress surface of MRTR (the seam the client-edge +// limitation in docs/arch/10-virtual-mcp-architecture.md names): the backend +// client decodes the envelope into this type and surfaces it through a typed +// error, so upper layers can relay a round to a Modern downstream client or +// fulfill it in-request for a Legacy one. Full flow design in +// docs/arch/16-vmcp-mrtr.md. +type InputRequiredResult struct { + // InputRequests maps the backend's server-assigned keys to raw request + // objects (ElicitRequest, CreateMessageRequest, or ListRootsRequest — + // schema/draft's InputRequest union). Values are deliberately opaque + // json.RawMessage: on the pass-through path vMCP relays them verbatim and + // must not reinterpret them. Use Methods to inspect only the wire method + // names (e.g. for capability gating). + InputRequests map[string]json.RawMessage + + // RequestState is the backend's opaque round-trip state. Per SEP-2322 the + // client "MUST echo back the exact value" on retry and "MUST NOT inspect, + // parse, modify, or make any assumptions about" it — vMCP relays it + // verbatim and never interprets it. Empty when the backend sent none. + RequestState string +} + +// Methods returns the JSON-RPC method name of each input request, keyed by +// the backend's request key. An entry whose value does not decode as an +// object with a "method" field maps to the empty string, so callers gating on +// method (capability checks) fail closed on malformed entries instead of +// skipping them. +func (r *InputRequiredResult) Methods() map[string]string { + if len(r.InputRequests) == 0 { + return nil + } + methods := make(map[string]string, len(r.InputRequests)) + for key, raw := range r.InputRequests { + var probe struct { + Method string `json:"method"` + } + // A failed decode leaves probe.Method empty — the fail-closed value. + _ = json.Unmarshal(raw, &probe) + methods[key] = probe.Method + } + return methods +} diff --git a/pkg/vmcp/mrtr_test.go b/pkg/vmcp/mrtr_test.go new file mode 100644 index 0000000000..e7e535cf9f --- /dev/null +++ b/pkg/vmcp/mrtr_test.go @@ -0,0 +1,53 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package vmcp + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestInputRequiredResultMethods(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + requests map[string]json.RawMessage + want map[string]string + }{ + { + name: "methods extracted per key", + requests: map[string]json.RawMessage{ + "a": json.RawMessage(`{"method":"elicitation/create","params":{}}`), + "b": json.RawMessage(`{"method":"sampling/createMessage"}`), + }, + want: map[string]string{"a": "elicitation/create", "b": "sampling/createMessage"}, + }, + { + // A malformed entry maps to "" — the fail-closed value for a + // capability gate — rather than being silently dropped. + name: "malformed entry fails closed as empty method", + requests: map[string]json.RawMessage{ + "bad": json.RawMessage(`[1,2]`), + "good": json.RawMessage(`{"method":"roots/list"}`), + }, + want: map[string]string{"bad": "", "good": "roots/list"}, + }, + { + name: "empty requests yield nil", + requests: nil, + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + r := &InputRequiredResult{InputRequests: tt.requests} + assert.Equal(t, tt.want, r.Methods()) + }) + } +} From 8a44318cca0dbb38326c433e5467af2b1fa61be9 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Mon, 27 Jul 2026 20:59:50 +0000 Subject: [PATCH 03/11] Flag the MRTR design's draft-only spec dependencies The 2026-07-28 revision finalizes one day after this design was written against schema/draft. The schema types and SEPs it cites are already Final, but four load-bearing points rest on the draft spec page's text, which refines the Final SEP: the three-method table (the SEP also allowed GetTaskPayloadRequest), the capability-gating MUST NOT, the at-least-one-field requirement, and the requestState security language. Name them explicitly with the design's exposure to each, so the re-verification against the final cut is a checklist rather than a re-read. Refs #5759 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n --- docs/arch/16-vmcp-mrtr.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/arch/16-vmcp-mrtr.md b/docs/arch/16-vmcp-mrtr.md index 55d06e4efc..67c99e5247 100644 --- a/docs/arch/16-vmcp-mrtr.md +++ b/docs/arch/16-vmcp-mrtr.md @@ -21,6 +21,31 @@ the revision's finalization date, when it still lived at `schema/draft` should be re-verified against the final text once cut; the MRTR SEP and schema types were already Final at time of writing. +**Draft-only dependencies (re-verify against the final cut).** The schema +types and the SEPs are Final and low-risk. Four load-bearing points in this +design rest on the draft *spec page's* text, which refines (or diverges from) +the Final SEP and could still move before the final cut: + +- **The three-method table.** SEP-2322 (Final) also allows + `InputRequiredResult` on `GetTaskPayloadRequest`; the draft page dropped it + when tasks moved to an extension. If the final restores a fourth method, + the relay's method allow-list grows — a table edit, not a design change. +- **Capability gating** (server requirement 7, "MUST NOT send an + `inputRequests` that the client has not declared") is page-only — SEP-2322 + does not state it. Capability mirroring is designed around it; if the final + weakens it to SHOULD, mirroring remains correct (it is also what makes the + relay deliverable), only the backend-violation error becomes discretionary. +- **Server requirement 6** ("at least one of `inputRequests` or + `requestState`") is page-only; it affects only what vMCP treats as a + backend protocol violation. +- **The `requestState` security language** differs between the two: SEP-2322 + says user-specific state "MUST use some mechanism to cryptographically + bind" it to the user, while the page allows omitting integrity protection + "only when tampering can cause nothing worse than request failure". The + workflow-handle design (below) satisfies the stricter reading — state never + leaves the server and the handle is owner-checked — so it is robust to + either landing in the final. + ## Protocol recap (what MRTR actually is) The 2026-07-28 revision removes server-initiated requests: "Servers **MUST** From 7e51fe2faee962fd66ec43f192e8b87a27700ad5 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 28 Jul 2026 06:01:34 +0000 Subject: [PATCH 04/11] Reconcile the MRTR design with merged #6061 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #6061 landed the two-path mid-call capability-refusal contract this design's sequencing anticipated: -32021 with data.requiredCapabilities at HTTP 200 (documented deviation, go-sdk#1117) for an undeclared capability, an explicit -32603 naming SEP-2322 for a declared one. Reconcile rather than duplicate: the contract is permanent for the Modern-client/Legacy-backend cell (the only cell its refusal recorder can fire on — the SDK adapters are the Legacy-session forwarding seams), superseded by capability mirroring only where the backend is Modern. Name writeModernCallFailure as slice 2's rendering hook — the input_required branch and the refusal branch cannot co-occur — and note the declared-case message needs rescoping when slice 3 lands. Also record the finalization re-check: on 2026-07-28 the revision is still not cut (no schema/2026-07-28 directory, /specification/2026-07-28 pages 404, schema/draft byte-identical to the copy designed against), so the draft-only-dependency checklist stays pending. Refs #5759, #6061 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n --- docs/arch/16-vmcp-mrtr.md | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/docs/arch/16-vmcp-mrtr.md b/docs/arch/16-vmcp-mrtr.md index 67c99e5247..14b888f33a 100644 --- a/docs/arch/16-vmcp-mrtr.md +++ b/docs/arch/16-vmcp-mrtr.md @@ -21,6 +21,12 @@ the revision's finalization date, when it still lived at `schema/draft` should be re-verified against the final text once cut; the MRTR SEP and schema types were already Final at time of writing. +*Re-checked 2026-07-28 (the nominal finalization date): the revision is +still not cut — no `schema/2026-07-28` directory exists, +`/specification/2026-07-28/` pages 404, and `schema/draft/schema.ts` is +byte-identical to the copy this design was written against. The checklist +below therefore remains pending the final cut.* + **Draft-only dependencies (re-verify against the final cut).** The schema types and the SEPs are Final and low-risk. Four load-bearing points in this design rest on the draft *spec page's* text, which refines (or diverges from) @@ -99,7 +105,7 @@ halves of these cells were built in #6006; this table is the MRTR overlay. | Downstream client | Backend | MRTR shape | |---|---|---| | Modern | Modern | **Stateless pass-through** (the core design, below) | -| Modern | Legacy | **No MRTR bridge — deliberate.** Parking a live Legacy backend call server-side to synthesize `input_required` rounds is per-round server state with token-capability and replica-affinity costs: the session the revision removed, in different clothes. Costed and rejected in the client-edge limitation section of doc 10; the sanctioned stateful path is the Tasks extension (SEP-2663). The call fails with the honest explicit error that section documents. | +| Modern | Legacy | **No MRTR bridge — deliberate.** Parking a live Legacy backend call server-side to synthesize `input_required` rounds is per-round server state with token-capability and replica-affinity costs: the session the revision removed, in different clothes. Costed and rejected in the client-edge limitation section of doc 10; the sanctioned stateful path is the Tasks extension (SEP-2663). The call fails with the two-path contract #6061 (merged) implements in `writeModernCallFailure`: `-32021` + `data.requiredCapabilities` when the client did not declare the capability, an explicit `-32603` naming SEP-2322 when it did. That contract is **permanent for this cell** — MRTR slices supersede it only where the backend is Modern. | | Legacy | Modern | **In-request bridge**: vMCP fulfills the backend's `inputRequests` through the existing Legacy forwarding seams and retries the backend call itself (below) | | Legacy | Legacy | Today's server-initiated forwarding, unchanged | @@ -306,9 +312,10 @@ Verified against go-sdk `v1.7.0-pre.3` (the transitive pin via 5. **`-32021` at HTTP 400 is unshippable** with current go-sdk clients (their transient set is 500/502/503/504/429; any other 4xx is permanent session death), which is why the undeclared-capability error lands at HTTP 200 as - a documented deviation — analysis in doc 10's client-edge limitation - section; MRTR does not change it, it *removes the case* for capabilities - the client does declare. + a documented deviation — implemented by #6061 + (`writeModernMissingCapability`), tracked upstream as go-sdk#1117; MRTR + does not change it, it *removes the case* for capabilities the client + does declare against Modern backends. ## Sequencing (collision-aware) @@ -331,12 +338,22 @@ its predecessors merge: 2. **Ingress envelope + retry params (after #6050/#6033).** `dispatchModern` accepts `inputResponses`/`requestState`, threads them through `core.CallTool`→`BackendClient`, and renders `input_required` envelopes; - parser vocabulary for the two params. + parser vocabulary for the two params. Post-#6061, the rendering branch + hooks into `writeModernCallFailure` via `InputRequiredFromError`, + alongside (never instead of) its authz-priority and capability-refusal + branches — the two cannot co-occur: the refusal recorder fires only on + the Legacy-backend forwarding seams, `input_required` only from a Modern + backend's envelope. 3. **Capability mirroring (activates cell 1).** `ModernRequestMeta` gains a capabilities argument; dispatch threads the downstream declaration to the - egress call; the relay goes live end-to-end. The `-32021`-shaped - undeclared-capability error contract from doc 10's follow-up lands with or - before this. + egress call (reusing #6061's `modernClientDeclaredCapability` reading of + `_meta`); the relay goes live end-to-end. The `-32021`-shaped + undeclared-capability error contract landed ahead of this in #6061 + (merged 2026-07-28; `-32021` at HTTP 200 as the documented deviation, + upstream go-sdk#1117). When this slice lands, #6061's declared-capability + `-32603` message ("multi-round retrieval … which this server does not + implement") stops being true for Modern backends and needs rescoping to + the Legacy-backend cell it permanently serves. 4. **Cell-3 bridge.** The in-request fulfillment loop over `ElicitationRequester`/`SamplingRequester` with go-sdk-mirrored round caps. 5. **Composite suspend/resume.** Durable `WorkflowStateStore` (Redis) behind From 71be2bae8315d4ad4a63d964f883cc79fba5a42d Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 28 Jul 2026 06:45:53 +0000 Subject: [PATCH 05/11] Regenerate CRD reference docs task crdref-gen output shifts by two trailing blank lines once this branch's new exported pkg/vmcp types are in the generator's scanned set. Verified the drift is this branch's consequence, not pre-existing staleness: regenerating in a clean origin/main worktree produces no diff. Generated file only; no generator or config changes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n --- docs/operator/crd-api.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 4a87c88f16..fe18cda44c 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -4871,6 +4871,8 @@ This type is shared with the Kubernetes operator CRD (VirtualMCPServer.Status.Di + + From 067002f24f7434a590fedaf191f3ba0aed2d6e25 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 28 Jul 2026 07:14:27 +0000 Subject: [PATCH 06/11] Track the MRTR design under issue #6059 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The human filed #6059 for exactly the pass-through this document designs, superseding the closed-not-planned #5759 the doc referenced. Point the design at the live issue, defer to doc 10's landed rationale for both current limitations instead of restating it, and delineate the adjacent Modern-path issues (#6058 streaming, #6064 ping/resultType, #6065 subscriptions) this design deliberately does not cover. One genuine divergence is flagged rather than papered over: #6059 sketches wrapping the backend's requestState with vMCP routing context, while this design relays it verbatim and re-derives routing from the capability name — because a vMCP-authored wrapper makes vMCP a state-minting server under MRTR server requirements 4-5, a key-management surface the verbatim relay avoids. Recorded as an open decision to resolve at slice 2/3. Part of #6059. Refs #5743. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n --- docs/arch/16-vmcp-mrtr.md | 53 ++++++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 9 deletions(-) diff --git a/docs/arch/16-vmcp-mrtr.md b/docs/arch/16-vmcp-mrtr.md index 14b888f33a..3b0d366d6e 100644 --- a/docs/arch/16-vmcp-mrtr.md +++ b/docs/arch/16-vmcp-mrtr.md @@ -3,9 +3,17 @@ Design for serving MCP 2026-07-28 Multi Round-Trip Requests (SEP-2322) through the Virtual MCP Server — the Modern replacement for the server-initiated elicitation/sampling forwarding that exists only on Legacy (2025-11-25) -sessions. This is the design issue #5759 asked for and the "coherent future -MRTR shape" the client-edge limitation section of -[10-virtual-mcp-architecture.md](10-virtual-mcp-architecture.md) names. +sessions. This work is tracked by issue **#6059** ("Support MRTR pass-through +for Modern backends in vMCP"), which supersedes the earlier design request +#5759 (closed as not-planned before #6059 was filed); it is the "coherent +future MRTR shape" the client-edge limitation section of +[10-virtual-mcp-architecture.md](10-virtual-mcp-architecture.md) names. That +document carries the **landed rationale** for both current limitations — the +backend-edge one ("elicitation and sampling are unavailable on Modern +backends": empty `clientCapabilities`, `-32021` surfacing) and the client-edge +one (the honest `-32603`/`-32021` contract) — and this document defers to it +for that ground rather than restating it: what follows is the design that +removes the limitations, cell by cell. Decision framing follows RFC-0083 (`stacklok/toolhive-rfcs`, `rfcs/THV-0083-stateless-vmcp.md`), which defers MRTR and fixes the @@ -140,6 +148,19 @@ append to the relayed `requestState`; the moment a design change requires vMCP-owned data inside it, that data moves to the workflow-handle machinery below (D5 protections), not into ad-hoc fields. +**Open decision vs #6059 (resolve at slice 2/3, flagged, not silently +diverged):** the tracking issue sketches step 2 as "wrap the backend's opaque +`requestState` with vMCP routing context (which backend it came from)". This +design deliberately relays it **verbatim** instead, re-deriving the backend on +the retry from the capability name through the routing table — because the +moment vMCP injects its own data into `requestState`, vMCP becomes a state- +minting server under MRTR server requirements 4–5 (attacker-controlled input, +integrity protection, principal binding), a key-management surface the +verbatim relay avoids entirely. The cost is the rerouted-between-rounds edge +case, which is client-recoverable (the new backend rejects foreign state and +re-elicits). If review concludes the wrapped form is preferable, the wrapper +must carry the D5-grade protections, not a bare backend ID. + Failure modes, all client-recoverable per the spec's error-handling guidance (a server that got unusable `inputResponses` "SHOULD respond with a new `InputRequiredResult` requesting the missing information again"): @@ -344,10 +365,12 @@ its predecessors merge: branches — the two cannot co-occur: the refusal recorder fires only on the Legacy-backend forwarding seams, `input_required` only from a Modern backend's envelope. -3. **Capability mirroring (activates cell 1).** `ModernRequestMeta` gains a - capabilities argument; dispatch threads the downstream declaration to the - egress call (reusing #6061's `modernClientDeclaredCapability` reading of - `_meta`); the relay goes live end-to-end. The `-32021`-shaped +3. **Capability mirroring (activates cell 1; with slice 2, this is what + issue #6059 tracks).** `ModernRequestMeta` gains a capabilities argument; + dispatch threads the downstream declaration to the egress call (reusing + #6061's `modernClientDeclaredCapability` reading of `_meta`); the relay + goes live end-to-end. The `requestState` open decision above (verbatim vs + #6059's routing-context wrapper) must be resolved here at the latest. The `-32021`-shaped undeclared-capability error contract landed ahead of this in #6061 (merged 2026-07-28; `-32021` at HTTP 200 as the documented deviation, upstream go-sdk#1117). When this slice lands, #6061's declared-capability @@ -390,5 +413,17 @@ from #6051 as their claims become false. - [10-virtual-mcp-architecture.md](10-virtual-mcp-architecture.md) — the client-edge limitation section (added by #6051) this design supersedes step by step; forwarding seams; #6006 bridge cells. -- Issues: #5743 (epic), #5759 (this design), #6018 (shim retirement), - #5959/#6033 (kill-switch), #6050 (pagination/subscriptions). +- Issues: #5743 (epic); **#6059 (tracks the MRTR pass-through this document + designs — supersedes #5759, the earlier not-planned design request)**; + #6018 (shim retirement); #5959/#6033 (kill-switch); #6050 + (pagination/subscriptions). +- Adjacent Modern-path issues this design deliberately does NOT cover: + **#6058** — per-request SSE streaming for `notifications/progress`/ + `notifications/message`; distinct from MRTR (those are notifications, not + server-initiated requests — MRTR does not touch them), but slice 2's + `input_required` envelope may ride the same future stream as its final + message, per the spec's "MAY be sent … as the final message on an SSE + stream". **#6064** — `ping` removal and the required `resultType` on the + dispatch path; pure conformance, orthogonal to MRTR. **#6065** — + `subscriptions/listen` push delivery; a different channel with a fixed + four-type subscribable set, structurally disjoint from MRTR rounds. From c92bd2de350345f545c5a1cc2c687f04077e3061 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 28 Jul 2026 10:43:52 +0000 Subject: [PATCH 07/11] Harden the MRTR egress seam per #6074 review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review (jhrozek) found the seam promising more than it enforced. Extraction is now fail-closed on three gates the design already stated but the code did not check: the method allow-list (only tools/call, resources/read, prompts/get may carry a round), strict payload decode (a wrong-typed inputRequests or requestState is a schema violation, not an empty round), and server requirement 6 (an envelope with nothing to fulfill and nothing to echo must not become a retry-forever round). RequestState becomes *string because client requirement 2 makes absent-vs-present-and-empty an observable distinction the retry must preserve. The carrier and extractor move to pkg/vmcp beside InputRequiredResult with exported fields, so slice 2's server-side rendering never has to import the concrete HTTP client and mock-based consumers can construct the error. Classification and message text are unchanged: the typed error still unwraps to the client's sentinel and renders byte- identically — except that a resultType beyond 64 bytes is now truncated to a prefix plus length, the #6079/#6066 rule, since the text reaches the downstream client. Also per review: modernResultTypeInputRequired constant beside the existing complete constant, the unrecognized-resultType comment cites the page's MUST rather than the SEP's SHOULD, the sentinel's comment explains why its message is frozen instead of claiming MRTR is deferred, the message-pinning test hardcodes the expected string so test and production cannot drift together, and the verbatim-relay test asserts raw bytes (assert.Equal) as its name promises. Part of #6059. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n --- pkg/vmcp/client/modern.go | 27 ++++-- pkg/vmcp/client/modern_mrtr.go | 102 +++++++++----------- pkg/vmcp/client/modern_mrtr_test.go | 143 +++++++++++++++++++++++----- pkg/vmcp/mrtr.go | 94 ++++++++++++++++-- pkg/vmcp/mrtr_test.go | 49 ++++++++++ 5 files changed, 316 insertions(+), 99 deletions(-) diff --git a/pkg/vmcp/client/modern.go b/pkg/vmcp/client/modern.go index 7e463fd91c..f5c703f7c6 100644 --- a/pkg/vmcp/client/modern.go +++ b/pkg/vmcp/client/modern.go @@ -32,6 +32,12 @@ const modernClientName = "toolhive-vmcp" // this shim does not drive — see errModernInputRequired. const modernResultTypeComplete = "complete" +// modernResultTypeInputRequired is the SEP-2322 envelope resultType announcing +// a Multi Round-Trip Request round. The only non-"complete" resultType whose +// payload survives decode (newInputRequiredError); everything else is sentinel- +// only. +const modernResultTypeInputRequired = "input_required" + // jsonRPCCodeMethodNotFound is the JSON-RPC "method not found" code. Declared // locally rather than imported (mcpcompat's METHOD_NOT_FOUND is the SDK's wire // vocabulary; the Modern layer sources its codes independently — see @@ -57,10 +63,14 @@ var errWrongEra = errors.New("backend response is not a Modern (2026-07-28) MCP // of a side-effecting tool). The cache may still be reclassified. var errLegacyResponseBody = errors.New("backend returned a Legacy-shaped body (no resultType); it may have executed") -// errModernInputRequired is returned when a Modern envelope decodes with a -// resultType other than "complete" (e.g. "input_required"). Multi-round tool -// retrieval is deferred; this shim detects and errors rather than returning a -// blank success. +// errModernInputRequired is the classification sentinel for a Modern envelope +// whose resultType is not "complete" (e.g. "input_required"). The message +// string is frozen for compatibility — it predates the MRTR seam, and both +// probeRevision's errors.Is classification and client-visible error text pin +// it — even though "unsupported" no longer tells the whole story: an +// input_required round's SEP-2322 payload now rides the typed +// vmcp.InputRequiredError wrapping this sentinel, and MRTR consumers branch on +// vmcp.InputRequiredFromError (docs/arch/16-vmcp-mrtr.md, slice 1). var errModernInputRequired = errors.New("modern response requires additional input (multi-round retrieval unsupported)") // errModernProtocolError wraps a well-formed JSON-RPC error whose code is one of @@ -268,10 +278,11 @@ func interpretModernResult(result json.RawMessage, rpcErr *modernRPCError, metho return errLegacyResponseBody default: // Typed so an "input_required" round's SEP-2322 payload (inputRequests, - // requestState) survives for MRTR consumers (InputRequiredFromError); - // unwraps to errModernInputRequired with an identical message, so - // classification and behavior are unchanged for everyone else. - return newInputRequiredError(envelope.ResultType, result) + // requestState) survives for MRTR consumers (vmcp.InputRequiredFromError) + // — gated on the three methods that may carry a round and on payload + // validity; unwraps to errModernInputRequired with an identical message, + // so classification and behavior are unchanged for everyone else. + return newInputRequiredError(method, envelope.ResultType, result) } if out != nil { diff --git a/pkg/vmcp/client/modern_mrtr.go b/pkg/vmcp/client/modern_mrtr.go index 80244e045c..b16061da86 100644 --- a/pkg/vmcp/client/modern_mrtr.go +++ b/pkg/vmcp/client/modern_mrtr.go @@ -5,74 +5,66 @@ package client import ( "encoding/json" - "errors" - "fmt" "github.com/stacklok/toolhive/pkg/vmcp" ) -// inputRequiredError is the typed form of errModernInputRequired: a Modern -// envelope decoded with a non-"complete" resultType. For -// resultType:"input_required" it carries the decoded SEP-2322 payload so -// upper layers can drive a Multi Round-Trip Request round (relay to a Modern -// downstream client, or fulfill in-request for a Legacy one — see -// docs/arch/16-vmcp-mrtr.md); for any other unrecognized resultType the -// payload is nil and only the sentinel classification remains. -// -// It unwraps to errModernInputRequired so every existing errors.Is check — -// notably probeRevision's Modern-positive classification (client.go) — keeps -// working unchanged, and Error() renders byte-identically to the previous -// fmt.Errorf wrapping so no client-visible message shifts. -type inputRequiredError struct { - resultType string - result *vmcp.InputRequiredResult -} - -func (e *inputRequiredError) Error() string { - return fmt.Sprintf("%s: resultType=%q", errModernInputRequired, e.resultType) +// mrtrMethods are the only client requests that may carry an input_required +// round: "Servers MUST NOT send InputRequiredResult responses on any other +// client requests" (spec, basic/patterns/mrtr). The set coincides with +// pkg/mcp's nameRequiredMethods today, but the concepts are independent — +// Mcp-Name validation is a header rule, this is the MRTR method allow-list, +// and the final 2026-07-28 cut may grow this one alone (SEP-2322 Final also +// allowed GetTaskPayloadRequest before tasks moved to an extension; see the +// drift list in docs/arch/16-vmcp-mrtr.md) — so they are deliberately not +// shared. +var mrtrMethods = map[string]bool{ + "tools/call": true, + "resources/read": true, + "prompts/get": true, } -func (*inputRequiredError) Unwrap() error { return errModernInputRequired } - -// newInputRequiredError builds the typed error for a non-"complete" Modern -// result envelope. Only resultType:"input_required" carries a payload: the -// spec says clients "SHOULD treat unrecognized values as invalid protocol -// responses", so other resultTypes keep pure sentinel semantics. The payload -// decode is tolerant — a malformed inputRequests/requestState still yields -// the typed error with whatever decoded (the round is undrivable either way, -// and the caller's error handling must not depend on backend well-formedness). -func newInputRequiredError(resultType string, result json.RawMessage) error { - e := &inputRequiredError{resultType: resultType} - if resultType != "input_required" { +// newInputRequiredError builds the typed error (vmcp.InputRequiredError, +// unwrapping to errModernInputRequired) for a non-"complete" Modern result +// envelope. The error always classifies and renders identically to the +// previous fmt.Errorf wrapping; whether it also carries an extractable round +// (vmcp.InputRequiredFromError reporting ok=true) is gated fail-closed: +// +// - resultType must be exactly "input_required" — the spec says an +// unrecognized resultType "MUST be considered invalid", so anything else +// (including the Tasks extension's "task", which vMCP never solicits — +// it advertises no extensions) keeps pure sentinel semantics; +// - method must be one of mrtrMethods — a round on any other request is a +// backend protocol violation, and relaying it would make vMCP the +// non-conformant server; +// - the payload must decode strictly and be drivable: server requirement 6 +// mandates at least one of inputRequests or requestState, and an envelope +// violating it (or one with a wrong-typed field) must not surface as a +// round — a consumer following client requirement 1 would retry it having +// gathered nothing, which the backend may answer with the same envelope +// again, a loop with no exit. Fail closed to the sentinel instead. +func newInputRequiredError(method, resultType string, result json.RawMessage) error { + e := &vmcp.InputRequiredError{ResultType: resultType, Sentinel: errModernInputRequired} + if resultType != modernResultTypeInputRequired || !mrtrMethods[method] { return e } var payload struct { InputRequests map[string]json.RawMessage `json:"inputRequests"` - RequestState string `json:"requestState"` + RequestState *string `json:"requestState"` + } + if err := json.Unmarshal(result, &payload); err != nil { + // Wrong-typed inputRequests/requestState: a schema violation, not a + // drivable round. The sentinel classification survives. + return e } - // Tolerant decode: see the doc comment. - _ = json.Unmarshal(result, &payload) - e.result = &vmcp.InputRequiredResult{ + if len(payload.InputRequests) == 0 && payload.RequestState == nil { + // Server requirement 6 violation ("at least one of inputRequests or + // requestState"): nothing to fulfill and nothing to echo. + return e + } + e.Result = &vmcp.InputRequiredResult{ InputRequests: payload.InputRequests, RequestState: payload.RequestState, } return e } - -// InputRequiredFromError extracts the SEP-2322 input_required payload from an -// error chain, however deeply the backend client wrapped it. It reports false -// for errors that are not an input_required round — including the -// unrecognized-resultType variant of the same sentinel — so callers can use -// it as the single MRTR branch point: -// -// if round, ok := client.InputRequiredFromError(err); ok { ... relay round ... } -// -// This is the seam the ingress half (dispatchModern's input_required envelope -// and the Legacy-client bridge; docs/arch/16-vmcp-mrtr.md slices 2-4) consumes. -func InputRequiredFromError(err error) (*vmcp.InputRequiredResult, bool) { - var ire *inputRequiredError - if errors.As(err, &ire) && ire.result != nil { - return ire.result, true - } - return nil, false -} diff --git a/pkg/vmcp/client/modern_mrtr_test.go b/pkg/vmcp/client/modern_mrtr_test.go index 4231b5464f..8b89f6479b 100644 --- a/pkg/vmcp/client/modern_mrtr_test.go +++ b/pkg/vmcp/client/modern_mrtr_test.go @@ -6,10 +6,13 @@ package client import ( "encoding/json" "fmt" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/stacklok/toolhive/pkg/vmcp" ) // TestInterpretModernResultInputRequired pins the egress MRTR seam @@ -17,20 +20,26 @@ import ( // surface as an error that (a) still satisfies every existing // errors.Is(err, errModernInputRequired) classification, (b) renders the // exact message the previous fmt.Errorf wrapping produced (no client-visible -// drift), and (c) now carries the decoded SEP-2322 payload, extractable -// through arbitrary wrapping via InputRequiredFromError. +// drift), and (c) carries an extractable SEP-2322 round — but only for a +// valid, drivable round on a method that may carry one; protocol violations +// fail closed to sentinel-only semantics. func TestInterpretModernResultInputRequired(t *testing.T) { t.Parallel() + strPtr := func(s string) *string { return &s } + tests := []struct { name string + method string result string + wantMsg string // "" means derive from the resultType template wantPayload bool wantKeys map[string]string // request key -> method, per Methods() - wantState string + wantState *string }{ { - name: "input_required with elicitation and state", + name: "input_required with elicitation and state", + method: "tools/call", result: `{ "resultType": "input_required", "inputRequests": { @@ -39,36 +48,90 @@ func TestInterpretModernResultInputRequired(t *testing.T) { }, "requestState": "opaque-blob" }`, + wantMsg: `modern response requires additional input (multi-round retrieval unsupported): ` + + `resultType="input_required"`, wantPayload: true, wantKeys: map[string]string{ "github_login": "elicitation/create", "summary": "sampling/createMessage", }, - wantState: "opaque-blob", + wantState: strPtr("opaque-blob"), }, { // SEP-2322's load-shedding shape: requestState only, no inputRequests. // The client may retry immediately; the payload must still decode. name: "load-shedding round (requestState only)", + method: "resources/read", result: `{"resultType": "input_required", "requestState": "resume-here"}`, wantPayload: true, wantKeys: nil, - wantState: "resume-here", + wantState: strPtr("resume-here"), }, { - // Malformed payload: the round is undrivable, but the typed error and - // sentinel classification must survive a backend that violates the - // schema (tolerant decode). - name: "malformed inputRequests still yields the typed error", - result: `{"resultType": "input_required", "inputRequests": "not-a-map"}`, + // requestState?: string has no minimum length: present-and-empty is + // legal, distinct from absent, and must survive as such — client + // requirement 2 makes the retry echo the exact value. + name: "present-and-empty requestState is a drivable round", + method: "prompts/get", + result: `{"resultType": "input_required", "requestState": ""}`, wantPayload: true, wantKeys: nil, - wantState: "", + wantState: strPtr(""), + }, + { + // Wrong-typed inputRequests: a schema violation. The typed error and + // sentinel classification survive, but no round is extractable — a + // consumer must fail closed on a protocol violation, not relay an + // empty envelope. + name: "malformed inputRequests fails closed to sentinel-only", + method: "tools/call", + result: `{"resultType": "input_required", "inputRequests": "not-a-map"}`, + wantPayload: false, + }, + { + // Server requirement 6 violation: neither inputRequests nor + // requestState. Indistinguishable-from-empty must not become a + // retry-forever round. + name: "empty envelope (server requirement 6 violation) fails closed", + method: "tools/call", + result: `{"resultType": "input_required"}`, + wantPayload: false, + }, + { + // A wrong-typed requestState (numeric) must not silently decode as + // absent or empty. + name: "wrong-typed requestState fails closed", + method: "tools/call", + result: `{"resultType": "input_required", "requestState": 42}`, + wantPayload: false, }, { - // An unrecognized resultType is an invalid protocol response per the + // Only tools/call, resources/read, and prompts/get may carry a round + // ("Servers MUST NOT send InputRequiredResult responses on any other + // client requests"). A valid-looking round on another method is a + // backend protocol violation; relaying it would make vMCP the + // non-conformant server. + name: "input_required on a disallowed method carries no round", + method: "completion/complete", + result: `{"resultType": "input_required", "requestState": "x"}`, + wantPayload: false, + }, + { + // An unrecognized resultType "MUST be considered invalid" per the // spec; it keeps pure sentinel semantics with NO extractable round. name: "unrecognized resultType carries no payload", + method: "tools/call", + result: `{"resultType": "partial"}`, + wantPayload: false, + }, + { + // "task" is the Tasks extension's CreateTaskResult discriminator, not + // an input round. vMCP advertises no extensions, so rejecting it here + // is correct — it lands on the same sentinel as a genuinely + // unrecognized value, even though the sentinel's "requires additional + // input" text names the wrong reason (message frozen for compat). + name: "Tasks-extension resultType carries no payload", + method: "tools/call", result: `{"resultType": "task"}`, wantPayload: false, }, @@ -78,25 +141,32 @@ func TestInterpretModernResultInputRequired(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := interpretModernResult(json.RawMessage(tt.result), nil, "tools/call", nil) + err := interpretModernResult(json.RawMessage(tt.result), nil, tt.method, nil) require.Error(t, err) // (a) classification is unchanged. require.ErrorIs(t, err, errModernInputRequired) - // (b) message is byte-identical to the previous wrapping. - var envelope struct { - ResultType string `json:"resultType"` + // (b) message is byte-identical to the previous wrapping. The + // expected text is hardcoded for the first case so the production + // template and this assertion cannot drift together; the rest derive + // it from the envelope like the old wrapping did. + if tt.wantMsg != "" { + assert.Equal(t, tt.wantMsg, err.Error()) + } else { + var envelope struct { + ResultType string `json:"resultType"` + } + require.NoError(t, json.Unmarshal([]byte(tt.result), &envelope)) + assert.Equal(t, + fmt.Errorf("%w: resultType=%q", errModernInputRequired, envelope.ResultType).Error(), + err.Error()) } - require.NoError(t, json.Unmarshal([]byte(tt.result), &envelope)) - assert.Equal(t, - fmt.Errorf("%w: resultType=%q", errModernInputRequired, envelope.ResultType).Error(), - err.Error()) // (c) payload extraction, through an extra wrapping layer to mirror // how modernCallTool wraps backend errors before they reach dispatch. wrapped := fmt.Errorf("tool call failed on backend %s: %w", "b1", err) - round, ok := InputRequiredFromError(wrapped) + round, ok := vmcp.InputRequiredFromError(wrapped) require.Equal(t, tt.wantPayload, ok) if !tt.wantPayload { assert.Nil(t, round) @@ -109,6 +179,26 @@ func TestInterpretModernResultInputRequired(t *testing.T) { } } +// TestInputRequiredErrorTruncatesHostileResultType pins that a +// backend-controlled resultType does not ride unbounded into the error text +// (which reaches the downstream client via writeModernDispatchError): beyond +// 64 bytes the message carries a prefix and the length, per the #6079/#6066 +// report-the-length rule. +func TestInputRequiredErrorTruncatesHostileResultType(t *testing.T) { + t.Parallel() + + hostile := strings.Repeat("A", 500) + result := fmt.Sprintf(`{"resultType": %q}`, hostile) + + err := interpretModernResult(json.RawMessage(result), nil, "tools/call", nil) + require.Error(t, err) + require.ErrorIs(t, err, errModernInputRequired) + assert.Equal(t, + "modern response requires additional input (multi-round retrieval unsupported): "+ + fmt.Sprintf("resultType=%q... (500 bytes)", strings.Repeat("A", 64)), + err.Error()) +} + // TestInputRequiredFromErrorRejectsForeignErrors pins that the extractor is a // safe single branch point: unrelated errors — including the OTHER Modern // sentinels — report ok=false. @@ -121,7 +211,7 @@ func TestInputRequiredFromErrorRejectsForeignErrors(t *testing.T) { errLegacyResponseBody, fmt.Errorf("wrapped: %w", errModernProtocolError), } { - round, ok := InputRequiredFromError(err) + round, ok := vmcp.InputRequiredFromError(err) assert.False(t, ok, "error %v must not extract as input_required", err) assert.Nil(t, round) } @@ -129,7 +219,8 @@ func TestInputRequiredFromErrorRejectsForeignErrors(t *testing.T) { // TestInputRequiredRequestsAreRelayedVerbatim pins the pass-through // invariant: the raw bytes of each input request survive the decode -// untouched, so the relay (slice 2) can forward them without reinterpretation. +// untouched (json.RawMessage preserves them exactly), so the relay (slice 2) +// can forward them without reinterpretation. func TestInputRequiredRequestsAreRelayedVerbatim(t *testing.T) { t.Parallel() @@ -137,8 +228,8 @@ func TestInputRequiredRequestsAreRelayedVerbatim(t *testing.T) { result := `{"resultType":"input_required","inputRequests":{"k1":` + rawReq + `}}` err := interpretModernResult(json.RawMessage(result), nil, "tools/call", nil) - round, ok := InputRequiredFromError(err) + round, ok := vmcp.InputRequiredFromError(err) require.True(t, ok) require.Contains(t, round.InputRequests, "k1") - assert.JSONEq(t, rawReq, string(round.InputRequests["k1"])) + assert.Equal(t, rawReq, string(round.InputRequests["k1"])) } diff --git a/pkg/vmcp/mrtr.go b/pkg/vmcp/mrtr.go index aa169d3c86..4bf4a99758 100644 --- a/pkg/vmcp/mrtr.go +++ b/pkg/vmcp/mrtr.go @@ -3,7 +3,11 @@ package vmcp -import "encoding/json" +import ( + "encoding/json" + "errors" + "fmt" +) // InputRequiredResult carries a Modern (2026-07-28) backend's // resultType:"input_required" envelope — a Multi Round-Trip Request round @@ -12,24 +16,94 @@ import "encoding/json" // // This is the domain-typed egress surface of MRTR (the seam the client-edge // limitation in docs/arch/10-virtual-mcp-architecture.md names): the backend -// client decodes the envelope into this type and surfaces it through a typed -// error, so upper layers can relay a round to a Modern downstream client or -// fulfill it in-request for a Legacy one. Full flow design in -// docs/arch/16-vmcp-mrtr.md. +// client decodes the envelope into this type and surfaces it through +// InputRequiredError, so upper layers can relay a round to a Modern +// downstream client or fulfill it in-request for a Legacy one. Full flow +// design in docs/arch/16-vmcp-mrtr.md. type InputRequiredResult struct { // InputRequests maps the backend's server-assigned keys to raw request // objects (ElicitRequest, CreateMessageRequest, or ListRootsRequest — // schema/draft's InputRequest union). Values are deliberately opaque // json.RawMessage: on the pass-through path vMCP relays them verbatim and // must not reinterpret them. Use Methods to inspect only the wire method - // names (e.g. for capability gating). + // names (e.g. for capability gating). Nil on a load-shedding round + // (RequestState only). InputRequests map[string]json.RawMessage // RequestState is the backend's opaque round-trip state. Per SEP-2322 the - // client "MUST echo back the exact value" on retry and "MUST NOT inspect, - // parse, modify, or make any assumptions about" it — vMCP relays it - // verbatim and never interprets it. Empty when the backend sent none. - RequestState string + // client "MUST echo back the exact value" on retry — vMCP relays it + // verbatim and never interprets it. Nil when the backend's envelope had no + // requestState field, which the retry must preserve: client requirement 2 + // says that if the result does not contain the field, the client "MUST NOT + // include one in the retry". The schema puts no minimum length on the + // field, so present-and-empty ("") is legal, distinct from absent, and + // must be echoed as the empty string. + RequestState *string +} + +// InputRequiredError is the typed error that carries an input_required round +// through the backend client's error return, so every existing +// complete-result path stays untouched. Result is non-nil only for a valid, +// drivable round (see the extraction rules on InputRequiredFromError); for +// any other non-"complete" envelope only the sentinel classification remains. +// +// Sentinel is the classification error this unwraps to — the backend client +// sets its errModernInputRequired so every existing errors.Is check keeps +// working unchanged. The fields are exported so tests (e.g. against +// mocks.MockBackendClient) can construct the error without going through the +// real client's envelope decode. +type InputRequiredError struct { + // ResultType is the envelope's raw resultType. Backend-controlled text: + // Error() truncates it, and it must never be interpolated elsewhere + // unbounded. + ResultType string + + // Result is the decoded round, nil unless the envelope was a valid + // input_required round on a method that may carry one. + Result *InputRequiredResult + + // Sentinel is the error Unwrap returns; required. It leads the Error() + // message, so the rendered text stays byte-identical to a plain + // fmt.Errorf("%w: ...", Sentinel, ...) wrapping. + Sentinel error +} + +// maxResultTypeInError bounds how much of the backend-controlled resultType +// Error() interpolates. Real resultTypes are short tokens ("complete", +// "input_required", "task"); anything longer is hostile or broken, and this +// error's text reaches the downstream client via writeModernDispatchError — +// report a prefix and the length instead of splicing the payload (the #6079 / +// #6066 rule). +const maxResultTypeInError = 64 + +func (e *InputRequiredError) Error() string { + if len(e.ResultType) > maxResultTypeInError { + return fmt.Sprintf("%s: resultType=%q... (%d bytes)", + e.Sentinel, e.ResultType[:maxResultTypeInError], len(e.ResultType)) + } + return fmt.Sprintf("%s: resultType=%q", e.Sentinel, e.ResultType) +} + +// Unwrap returns the classification sentinel, keeping every existing +// errors.Is check on the untyped sentinel working unchanged. +func (e *InputRequiredError) Unwrap() error { return e.Sentinel } + +// InputRequiredFromError extracts the SEP-2322 input_required payload from an +// error chain, however deeply the backend client wrapped it. It reports false +// for errors that are not a drivable input_required round — including the +// unrecognized-resultType, disallowed-method, and invalid-payload variants of +// the same sentinel — so callers can use it as the single MRTR branch point: +// +// if round, ok := vmcp.InputRequiredFromError(err); ok { ... relay round ... } +// +// This is the seam the ingress half (dispatchModern's input_required envelope +// and the Legacy-client bridge; docs/arch/16-vmcp-mrtr.md slices 2-4) consumes. +func InputRequiredFromError(err error) (*InputRequiredResult, bool) { + var ire *InputRequiredError + if errors.As(err, &ire) && ire.Result != nil { + return ire.Result, true + } + return nil, false } // Methods returns the JSON-RPC method name of each input request, keyed by diff --git a/pkg/vmcp/mrtr_test.go b/pkg/vmcp/mrtr_test.go index e7e535cf9f..e62a4bda4b 100644 --- a/pkg/vmcp/mrtr_test.go +++ b/pkg/vmcp/mrtr_test.go @@ -5,11 +5,60 @@ package vmcp import ( "encoding/json" + "errors" + "fmt" + "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +// TestInputRequiredError pins the carrier's contract for consumers that +// construct it directly (e.g. tests against mocks.MockBackendClient, which +// cannot reach the backend client's unexported sentinel): it unwraps to +// whatever Sentinel it was built with, renders the sentinel-prefixed message, +// and InputRequiredFromError extracts the round only when Result is non-nil. +func TestInputRequiredError(t *testing.T) { + t.Parallel() + + sentinel := errors.New("some classification") + state := "s1" + withRound := &InputRequiredError{ + ResultType: "input_required", + Result: &InputRequiredResult{RequestState: &state}, + Sentinel: sentinel, + } + require.ErrorIs(t, withRound, sentinel) + assert.Equal(t, `some classification: resultType="input_required"`, withRound.Error()) + + round, ok := InputRequiredFromError(fmt.Errorf("wrapped: %w", withRound)) + require.True(t, ok) + assert.Equal(t, &state, round.RequestState) + + // Result nil (unrecognized resultType, disallowed method, invalid + // payload): classification survives, extraction reports false. + sentinelOnly := &InputRequiredError{ResultType: "task", Sentinel: sentinel} + require.ErrorIs(t, sentinelOnly, sentinel) + round, ok = InputRequiredFromError(sentinelOnly) + assert.False(t, ok) + assert.Nil(t, round) +} + +// TestInputRequiredErrorBoundsResultType pins that Error() never interpolates +// more than 64 bytes of the backend-controlled resultType. +func TestInputRequiredErrorBoundsResultType(t *testing.T) { + t.Parallel() + + e := &InputRequiredError{ + ResultType: strings.Repeat("x", 200), + Sentinel: errors.New("sentinel"), + } + assert.Equal(t, + fmt.Sprintf("sentinel: resultType=%q... (200 bytes)", strings.Repeat("x", 64)), + e.Error()) +} + func TestInputRequiredResultMethods(t *testing.T) { t.Parallel() From 10d08c79c7d5eb0bd2c2a4e6490bb75c299730ce Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 28 Jul 2026 10:44:09 +0000 Subject: [PATCH 08/11] Correct MRTR design claims found false in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The principal-binding claim in cell 1 was false for two of six egress strategies: header_injection (static env-mounted secret) and unauthenticated (no-op) present one vMCP identity for every downstream user, so the backend's requestState principal binding cannot protect one downstream user from another. Scope the claim to the user-derived strategies and state the consequence — under shared-credential egress vMCP must bind the round itself, because no other component can — which turns wrap-vs-verbatim into a configuration-dependent obligation. Record the third arm the framing missed: a server-side D5 handle over a verbatim backend round, which closes that gap with no keys at the cost of a durable store on the pass-through path. Also from review: the D5 owner check is vacuous under incomingAuth: anonymous (every session stores the unauthenticated sentinel), so slice 5 documents that as a single-tenant caveat and the stronger-than-AEAD claim is scoped to authenticated incoming auth; the drift list gains the fifth SEP-vs-page divergence (unrecognized resultType SHOULD vs MUST) and stops calling requirements 6 and 7 page-only (one is in schema.ts, the other a core-page MUST); SEP-2577's union freeze does not name InputRequest, so that citation is scoped to what the SEP actually says; a cell-1 sequence diagram per the arch-docs convention; and the in-flight list reflects #6050/#6051 having merged. Doc 10's client-edge section still called the -32021 contract 'the planned follow-up' — #6061 landed it, and this PR is the reconciliation pass, so the refresh rides here. Part of #6059. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n --- docs/arch/10-virtual-mcp-architecture.md | 30 +-- docs/arch/16-vmcp-mrtr.md | 222 +++++++++++++++++------ 2 files changed, 184 insertions(+), 68 deletions(-) diff --git a/docs/arch/10-virtual-mcp-architecture.md b/docs/arch/10-virtual-mcp-architecture.md index f083b96a90..eaf8469c84 100644 --- a/docs/arch/10-virtual-mcp-architecture.md +++ b/docs/arch/10-virtual-mcp-architecture.md @@ -310,7 +310,7 @@ The client edge mirrors the backend edge. The Modern dispatcher (SEP-2322) is unimplemented on this edge too. When a backend tool issues a mid-call server-initiated request during a **Modern** client's `tools/call`, there is no client session to forward it to, so the call fails with an explicit -`-32603` whose message names the refused request (pinned by +error naming the refused request (pinned by `TestIntegration_Modern_RealBackend_ElicitingToolFailsCleanly`). This is a deliberate honest-unsupported error, not a gap left by accident: @@ -328,12 +328,16 @@ so a conformant 400 would tear down the entire client session to punish one call. And for a client that DID declare the capability, the 2026-07-28 vocabulary has no conformant code at all: no "operation not supported", MRTR is not a server-advertised capability, and SEP-2322 has no decline mechanism. -The planned follow-up therefore serves `-32021` (with -`data.requiredCapabilities`, and a message naming both the capability and the -gateway limitation) at **HTTP 200** for the undeclared case — deviating from -the mandated 400 for exactly the reason above — and keeps `-32603` for the -declared case as a documented spec gap. Until that lands, both cases surface -as `-32603`. +**#6061 (merged) implements exactly that two-path contract** in +`writeModernCallFailure`/`writeModernMissingCapability` (`pkg/vmcp/server`): +`-32021` with `data.requiredCapabilities` and a message naming both the +capability and the gateway limitation, served at **HTTP 200** for the +undeclared case — deviating from the mandated 400 for exactly the reason +above (tracked upstream as go-sdk#1117) — and an explicit `-32603` naming +SEP-2322 for the declared case, as a documented spec gap. The MRTR design +([16-vmcp-mrtr.md](16-vmcp-mrtr.md)) supersedes this contract slice by slice +where the backend is Modern; it is permanent for the Modern-client ↔ Legacy- +backend cell. **A clean error does not mean nothing happened.** The refusal reaches the backend mid-call, so a real backend tool may have executed — including side @@ -381,11 +385,13 @@ not parked `tools/call`. The coherent future MRTR shape for a re-aggregating gateway is **Modern-client ↔ Modern-backend pass-through** — relay a Modern backend's `inputRequests`/`requestState` to the client and the client's -`inputResponses` back, genuinely stateless at vMCP. It requires the egress -half first (today a Modern backend's `input_required` surfaces as -`errModernInputRequired`, the seam left in `pkg/vmcp/client`), and by the time -Modern backends exist to relay from, SEP-2577's deprecations make elicitation -its only durable consumer; see #5743. +`inputResponses` back, genuinely stateless at vMCP. The full design is +[16-vmcp-mrtr.md](16-vmcp-mrtr.md); its slice 1 (the egress half) has landed — +a Modern backend's `input_required` still classifies as +`errModernInputRequired`, but the typed `vmcp.InputRequiredError` now carries +the decoded round for the relay slices to consume. By the time Modern +backends exist to relay from, SEP-2577's deprecations make elicitation its +only durable consumer; see #5743 and #6059. Progress and log notifications toward Modern clients are a separate concern from MRTR: they remain spec-legal as request-scoped notifications on the diff --git a/docs/arch/16-vmcp-mrtr.md b/docs/arch/16-vmcp-mrtr.md index 3b0d366d6e..6b29effb8f 100644 --- a/docs/arch/16-vmcp-mrtr.md +++ b/docs/arch/16-vmcp-mrtr.md @@ -36,8 +36,8 @@ byte-identical to the copy this design was written against. The checklist below therefore remains pending the final cut.* **Draft-only dependencies (re-verify against the final cut).** The schema -types and the SEPs are Final and low-risk. Four load-bearing points in this -design rest on the draft *spec page's* text, which refines (or diverges from) +types and the SEPs are Final and low-risk. Five load-bearing points in this +design rest on the draft *spec pages'* text, which refines (or diverges from) the Final SEP and could still move before the final cut: - **The three-method table.** SEP-2322 (Final) also allows @@ -45,13 +45,27 @@ the Final SEP and could still move before the final cut: when tasks moved to an extension. If the final restores a fourth method, the relay's method allow-list grows — a table edit, not a design change. - **Capability gating** (server requirement 7, "MUST NOT send an - `inputRequests` that the client has not declared") is page-only — SEP-2322 - does not state it. Capability mirroring is designed around it; if the final - weakens it to SHOULD, mirroring remains correct (it is also what makes the - relay deliverable), only the backend-violation error becomes discretionary. + `inputRequests` that the client has not declared") is not stated by + SEP-2322, but it is not merely page-local either: the core `_meta` section + of `/specification/draft/basic` carries the general form as a MUST — a + server "MUST NOT rely on capabilities the client has not declared" and MUST + answer a request that needs one with `MissingRequiredClientCapabilityError` + (`-32021`) listing the missing capabilities in `data.requiredCapabilities`. + The MRTR requirement is the pattern-specific instance. Capability mirroring + is designed around it; the risk is only that the final cut reshuffles which + page states it. - **Server requirement 6** ("at least one of `inputRequests` or - `requestState`") is page-only; it affects only what vMCP treats as a - backend protocol violation. + `requestState`") is stated on the page *and* in `schema/draft/schema.ts`'s + `InputRequiredResult` doc comment ("At least one of `inputRequests` or + `requestState` MUST be present") — anchored in the Final-typed schema, so + the least likely of these to move. It governs what vMCP treats as a backend + protocol violation (the egress decode fails closed on it — see slice 1). +- **Unrecognized `resultType` severity.** SEP-2322 says the client "SHOULD + treat unrecognized values as invalid protocol responses"; the draft basic + page's ResultType section tightens it: any unrecognized value "MUST be + considered invalid". The egress decode implements the MUST (sentinel-only, + no extractable round); a final cut that reverts to SHOULD costs nothing, + since rejecting remains SHOULD-conformant. - **The `requestState` security language** differs between the two: SEP-2322 says user-specific state "MUST use some mechanism to cryptographically bind" it to the user, while the page allows omitting integrity protection @@ -119,6 +133,23 @@ halves of these cells were built in #6006; this table is the MRTR overlay. ### Cell 1 — Modern client ↔ Modern backend: stateless pass-through +```mermaid +sequenceDiagram + participant C as Modern client + participant V as vMCP + participant B as Modern backend + C->>V: tools/call (id: 1, _meta.clientCapabilities) + V->>B: tools/call (id: v1, mirrored clientCapabilities) + B-->>V: input_required (inputRequests, requestState) + V-->>C: input_required (relayed verbatim, id: 1) + note over C: fulfills inputRequests locally + C->>V: tools/call retry (id: 2, inputResponses, echoed requestState) + note over V: fresh request — any replica;
backend re-derived from routing table + V->>B: tools/call retry (id: v2, both fields verbatim) + B-->>V: complete (id: v2) + V-->>C: complete (id: 2) +``` + For a plain (non-composite) `tools/call` / `resources/read` / `prompts/get`, vMCP adds **no state of its own**: @@ -127,7 +158,8 @@ vMCP adds **no state of its own**: capabilities the downstream client declared on this request — never more (vMCP must not solicit `inputRequests` it cannot deliver downstream), never less (an empty declaration makes a compliant backend that needs input - return `-32021 MissingRequiredClientCapability`, which is today's behavior: + return `-32021 MissingRequiredClientCapability` — mandated by the core + `_meta` section's MUST, not merely predicted — which is today's behavior: `mcpparser.ModernRequestMeta` hardcodes `clientCapabilities: {}`). 2. **`input_required` relay (ingress ← egress).** When the backend returns `resultType:"input_required"`, vMCP relays `inputRequests` (values opaque, @@ -140,26 +172,68 @@ vMCP adds **no state of its own**: Round-trip state lives **only** in the backend's `requestState`, whose integrity/user-binding obligations sit with the backend that minted it, per -the spec. The end-to-end principal binding survives because vMCP's outgoing -auth (OBO / header injection / token exchange) presents the calling -principal's identity to the backend on every round — the backend binds its -state to the same principal each time. vMCP MUST NOT interpret, rewrite, or -append to the relayed `requestState`; the moment a design change requires -vMCP-owned data inside it, that data moves to the workflow-handle machinery -below (D5 protections), not into ad-hoc fields. +the spec. Whether the end-to-end principal binding survives that division of +labor **depends on the egress auth strategy**: + +- Under the **user-derived** strategies (`token_exchange`, `xaa`, + `upstream_inject`, `aws_sts`), vMCP's outgoing auth presents the calling + principal's identity to the backend on every round, so the backend can bind + its `requestState` to the same principal each time (server requirement 5) + and reject another user's echo. The claim above holds. +- Under the **shared-credential** strategies — `header_injection` (a static, + config-mounted secret whose own docstring says it "does not depend on user + identity") and `unauthenticated` (a no-op) — the backend sees **one vMCP + identity for every downstream user**. Its principal binding then binds all + rounds to that shared identity: it cannot protect one downstream user from + another, because no signal distinguishing them ever reaches it. If + cross-user misuse of a relayed `requestState` matters for such a backend, + **vMCP must bind the round to the downstream principal itself, because no + other component can** — which turns the wrap-vs-verbatim decision below + from a preference into a configuration-dependent obligation for + shared-credential egress. + +vMCP MUST NOT interpret, rewrite, or append to the relayed `requestState`; +the moment a design change requires vMCP-owned data inside it, that data +moves to the workflow-handle machinery below (D5 protections) or an +equivalent server-side entry, not into ad-hoc fields. **Open decision vs #6059 (resolve at slice 2/3, flagged, not silently diverged):** the tracking issue sketches step 2 as "wrap the backend's opaque -`requestState` with vMCP routing context (which backend it came from)". This -design deliberately relays it **verbatim** instead, re-deriving the backend on -the retry from the capability name through the routing table — because the -moment vMCP injects its own data into `requestState`, vMCP becomes a state- -minting server under MRTR server requirements 4–5 (attacker-controlled input, -integrity protection, principal binding), a key-management surface the -verbatim relay avoids entirely. The cost is the rerouted-between-rounds edge -case, which is client-recoverable (the new backend rejects foreign state and -re-elicits). If review concludes the wrapped form is preferable, the wrapper -must carry the D5-grade protections, not a bare backend ID. +`requestState` with vMCP routing context (which backend it came from)". Three +arms, no winner picked here: + +1. **Verbatim relay** (this document's default): re-derive the backend on the + retry from the capability name through the routing table. vMCP never mints + state, so MRTR server requirements 4–5 (attacker-controlled input, + integrity protection, principal binding) stay entirely with the backend — + no vMCP key-management surface. Costs: the rerouted-between-rounds edge + case, which is client-recoverable (the new backend rejects foreign state + and re-elicits); and under shared-credential egress it leaves the + cross-downstream-user gap above unclosed, since the backend cannot close + it and vMCP added nothing that could. +2. **#6059's wrapper**: vMCP wraps the backend's `requestState` with routing + context. The moment vMCP injects its own data, it becomes a state-minting + server under requirements 4–5; the wrapper must carry D5-grade protections + (integrity, principal binding, TTL) — cryptographic keys and their + management, the surface arm 1 avoids. +3. **Server-side handle over a verbatim backend round**: relay verbatim *to + the backend*, but make the **downstream-visible** `requestState` an opaque + handle into a server-side entry `{backend, capability, + owner=binding.Format(iss, sub), backend requestState, TTL}` — the same D5 + handle machinery the composite slice below already builds, and **no keys + at all** (the state never leaves the server, so there is nothing to sign + or seal). This closes cross-principal replay, cross-backend A→B + substitution, and the shared-credential gap above in one move. Its cost is + exactly the durable store this design otherwise keeps off the pass-through + path: every `input_required` round writes an entry, and plain-tool relay + stops being stateless at vMCP. + +Arm 1 maximizes statelessness, arm 3 maximizes containment with composite- +slice machinery instead of cryptography, arm 2 buys arm 3's properties at the +price of a key-management surface and is dominated by it unless the store is +unavailable. If shared-credential egress must support MRTR with cross-user +protection, arm 1 alone cannot deliver it (see above); the resolution at +slice 2/3 must say which arm serves that configuration. Failure modes, all client-recoverable per the spec's error-handling guidance (a server that got unusable `inputResponses` "SHOULD respond with a new @@ -230,18 +304,35 @@ RFC-0083 D5 sequences the durable machinery on: ≥128-bit opaque random token; an owner field on `WorkflowStatus` holding the authenticated `(iss, sub)` pair encoded exactly as the existing identity binding does — `pkg/vmcp/session/binding`'s - `Format(iss, sub)` → `iss + "\x00" + sub`, plaintext, not hashed — so the - two owner-binding mechanisms stay consistent. + `Format(iss, sub)`, which returns `iss + "\x00" + sub` (and an error for + empty components), plaintext, not hashed — so the two owner-binding + mechanisms stay consistent. - **Owner check on resume**: the retry's authenticated identity must satisfy `binding.Format(iss, sub) == status.Owner` before any state is loaded; a mismatch is an audit-visible security signal (owner-mismatch resume), fails closed, and reveals nothing about the handle's existence. - **TTL at write** (bounding the replay window per MRTR server requirement 5) and **redaction**: the handle never appears in audit or telemetry output. -- This satisfies the spec's requirement to "cryptographically bind the data - to the original user": the state never leaves the server, and the handle is - an unguessable capability whose owner is checked server-side — stronger - than client-side AEAD, with no key-management surface. +- **With authenticated incoming auth**, this satisfies the spec's requirement + to "cryptographically bind the data to the original user": the state never + leaves the server, and the handle is an unguessable capability whose owner + is checked server-side — stronger than client-side AEAD, with no + key-management surface. +- **Under `incomingAuth: anonymous`** the owner check is vacuous: every + anonymous session stores the same `binding.UnauthenticatedSentinel`, and + `validateCallerBinding` accepts any anonymous caller + (`pkg/vmcp/session/internal/security`), so "the original user" is not a + concept the deployment has — the owner-mismatch audit signal can never + fire, and protection reduces to handle unguessability plus TTL. Slice 5 + **accepts this as a documented single-tenant caveat rather than refusing to + suspend**: anonymous auth already grants any caller the tools themselves, + so a stolen handle adds resumption of a workflow the thief could have run + outright — no privilege the deployment's trust model doesn't already + concede. Refusing would break composites for exactly the single-operator + deployments anonymous auth exists for. A multi-user deployment fronting + vMCP with anything but authenticated incoming auth gets no cross-user + workflow isolation, and slice 5's docs must say so where `anonymous` is + documented. **The distinction that sizes the infrastructure**: plain-tool pass-through rounds and the Legacy-client bridge need *no* durable store; only @@ -272,14 +363,22 @@ Explicit answer to "does sampling deserve an MRTR path": **vMCP builds no sampling feature; sampling flows through the generic relay only.** - The MRTR union still carries it: `InputRequest = CreateMessageRequest | - ListRootsRequest | ElicitRequest` (schema/draft, above) — deprecated types - stay in the unions during the window ("The following union types reference - deprecated types but MUST NOT be modified during the deprecation period", - SEP-2577). -- SEP-2577 (Final) deprecates sampling as of 2026-07-28: "New implementations - SHOULD NOT add support for deprecated features unless needed for backward - compatibility with existing counterparts", with "integrate directly with - LLM provider APIs" as the sanctioned replacement. + ListRootsRequest | ElicitRequest` (schema/draft, above). SEP-2577's + explicit union freeze ("The following union types reference deprecated + types but MUST NOT be modified during the deprecation period", + `seps/2577-deprecate-roots-sampling-and-logging.md`) enumerates only + `ClientNotification`, `ClientResult`, `ServerRequest`, and + `ServerNotification` — `InputRequest` is **not** on that list (it + postdates the SEP), so its shape rests on the schema as published plus the + SEP's blanket "features remain fully functional during the deprecation + window" guarantee, not on a named freeze. Same practical effect during the + window; re-check the union at each revision rather than assuming it frozen. +- SEP-2577 (Final) deprecates sampling — along with roots and logging, the + latter irrelevant to MRTR — as of 2026-07-28: "New implementations SHOULD + NOT add support for deprecated features unless needed for backward + compatibility with existing counterparts" (SEP-2577, Capability + negotiation section), with "integrate directly with LLM provider APIs" as + the sanctioned replacement. - vMCP's position under that carve-out: the pass-through relay is **type-agnostic** — vMCP forwards whatever `inputRequests` the backend sent and the client declared capability for, without interpreting them, so @@ -340,27 +439,37 @@ Verified against go-sdk `v1.7.0-pre.3` (the transitive pin via ## Sequencing (collision-aware) -In-flight at time of writing: #6050 (modern pagination/subscriptions — -rewrites `modern_dispatch.go`/`modern_envelope.go`), #6033 (kill-switch -removal — `classification.go`, `server.go`, `serve.go` and test helpers), -#6051 (Legacy-pins the forwarding tests, adds doc 10's client-edge limitation -section). Slices are ordered so each lands without touching those files until -its predecessors merge: +Collision set at time of writing: #6050 (modern pagination/subscriptions — +rewrites `modern_dispatch.go`/`modern_envelope.go`) and #6051 (Legacy-pins +the forwarding tests, adds doc 10's client-edge limitation section) have +both **merged** (2026-07-28); #6033 (kill-switch removal — +`classification.go`, `server.go`, `serve.go` and test helpers) remains open. +Slices are ordered so each lands without touching #6033's files until it +merges: 1. **Egress MRTR surface (safe now; implemented alongside this doc).** Domain - type `vmcp.InputRequiredResult`; `pkg/vmcp/client`'s Modern shim decodes an - `input_required` envelope into a typed, `errors.Is`-compatible error - carrying the payload instead of the opaque - `errModernInputRequired`. No behavior change: capabilities are still - declared empty, so a compliant backend cannot yet send `input_required`; - the seam #6051 calls "the egress half" simply becomes load-bearing. - Files: `pkg/vmcp/mrtr.go`, `pkg/vmcp/client/modern.go` (one hook), - `pkg/vmcp/client/modern_mrtr.go` — none touched by the in-flight PRs. + types `vmcp.InputRequiredResult` and `vmcp.InputRequiredError` (carrier and + `InputRequiredFromError` extractor live beside the domain type, so slice + 2's server-side rendering never imports the concrete HTTP client and mock + consumers can construct the error); `pkg/vmcp/client`'s Modern shim decodes + an `input_required` envelope into that typed, `errors.Is`-compatible error + instead of the opaque `errModernInputRequired`. Extraction is + **fail-closed**: a round is extractable only on the three methods that may + carry one, only for `resultType:"input_required"` exactly, and only when + the payload decodes strictly and satisfies server requirement 6 — every + violation keeps pure sentinel semantics, so a slice-2 consumer can never + relay a malformed or misplaced round. `RequestState` is a `*string`, since + client requirement 2 makes absent-vs-present-and-empty an observable + distinction the retry must preserve. No behavior change: capabilities are + still declared empty, so a compliant backend cannot yet send + `input_required`; the seam #6051 calls "the egress half" simply becomes + load-bearing. Files: `pkg/vmcp/mrtr.go`, `pkg/vmcp/client/modern.go` (one + hook), `pkg/vmcp/client/modern_mrtr.go` — none touched by #6033. 2. **Ingress envelope + retry params (after #6050/#6033).** `dispatchModern` accepts `inputResponses`/`requestState`, threads them through `core.CallTool`→`BackendClient`, and renders `input_required` envelopes; parser vocabulary for the two params. Post-#6061, the rendering branch - hooks into `writeModernCallFailure` via `InputRequiredFromError`, + hooks into `writeModernCallFailure` via `vmcp.InputRequiredFromError`, alongside (never instead of) its authz-priority and capability-refusal branches — the two cannot co-occur: the refusal recorder fires only on the Legacy-backend forwarding seams, `input_required` only from a Modern @@ -390,7 +499,8 @@ from #6051 as their claims become false. ## Testing strategy - Slice 1: envelope decode (payload extraction, `errors.Is` back-compat with - the sentinel, malformed-payload tolerance), no-behavior-change pins. + the sentinel, fail-closed rejection of malformed/empty/misplaced rounds), + no-behavior-change pins. - Slice 2/3: dispatcher round-trip — `input_required` envelope shape (resultType, echoed id, relayed keys), retry threading, capability-gate refusals (undeclared → no forward; backend violation → explicit error). From e829b31b7c7744e0af970a1643937ccc4af0c5e9 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 28 Jul 2026 11:06:46 +0000 Subject: [PATCH 09/11] Regenerate CRD reference docs after main merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same cause as the earlier regen on this branch, not pre-existing staleness: a clean origin/main (92c70db2) worktree regenerates with no diff, while this branch's additions to pkg/vmcp — whose shared types the VirtualMCPServer CRD reference renders — make the generator emit two extra blank lines. The merge of main resolved docs/operator/crd-api.md to main's copy, dropping the lines the earlier regen commit had added, so the check failed again on the merge result. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n --- docs/operator/crd-api.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index c04ee2c48b..741aaa1299 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -4874,6 +4874,8 @@ This type is shared with the Kubernetes operator CRD (VirtualMCPServer.Status.Di + + From b12b13272c6b5860db816a9dd23d716c4d05180b Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 28 Jul 2026 11:24:59 +0000 Subject: [PATCH 10/11] Narrow anonymous-auth handle-theft rationale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The D5 caveat claimed a stolen resume handle concedes no privilege the anonymous trust model doesn't already grant. That holds for invoking the tools, but not for the data: resuming loads accumulated step outputs produced from the original caller's arguments, which a thief running the same workflow could not necessarily reproduce. Scope the claim to tool-invocation privilege and name the residual data-read exposure, which rests on handle secrecy and the existing redaction requirement. The decision — accept as a single-tenant caveat rather than refuse to suspend — is unchanged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n --- docs/arch/16-vmcp-mrtr.md | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/arch/16-vmcp-mrtr.md b/docs/arch/16-vmcp-mrtr.md index 6b29effb8f..cd69567549 100644 --- a/docs/arch/16-vmcp-mrtr.md +++ b/docs/arch/16-vmcp-mrtr.md @@ -326,13 +326,18 @@ RFC-0083 D5 sequences the durable machinery on: fire, and protection reduces to handle unguessability plus TTL. Slice 5 **accepts this as a documented single-tenant caveat rather than refusing to suspend**: anonymous auth already grants any caller the tools themselves, - so a stolen handle adds resumption of a workflow the thief could have run - outright — no privilege the deployment's trust model doesn't already - concede. Refusing would break composites for exactly the single-operator - deployments anonymous auth exists for. A multi-user deployment fronting - vMCP with anything but authenticated incoming auth gets no cross-user - workflow isolation, and slice 5's docs must say so where `anonymous` is - documented. + so a stolen handle adds no *tool-invocation* privilege the trust model + doesn't already concede — the thief could have run the workflow outright. + It does add a *data-read* privilege the trust model does not concede: + resuming loads another caller's accumulated step outputs, produced from + arguments the thief could not necessarily have supplied, so that + intermediate data is not something they could have obtained on their own — + a real residual exposure, gated on handle secrecy alone, which the + redaction requirement above covers. Refusing would break composites for + exactly the single-operator deployments anonymous auth exists for. A + multi-user deployment fronting vMCP with anything but authenticated + incoming auth gets no cross-user workflow isolation, and slice 5's docs + must say so where `anonymous` is documented. **The distinction that sizes the infrastructure**: plain-tool pass-through rounds and the Legacy-client bridge need *no* durable store; only From 34c4de630868c61d33202efaa7e15509b263e4ca Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 28 Jul 2026 11:24:59 +0000 Subject: [PATCH 11/11] Name missing sentinel in InputRequiredError InputRequiredError.Sentinel is documented as required, but the fields are exported so callers can build the error without the real client's envelope decode, and nothing enforces it: a literal omitting Sentinel rendered "%!s()" into text that reaches the downstream client. Guard both Error() branches with a named fallback so the omission is diagnosable instead of cryptic. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n --- pkg/vmcp/mrtr.go | 21 ++++++++++++++++++--- pkg/vmcp/mrtr_test.go | 22 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/pkg/vmcp/mrtr.go b/pkg/vmcp/mrtr.go index 4bf4a99758..231f2416d2 100644 --- a/pkg/vmcp/mrtr.go +++ b/pkg/vmcp/mrtr.go @@ -64,7 +64,10 @@ type InputRequiredError struct { // Sentinel is the error Unwrap returns; required. It leads the Error() // message, so the rendered text stays byte-identical to a plain - // fmt.Errorf("%w: ...", Sentinel, ...) wrapping. + // fmt.Errorf("%w: ...", Sentinel, ...) wrapping. Because the field is + // exported for direct construction, nothing enforces "required" at compile + // time: a literal that omits it unwraps to nil and renders + // missingSentinelText in place of the classification. Sentinel error } @@ -76,12 +79,24 @@ type InputRequiredError struct { // #6066 rule). const maxResultTypeInError = 64 +// missingSentinelText leads Error() when Sentinel is nil, which a direct +// construction that omits the required field can produce. Naming the mistake +// beats letting fmt splice "%!s()" into a message that reaches the +// downstream client. +const missingSentinelText = "input_required error with no classification sentinel" + func (e *InputRequiredError) Error() string { + sentinel := func() string { + if e.Sentinel == nil { + return missingSentinelText + } + return e.Sentinel.Error() + }() if len(e.ResultType) > maxResultTypeInError { return fmt.Sprintf("%s: resultType=%q... (%d bytes)", - e.Sentinel, e.ResultType[:maxResultTypeInError], len(e.ResultType)) + sentinel, e.ResultType[:maxResultTypeInError], len(e.ResultType)) } - return fmt.Sprintf("%s: resultType=%q", e.Sentinel, e.ResultType) + return fmt.Sprintf("%s: resultType=%q", sentinel, e.ResultType) } // Unwrap returns the classification sentinel, keeping every existing diff --git a/pkg/vmcp/mrtr_test.go b/pkg/vmcp/mrtr_test.go index e62a4bda4b..cec6723ed1 100644 --- a/pkg/vmcp/mrtr_test.go +++ b/pkg/vmcp/mrtr_test.go @@ -59,6 +59,28 @@ func TestInputRequiredErrorBoundsResultType(t *testing.T) { e.Error()) } +// TestInputRequiredErrorNilSentinel pins what a construction that omits the +// required Sentinel renders: nothing enforces the field, so Error() must name +// the missing classification rather than splicing fmt's "%!s()" into text +// that reaches the downstream client. Unwrap still reports nil — the guard +// makes the mistake legible, it does not invent a classification. +func TestInputRequiredErrorNilSentinel(t *testing.T) { + t.Parallel() + + e := &InputRequiredError{ResultType: "input_required"} + assert.Equal(t, + missingSentinelText+`: resultType="input_required"`, + e.Error()) + assert.NotContains(t, e.Error(), "%!s") + assert.NoError(t, e.Unwrap()) + + // The bounded branch guards the sentinel too. + long := &InputRequiredError{ResultType: strings.Repeat("x", 200)} + assert.Equal(t, + fmt.Sprintf("%s: resultType=%q... (200 bytes)", missingSentinelText, strings.Repeat("x", 64)), + long.Error()) +} + func TestInputRequiredResultMethods(t *testing.T) { t.Parallel()