Merge branch 'main' into main

Author: anuraj

src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs

@@ -99,15 +99,33 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can
                 // behavior. Capture the underlying error (status + body) before falling back so that,
                 // if SSE also fails, we can surface the real Streamable HTTP diagnostic to the caller
                 // instead of dropping it on the floor (see https://github.com/modelcontextprotocol/csharp-sdk/issues/1526).
-                LogStreamableHttpFailed(_name, response.StatusCode);
-
                 // This reads the response body a second time for the application/json case, where
                 // TryReadJsonRpcErrorAsync above already read it. HttpContent buffers after the first
                 // read, so this returns the same buffered content and is safe (not a second stream
                 // consumption). For the common non-JSON error responses (415, 405, plain text)
                 // TryReadJsonRpcErrorAsync returns early on the content type, so there is no double read.
                 var streamableHttpError = await HttpResponseMessageExtensions.CreateHttpRequestExceptionWithBodyAsync(response, cancellationToken).ConfigureAwait(false);
 
+                if (IsDiscoverProbeRejection(message, response.StatusCode))
+                {
+                    // The server/discover probe is protocol negotiation, not transport detection. A server
+                    // predating SEP-2575 rejects the session-less POST with 400 (can't parse the request) or
+                    // 404 (requires Mcp-Session-Id on every non-initialize POST) whether it speaks Streamable
+                    // HTTP or SSE, so neither status is evidence about which transport to use. McpClientImpl
+                    // .ConnectAsync treats exactly these two statuses as "initialize-handshake server" and
+                    // immediately retries with initialize on this same transport — and that attempt still
+                    // falls back to SSE, so an SSE-only server is reached one POST later rather than not at
+                    // all. Attempting SSE here instead spends a GET whose result is discarded on every
+                    // connect to a Streamable-HTTP-only server that predates SEP-2575, and logs a "falling
+                    // back to SSE transport" line that misreports settled protocol negotiation as a failure.
+                    LogSkippingSseFallbackForDiscoverProbe(_name, response.StatusCode);
+
+                    await streamableHttpTransport.DisposeAsync().ConfigureAwait(false);
+                    throw streamableHttpError;
+                }
+
+                LogStreamableHttpFailed(_name, response.StatusCode);
+
                 await streamableHttpTransport.DisposeAsync().ConfigureAwait(false);
                 await InitializeSseTransportAsync(message, streamableHttpError, cancellationToken).ConfigureAwait(false);
             }
@@ -122,6 +140,15 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can
         }
     }
 
+    /// <summary>
+    /// Returns <see langword="true"/> when the failed request was the SEP-2575 <c>server/discover</c> probe and the
+    /// status is one of the two <see cref="McpClientImpl"/> already reads as "this server requires the initialize
+    /// handshake", meaning the SSE fallback cannot contribute anything the initialize retry won't.
+    /// </summary>
+    private static bool IsDiscoverProbeRejection(JsonRpcMessage message, HttpStatusCode statusCode) =>
+        statusCode is HttpStatusCode.BadRequest or HttpStatusCode.NotFound &&
+        message is JsonRpcRequest { Method: RequestMethods.ServerDiscover };
+
     private async Task InitializeSseTransportAsync(JsonRpcMessage message, HttpRequestException? streamableHttpError, CancellationToken cancellationToken)
     {
         if (_options.KnownSessionId is not null)
@@ -184,6 +211,9 @@ public async ValueTask DisposeAsync()
     [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} streamable HTTP transport failed with status code {StatusCode}, falling back to SSE transport.")]
     private partial void LogStreamableHttpFailed(string endpointName, HttpStatusCode statusCode);
 
+    [LoggerMessage(Level = LogLevel.Debug, Message = "{EndpointName} server/discover probe rejected with status code {StatusCode}; skipping the SSE fallback so the initialize handshake is attempted instead.")]
+    private partial void LogSkippingSseFallbackForDiscoverProbe(string endpointName, HttpStatusCode statusCode);
+
     [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} using Streamable HTTP transport.")]
     private partial void LogUsingStreamableHttp(string endpointName);
 

tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs

@@ -505,4 +505,155 @@ await Assert.ThrowsAnyAsync<Exception>(() =>
             MockLoggerProvider.LogMessages,
             m => m.LogLevel == LogLevel.Warning && m.Message.Contains("SSE fallback failed"));
     }
