Multi Round-Trip Requests (MRTR)#1458
Conversation
f1dd4c4 to
5845866
Compare
Resolves conflicts from rebasing the MRTR work (originally branched from 4140c6d) onto the current main (b8c4d95). Key conflict resolutions: - McpClientImpl.SendRequestAsync: combine SEP-2243 tool-context attachment with MRTR retry loop for IncompleteResult. - McpSessionHandler.SendRequestAsync: take MRTR's outgoing filter and request logging. - McpServerImpl.InvokeHandlerAsync: take MRTR's CreateDestinationBoundServer. - docs/concepts/index.md: combine main's Tasks entry with MRTR additions. - MapMcpTests.cs: keep main's new IncomingFilter/OutgoingFilter tests in full, drop MRTR's outdated overload usage by going through configureClient. - MrtrIntegrationTests.cs: gate with #if !NET472 (uses ReadLineAsync(CT)). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- IncompleteResult/IncompleteResultException -> InputRequiredResult/InputRequiredException - Wire format: result_type -> resultType, `incomplete` -> `input_required` - Drop ExperimentalProtocolVersion option; opt in via ProtocolVersion = `DRAFT-2026-v1` - Add DraftProtocolVersion constant and include in SupportedProtocolVersions - Restrict implicit MRTR continuation path to legacy stateful sessions; DRAFT-2026-v1 and stateless sessions always use the exception-based path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Implicit MRTR (handler suspension via ElicitAsync) requires both client support (DRAFT-2026-v1) and a stateful session. All other cases fall through to the exception-based path, which transparently resolves InputRequiredException via legacy JSON-RPC requests for clients that don't speak MRTR. - Drop the now-redundant ProtocolVersion pin from ConfigureExperimentalServer in MapMcpTests.Mrtr; server uses the negotiated version like any other server. - Rewrite the obsolete WithoutExperimental low-level test now that the experimental flag is gone; it now verifies retry exhaustion when no input requests are supplied. - Update other test assertions to use the literal DRAFT-2026-v1 string. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| public CreateMessageResult? SamplingResult => | ||
| JsonSerializer.Deserialize(RawValue, McpJsonUtilities.JsonContext.Default.CreateMessageResult); | ||
|
|
||
| /// <summary> | ||
| /// Gets the response as an <see cref="ElicitResult"/>. | ||
| /// </summary> | ||
| /// <returns>The deserialized elicitation result, or <see langword="null"/> if deserialization fails.</returns> | ||
| [JsonIgnore] | ||
| public ElicitResult? ElicitationResult => | ||
| JsonSerializer.Deserialize(RawValue, McpJsonUtilities.JsonContext.Default.ElicitResult); | ||
|
|
||
| /// <summary> | ||
| /// Gets the response as a <see cref="ListRootsResult"/>. | ||
| /// </summary> | ||
| /// <returns>The deserialized roots list result, or <see langword="null"/> if deserialization fails.</returns> | ||
| [JsonIgnore] | ||
| public ListRootsResult? RootsResult => | ||
| JsonSerializer.Deserialize(RawValue, McpJsonUtilities.JsonContext.Default.ListRootsResult); |
There was a problem hiding this comment.
Typed accessors: silent garbage on wrong call + reparse on every access
These three properties each call JsonSerializer.Deserialize(RawValue, ...) unconditionally on every read.
- No type guard. Unlike
InputRequest, which gates onMethod,InputResponsehas no discriminator on the wire (per spec, the type is implied by the matchingInputRequest.Method). Since none ofElicitResult/CreateMessageResult/ListRootsResultmark any property asrequiredand STJ ignores unknown properties by default, reading the wrong accessor returns a fully-populated-but-empty object (e.g.,ElicitResult { Action = "cancel", Content = null }) instead of throwing. Callers cannot distinguish a real "cancel" from picking the wrong accessor. - Re-deserializes on every access. Properties look like cheap field reads, but each one parses the
JsonElementand allocates a fresh result object.result.SamplingResult.Modelfollowed byresult.SamplingResult.Roleis two full deserializations.
Suggestion: drop the three typed properties; keep only the generic helper
public T? Deserialize<T>(JsonTypeInfo<T> typeInfo)
{
Throw.IfNull(typeInfo);
return JsonSerializer.Deserialize(RawValue, typeInfo);
}Call sites change from response.ElicitationResult?.Action to:
var elicit = response.Deserialize(McpJsonUtilities.JsonContext.Default.ElicitResult);
var action = elicit?.Action;Why:
- The caller already knows the method from the
InputRequiredResult.InputRequestskey, so the typed properties save nothing and add a footgun. - One explicit deserialization per caller (callers can cache locally), with no hidden per-access cost.
- Smaller
[Experimental]surface to evolve as SEP-2322 settles. - Consistent with the spec keeping the discriminator on the request side.
Keep FromSamplingResult / FromElicitResult / FromRootsResult factories. They still document expected types and stay symmetric with InputRequest.For*.
In-SDK consumers are tests and the example in InputRequiredException XML doc; both updates are mechanical.
| private async ValueTask<IDictionary<string, InputResponse>> ResolveInputRequestsAsync( | ||
| IDictionary<string, InputRequest> inputRequests, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| var responses = new Dictionary<string, InputResponse>(inputRequests.Count); | ||
|
|
||
| // Resolve all input requests concurrently | ||
| var tasks = new List<(string Key, Task<InputResponse> Task)>(inputRequests.Count); | ||
| foreach (var kvp in inputRequests) | ||
| { | ||
| tasks.Add((kvp.Key, ResolveInputRequestAsync(kvp.Value, cancellationToken))); | ||
| } | ||
|
|
||
| foreach (var entry in tasks) | ||
| { | ||
| responses[entry.Key] = await entry.Task.ConfigureAwait(false); | ||
| } | ||
|
|
There was a problem hiding this comment.
Failure-mode cleanup: remaining handlers leak on first failure
Tasks are started concurrently (good), then gathered sequentially. On the happy path this is equivalent to Task.WhenAll. On failure, the first await entry.Task rethrows and the method exits, but:
- Remaining handlers keep running. Sampling/elicitation/roots are user-facing (model calls, UI prompts); the caller has given up, yet dialogs stay open and tokens keep burning. There is no proactive cancellation of
tasks[1..N]. - Late successes are dropped silently. Anything that completes after we bail is thrown away (handler ran for nothing).
- Late exceptions risk being unobserved.
Task.WhenAllwould aggregate them onto a single observed task; here each remaining task is observed only if someone awaits it, which nobody does.
The mirror loop in McpServerImpl.InvokeWithInputRequiredResultHandlingAsync (back-compat shim) is worse: it is fully sequential, so each elicitation/create / sampling/createMessage to the client waits for the previous one to return, multiplying round-trip latency.
Suggestion
Use Task.WhenAll with a linked CTS so a single failure stops the rest cleanly and all exceptions are observed:
private async ValueTask<IDictionary<string, InputResponse>> ResolveInputRequestsAsync(
IDictionary<string, InputRequest> inputRequests,
CancellationToken cancellationToken)
{
using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
var keyed = new (string Key, Task<InputResponse> Task)[inputRequests.Count];
int i = 0;
foreach (var kvp in inputRequests)
keyed[i++] = (kvp.Key, ResolveInputRequestAsync(kvp.Value, linkedCts.Token));
try
{
await Task.WhenAll(keyed.Select(k => k.Task)).ConfigureAwait(false);
}
catch
{
linkedCts.Cancel(); // stop any handlers still running
try { await Task.WhenAll(keyed.Select(k => k.Task)).ConfigureAwait(false); } catch { }
throw;
}
var responses = new Dictionary<string, InputResponse>(keyed.Length);
foreach (var (key, task) in keyed)
responses[key] = task.Result;
return responses;
}Apply the same shape to the server-side back-compat resolver in McpServerImpl so its requests start concurrently too.
Summary
This PR implements Multi Round-Trip Requests (MRTR) in the C# MCP SDK, demonstrating that the SEP-2322 proposal can be implemented in a fully backwards-compatible way. The existing
await-based server APIs (ElicitAsync,SampleAsync,RequestRootsAsync) continue to work identically — the SDK transparently handles the new wire protocol when both sides opt in, and falls back to legacy JSON-RPC requests when they don't.This implementation is intended to serve as a reference for other SDK maintainers (TypeScript, Python, Go, Java) implementing MRTR, particularly around the backwards compatibility story and the interplay between protocol negotiation and handler behavior.
35 files changed, 4,725 lines added across 10 commits. ~2,200 lines of test coverage.
Motivation
As discussed in the Core Maintainer's meeting (accepted with changes, 🟢1 🟡7 🔴0), MRTR addresses fundamental scalability issues with the current server-to-client request model:
What This PR Demonstrates
1. Full Backwards Compatibility via Protocol Negotiation
This was the most debated topic in the spec PR (felixweinberger, maciej-kisiel, CaitieM20). The C# SDK proves all four combinations work seamlessly:
ElicitAsync/SampleAsyncautomatically send legacy JSON-RPC requests, as do IncompleteResultExceptionsKey insight: The existing
await server.ElicitAsync(...)API doesn't change at all. When the connected client supports MRTR, the SDK returns anIncompleteResultwithinputRequestsinstead of sending aelicitation/createJSON-RPC request. When the client doesn't support MRTR, it sends the legacy request. Tool authors don't need to know or care which path is taken.The determination is made via protocol version negotiation during
initialize:This directly answers Randgalt's question — yes, the server knows the client supports MRTR purely from the negotiated protocol version. No new capabilities are needed.
2. Type Discrimination via
result_typeThis was flagged as a critical issue by maxisbey: since
Resultallows arbitrary extra fields, anIncompleteResultwith only optional fields is indistinguishable from aCallToolResult. The spec resolved this with aresult_typediscriminator field.The C# SDK implements this cleanly:
IncompleteResultalways serializes with"result_type": "incomplete"McpClientImpl.SendRequestAsync()checks forresult_type == "incomplete"on every response, triggers the retry loop if foundresult_typeis absent or any other value, the result deserializes as the expected type (backwards compatible)This approach is extensible for future result types (tasks, callbacks, streaming) as CaitieM20 noted.
3. Two Server-Side API Levels
High-Level API (Stateful Servers)
Tool handlers use
await— the SDK suspends the handler in memory and resumes it when the client retries with responses:Internally, the handler task is suspended via
MrtrContextand stored in aConcurrentDictionary<string, MrtrContinuation>keyed by a generated continuation ID. On retry, the continuation is looked up, the handler is resumed with the client's response, and execution continues from whereElicitAsyncwas awaited.Low-Level API (Stateless Servers)
For servers that can't keep handler state in memory (stateless HTTP, serverless functions), handlers throw
IncompleteResultExceptionand manage their own state viarequestState:The
IncompleteResultExceptionapproach was chosen because C# doesn't yet have discriminated unions. However, C# unions are in active development, and when available,IncompleteResultreturn types will be a natural fit — returning either the final result or anIncompleteResultfrom a single method without exceptions. The exception-based API will remain supported but we expect the union-based approach to be preferred.4. Client-Side Transparency
The client retry loop is fully automatic.
CallToolAsynclooks the same regardless of whether MRTR is active:Under the hood,
McpClientImpl.SendRequestAsyncdetectsresult_type: "incomplete", resolves allinputRequestsby dispatching to the registered handlers (ElicitationHandler,SamplingHandler,RootsHandler), and retries withinputResponsesattached. Multiple input requests in a singleIncompleteResultare resolved concurrently — all handler tasks are started immediately, then awaited.The retry loop has a maximum of 10 attempts (not currently user-configurable). The escape hatch is
CancellationToken.5. Concurrent Multi-Input Resolution
A single
IncompleteResultcan request multiple types of input simultaneously:The client resolves all three concurrently and retries with all responses in one request. This is verified by tests using
TaskCompletionSourcebarriers that prove all three handlers run simultaneously.6. No Old-Style Requests with MRTR
When MRTR is negotiated, the server never sends
elicitation/create,sampling/createMessage, orroots/listJSON-RPC requests. This is verified by message filter tests that inspect every outgoing message and confirm onlyIncompleteResultresponses are used.This is important for clients that can't support SSE streams (cloud-hosted clients) — MRTR means they can support elicitation and sampling via standard HTTP request/response.
7. MRTR-Native Backward Compatibility for the Low-Level API
Tools written with the low-level
IncompleteResultExceptionpattern work automatically with clients that don't support MRTR. When a tool throwsIncompleteResultExceptionand the client hasn't negotiated MRTR, the SDK resolves eachInputRequestby sending the corresponding standard JSON-RPC call (elicitation, sampling, or roots) to the client, then retries the handler with the resolved responses — all internal to the server. The client never sees theIncompleteResult.This mirrors the Python SDK's
sse_retry_shimapproach, but is built into the SDK rather than requiring an explicit wrapper. It means authors can write a single tool implementation using the MRTR-native pattern and it works with any client:IncompleteResultsent over the wire → client resolves and retrieselicitation/createJSON-RPC to the client, collects the response, and retries the handler internallyThe
IsMrtrSupportedcheck is no longer required for basic functionality — it remains useful for tools that want to provide a different experience when MRTR isn't available (e.g., a richer fallback message), but tools that just want elicitation/sampling can throwIncompleteResultExceptionunconditionally.New Protocol Types
IncompleteResultresult_type: "incomplete",inputRequests, and/orrequestStateIncompleteResultExceptionIncompleteResultInputRequestForElicitation(),ForSampling(),ForRootsList()InputResponseElicitationResult,SamplingResult,RootsResultAll new types are marked
[Experimental(MCPEXP001)]and gated behindExperimentalProtocolVersion = "2026-06-XX".RequestParamsExtensionsAll request parameter types (
CallToolRequestParams,GetPromptRequestParams,ReadResourceRequestParams, etc.) inherit two new optional properties fromRequestParams:InputResponses— Client's responses to the server's input requests, keyed by the same keys frominputRequestsRequestState— Opaque string echoed back from the previousIncompleteResultThese are populated only on retries. On initial requests, both are null.
Test Coverage
~2,200 lines of tests across 8 test files covering:
result_typediscriminator, serialization/deserializationElicitAsync/SampleAsyncwith MRTR, automatic fallback to legacyIncompleteResultException,requestStatemanagement, multi-round tripsinputRequestsin a singleIncompleteResult, verified concurrent executionElicitAsync/SampleAsynccalls (prevented), message filter verificationDocumentation
docs/concepts/mrtr/mrtr.md— comprehensive guide with high-level and low-level API examples, compatibility matrix, backward compatibility sectionelicitation.md,sampling.md,roots.md— each now includes an MRTR section showing bothawait-based andIncompleteResultException-based approachestoc.ymlandindex.mdnavigation entriesOpen Questions for the Spec
InputRequestmethod field: The spec'sInputRequesttype is a union ofCreateMessageRequest | ElicitRequest | ListRootsRequest. In practice, themethodfield is needed for deserialization since the params shapes can overlap. The C# SDK usesInputRequest.Methodas the discriminator for typed deserialization. The spec should be explicit thatmethodis required.inputResponses. This could make the upgrade scenario for servers relying on errors from upgrading to the new protocol if they're capable of maintaining the stateful session required for such an interaction to makes sense even in stdio. While this isn't as critical as keeping sessions in the protocol in general, I strongly agree with this feedback.[Experimental(MCPEXP001)]and gated behindExperimentalProtocolVersion = "2026-06-XX", so thatMcp-Protocol-Headerversion is what I'm doing for protocol negotiation, and naturally the initialize handshake in stateful modes. I think we have conformance tests these must match in stateful mode). Anyway, is that how we're going to do this before the next protocol version is ratified?