diff --git a/internal/mcp/connection.go b/internal/mcp/connection.go index 74efaf8f..769badb6 100644 --- a/internal/mcp/connection.go +++ b/internal/mcp/connection.go @@ -377,9 +377,10 @@ func (c *Connection) reconnectSDKTransport() error { switch c.httpTransportType { case HTTPTransportStreamable: transport = &sdk.StreamableClientTransport{ - Endpoint: c.httpURL, - HTTPClient: headerClient, - MaxRetries: 0, + Endpoint: c.httpURL, + HTTPClient: headerClient, + MaxRetries: -1, // Disable retries; reconnect logic is handled by the gateway. + DisableStandaloneSSE: true, } case HTTPTransportSSE: transport = &sdk.SSEClientTransport{ diff --git a/internal/mcp/http_connection_test.go b/internal/mcp/http_connection_test.go index b6c1fe22..5bf251ec 100644 --- a/internal/mcp/http_connection_test.go +++ b/internal/mcp/http_connection_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "sync/atomic" "testing" "time" @@ -582,3 +583,61 @@ func TestNewHTTPConnection_GettersAfterCreation(t *testing.T) { assert.Equal(t, expectedValue, returnedHeaders[key], "Header %s should match", key) } } + +// TestNewHTTPConnection_StreamableTransport_BadSSEEndpoint verifies that the streamable +// HTTP transport succeeds even when the server's SSE endpoint (GET) returns errors. +// +// This tests the fix for the Unwrap MCP server issue: some cloud API MCP servers +// correctly respond to POST (initialize) but return 5xx or unexpected responses to GET +// requests. Before the fix, the SDK's standalone SSE stream would call c.fail() on the +// connection, causing the initialized notification to fail and the connection to be +// reported as "error". With DisableStandaloneSSE: true, the GET is never issued and +// the connection succeeds on the POST-only path. +func TestNewHTTPConnection_StreamableTransport_BadSSEEndpoint(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + var getMethodCount atomic.Int32 + + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + // Simulate a server that returns 500 for the SSE GET stream. + // Before the fix this would call c.fail() and break the connection; + // after the fix the GET is never issued. + getMethodCount.Add(1) + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(`{"error":"internal server error"}`)) //nolint:errcheck + return + } + + // POST: respond with a valid JSON-RPC initialize result. + response := map[string]interface{}{ + "jsonrpc": "2.0", + "id": 1, + "result": map[string]interface{}{ + "protocolVersion": "2024-11-05", + "serverInfo": map[string]interface{}{ + "name": "unwrap-mcp", + "version": "1.0.0", + }, + }, + } + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Mcp-Session-Id", "unwrap-session-1") + w.WriteHeader(http.StatusOK) + json.NewEncoder(w).Encode(response) //nolint:errcheck + })) + defer testServer.Close() + + conn, err := NewHTTPConnection(context.Background(), "unwrap", testServer.URL, map[string]string{ + "Authorization": "Bearer secret-token", + }, nil, "", 0, 0) + + require.NoError(err, "Connection must succeed even when the server's GET SSE endpoint returns 500") + require.NotNil(conn) + defer conn.Close() + + assert.Equal(HTTPTransportStreamable, conn.httpTransportType, "Should use streamable transport") + assert.Equal("unwrap-session-1", conn.httpSessionID, "Session ID should be captured from POST response") + assert.Equal(int32(0), getMethodCount.Load(), "No GET requests should be issued (standalone SSE is disabled)") +} diff --git a/internal/mcp/http_transport.go b/internal/mcp/http_transport.go index 4d0ba07a..6f406e38 100644 --- a/internal/mcp/http_transport.go +++ b/internal/mcp/http_transport.go @@ -445,7 +445,15 @@ func tryStreamableHTTPTransport(ctx context.Context, cancel context.CancelFunc, return &sdk.StreamableClientTransport{ Endpoint: url, HTTPClient: httpClient, - MaxRetries: 0, // Don't retry on failure - we'll try other transports + MaxRetries: -1, // Disable retries (-1 = 0 retries; SDK treats 0 as "use default: 5"). We try other transports on failure. + // DisableStandaloneSSE prevents the SDK from issuing a GET request for a + // persistent server-sent events stream immediately after initialization. + // Some HTTP MCP servers (e.g. cloud APIs) return 5xx or keep the GET + // request open indefinitely, which causes the SDK to call c.fail() and + // break the connection before the gateway can send the initialized + // notification. The gateway operates in request-response mode only and + // does not need server-initiated messages, so this stream is unnecessary. + DisableStandaloneSSE: true, } }, keepAlive,