From 3a6a67f68569e2dbd1dc1f30b27a4633145ee6f1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:39:10 +0000 Subject: [PATCH 1/4] Initial plan From 2156729485dafc213f42cdbc11c99036a199aa54 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:55:27 +0000 Subject: [PATCH 2/4] fix: disable standalone SSE stream for streamable HTTP transport to prevent startup failures Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/b3a7a4cf-4e3a-41da-90f0-b7c5624d2a27 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- internal/mcp/connection.go | 7 ++-- internal/mcp/http_connection_test.go | 58 ++++++++++++++++++++++++++++ internal/mcp/http_transport.go | 10 ++++- 3 files changed, 71 insertions(+), 4 deletions(-) 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..8257b131 100644 --- a/internal/mcp/http_connection_test.go +++ b/internal/mcp/http_connection_test.go @@ -582,3 +582,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) + + getRequestCount := 0 + + 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. + getRequestCount++ + 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(0, getRequestCount, "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..3adad174 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; 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, From 7838d27f227cef9180e4c9e96bc6ac3eb7f4a354 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:57:12 +0000 Subject: [PATCH 3/4] fix: address code review comments - improve MaxRetries comment and rename variable Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/b3a7a4cf-4e3a-41da-90f0-b7c5624d2a27 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- internal/mcp/http_connection_test.go | 6 +++--- internal/mcp/http_transport.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/mcp/http_connection_test.go b/internal/mcp/http_connection_test.go index 8257b131..fcc34d18 100644 --- a/internal/mcp/http_connection_test.go +++ b/internal/mcp/http_connection_test.go @@ -596,14 +596,14 @@ func TestNewHTTPConnection_StreamableTransport_BadSSEEndpoint(t *testing.T) { require := require.New(t) assert := assert.New(t) - getRequestCount := 0 + getMethodCount := 0 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. - getRequestCount++ + getMethodCount++ w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(`{"error":"internal server error"}`)) //nolint:errcheck return @@ -638,5 +638,5 @@ func TestNewHTTPConnection_StreamableTransport_BadSSEEndpoint(t *testing.T) { 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(0, getRequestCount, "No GET requests should be issued (standalone SSE is disabled)") + assert.Equal(0, getMethodCount, "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 3adad174..6f406e38 100644 --- a/internal/mcp/http_transport.go +++ b/internal/mcp/http_transport.go @@ -445,7 +445,7 @@ func tryStreamableHTTPTransport(ctx context.Context, cancel context.CancelFunc, return &sdk.StreamableClientTransport{ Endpoint: url, HTTPClient: httpClient, - MaxRetries: -1, // Disable retries; we try other transports on failure. + 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 From fc18b1e8aebbd1acb4f47ccb753eeb71d7da2509 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 23:12:37 +0000 Subject: [PATCH 4/4] fix: use atomic.Int32 for getMethodCount to prevent data race in test Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/cc87c7cf-5340-4ef1-bedc-58d9feaffc20 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- internal/mcp/http_connection_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/mcp/http_connection_test.go b/internal/mcp/http_connection_test.go index fcc34d18..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" @@ -596,14 +597,14 @@ func TestNewHTTPConnection_StreamableTransport_BadSSEEndpoint(t *testing.T) { require := require.New(t) assert := assert.New(t) - getMethodCount := 0 + 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++ + getMethodCount.Add(1) w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(`{"error":"internal server error"}`)) //nolint:errcheck return @@ -638,5 +639,5 @@ func TestNewHTTPConnection_StreamableTransport_BadSSEEndpoint(t *testing.T) { 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(0, getMethodCount, "No GET requests should be issued (standalone SSE is disabled)") + assert.Equal(int32(0), getMethodCount.Load(), "No GET requests should be issued (standalone SSE is disabled)") }