diff --git a/src/ModelContextProtocol.Core/Server/McpServer.Methods.cs b/src/ModelContextProtocol.Core/Server/McpServer.Methods.cs index 3caaca5a6..fe99794c8 100644 --- a/src/ModelContextProtocol.Core/Server/McpServer.Methods.cs +++ b/src/ModelContextProtocol.Core/Server/McpServer.Methods.cs @@ -54,9 +54,18 @@ public static McpServer Create( /// The client does not support sampling. /// The request failed or the client returned an error response. /// + /// + /// When the server is using the Streamable HTTP transport, prefer calling this method on the + /// instance available via RequestContext from inside a tool, prompt, + /// or resource handler. That routes the request through the originating POST response stream via + /// , which is always open for the duration of + /// the request, rather than relying on the optional standalone GET SSE stream. + /// + /// /// When called during task-augmented tool execution, this method automatically updates the task /// status to while waiting for the client response, /// then returns to when the response is received. + /// /// public async ValueTask SampleAsync( CreateMessageRequestParams requestParams, @@ -252,6 +261,13 @@ public async Task SampleAsync( /// The to use for serialization. If , is used. /// The that can be used to issue sampling requests to the client. /// The client does not support sampling. + /// + /// When the server is using the Streamable HTTP transport, prefer obtaining this chat client from the + /// instance available via RequestContext from inside a tool, prompt, + /// or resource handler. That routes sampling requests through the originating POST response stream via + /// , which is always open for the duration of + /// the request, rather than relying on the optional standalone GET SSE stream. + /// public IChatClient AsSamplingChatClient(JsonSerializerOptions? serializerOptions = null) { ThrowIfSamplingUnsupported(); @@ -273,6 +289,13 @@ public ILoggerProvider AsClientLoggerProvider() => /// is . /// The client does not support roots. /// The request failed or the client returned an error response. + /// + /// When the server is using the Streamable HTTP transport, prefer calling this method on the + /// instance available via RequestContext from inside a tool, prompt, + /// or resource handler. That routes the request through the originating POST response stream via + /// , which is always open for the duration of + /// the request, rather than relying on the optional standalone GET SSE stream. + /// public ValueTask RequestRootsAsync( ListRootsRequestParams requestParams, CancellationToken cancellationToken = default) @@ -298,9 +321,18 @@ public ValueTask RequestRootsAsync( /// The client does not support elicitation. /// The request failed or the client returned an error response. /// + /// + /// When the server is using the Streamable HTTP transport, prefer calling this method on the + /// instance available via RequestContext from inside a tool, prompt, + /// or resource handler. That routes the request through the originating POST response stream via + /// , which is always open for the duration of + /// the request, rather than relying on the optional standalone GET SSE stream. + /// + /// /// When called during task-augmented tool execution, this method automatically updates the task /// status to while waiting for user input, /// then returns to when the response is received. + /// /// public async ValueTask ElicitAsync( ElicitRequestParams requestParams, diff --git a/src/ModelContextProtocol.Core/Server/StreamableHttpServerTransport.cs b/src/ModelContextProtocol.Core/Server/StreamableHttpServerTransport.cs index 71c366e83..6b9807c41 100644 --- a/src/ModelContextProtocol.Core/Server/StreamableHttpServerTransport.cs +++ b/src/ModelContextProtocol.Core/Server/StreamableHttpServerTransport.cs @@ -201,6 +201,34 @@ public async Task HandlePostRequestAsync(JsonRpcMessage message, Stream re } /// + /// + /// + /// This method sends server-to-client messages via the standalone SSE stream opened by an + /// optional HTTP GET request (see ). + /// + /// + /// This is generally the wrong channel for server-to-client requests. Requests + /// sent via the GET stream depend on the client keeping a long-lived GET open, have no per-request + /// correlation to a caller, and race with GET startup and teardown. When called from inside a + /// tool, prompt, or resource handler, use the instance available via + /// RequestContext instead — it routes through the originating POST response stream via + /// , which is always open for the duration of + /// the request. A diagnostic is emitted whenever a + /// is sent through this method. + /// + /// + /// If no GET SSE stream has yet been opened on this session, behavior depends on the message kind: + /// messages throw because the + /// awaiting caller has no way to receive a response; messages are + /// dropped (notifications are best-effort and the spec does not require clients to issue a GET) + /// and a diagnostic is logged; other messages are dropped and a + /// diagnostic is logged. + /// + /// + /// + /// is , or is a + /// and no GET SSE stream has been opened on this session. + /// public async Task SendMessageAsync(JsonRpcMessage message, CancellationToken cancellationToken = default) { Throw.IfNull(message); @@ -214,9 +242,32 @@ public async Task SendMessageAsync(JsonRpcMessage message, CancellationToken can if (!_getHttpRequestStarted) { - // Clients are not required to make a GET request for unsolicited messages. - // If no GET request has been made, drop the message. - return; + switch (message) + { + case JsonRpcRequest request: + throw new InvalidOperationException( + $"Cannot send server-to-client JSON-RPC request '{request.Method}' because no GET SSE stream has been opened on this session " + + $"(SessionId: '{SessionId}'). " + + "Inside a tool, prompt, or resource handler, use the IMcpServer instance from RequestContext (or any IMcpServer obtained via DI from a request-scoped service provider) so the request is routed through the originating POST response stream via JsonRpcMessageContext.RelatedTransport. " + + "The standalone GET SSE stream is optional for clients and is not a reliable channel for server-to-client requests."); + + case JsonRpcNotification notification: + // Clients are not required to make a GET request for unsolicited messages. + // If no GET request has been made, drop the notification (best-effort). + LogNotificationDroppedNoGetStream(notification.Method, SessionId ?? string.Empty); + return; + + default: + // JsonRpcResponse / JsonRpcError generally flow through the originating POST response + // stream, so receiving one here without a GET is unexpected. Log loudly and drop. + LogMessageDroppedNoGetStream(message.GetType().Name, GetMessageId(message), SessionId ?? string.Empty); + return; + } + } + + if (message is JsonRpcRequest openRequest) + { + LogServerRequestOverGetStream(openRequest.Method, SessionId ?? string.Empty); } Debug.Assert(_httpSseWriter is not null); @@ -244,6 +295,9 @@ public async Task SendMessageAsync(JsonRpcMessage message, CancellationToken can } } + private static string GetMessageId(JsonRpcMessage message) => + message is JsonRpcMessageWithId withId ? withId.Id.ToString() : string.Empty; + /// public async ValueTask DisposeAsync() { @@ -300,4 +354,18 @@ public async ValueTask DisposeAsync() return sseEventStreamWriter; } + + [LoggerMessage(Level = LogLevel.Warning, + Message = "Sending server-to-client JSON-RPC request '{Method}' over the standalone GET SSE stream (SessionId: '{SessionId}'). " + + "Consider using the IMcpServer instance from RequestContext inside a tool, prompt, or resource handler so the request is routed through the originating POST response stream via JsonRpcMessageContext.RelatedTransport, which is more reliable than the optional GET SSE stream.")] + private partial void LogServerRequestOverGetStream(string method, string sessionId); + + [LoggerMessage(Level = LogLevel.Debug, + Message = "Dropping server-to-client JSON-RPC notification '{Method}' because no GET SSE stream has been opened on this session (SessionId: '{SessionId}').")] + private partial void LogNotificationDroppedNoGetStream(string method, string sessionId); + + [LoggerMessage(Level = LogLevel.Warning, + Message = "Dropping unexpected server-to-client {MessageType} (Id: '{MessageId}') because no GET SSE stream has been opened on this session (SessionId: '{SessionId}'). " + + "Responses normally flow through the originating POST response stream via JsonRpcMessageContext.RelatedTransport.")] + private partial void LogMessageDroppedNoGetStream(string messageType, string messageId, string sessionId); } diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/StreamableHttpServerConformanceTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/StreamableHttpServerConformanceTests.cs index 38b1ca696..ef8256993 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/StreamableHttpServerConformanceTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/StreamableHttpServerConformanceTests.cs @@ -384,7 +384,8 @@ async Task GetFirstNotificationAsync() public async Task SendNotificationAsync_DoesNotThrow_WhenNoGetRequestHasBeenMade() { // Clients are not required to make a GET request for unsolicited messages. - // If no GET request has been made, the messages should be dropped rather than throwing. + // If no GET request has been made, the messages should be dropped rather than throwing, + // and the drop should be visible as a Debug-level log so it can be diagnosed. McpServer? server = null; Builder.Services.AddMcpServer() @@ -409,6 +410,150 @@ public async Task SendNotificationAsync_DoesNotThrow_WhenNoGetRequestHasBeenMade var exception = await Record.ExceptionAsync(() => server.SendNotificationAsync("test-method", TestContext.Current.CancellationToken)); Assert.Null(exception); + + Assert.Contains(MockLoggerProvider.LogMessages, log => + log.Category == typeof(StreamableHttpServerTransport).FullName && + log.LogLevel == LogLevel.Debug && + log.Message.Contains("test-method") && + log.Message.Contains("no GET SSE stream")); + } + + [Fact] + public async Task SendMessageAsync_Throws_OnRequest_WhenNoGetRequestHasBeenMade() + { + // A server-to-client request sent before any GET SSE stream is opened can never + // receive a response, so SendMessageAsync should fail fast with InvalidOperationException + // instead of silently dropping the message and leaving the caller hanging. + McpServer? server = null; + + Builder.Services.AddMcpServer() + .WithHttpTransport(options => + { +#pragma warning disable MCPEXP002 // RunSessionHandler is experimental + options.RunSessionHandler = (httpContext, mcpServer, cancellationToken) => + { + server = mcpServer; + return mcpServer.RunAsync(cancellationToken); + }; +#pragma warning restore MCPEXP002 + }); + + await StartAsync(); + + await CallInitializeAndValidateAsync(); + Assert.NotNull(server); + + var request = new JsonRpcRequest + { + Method = "roots/list", + Id = new RequestId(42), + }; + + var ex = await Assert.ThrowsAsync(() => + server.SendMessageAsync(request, TestContext.Current.CancellationToken)); + + Assert.Contains("roots/list", ex.Message); + Assert.Contains("no GET SSE stream", ex.Message); + Assert.Contains("RequestContext", ex.Message); + Assert.Contains("RelatedTransport", ex.Message); + } + + [Fact] + public async Task SendMessageAsync_LogsWarning_OnUnexpectedResponse_WhenNoGetRequestHasBeenMade() + { + // Responses normally ride the originating POST response stream via RelatedTransport, so + // receiving one through the GET path without an open GET is unexpected. The message is + // dropped (preserving best-effort semantics) but a warning is logged so the situation is + // visible. + McpServer? server = null; + + Builder.Services.AddMcpServer() + .WithHttpTransport(options => + { +#pragma warning disable MCPEXP002 // RunSessionHandler is experimental + options.RunSessionHandler = (httpContext, mcpServer, cancellationToken) => + { + server = mcpServer; + return mcpServer.RunAsync(cancellationToken); + }; +#pragma warning restore MCPEXP002 + }); + + await StartAsync(); + + await CallInitializeAndValidateAsync(); + Assert.NotNull(server); + + var response = new JsonRpcResponse + { + Id = new RequestId(7), + Result = JsonSerializer.SerializeToNode(new { }, GetJsonTypeInfo()), + }; + + var exception = await Record.ExceptionAsync(() => + server.SendMessageAsync(response, TestContext.Current.CancellationToken)); + Assert.Null(exception); + + Assert.Contains(MockLoggerProvider.LogMessages, log => + log.Category == typeof(StreamableHttpServerTransport).FullName && + log.LogLevel == LogLevel.Warning && + log.Message.Contains(nameof(JsonRpcResponse)) && + log.Message.Contains("no GET SSE stream")); + } + + [Fact] + public async Task SendMessageAsync_LogsWarning_OnRequest_WhenGetRequestIsOpen() + { + // Even when the GET SSE stream is open and the request is delivered, server-to-client + // requests sent via the GET path are fragile (no per-request correlation, depend on a + // long-lived GET, race with startup/teardown). A warning is logged to direct callers at + // the RequestContext.RelatedTransport channel instead, without changing behavior. + McpServer? server = null; + + Builder.Services.AddMcpServer() + .WithHttpTransport(options => + { +#pragma warning disable MCPEXP002 // RunSessionHandler is experimental + options.RunSessionHandler = (httpContext, mcpServer, cancellationToken) => + { + server = mcpServer; + return mcpServer.RunAsync(cancellationToken); + }; +#pragma warning restore MCPEXP002 + }); + + await StartAsync(); + + await CallInitializeAndValidateAsync(); + Assert.NotNull(server); + + using var getResponse = await HttpClient.GetAsync("", HttpCompletionOption.ResponseHeadersRead, TestContext.Current.CancellationToken); + Assert.Equal(HttpStatusCode.OK, getResponse.StatusCode); + + // Send a request via the GET stream and assert it lands on the wire (proving behavior is unchanged). + var request = new JsonRpcRequest + { + Method = "roots/list", + Id = new RequestId(99), + }; + + var sendTask = server.SendMessageAsync(request, TestContext.Current.CancellationToken); + + await foreach (var sseEvent in ReadSseAsync(getResponse.Content)) + { + var received = JsonSerializer.Deserialize(sseEvent, GetJsonTypeInfo()); + Assert.NotNull(received); + Assert.Equal("roots/list", received.Method); + break; + } + + await sendTask; + + Assert.Contains(MockLoggerProvider.LogMessages, log => + log.Category == typeof(StreamableHttpServerTransport).FullName && + log.LogLevel == LogLevel.Warning && + log.Message.Contains("roots/list") && + log.Message.Contains("RequestContext")); } [Fact]