Skip to content

Commit a8c60d0

Browse files
halter73Copilot
andcommitted
Pre-undraft cleanup: revert unrelated noise, drop low-level/high-level framing, restore lost theory coverage
- Revert BOM-only diffs on AIFunctionMcpServerTool.cs and DelegatingMcpServerTool.cs. - Drop the unused System.Diagnostics.CodeAnalysis using in McpServerTool.cs. - Restore the trailing newline in McpServerToolAttribute.cs. - Revert the NegotiatedProtocolVersion stub change in McpServerTests.cs (only the deleted ThrowIfDraftProtocol gate needed it). - Drop the stray blank line in MapMcpTests.cs. - Inline IsLowLevelMrtrAvailable into a public override IsMrtrSupported on McpServerImpl; DestinationBoundMcpServer.IsMrtrSupported is now a simple proxy. - Rewrite the stale IsStatefulSession XML doc. - Rename MrtrLowLevelApiTests -> MrtrInputRequiredExceptionTests, and drop low-level/high-level adjectives from MRTR tests + docs. - Restore InlineData(true) on Mrtr_MixedExceptionAndAwaitStyle (covers draft+stateful mixed mode); add AssertMrtrUsedAtLeastOnce helper. - Collapse Mrtr_ParallelAwaits to a Fact (under the new contract draft+stateful behaves the same as legacy+stateful). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5db1781 commit a8c60d0

14 files changed

Lines changed: 59 additions & 87 deletions

File tree

src/ModelContextProtocol.Core/Protocol/InputRequiredException.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace ModelContextProtocol.Protocol;
88
/// </summary>
99
/// <remarks>
1010
/// <para>
11-
/// This exception is part of the low-level Multi Round-Trip Requests (MRTR) API. Tool handlers
11+
/// This exception is part of the Multi Round-Trip Requests (MRTR) API. Tool handlers
1212
/// throw this exception to directly control the input-required result payload, including
1313
/// <see cref="Protocol.InputRequiredResult.InputRequests"/> and <see cref="Protocol.InputRequiredResult.RequestState"/>.
1414
/// </para>
@@ -30,7 +30,7 @@ namespace ModelContextProtocol.Protocol;
3030
/// </remarks>
3131
/// <example>
3232
/// <code>
33-
/// [McpServerTool, Description("A stateless tool using low-level MRTR")]
33+
/// [McpServerTool, Description("A stateless tool using MRTR")]
3434
/// public static string MyTool(McpServer server, RequestContext&lt;CallToolRequestParams&gt; context)
3535
/// {
3636
/// if (context.Params.RequestState is { } state)

src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using Microsoft.Extensions.AI;
1+
using Microsoft.Extensions.AI;
22
using Microsoft.Extensions.DependencyInjection;
33
using ModelContextProtocol.Protocol;
44
using System.ComponentModel;

src/ModelContextProtocol.Core/Server/DelegatingMcpServerTool.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using ModelContextProtocol.Protocol;
1+
using ModelContextProtocol.Protocol;
22

33
namespace ModelContextProtocol.Server;
44

src/ModelContextProtocol.Core/Server/DestinationBoundMcpServer.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ internal sealed class DestinationBoundMcpServer(McpServerImpl server, ITransport
1414
public override IServiceProvider? Services => server.Services;
1515
public override LoggingLevel? LoggingLevel => server.LoggingLevel;
1616

17-
public override bool IsMrtrSupported => server.IsLowLevelMrtrAvailable();
17+
public override bool IsMrtrSupported => server.IsMrtrSupported;
1818

1919
public override ValueTask DisposeAsync() => server.DisposeAsync();
2020

src/ModelContextProtocol.Core/Server/McpServerImpl.cs

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -781,7 +781,7 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false)
781781
// (client request cancellation, session shutdown, MRTR teardown), not a
782782
// tool error.
783783
// Skip logging for InputRequiredException — it's normal MRTR control flow,
784-
// not an error (the low-level API uses it to signal an InputRequiredResult).
784+
// not an error (tools throw it to signal an InputRequiredResult).
785785
if (!(e is OperationCanceledException && cancellationToken.IsCancellationRequested) && e is not InputRequiredException)
786786
{
787787
ToolCallError(request.Params?.Name ?? string.Empty, e);
@@ -1134,26 +1134,17 @@ internal bool ClientSupportsMrtr() =>
11341134
_negotiatedProtocolVersion == McpSessionHandler.DraftProtocolVersion;
11351135

11361136
/// <summary>
1137-
/// Returns <see langword="true"/> when the session is stateful (i.e., the same server instance
1138-
/// will handle subsequent requests). The implicit MRTR path — where a handler can call
1139-
/// <c>ElicitAsync</c>/<c>SampleAsync</c> and the SDK suspends/resumes the handler across an
1140-
/// <see cref="InputRequiredResult"/> round trip — requires the continuation map to outlive the
1141-
/// initial response, so it is only available on stateful sessions. Stateless transports always
1142-
/// go through the exception-based path.
1137+
/// Returns <see langword="true"/> when the session is stateful — the same server instance handles
1138+
/// subsequent requests on the same session. The legacy backcompat resolver in
1139+
/// <see cref="InvokeWithInputRequiredResultHandlingAsync"/> needs a stateful session so it can send
1140+
/// <c>elicitation/create</c> / <c>sampling/createMessage</c> / <c>roots/list</c> to the client and
1141+
/// retry the handler with the responses.
11431142
/// </summary>
11441143
internal bool IsStatefulSession() =>
11451144
_sessionTransport is not StreamableHttpServerTransport { Stateless: true };
11461145

1147-
/// <summary>
1148-
/// Checks whether the low-level MRTR API (<see cref="InputRequiredException"/>) is available
1149-
/// for the current request. Returns <see langword="true"/> in all cases except stateless mode
1150-
/// with a client that hasn't negotiated MRTR — that's the one configuration where nobody can
1151-
/// drive the retry loop (the server can't send JSON-RPC requests to the client, and the client
1152-
/// doesn't know about <c>InputRequiredResult</c>).
1153-
/// </summary>
1154-
internal bool IsLowLevelMrtrAvailable() =>
1155-
ClientSupportsMrtr() ||
1156-
_sessionTransport is not StreamableHttpServerTransport { Stateless: true };
1146+
/// <inheritdoc />
1147+
public override bool IsMrtrSupported => ClientSupportsMrtr() || IsStatefulSession();
11571148

11581149
/// <summary>
11591150
/// Invokes a handler and catches <see cref="InputRequiredException"/> to convert it to an

src/ModelContextProtocol.Core/Server/McpServerTool.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
using Microsoft.Extensions.DependencyInjection;
33
using ModelContextProtocol.Client;
44
using ModelContextProtocol.Protocol;
5-
using System.Diagnostics.CodeAnalysis;
65
using System.Reflection;
76
using System.Text.Json;
87

src/ModelContextProtocol.Core/Server/McpServerToolAttribute.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,5 +325,4 @@ public ToolTaskSupport TaskSupport
325325
get => _taskSupport ?? ToolTaskSupport.Forbidden;
326326
set => _taskSupport = value;
327327
}
328-
329-
}
328+
}

tests/Common/Utils/ServerMessageTracker.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,16 @@ public void AssertMrtrUsed()
7272
Assert.Empty(_legacyRequestMethods);
7373
}
7474

75+
/// <summary>
76+
/// Asserts that MRTR was used at least once (at least one InputRequiredResult response was sent),
77+
/// independent of whether the session also issued any legacy server-to-client requests.
78+
/// </summary>
79+
public void AssertMrtrUsedAtLeastOnce()
80+
{
81+
Assert.True(_incompleteResultCount > 0,
82+
"Expected at least one InputRequiredResult response (MRTR mode), but none were detected.");
83+
}
84+
7585
/// <summary>
7686
/// Asserts that legacy mode was used: at least one legacy JSON-RPC request was sent
7787
/// and no MRTR retries or InputRequiredResult responses were detected.

tests/ModelContextProtocol.AspNetCore.Tests/MapMcpTests.Mrtr.cs

Lines changed: 28 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ private static async Task<string> MrtrMixed(McpServer server, RequestContext<Cal
153153

154154
[Theory]
155155
[InlineData(false)]
156+
[InlineData(true)]
156157
public async Task Mrtr_MixedExceptionAndAwaitStyle(bool experimentalClient)
157158
{
158159
// The server always supports DRAFT-2026-v1 (it's in SupportedProtocolVersions). The
@@ -217,7 +218,10 @@ public async Task Mrtr_MixedExceptionAndAwaitStyle(bool experimentalClient)
217218

218219
if (experimentalClient)
219220
{
220-
messageTracker.AssertMrtrUsed();
221+
// Rounds 1-2 use wire-format MRTR (InputRequiredResult), but round 3's await calls
222+
// still issue legacy elicitation/create + sampling/createMessage requests, so this
223+
// configuration is mixed-mode.
224+
messageTracker.AssertMrtrUsedAtLeastOnce();
221225
}
222226
else
223227
{
@@ -228,75 +232,45 @@ public async Task Mrtr_MixedExceptionAndAwaitStyle(bool experimentalClient)
228232
[McpServerTool(Name = "mrtr-parallel-await")]
229233
private static async Task<string> MrtrParallelAwait(McpServer server, CancellationToken ct)
230234
{
231-
// Start the first await — succeeds with MRTR (creates exchange)
232235
var elicitTask = server.ElicitAsync(new ElicitRequestParams
233236
{
234237
Message = "Parallel elicit",
235238
RequestedSchema = new()
236239
}, ct);
237240

238-
// Start the second await. This path is only exercised for legacy clients now
239-
// that draft clients must use InputRequiredException instead of await-style requests.
240-
try
241+
var sampleTask = server.SampleAsync(new CreateMessageRequestParams
241242
{
242-
var sampleTask = server.SampleAsync(new CreateMessageRequestParams
243-
{
244-
Messages = [new SamplingMessage { Role = Role.User, Content = [new TextContentBlock { Text = "Parallel sample" }] }],
245-
MaxTokens = 100
246-
}, ct);
243+
Messages = [new SamplingMessage { Role = Role.User, Content = [new TextContentBlock { Text = "Parallel sample" }] }],
244+
MaxTokens = 100
245+
}, ct);
247246

248-
// If we get here, both calls succeeded (non-MRTR path)
249-
var sampleResult = await sampleTask;
250-
var elicitResult = await elicitTask;
251-
return $"parallel-ok:{elicitResult.Action}:{sampleResult.Content.OfType<TextContentBlock>().First().Text}";
252-
}
253-
catch (InvalidOperationException ex)
254-
{
255-
return ex.Message;
256-
}
247+
var sampleResult = await sampleTask;
248+
var elicitResult = await elicitTask;
249+
return $"parallel-ok:{elicitResult.Action}:{sampleResult.Content.OfType<TextContentBlock>().First().Text}";
257250
}
258251

259-
[Theory]
260-
[InlineData(false)]
261-
public async Task Mrtr_ParallelAwaits(bool experimentalClient)
252+
[Fact]
253+
public async Task Mrtr_ParallelAwaits()
262254
{
263-
// Parallel awaits work with regular JSON-RPC for legacy clients.
264-
Assert.SkipWhen(Stateless, "Await-style API requires handler suspension (stateful only).");
255+
// Server-side parallel ElicitAsync + SampleAsync awaits use the legacy server-to-client
256+
// request path on stateful sessions, which works the same under either negotiated revision
257+
// (the spec only removes those request methods from Streamable HTTP under draft, which is
258+
// stateless-only territory). Stateless servers can't issue server-to-client requests at all.
259+
Assert.SkipWhen(Stateless, "Server-side awaits require stateful server-to-client requests.");
265260

266-
var messageTracker = ConfigureServer(MrtrParallelAwait);
261+
ConfigureServer(MrtrParallelAwait);
267262
await using var app = Builder.Build();
268263
app.MapMcp();
269264
await app.StartAsync(TestContext.Current.CancellationToken);
270265

271-
Action<McpClientOptions> configureClient = experimentalClient
272-
? options => { ConfigureMrtrHandlers(options); options.ProtocolVersion = "DRAFT-2026-v1"; }
273-
: ConfigureMrtrHandlers;
274-
await using var client = await ConnectAsync(configureClient: configureClient);
275-
276-
if (experimentalClient)
277-
{
278-
// Draft clients must use InputRequiredException instead of await-style requests.
279-
Assert.Equal("DRAFT-2026-v1", client.NegotiatedProtocolVersion);
280-
281-
var result = await client.CallToolAsync("mrtr-parallel-await",
282-
cancellationToken: TestContext.Current.CancellationToken);
266+
await using var client = await ConnectAsync(configureClient: ConfigureMrtrHandlers);
283267

284-
var text = Assert.IsType<TextContentBlock>(Assert.Single(result.Content)).Text;
285-
Assert.Contains("Concurrent server-to-client requests are not supported", text);
286-
Assert.True(result.IsError is not true);
287-
}
288-
else
289-
{
290-
// Non-MRTR: awaits go through regular JSON-RPC — concurrent calls work.
291-
Assert.Equal("2025-11-25", client.NegotiatedProtocolVersion);
292-
293-
var result = await client.CallToolAsync("mrtr-parallel-await",
294-
cancellationToken: TestContext.Current.CancellationToken);
268+
var result = await client.CallToolAsync("mrtr-parallel-await",
269+
cancellationToken: TestContext.Current.CancellationToken);
295270

296-
var text = Assert.IsType<TextContentBlock>(Assert.Single(result.Content)).Text;
297-
Assert.StartsWith("parallel-ok:", text);
298-
Assert.True(result.IsError is not true);
299-
}
271+
var text = Assert.IsType<TextContentBlock>(Assert.Single(result.Content)).Text;
272+
Assert.StartsWith("parallel-ok:", text);
273+
Assert.True(result.IsError is not true);
300274
}
301275

302276
[McpServerTool(Name = "mrtr-elicit")]
@@ -321,7 +295,7 @@ private static string MrtrElicit(RequestContext<CallToolRequestParams> context)
321295
}
322296

323297
[Fact]
324-
public async Task Mrtr_LowLevel_Roots_CompletesViaMrtr()
298+
public async Task Mrtr_Roots_CompletesViaMrtr()
325299
{
326300
var messageTracker = ConfigureServer(
327301
[McpServerTool(Name = "mrtr-roots")] (RequestContext<CallToolRequestParams> context) =>
@@ -558,7 +532,7 @@ public async Task Mrtr_ConcurrentThreeInputs_ResolvedSimultaneously()
558532
}
559533

560534
[Fact]
561-
public async Task Mrtr_Experimental_LoadShedding_RequestStateOnly_CompletesViaMrtr()
535+
public async Task Mrtr_LoadShedding_RequestStateOnly_CompletesViaMrtr()
562536
{
563537
var messageTracker = ConfigureServer(
564538
[McpServerTool(Name = "mrtr-loadshed")] (RequestContext<CallToolRequestParams> context) =>

tests/ModelContextProtocol.AspNetCore.Tests/MapMcpTests.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,6 @@ public async Task LongRunningToolCall_DoesNotTimeout_WhenNoEventStreamStore()
288288

289289
}
290290

291-
292291
[Fact]
293292
public async Task IncomingFilter_SeesClientRequests()
294293
{

0 commit comments

Comments
 (0)