Skip to content

Commit 5487d0a

Browse files
halter73Copilot
andcommitted
Add session DELETE mid-MRTR tests and fix disposal cleanup
- Add SessionDelete_CancelsPendingMrtrContinuation test verifying: - MRTR continuation is cancelled on session DELETE - Debug-level log emitted for cancelled continuations - No Error-level log noise from handler cancellation - Add SessionDelete_RetryAfterDelete_ReturnsSessionNotFound test verifying retry with stale requestState returns 404 - Add MrtrContinuationsCancelled debug log in DisposeAsync - Skip ToolCallError log for OperationCanceledException during disposal (not a tool bug, just session teardown) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6435257 commit 5487d0a

2 files changed

Lines changed: 95 additions & 1 deletion

File tree

src/ModelContextProtocol.Core/Server/McpServerImpl.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,14 +206,21 @@ public override async ValueTask DisposeAsync()
206206
_disposed = true;
207207

208208
// Cancel all suspended MRTR handlers by faulting their pending exchanges.
209+
int cancelledCount = 0;
209210
foreach (var kvp in _mrtrContinuations)
210211
{
211212
if (_mrtrContinuations.TryRemove(kvp.Key, out var continuation))
212213
{
213214
continuation.PendingExchange.ResponseTcs.TrySetCanceled();
215+
cancelledCount++;
214216
}
215217
}
216218

219+
if (cancelledCount > 0)
220+
{
221+
MrtrContinuationsCancelled(cancelledCount);
222+
}
223+
217224
_taskCancellationTokenProvider?.Dispose();
218225
_disposables.ForEach(d => d());
219226
await _sessionHandler.DisposeAsync().ConfigureAwait(false);
@@ -775,7 +782,12 @@ await originalListToolsHandler(request, cancellationToken).ConfigureAwait(false)
775782
}
776783
catch (Exception e)
777784
{
778-
ToolCallError(request.Params?.Name ?? string.Empty, e);
785+
// Skip logging for OperationCanceledException during server disposal —
786+
// MRTR handler cancellation during session teardown is expected, not an error.
787+
if (!(e is OperationCanceledException && _disposed))
788+
{
789+
ToolCallError(request.Params?.Name ?? string.Empty, e);
790+
}
779791

780792
if ((e is OperationCanceledException && cancellationToken.IsCancellationRequested) || e is McpProtocolException || e is IncompleteResultException)
781793
{
@@ -1142,6 +1154,9 @@ internal static LoggingLevel ToLoggingLevel(LogLevel level) =>
11421154
[LoggerMessage(Level = LogLevel.Information, Message = "ReadResource \"{ResourceUri}\" completed.")]
11431155
private partial void ReadResourceCompleted(string resourceUri);
11441156

1157+
[LoggerMessage(Level = LogLevel.Debug, Message = "Cancelled {Count} pending MRTR continuation(s) during session disposal.")]
1158+
private partial void MrtrContinuationsCancelled(int count);
1159+
11451160
/// <summary>
11461161
/// Checks whether the negotiated protocol version enables MRTR.
11471162
/// </summary>

tests/ModelContextProtocol.AspNetCore.Tests/MrtrProtocolTests.cs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using Microsoft.AspNetCore.Builder;
22
using Microsoft.Extensions.DependencyInjection;
3+
using Microsoft.Extensions.Logging;
34
using ModelContextProtocol.AspNetCore.Tests.Utils;
45
using ModelContextProtocol.Protocol;
56
using ModelContextProtocol.Server;
@@ -829,6 +830,84 @@ public async Task LowLevel_ToolFallsBackGracefully_WithoutMrtr()
829830
Assert.Equal("lowlevel-unsupported:MRTR is not available", text);
830831
}
831832

833+
[Fact]
834+
public async Task SessionDelete_CancelsPendingMrtrContinuation()
835+
{
836+
await StartAsync();
837+
await InitializeWithMrtrAsync();
838+
839+
// 1. Call a tool that suspends at ElicitAsync (high-level MRTR path).
840+
var response = await PostJsonRpcAsync(CallTool("elicit-tool", """{"message":"Please confirm"}"""));
841+
var rpcResponse = await AssertSingleSseResponseAsync(response);
842+
843+
// Verify we got an IncompleteResult (handler is now suspended, continuation stored).
844+
var resultObj = Assert.IsType<JsonObject>(rpcResponse.Result);
845+
Assert.Equal("incomplete", resultObj["result_type"]?.GetValue<string>());
846+
var requestState = resultObj["requestState"]!.GetValue<string>();
847+
Assert.False(string.IsNullOrEmpty(requestState));
848+
849+
// 2. DELETE the session while the handler is suspended.
850+
using var deleteResponse = await HttpClient.DeleteAsync("", TestContext.Current.CancellationToken);
851+
Assert.Equal(HttpStatusCode.OK, deleteResponse.StatusCode);
852+
853+
// Allow a moment for the async cancellation to propagate through the handler task.
854+
await Task.Delay(100, TestContext.Current.CancellationToken);
855+
856+
// 3. Verify that the MRTR cancellation was logged at Debug level.
857+
var mrtrCancelledLog = MockLoggerProvider.LogMessages
858+
.Where(m => m.Message.Contains("pending MRTR continuation"))
859+
.ToList();
860+
Assert.Single(mrtrCancelledLog);
861+
Assert.Equal(LogLevel.Debug, mrtrCancelledLog[0].LogLevel);
862+
Assert.Contains("1", mrtrCancelledLog[0].Message);
863+
864+
// 4. Verify no error-level log was emitted for the cancellation.
865+
// The handler's OperationCanceledException should be silently observed, not logged as an error.
866+
var errorLogs = MockLoggerProvider.LogMessages
867+
.Where(m => m.LogLevel >= LogLevel.Error && m.Message.Contains("elicit"))
868+
.ToList();
869+
Assert.Empty(errorLogs);
870+
}
871+
872+
[Fact]
873+
public async Task SessionDelete_RetryAfterDelete_ReturnsSessionNotFound()
874+
{
875+
await StartAsync();
876+
await InitializeWithMrtrAsync();
877+
878+
// 1. Call a tool that suspends at ElicitAsync.
879+
var response = await PostJsonRpcAsync(CallTool("elicit-tool", """{"message":"Please confirm"}"""));
880+
var rpcResponse = await AssertSingleSseResponseAsync(response);
881+
882+
var resultObj = Assert.IsType<JsonObject>(rpcResponse.Result);
883+
var requestState = resultObj["requestState"]!.GetValue<string>();
884+
var inputRequests = resultObj["inputRequests"]!.AsObject();
885+
var inputKey = inputRequests.First().Key;
886+
887+
// 2. DELETE the session.
888+
using var deleteResponse = await HttpClient.DeleteAsync("", TestContext.Current.CancellationToken);
889+
Assert.Equal(HttpStatusCode.OK, deleteResponse.StatusCode);
890+
891+
// 3. Attempt to retry with the old requestState — session is gone.
892+
var inputResponse = InputResponse.FromElicitResult(new ElicitResult { Action = "accept" });
893+
var retryParams = new JsonObject
894+
{
895+
["name"] = "elicit-tool",
896+
["arguments"] = new JsonObject { ["message"] = "Please confirm" },
897+
["requestState"] = requestState,
898+
["inputResponses"] = new JsonObject
899+
{
900+
[inputKey] = JsonSerializer.SerializeToNode(inputResponse, McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(InputResponse)))
901+
},
902+
};
903+
904+
using var retryResponse = await PostJsonRpcAsync(Request("tools/call", retryParams.ToJsonString()));
905+
906+
// The session was deleted, so we should get a 404 with a JSON-RPC error.
907+
Assert.Equal(HttpStatusCode.NotFound, retryResponse.StatusCode);
908+
Assert.Equal("application/json", retryResponse.Content.Headers.ContentType?.MediaType);
909+
}
910+
832911
// --- Helpers ---
833912

834913
private static StringContent JsonContent(string json) => new(json, Encoding.UTF8, "application/json");

0 commit comments

Comments
 (0)