+
+    // A 400/404 on the SEP-2575 server/discover probe is protocol negotiation, not transport detection:
+    // a pre-SEP-2575 server rejects the session-less POST whether it speaks Streamable HTTP or SSE, and
+    // McpClientImpl.ConnectAsync reads exactly those two statuses as "initialize-handshake server" and
+    // retries with initialize. The SSE GET can therefore only waste a round trip here.
+    [Theory]
+    [InlineData(HttpStatusCode.NotFound)]
+    [InlineData(HttpStatusCode.BadRequest)]
+    public async Task AutoDetectMode_SkipsSseFallback_WhenDiscoverProbeIsRejected(HttpStatusCode statusCode)
+    {
+        var options = new HttpClientTransportOptions
+        {
+            Endpoint = new Uri("http://localhost"),
+            TransportMode = HttpTransportMode.AutoDetect,
+            Name = "AutoDetect discover probe test client"
+        };
+
+        using var mockHttpHandler = new MockHttpHandler();
+        using var httpClient = new HttpClient(mockHttpHandler);
+        await using var transport = new HttpClientTransport(options, httpClient, LoggerFactory);
+        var getCount = 0;
+
+        mockHttpHandler.RequestHandler = request =>
+        {
+            if (request.Method == HttpMethod.Get)
+            {
+                getCount++;
+                return Task.FromResult(new HttpResponseMessage(HttpStatusCode.MethodNotAllowed));
+            }
+
+            return Task.FromResult(new HttpResponseMessage(statusCode)
+            {
+                Content = new StringContent("Not Found"),
+            });
+        };
+
+        await using var session = await transport.ConnectAsync(TestContext.Current.CancellationToken);
+
+        var ex = await Assert.ThrowsAsync<HttpRequestException>(() =>
+            session.SendMessageAsync(
+                new JsonRpcRequest { Method = RequestMethods.ServerDiscover, Id = new RequestId(1) },
+                TestContext.Current.CancellationToken));
+
+        Assert.Equal(0, getCount);
+        Assert.Equal(statusCode, ex.Data["ModelContextProtocol.HttpStatusCode"]);
+        Assert.DoesNotContain(
+            MockLoggerProvider.LogMessages,
+            m => m.Message.Contains("falling back to SSE transport"));
+    }
+
+    // Skipping the SSE attempt on the discover probe must not strand an SSE-only server: the initialize
+    // retry that McpClientImpl.ConnectAsync issues next goes through this same transport and still falls
+    // back to SSE, so such a server is reached one POST later rather than not at all.
+    [Fact]
+    public async Task AutoDetectMode_StillFallsBackToSse_WhenInitializeFollowsRejectedDiscoverProbe()
+    {
+        var options = new HttpClientTransportOptions
+        {
+            Endpoint = new Uri("http://localhost"),
+            TransportMode = HttpTransportMode.AutoDetect,
+            Name = "AutoDetect discover probe then initialize test client"
+        };
+
+        using var mockHttpHandler = new MockHttpHandler();
+        using var httpClient = new HttpClient(mockHttpHandler);
+        await using var transport = new HttpClientTransport(options, httpClient, LoggerFactory);
+        var ssePipe = new Pipe();
+        var sseEndpointPostCount = 0;
+
+        await ssePipe.Writer.WriteAsync(
+            System.Text.Encoding.UTF8.GetBytes("event: endpoint\r\ndata: /sse-endpoint\r\n\r\n"),
+            TestContext.Current.CancellationToken);
+        await ssePipe.Writer.FlushAsync(TestContext.Current.CancellationToken);
+
+        mockHttpHandler.RequestHandler = request =>
+        {
+            if (request.Method == HttpMethod.Get)
+            {
+                var content = new StreamContent(ssePipe.Reader.AsStream());
+                content.Headers.ContentType = new("text/event-stream");
+                return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = content });
+            }
+
+            if (request.RequestUri?.AbsolutePath == "/sse-endpoint")
+            {
+                sseEndpointPostCount++;
+                return Task.FromResult(new HttpResponseMessage(HttpStatusCode.Accepted));
+            }
+
+            return Task.FromResult(new HttpResponseMessage(HttpStatusCode.NotFound)
+            {
+                Content = new StringContent("Mcp-Session-Id required"),
+            });
+        };
+
+        await using var session = await transport.ConnectAsync(TestContext.Current.CancellationToken);
+
+        await Assert.ThrowsAsync<HttpRequestException>(() =>
+            session.SendMessageAsync(
+                new JsonRpcRequest { Method = RequestMethods.ServerDiscover, Id = new RequestId(1) },
+                TestContext.Current.CancellationToken));
+
+        await session.SendMessageAsync(
+            new JsonRpcRequest { Method = RequestMethods.Initialize, Id = new RequestId(2) },
+            TestContext.Current.CancellationToken);
+
+        Assert.Equal(1, sseEndpointPostCount);
+
+        await ssePipe.Writer.CompleteAsync();
+    }
+
+    // The skip is scoped to the two statuses ConnectAsync acts on. Any other failure on the discover probe
+    // keeps the original fallback, because it is not evidence that an initialize retry is coming.
+    [Fact]
+    public async Task AutoDetectMode_FallsBackToSse_WhenDiscoverProbeFailsWithUnrelatedStatus()
+    {
+        var options = new HttpClientTransportOptions
+        {
+            Endpoint = new Uri("http://localhost"),
+            TransportMode = HttpTransportMode.AutoDetect,
+            Name = "AutoDetect discover probe 415 test client"
+        };
+
+        using var mockHttpHandler = new MockHttpHandler();
+        using var httpClient = new HttpClient(mockHttpHandler);
+        await using var transport = new HttpClientTransport(options, httpClient, LoggerFactory);
+        var getCount = 0;
+
+        mockHttpHandler.RequestHandler = request =>
+        {
+            if (request.Method == HttpMethod.Get)
+            {
+                getCount++;
+                return Task.FromResult(new HttpResponseMessage(HttpStatusCode.MethodNotAllowed));
+            }
+
+            return Task.FromResult(new HttpResponseMessage(HttpStatusCode.UnsupportedMediaType)
+            {
+                Content = new StringContent("Content-Type must be 'application/json'"),
+            });
+        };
+
+        await using var session = await transport.ConnectAsync(TestContext.Current.CancellationToken);
+
+        await Assert.ThrowsAsync<HttpRequestException>(() =>
+            session.SendMessageAsync(
+                new JsonRpcRequest { Method = RequestMethods.ServerDiscover, Id = new RequestId(1) },
+                TestContext.Current.CancellationToken));
+
+        Assert.Equal(1, getCount);
+    }
 }