Skip to content

MCP 2026-07-28 (5/11): multi round-trip requests — InputRequiredResult, inputResponses, requestState - #228

Open
simonx1 wants to merge 9 commits into
mcp-2026/x-mcp-headerfrom
mcp-2026/mrtr
Open

MCP 2026-07-28 (5/11): multi round-trip requests — InputRequiredResult, inputResponses, requestState#228
simonx1 wants to merge 9 commits into
mcp-2026/x-mcp-headerfrom
mcp-2026/mrtr

Conversation

@simonx1

@simonx1 simonx1 commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Summary

Fifth PR of the MCP 2026-07-28 series (stacked on #227). Implements Multi Round-Trip Requests (SEP-2322), which replaces server-initiated roots/list, sampling/createMessage and elicitation/create requests on modern servers.

  • Resolver (JsonRpcCommon#resolve_input_round_trips) — while tools/call, resources/read or prompts/get answers with resultType: "input_required", fulfil every inputRequests entry through the transport's registered handler (on_elicitation_request, on_sampling_request, on_roots_list_request — the same callbacks MCPClient::Client already wires for its elicitation handler, sampling handler and roots) and retry the original request as an independent request: new JSON-RPC id, same params, inputResponses keyed like the requests, requestState echoed verbatim or omitted when the server sent none. A result without inputRequests is retried immediately. Wired into both the stdio and HTTP transports' request path (so version renegotiation, HeaderMismatch refresh and stream re-issue all go through it).
  • Client requirements enforced — ids differ between attempts; inputResponses/requestState affect only the retry; input_required on any other method is an InvalidResultError (servers MUST NOT send it there); more than 10 consecutive round trips, an unknown input request method, a missing handler, a handler error or a malformed inputRequests raise InputRequiredError (with input_requests/request_state) without retrying.
  • Capabilities — modern requests declare elicitation (form+url), roots ({}, no listChanged) and sampling (tools when opted in) again once a handler is registered; PR 2's placeholder that withheld them is removed.

Test plan

  • bundle exec rspec — 1949 examples, 0 failures (17 new in mrtr_2026_spec.rb, incl. an end-to-end MCPClient::Client elicitation + roots round trip and a Streamable HTTP retry)
  • bundle exec rubocop — clean

🤖 Generated with Claude Code

https://claude.ai/code/session_01MoErzDypnq7hhuFBtueML7

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

simonx1 and others added 9 commits September 5, 2026 21:13
tools/call, resources/read and prompts/get may answer with resultType
"input_required". The transports now fulfil each inputRequests entry
through the registered handlers (elicitation, sampling, roots) and retry
the original request as a new request carrying inputResponses and the
opaque requestState (echoed verbatim, omitted when absent); a result
without inputRequests is retried immediately. Modern requests declare
elicitation/roots/sampling again when a handler is registered.

Round trips are bounded (10); an unfulfillable or malformed input request
raises InputRequiredError without a retry; input_required on any other
method is an InvalidResultError.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoErzDypnq7hhuFBtueML7
…plain HTTP handlers

Codex + grok findings on multi round-trip requests:
- The resolver now sits outside the per-attempt recovery on both
  transports, so retries, version renegotiation, the HeaderMismatch refresh
  and a re-issued stream keep the attempt's inputResponses/requestState.
- A handler that raises becomes InputRequiredError (its message stays in
  the local log); an explicit null inputRequests is malformed; a legacy
  session answering input_required gets InvalidResultError (the result
  type is not accepted there, on any transport).
- requestState-only answers are retried with a growing pause instead of a
  tight loop; peer-controlled keys and methods in error text are bounded.
- ServerHTTP exposes on_elicitation_request / on_roots_list_request /
  on_sampling_request so MCPClient::Client wires its handlers there too.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoErzDypnq7hhuFBtueML7
Codex second-round finding: with the new handlers registered, a legacy
ServerHTTP session would declare capabilities during initialize that it
has no server-request channel to serve. The capabilities are now omitted
on legacy sessions and kept for the modern multi round-trip pattern.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoErzDypnq7hhuFBtueML7
… log sanitizing

- A URL-mode elicitation is answered with cancel unless the handler
  explicitly accepts (true or an ElicitResult action); decline stays
  decline.
- Input requests whose params are not an object are malformed.
- Peer-controlled mode, schema text and action are sanitized in the
  client elicitation warnings.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoErzDypnq7hhuFBtueML7
A verification pass over the multi round-trip work found three contract
violations; each was reproduced before being fixed.

- The transport's own round-trip path invoked a registered sampling
  callback for an input request carrying `tools`/`toolChoice` without
  checking the declared sampling.tools sub-capability (SEP-1577). Only
  MCPClient::Client performed that check, so a host that registers
  ServerHTTP#on_sampling_request itself sampled — and answered — a request
  it had advertised no support for. InputResponses has no per-request error
  channel, so the round trip now fails with InputRequiredError and the
  sampler is never invoked.

- Adding the plain HTTP roots handler made respond_to?(:on_roots_list_request)
  true, which was the eligibility test for notifications/roots/list_changed:
  a legacy plain HTTP session, whose initialize correctly declares
  `capabilities: {}`, was told the roots list had changed. Eligibility now
  reads the transport's declared client capabilities, as the lifecycle
  requires.

- URL-mode elicitation normalization rebuilt an explicit action as an
  action-only hash, dropping the handler's `_meta` on both the round-trip
  and the legacy server-request path. ElicitResult carries `_meta` in every
  mode, so it is preserved; form-mode-only `content` is still stripped.

New examples in spec/lib/mcp_client/mrtr_2026_verify_spec.rb fail if any of
the three fixes is reverted, and close the coverage gaps the same pass
named: retries now run past the five-second pacing ceiling, a malicious
requestedSchema pins the escaped schema warning, a real MCPClient::Client
round trip runs over plain HTTP instead of asserting on callback ivars, and
concurrent round trips, a continuation surviving the HeaderMismatch
refresh, and the legacy paths are covered.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoErzDypnq7hhuFBtueML7
…e gaps

Review round 3 (codex) found one correctness defect and four
behaviour-breaking mutations that the suite did not catch.

Correctness: an InputRequiredResult is defined only for tools/call,
resources/read and prompts/get (basic/patterns/mrtr "Supported
Requests"). server/discover is not one of them, but the probe went
straight through process_jsonrpc_response — which accepts input_required
on a modern session — into apply_discover_result, so a discover answer
carrying resultType "input_required" was accepted as successful
discovery: the version and capabilities out of an unfinished result were
adopted and cached, and the first ping (which is answered from the
probe's own round trip) handed that result back to the caller.
apply_discover_result now refuses such a result before applying or
caching anything. The refusal is a ConnectionError, not an
InvalidResultError, because a server that answered server/discover at
all is modern and must never fall back to the initialize handshake.

Coverage for the four mutations that previously left every example
green:

* attempts are rebuilt from the CALLER's params — a continuation field
  the server stops sending is dropped, fulfilled inputResponses give way
  to a state-only round, and continuation fields the caller supplied (in
  either key spelling) are replaced rather than kept
* the caller's params hash is never mutated (params.dup)
* tool-enabled sampling actually delivers `tools`, `toolChoice`,
  `maxTokens` and the messages to the sampler, including a multi
  tool_use answer and the matching tool_result messages on the next
  round, and through MCPClient::Client
* a handler result that is not a result object (nil, a scalar, an array)
  fails the round trip

Plus the coverage the same pass found thin: symbol-keyed handler errors,
a multi-entry map where one entry fails (no partial replay), a
continuation re-issued after a Streamable HTTP response stream closed
empty, a continuation through a version renegotiation, a plain HTTP
prompts/get round trip, round trips over real stdio response handling
(reader thread and id correlation, not a stubbed wait_response), the
ten/eleven round-trip boundary, the prompts/get retry's own
inputResponses/requestState/arguments, per-thread results and
overlapping round trips that share an input key with one failing,
roots/list_changed across a mixed fleet, and the URL-mode consent
normalization on the legacy path with symbol-keyed answers.

The "retries immediately" example is renamed: the retry is paced, and
the doc comment and CHANGELOG said the same thing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoErzDypnq7hhuFBtueML7
The branches below added two guards for an unfinished answer, written
when nothing could drive one: the response parser refused it, and the
wrappers that project a payload out of a result refused it again. This
branch implements the round trips, so the resolver that wraps every
request is what drives an input_required result to a finished answer —
and what reports the condition when it cannot. The parser hands it on;
the wrapper guard stays as the backstop for a discriminator no resolver
claims, without the message's stale claim that round trips are
unimplemented.

The recoveries an attempt may need — version renegotiation, the
HeaderMismatch refresh, a re-issued stream — move into their own mixin;
they sit under the resolver, since all three re-send the same params.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoErzDypnq7hhuFBtueML7
… alive

An InputRequiredResult may carry only requestState, so an unfinished
server/discover answer need not look like a DiscoverResult at all. The
shape test ran first, so that answer was read as a permissive legacy
endpoint and the probe sent initialize to a modern server. Check the
2026-only discriminator before the shape (require_discover_result!), and
raise ModernServerError rather than a bare ConnectionError so the verdict
survives MCPClient.connect's transport detector instead of being handed on
to the legacy SSE and HTTP+POST fallbacks.

Read the round-trip payload in the protocol's own key spelling. ::result_type
already tolerates a symbolizing JSON middleware for the discriminator; the
rest of the result did not, so a symbolized InputRequiredResult entered the
loop and then retried with neither the fulfilled inputResponses nor the
requestState the server MUST get back.

On stdio, re-establish a retired transport before each round. The handler
that gathers the input waits for a person, and a subprocess that exits under
it left the continuation writing to a dead pipe; the stdio spec says to
restart such a server, so the continuation reaches a fresh process with the
answers and state it was gathered for.

An elicitation action outside accept/decline/cancel is now answered cancel
instead of being rewritten to accept — consent is explicit, as it already was
in URL mode. A handler that returns bare content and no action is unchanged.

Coverage: the trip-cap constant assertion and the plain HTTP ivar check are
replaced by behaviour (a Client sampling round trip over plain HTTP); the
caller-supplied continuation fields example now makes all four deletions
load-bearing; the concurrency examples fail when the resolver is serialized;
a new example pins the single HeaderMismatch refresh across rounds. The
unfinished read_resource suite drives the resolver on stdio with one scripted
answer instead of sleeping 38s to the ceiling. New: an SSE-delivered round
trip, an empty inputRequests map, an empty requestState, a timed-out
continuation cancelled by its own id, schema-invalid content, a nil sampling
rejection and a mode-less elicitation through Client.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoErzDypnq7hhuFBtueML7
The branches below refuse an unfinished answer wherever they meet one,
because nothing could drive it. This branch drives it, so their examples
say what happens now: a state-only continuation is re-sent with the state
echoed under a new id, which is the retry their own comment said would
arrive; a method the pattern does not cover is named as such and carries
the whole answer on the error, so nothing is discarded whichever guard
reports it; and the continuation fixture is a real InputRequests map,
keyed the way the resolver keys its responses, rather than a list.

The recovery an attempt may need keeps the nested-exchange guards from
the branch below: a rejection that escaped a listener's own call is that
call's, and re-sending this one on it would execute it twice.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoErzDypnq7hhuFBtueML7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant