Skip to content

Commit 27badfe

Browse files
halter73Copilot
andcommitted
Consolidate and tighten MRTR test coverage
Refactor MRTR tests to provide extensive coverage with less code, making it easy for reviewers and other SDK implementers to see edge case coverage at a glance. Test consolidation: - Move MRTR tests into MapMcpTests.Mrtr.cs partial class, which runs every test across StreamableHttp, SSE, and Stateless transports - Use Theory tests with (experimentalServer, experimentalClient) bools to cover all 4 MRTR/backcompat protocol combinations - Delete MrtrBackcompatTests.cs, StatelessMrtrTests.cs, and StreamableHttpMrtrTests.cs (coverage subsumed by MapMcpTests) - Reduce MrtrLowLevelApiTests.cs and MrtrProtocolTests.cs to unique protocol-level tests not covered by McpClient-based tests - Update MrtrIntegrationTests doc to reflect edge-case focus and add simple happy-path smoke test for reviewer reference Test improvements: - Add ServerMessageTracker with AssertMrtrUsed()/AssertMrtrNotUsed() to verify correct protocol mode in every test - Add NegotiatedProtocolVersion assertions to verify negotiation - Add result.IsError checks on all success-path tool results - Add ErrorCode assertions on all error-path tests - Tighten all version assertions to exact values (no NotEqual) Helpers: - ConnectAsync takes Action<McpClientOptions>? to prevent bypassing transport config (path/TransportMode defaults) - ConfigureExperimentalServer/ConfigureDefaultServer with distinct Implementation names for clear test output - ConfigureMrtrHandlers configures elicitation, sampling, and roots handlers on an existing McpClientOptions object - ConnectExperimentalAsync/ConnectDefaultAsync for common patterns - Move EnablePollingAsync stateless test to MapMcpStatelessTests - Add client-side warnings for legacy requests on MRTR sessions and IncompleteResult on non-MRTR sessions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1fff8c5 commit 27badfe

File tree

16 files changed

+1285
-3162
lines changed

16 files changed

+1285
-3162
lines changed

src/ModelContextProtocol.Core/Client/McpClientImpl.cs

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ private void RegisterHandlers(McpClientOptions options, NotificationHandlers not
120120
RequestMethods.SamplingCreateMessage,
121121
async (request, jsonRpcRequest, cancellationToken) =>
122122
{
123+
WarnIfLegacyRequestOnMrtrSession(RequestMethods.SamplingCreateMessage);
124+
123125
// Check if this is a task-augmented request
124126
if (request?.Task is { } taskMetadata)
125127
{
@@ -154,10 +156,14 @@ private void RegisterHandlers(McpClientOptions options, NotificationHandlers not
154156
{
155157
requestHandlers.Set(
156158
RequestMethods.SamplingCreateMessage,
157-
(request, _, cancellationToken) => samplingHandler(
158-
request,
159-
request?.ProgressToken is { } token ? new TokenProgress(this, token) : NullProgress.Instance,
160-
cancellationToken),
159+
(request, _, cancellationToken) =>
160+
{
161+
WarnIfLegacyRequestOnMrtrSession(RequestMethods.SamplingCreateMessage);
162+
return samplingHandler(
163+
request,
164+
request?.ProgressToken is { } token ? new TokenProgress(this, token) : NullProgress.Instance,
165+
cancellationToken);
166+
},
161167
McpJsonUtilities.JsonContext.Default.CreateMessageRequestParams,
162168
McpJsonUtilities.JsonContext.Default.CreateMessageResult);
163169
}
@@ -170,7 +176,11 @@ private void RegisterHandlers(McpClientOptions options, NotificationHandlers not
170176
{
171177
requestHandlers.Set(
172178
RequestMethods.RootsList,
173-
(request, _, cancellationToken) => rootsHandler(request, cancellationToken),
179+
(request, _, cancellationToken) =>
180+
{
181+
WarnIfLegacyRequestOnMrtrSession(RequestMethods.RootsList);
182+
return rootsHandler(request, cancellationToken);
183+
},
174184
McpJsonUtilities.JsonContext.Default.ListRootsRequestParams,
175185
McpJsonUtilities.JsonContext.Default.ListRootsResult);
176186

@@ -187,6 +197,8 @@ private void RegisterHandlers(McpClientOptions options, NotificationHandlers not
187197
RequestMethods.ElicitationCreate,
188198
async (request, jsonRpcRequest, cancellationToken) =>
189199
{
200+
WarnIfLegacyRequestOnMrtrSession(RequestMethods.ElicitationCreate);
201+
190202
// Check if this is a task-augmented request
191203
if (request?.Task is { } taskMetadata)
192204
{
@@ -219,6 +231,7 @@ private void RegisterHandlers(McpClientOptions options, NotificationHandlers not
219231
RequestMethods.ElicitationCreate,
220232
async (request, _, cancellationToken) =>
221233
{
234+
WarnIfLegacyRequestOnMrtrSession(RequestMethods.ElicitationCreate);
222235
var result = await elicitationHandler(request, cancellationToken).ConfigureAwait(false);
223236
return ElicitResult.WithDefaults(request, result);
224237
},
@@ -718,6 +731,8 @@ public override async Task<JsonRpcResponse> SendRequestAsync(JsonRpcRequest requ
718731
resultObj.TryGetPropertyValue("result_type", out var resultTypeNode) &&
719732
resultTypeNode?.GetValue<string>() is "incomplete")
720733
{
734+
WarnIfIncompleteResultOnNonMrtrSession(request.Method);
735+
721736
var incompleteResult = JsonSerializer.Deserialize(response.Result, McpJsonUtilities.JsonContext.Default.IncompleteResult)
722737
?? throw new JsonException("Failed to deserialize IncompleteResult.");
723738

@@ -795,6 +810,32 @@ public override async ValueTask DisposeAsync()
795810
await Completion.ConfigureAwait(false);
796811
}
797812

813+
/// <summary>Logs a warning if the session negotiated MRTR but the server sent a legacy JSON-RPC request.</summary>
814+
private void WarnIfLegacyRequestOnMrtrSession(string method)
815+
{
816+
if (_options.ExperimentalProtocolVersion is not null &&
817+
_negotiatedProtocolVersion == _options.ExperimentalProtocolVersion)
818+
{
819+
LogLegacyRequestOnMrtrSession(_endpointName, method);
820+
}
821+
}
822+
823+
/// <summary>Logs a warning if the session did not negotiate MRTR but the server sent an IncompleteResult.</summary>
824+
private void WarnIfIncompleteResultOnNonMrtrSession(string method)
825+
{
826+
if (_options.ExperimentalProtocolVersion is null ||
827+
_negotiatedProtocolVersion != _options.ExperimentalProtocolVersion)
828+
{
829+
LogIncompleteResultOnNonMrtrSession(_endpointName, method, _negotiatedProtocolVersion);
830+
}
831+
}
832+
833+
[LoggerMessage(Level = LogLevel.Warning, Message = "{EndpointName} received legacy '{Method}' JSON-RPC request on session that negotiated MRTR. The server should use IncompleteResult instead of sending direct requests.")]
834+
private partial void LogLegacyRequestOnMrtrSession(string endpointName, string method);
835+
836+
[LoggerMessage(Level = LogLevel.Warning, Message = "{EndpointName} received IncompleteResult for '{Method}' on session that did not negotiate MRTR (protocol version '{ProtocolVersion}'). The server may not be spec-compliant.")]
837+
private partial void LogIncompleteResultOnNonMrtrSession(string endpointName, string method, string? protocolVersion);
838+
798839
[LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} client received server '{ServerInfo}' capabilities: '{Capabilities}'.")]
799840
private partial void LogServerCapabilitiesReceived(string endpointName, string capabilities, string serverInfo);
800841

@@ -812,5 +853,4 @@ public override async ValueTask DisposeAsync()
812853

813854
[LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} client resumed existing session.")]
814855
private partial void LogClientSessionResumed(string endpointName);
815-
816856
}

src/ModelContextProtocol.Core/McpSession.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public abstract partial class McpSession : IAsyncDisposable
6868
/// </param>
6969
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests. The default is <see cref="CancellationToken.None"/>.</param>
7070
/// <returns>A task that represents the asynchronous send operation.</returns>
71-
/// <exception cref="InvalidOperationException">The transport is not connected.</exception>
71+
/// <exception cref="InvalidOperationException">The transport is not connected, or <paramref name="message"/> is a <see cref="JsonRpcRequest"/>. Use <see cref="SendRequestAsync"/> for requests.</exception>
7272
/// <exception cref="ArgumentNullException"><paramref name="message"/> is <see langword="null"/>.</exception>
7373
/// <remarks>
7474
/// <para>

src/ModelContextProtocol.Core/McpSessionHandler.cs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -593,10 +593,7 @@ public async Task<JsonRpcResponse> SendRequestAsync(JsonRpcRequest request, Canc
593593
LogSendingRequest(EndpointName, request.Method);
594594
}
595595

596-
await _outgoingMessageFilter(async (msg, ct) =>
597-
{
598-
await SendToRelatedTransportAsync(msg, ct).ConfigureAwait(false);
599-
})(request, cancellationToken).ConfigureAwait(false);
596+
await _outgoingMessageFilter(SendToRelatedTransportAsync)(request, cancellationToken).ConfigureAwait(false);
600597

601598
// Now that the request has been sent, register for cancellation. If we registered before,
602599
// a cancellation request could arrive before the server knew about that request ID, in which
@@ -654,6 +651,13 @@ public async Task SendMessageAsync(JsonRpcMessage message, CancellationToken can
654651
{
655652
Throw.IfNull(message);
656653

654+
if (message is JsonRpcRequest request)
655+
{
656+
throw new InvalidOperationException(
657+
$"Cannot send '{request.Method}' request via {nameof(SendMessageAsync)}. " +
658+
$"Use {nameof(SendRequestAsync)} instead to get a correlated response.");
659+
}
660+
657661
cancellationToken.ThrowIfCancellationRequested();
658662

659663
Histogram<double> durationMetric = _isServer ? s_serverOperationDuration : s_clientOperationDuration;
Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,45 @@
1-
namespace ModelContextProtocol.AspNetCore.Tests;
1+
using Microsoft.AspNetCore.Builder;
2+
using Microsoft.Extensions.DependencyInjection;
3+
using ModelContextProtocol.Protocol;
4+
using ModelContextProtocol.Server;
5+
6+
namespace ModelContextProtocol.AspNetCore.Tests;
27

38
public class MapMcpStatelessTests(ITestOutputHelper outputHelper) : MapMcpStreamableHttpTests(outputHelper)
49
{
510
protected override bool UseStreamableHttp => true;
611
protected override bool Stateless => true;
12+
13+
[Fact]
14+
public async Task EnablePollingAsync_ThrowsInvalidOperationException_InStatelessMode()
15+
{
16+
InvalidOperationException? capturedException = null;
17+
var pollingTool = McpServerTool.Create(async (RequestContext<CallToolRequestParams> context) =>
18+
{
19+
try
20+
{
21+
await context.EnablePollingAsync(retryInterval: TimeSpan.FromSeconds(1));
22+
}
23+
catch (InvalidOperationException ex)
24+
{
25+
capturedException = ex;
26+
}
27+
28+
return "Complete";
29+
}, options: new() { Name = "polling_tool" });
30+
31+
Builder.Services.AddMcpServer().WithHttpTransport(ConfigureStateless).WithTools([pollingTool]);
32+
33+
await using var app = Builder.Build();
34+
app.MapMcp();
35+
36+
await app.StartAsync(TestContext.Current.CancellationToken);
37+
38+
await using var mcpClient = await ConnectAsync();
39+
40+
await mcpClient.CallToolAsync("polling_tool", cancellationToken: TestContext.Current.CancellationToken);
41+
42+
Assert.NotNull(capturedException);
43+
Assert.Contains("stateless", capturedException.Message, StringComparison.OrdinalIgnoreCase);
44+
}
745
}

tests/ModelContextProtocol.AspNetCore.Tests/MapMcpStreamableHttpTests.cs

Lines changed: 2 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,9 @@ public async Task StreamableHttpClient_SendsMcpProtocolVersionHeader_AfterInitia
178178

179179
await app.StartAsync(TestContext.Current.CancellationToken);
180180

181-
await using var mcpClient = await ConnectAsync(clientOptions: new()
181+
await using var mcpClient = await ConnectAsync(configureClient: options =>
182182
{
183-
ProtocolVersion = "2025-06-18",
183+
options.ProtocolVersion = "2025-06-18";
184184
});
185185

186186
Assert.Equal("2025-06-18", mcpClient.NegotiatedProtocolVersion);
@@ -288,41 +288,6 @@ public async Task CanResumeSessionWithMapMcpAndRunSessionHandler()
288288
Assert.Equal(1, runSessionCount);
289289
}
290290

291-
[Fact]
292-
public async Task EnablePollingAsync_ThrowsInvalidOperationException_InStatelessMode()
293-
{
294-
Assert.SkipUnless(Stateless, "This test only applies to stateless mode.");
295-
296-
InvalidOperationException? capturedException = null;
297-
var pollingTool = McpServerTool.Create(async (RequestContext<CallToolRequestParams> context) =>
298-
{
299-
try
300-
{
301-
await context.EnablePollingAsync(retryInterval: TimeSpan.FromSeconds(1));
302-
}
303-
catch (InvalidOperationException ex)
304-
{
305-
capturedException = ex;
306-
}
307-
308-
return "Complete";
309-
}, options: new() { Name = "polling_tool" });
310-
311-
Builder.Services.AddMcpServer().WithHttpTransport(ConfigureStateless).WithTools([pollingTool]);
312-
313-
await using var app = Builder.Build();
314-
app.MapMcp();
315-
316-
await app.StartAsync(TestContext.Current.CancellationToken);
317-
318-
await using var mcpClient = await ConnectAsync();
319-
320-
await mcpClient.CallToolAsync("polling_tool", cancellationToken: TestContext.Current.CancellationToken);
321-
322-
Assert.NotNull(capturedException);
323-
Assert.Contains("stateless", capturedException.Message, StringComparison.OrdinalIgnoreCase);
324-
}
325-
326291
[Fact]
327292
public async Task EnablePollingAsync_ThrowsInvalidOperationException_WhenNoEventStreamStoreConfigured()
328293
{

0 commit comments

Comments
 (0)