From 481ba155a18a09aa69ea466151ddc50edf254376 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Mon, 24 Aug 2026 12:22:26 +0800 Subject: [PATCH] mcp: tolerate 404 on subscriptions/listen in stateless mode When connecting to remote stateless servers (such as GitHub's MCP server), optional streams like subscriptions/listen may be rejected by middleware with a plain-text HTTP 404 before reaching a JSON-RPC handler. Previously, checkResponse misclassified these 404s as lost stateful sessions (ErrSessionMissing) and tore down the transport connection because the error didn't wrap jsonrpc2.ErrRejected. This change aligns with the MCP 2026-07-28 Streamable HTTP specification: 1. Conditionally check for an active session ID before returning ErrSessionMissing, since a 404 on a stateless connection cannot mean a session was lost. 2. Wrap subscriptions/listen failures with ErrRejected (matching server/discover) so the jsonrpc2 layer doesn't permanently break the transport when encountering non-compliant plain-text 404s. --- mcp/streamable.go | 20 ++++++----- mcp/streamable_client_test.go | 64 +++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 9 deletions(-) diff --git a/mcp/streamable.go b/mcp/streamable.go index 6ed5d3d3..8729bb7c 100644 --- a/mcp/streamable.go +++ b/mcp/streamable.go @@ -2347,13 +2347,14 @@ func (c *streamableClientConn) Write(ctx context.Context, msg jsonrpc.Message) e } if err := c.checkResponse(ctx, requestSummary, resp); err != nil { - if requestMethod == methodDiscover && !errors.Is(err, jsonrpc2.ErrRejected) { - // Wrap the discover failure with ErrRejected so the jsonrpc2 layer - // doesn't set writeErr, which would prevent the legacy initialize - // fallback from succeeding on the same connection. This covers the - // case where a legacy server rejects server/discover with a - // non-JSON-RPC body (e.g. plain text 400), which checkResponse - // cannot classify as a per-call rejection on its own. + if (requestMethod == methodDiscover || requestMethod == methodSubscriptionsListen) && !errors.Is(err, jsonrpc2.ErrRejected) { + // Wrap the discover or subscriptions/listen failure with ErrRejected so + // the jsonrpc2 layer doesn't set writeErr, which would break the connection + // (preventing the legacy initialize fallback from succeeding, or breaking + // subsequent RPCs if subscriptions/listen fails). This covers the case + // where a server rejects these requests with a non-JSON-RPC body (e.g. + // plain text 404 or 400), which checkResponse cannot classify as a + // per-call rejection on its own. err = fmt.Errorf("%w: %w", err, jsonrpc2.ErrRejected) } else if !errors.Is(err, jsonrpc2.ErrRejected) { // Only fail the connection for non-transient errors. @@ -2592,10 +2593,11 @@ func (c *streamableClientConn) checkResponse(ctx context.Context, requestSummary // ยง2.5.3: "The server MAY terminate the session at any time, after // which it MUST respond to requests containing that session ID with HTTP // 404 Not Found." - if resp.StatusCode == http.StatusNotFound { + // Only a stateful connection with a known session ID can experience ErrSessionMissing. + if sessionID := c.SessionID(); resp.StatusCode == http.StatusNotFound && sessionID != "" { // Return an ErrSessionMissing to avoid sending a redundant DELETE when the // session is already gone. - return fmt.Errorf("%s: failed to connect (session ID: %v): %w", requestSummary, c.sessionID, ErrSessionMissing) + return fmt.Errorf("%s: failed to connect (session ID: %v): %w", requestSummary, sessionID, ErrSessionMissing) } if resp.StatusCode < 200 || resp.StatusCode >= 300 { return fmt.Errorf("%s: %v", requestSummary, http.StatusText(resp.StatusCode)) diff --git a/mcp/streamable_client_test.go b/mcp/streamable_client_test.go index 64c4da56..77ff81a2 100644 --- a/mcp/streamable_client_test.go +++ b/mcp/streamable_client_test.go @@ -1965,3 +1965,67 @@ func TestStreamableClientHandlerErrorPropagation(t *testing.T) { }) } } + +// TestStreamableClient_StatelessSubscriptionsListen404 verifies that in stateless mode +// (protocol 2026-07-28), when a server rejects subscriptions/listen with 404 (e.g. Copilot MCP server), +// the client connection survives and subsequent RPCs (such as tools/list) succeed. +func TestStreamableClient_StatelessSubscriptionsListen404(t *testing.T) { + ctx := t.Context() + + var listenServed atomic.Bool + fake := &fakeStreamableServer{ + t: t, + responses: fakeResponses{ + {"POST", "", methodDiscover, ""}: { + header: header{ + "Content-Type": "application/json", + }, + wantProtocolVersion: protocolVersion20260728, + responseFunc: func(r *jsonrpc.Request) (string, int) { + return jsonBody(t, resp(r.ID.Raw().(int64), discoverResult, nil)), http.StatusOK + }, + }, + {"POST", "", methodSubscriptionsListen, ""}: { + header: header{"Content-Type": "text/plain"}, + responseFunc: func(r *jsonrpc.Request) (string, int) { + listenServed.Store(true) + return "404 Not Found", http.StatusNotFound + }, + optional: true, + }, + {"POST", "", methodListTools, ""}: { + header: header{"Content-Type": "application/json"}, + responseFunc: func(r *jsonrpc.Request) (string, int) { + return jsonBody(t, resp(r.ID.Raw().(int64), &ListToolsResult{Tools: []*Tool{}}, nil)), http.StatusOK + }, + optional: true, + }, + }, + } + + httpServer := httptest.NewServer(fake) + defer httpServer.Close() + + transport := &StreamableClientTransport{Endpoint: httpServer.URL} + client := NewClient(testImpl, &ClientOptions{ + ToolListChangedHandler: func(ctx context.Context, req *ToolListChangedRequest) {}, + }) + + session, err := client.Connect(ctx, transport, &ClientSessionOptions{ProtocolVersion: protocolVersion20260728}) + if err != nil { + t.Fatalf("Connect failed: %v", err) + } + defer session.Close() + + // tools/list must succeed even though subscriptions/listen returned 404. + res, err := session.ListTools(ctx, nil) + if err != nil { + t.Fatalf("ListTools failed: %v", err) + } + if res == nil { + t.Fatal("ListTools result is nil") + } + if !listenServed.Load() { + t.Fatal("subscriptions/listen was not called") + } +}