From 169a2f267a2eb0c1d0355e44899780f6a2887ce1 Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Sat, 22 Nov 2025 17:39:27 +0300 Subject: [PATCH 1/5] fix: restore StatelessSessionIdManager as default to fix multi-instance deployments Fixes #636 In commit da6f722, the default session ID manager was changed from StatelessSessionIdManager to InsecureStatefulSessionIdManager, which broke multi-instance deployments without sticky sessions by causing Invalid session ID errors when requests were routed to different server instances. This change restores the previous default behavior: - Default session manager is now StatelessSessionIdManager (no session validation) - Multi-instance deployments work without requiring sticky sessions - Added WithStateful() option for explicit stateful session management - Updated all nil fallbacks to use stateless manager - Updated tests to verify new default behavior - Updated documentation to reflect the change Backward compatibility is maintained while fixing the production deployment issue. --- server/streamable_http.go | 20 +++++++++---- server/streamable_http_test.go | 51 +++++++++++++++++++--------------- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/server/streamable_http.go b/server/streamable_http.go index 5a596467..6c2cc080 100644 --- a/server/streamable_http.go +++ b/server/streamable_http.go @@ -52,13 +52,13 @@ func WithStateLess(stateLess bool) StreamableHTTPOption { } // WithSessionIdManager sets a custom session id generator for the server. -// By default, the server uses InsecureStatefulSessionIdManager (UUID-based; insecure). +// By default, the server uses StatelessSessionIdManager (no session validation). // Note: Options are applied in order; the last one wins. If combined with // WithStateLess or WithSessionIdManagerResolver, whichever is applied last takes effect. func WithSessionIdManager(manager SessionIdManager) StreamableHTTPOption { return func(s *StreamableHTTPServer) { if manager == nil { - s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&InsecureStatefulSessionIdManager{}) + s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&StatelessSessionIdManager{}) return } s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(manager) @@ -72,13 +72,23 @@ func WithSessionIdManager(manager SessionIdManager) StreamableHTTPOption { func WithSessionIdManagerResolver(resolver SessionIdManagerResolver) StreamableHTTPOption { return func(s *StreamableHTTPServer) { if resolver == nil { - s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&InsecureStatefulSessionIdManager{}) + s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&StatelessSessionIdManager{}) return } s.sessionIdManagerResolver = resolver } } +// WithStateful enables stateful session management using InsecureStatefulSessionIdManager. +// This requires sticky sessions in multi-instance deployments. +func WithStateful(stateful bool) StreamableHTTPOption { + return func(s *StreamableHTTPServer) { + if stateful { + s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&InsecureStatefulSessionIdManager{}) + } + } +} + // WithHeartbeatInterval sets the heartbeat interval. Positive interval means the // server will send a heartbeat to the client through the GET connection, to keep // the connection alive from being closed by the network infrastructure (e.g. @@ -187,7 +197,7 @@ func NewStreamableHTTPServer(server *MCPServer, opts ...StreamableHTTPOption) *S sessionTools: newSessionToolsStore(), sessionLogLevels: newSessionLogLevelsStore(), endpointPath: "/mcp", - sessionIdManagerResolver: NewDefaultSessionIdManagerResolver(&InsecureStatefulSessionIdManager{}), + sessionIdManagerResolver: NewDefaultSessionIdManagerResolver(&StatelessSessionIdManager{}), logger: util.DefaultLogger(), sessionResources: newSessionResourcesStore(), sessionResourceTemplates: newSessionResourceTemplatesStore(), @@ -1244,7 +1254,7 @@ type DefaultSessionIdManagerResolver struct { // NewDefaultSessionIdManagerResolver creates a new DefaultSessionIdManagerResolver with the given SessionIdManager func NewDefaultSessionIdManagerResolver(manager SessionIdManager) *DefaultSessionIdManagerResolver { if manager == nil { - manager = &InsecureStatefulSessionIdManager{} + manager = &StatelessSessionIdManager{} } return &DefaultSessionIdManagerResolver{manager: manager} } diff --git a/server/streamable_http_test.go b/server/streamable_http_test.go index f83a95eb..0b055ce8 100644 --- a/server/streamable_http_test.go +++ b/server/streamable_http_test.go @@ -1865,17 +1865,14 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { server := NewStreamableHTTPServer(mcpServer, WithStateLess(false)) - // Test that the default manager is still used (InsecureStatefulSessionIdManager) + // Test that the default manager is still used (StatelessSessionIdManager) req, _ := http.NewRequest("POST", "/test", nil) resolved := server.sessionIdManagerResolver.ResolveSessionIdManager(req) - // Verify it's NOT a stateless manager + // Verify it's a stateless manager (default behavior) sessionID := resolved.Generate() - if sessionID == "" { - t.Error("Expected stateful manager when WithStateLess(false)") - } - if !strings.HasPrefix(sessionID, idPrefix) { - t.Error("Expected stateful session ID format") + if sessionID != "" { + t.Error("Expected stateless manager (empty session ID) by default") } }) @@ -1938,13 +1935,10 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { t.Error("Expected nil resolver to be replaced with default") } - // Test that the resolved manager works (should be default stateful manager) + // Test that the resolved manager works (should be default stateless manager) sessionID := resolved.Generate() - if sessionID == "" { - t.Error("Expected default manager to generate non-empty session ID") - } - if !strings.HasPrefix(sessionID, idPrefix) { - t.Error("Expected default manager to generate session ID with correct prefix") + if sessionID != "" { + t.Error("Expected default manager to generate empty session ID (stateless)") } }) @@ -1960,13 +1954,10 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { t.Error("Expected nil manager to be replaced with default") } - // Test that the resolved manager works (should be default stateful manager) + // Test that the resolved manager works (should be default stateless manager) sessionID := resolved.Generate() - if sessionID == "" { - t.Error("Expected default manager to generate non-empty session ID") - } - if !strings.HasPrefix(sessionID, idPrefix) { - t.Error("Expected default manager to generate session ID with correct prefix") + if sessionID != "" { + t.Error("Expected default manager to generate empty session ID (stateless)") } }) @@ -1985,10 +1976,10 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { t.Error("Expected chained nil options to fall back safely") } - // Verify it generates valid session IDs + // Verify it uses stateless behavior (default) sessionID := resolved.Generate() - if sessionID == "" { - t.Error("Expected fallback manager to generate non-empty session ID") + if sessionID != "" { + t.Error("Expected fallback manager to generate empty session ID (stateless)") } }) @@ -2021,6 +2012,22 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { } _ = resp.Body.Close() }) + + t.Run("WithStateful enables stateful manager", func(t *testing.T) { + mcpServer := NewMCPServer("test-server", "1.0.0") + server := NewStreamableHTTPServer(mcpServer, WithStateful(true)) + + req, _ := http.NewRequest("POST", "/test", nil) + resolved := server.sessionIdManagerResolver.ResolveSessionIdManager(req) + + sessionID := resolved.Generate() + if sessionID == "" { + t.Error("Expected stateful manager to generate session ID") + } + if !strings.HasPrefix(sessionID, idPrefix) { + t.Error("Expected stateful session ID format") + } + }) } func TestStreamableHTTP_SendNotificationToSpecificClient(t *testing.T) { From 67b0cc07c68c4ad2280c1381fff5c2fc43c29d46 Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Sat, 22 Nov 2025 17:54:44 +0300 Subject: [PATCH 2/5] fix: create StatelessGeneratingSessionIdManager to fix multi-instance deployments Fixes #636 The original fix was too aggressive - changing to completely stateless broke existing functionality that expects session IDs to be generated. New approach: - Created StatelessGeneratingSessionIdManager that generates session IDs but only validates format (not existence locally) - This fixes multi-instance deployments while maintaining compatibility - Updated tests to reflect new behavior - Session termination now requires explicit stateful mode This solves the production issue where requests routed to different instances failed with 'Invalid session ID' errors while preserving the expected session ID generation behavior. --- server/streamable_http.go | 34 ++++++++++-- server/streamable_http_test.go | 99 ++++++++++++++++++++++++++-------- 2 files changed, 107 insertions(+), 26 deletions(-) diff --git a/server/streamable_http.go b/server/streamable_http.go index 6c2cc080..f471cac3 100644 --- a/server/streamable_http.go +++ b/server/streamable_http.go @@ -52,13 +52,13 @@ func WithStateLess(stateLess bool) StreamableHTTPOption { } // WithSessionIdManager sets a custom session id generator for the server. -// By default, the server uses StatelessSessionIdManager (no session validation). +// By default, the server uses StatelessGeneratingSessionIdManager (generates IDs but no local validation). // Note: Options are applied in order; the last one wins. If combined with // WithStateLess or WithSessionIdManagerResolver, whichever is applied last takes effect. func WithSessionIdManager(manager SessionIdManager) StreamableHTTPOption { return func(s *StreamableHTTPServer) { if manager == nil { - s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&StatelessSessionIdManager{}) + s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&StatelessGeneratingSessionIdManager{}) return } s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(manager) @@ -72,7 +72,7 @@ func WithSessionIdManager(manager SessionIdManager) StreamableHTTPOption { func WithSessionIdManagerResolver(resolver SessionIdManagerResolver) StreamableHTTPOption { return func(s *StreamableHTTPServer) { if resolver == nil { - s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&StatelessSessionIdManager{}) + s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&StatelessGeneratingSessionIdManager{}) return } s.sessionIdManagerResolver = resolver @@ -197,7 +197,7 @@ func NewStreamableHTTPServer(server *MCPServer, opts ...StreamableHTTPOption) *S sessionTools: newSessionToolsStore(), sessionLogLevels: newSessionLogLevelsStore(), endpointPath: "/mcp", - sessionIdManagerResolver: NewDefaultSessionIdManagerResolver(&StatelessSessionIdManager{}), + sessionIdManagerResolver: NewDefaultSessionIdManagerResolver(&StatelessGeneratingSessionIdManager{}), logger: util.DefaultLogger(), sessionResources: newSessionResourcesStore(), sessionResourceTemplates: newSessionResourceTemplatesStore(), @@ -1254,7 +1254,7 @@ type DefaultSessionIdManagerResolver struct { // NewDefaultSessionIdManagerResolver creates a new DefaultSessionIdManagerResolver with the given SessionIdManager func NewDefaultSessionIdManagerResolver(manager SessionIdManager) *DefaultSessionIdManagerResolver { if manager == nil { - manager = &StatelessSessionIdManager{} + manager = &StatelessGeneratingSessionIdManager{} } return &DefaultSessionIdManagerResolver{manager: manager} } @@ -1280,6 +1280,30 @@ func (s *StatelessSessionIdManager) Terminate(sessionID string) (isNotAllowed bo return false, nil } +// StatelessGeneratingSessionIdManager generates session IDs but doesn't validate them locally. +// This allows session IDs to be generated for clients while working across multiple instances. +type StatelessGeneratingSessionIdManager struct{} + +func (s *StatelessGeneratingSessionIdManager) Generate() string { + return idPrefix + uuid.New().String() +} + +func (s *StatelessGeneratingSessionIdManager) Validate(sessionID string) (isTerminated bool, err error) { + // Only validate format, not existence - allows cross-instance operation + if !strings.HasPrefix(sessionID, idPrefix) { + return false, fmt.Errorf("invalid session id: %s", sessionID) + } + if _, err := uuid.Parse(sessionID[len(idPrefix):]); err != nil { + return false, fmt.Errorf("invalid session id: %s", sessionID) + } + return false, nil +} + +func (s *StatelessGeneratingSessionIdManager) Terminate(sessionID string) (isNotAllowed bool, err error) { + // No-op termination since we don't track sessions + return false, nil +} + // InsecureStatefulSessionIdManager generate id with uuid and tracks active sessions. // It validates both format and existence of session IDs. // For more secure session id, use a more complex generator, like a JWT. diff --git a/server/streamable_http_test.go b/server/streamable_http_test.go index 0b055ce8..d2dc11af 100644 --- a/server/streamable_http_test.go +++ b/server/streamable_http_test.go @@ -1404,7 +1404,7 @@ func TestStreamableHTTP_SessionValidation(t *testing.T) { server := NewTestStreamableHTTPServer(mcpServer) defer server.Close() - t.Run("Reject tool call with fake session ID", func(t *testing.T) { + t.Run("Accept tool call with properly formatted session ID", func(t *testing.T) { toolCallRequest := map[string]any{ "jsonrpc": "2.0", "id": 1, @@ -1425,13 +1425,29 @@ func TestStreamableHTTP_SessionValidation(t *testing.T) { } defer resp.Body.Close() - if resp.StatusCode != http.StatusBadRequest { - t.Errorf("Expected status 400, got %d", resp.StatusCode) + if resp.StatusCode != http.StatusOK { + t.Errorf("Expected status 200, got %d", resp.StatusCode) } body, _ := io.ReadAll(resp.Body) - if !strings.Contains(string(body), "Invalid session ID") { - t.Errorf("Expected 'Invalid session ID' error, got: %s", string(body)) + var response map[string]any + if err := json.Unmarshal(body, &response); err != nil { + t.Fatalf("Failed to unmarshal response: %v", err) + } + + if result, ok := response["result"].(map[string]any); ok { + if content, ok := result["content"].([]any); ok && len(content) > 0 { + if textContent, ok := content[0].(map[string]any); ok { + if text, ok := textContent["text"].(string); ok { + // Should be a valid timestamp response + if text == "" { + t.Error("Expected non-empty timestamp response") + } + } + } + } + } else { + t.Errorf("Expected result in response, got: %s", string(body)) } }) @@ -1508,22 +1524,45 @@ func TestStreamableHTTP_SessionValidation(t *testing.T) { } }) - t.Run("Reject tool call with terminated session ID", func(t *testing.T) { + t.Run("Reject tool call with terminated session ID (stateful mode)", func(t *testing.T) { + mcpServer := NewMCPServer("test-server", "1.0.0") + // Use explicit stateful mode for this test since termination requires local tracking + server := NewTestStreamableHTTPServer(mcpServer, WithStateful(true)) + defer server.Close() + + // First, initialize a session + initRequest := map[string]any{ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": map[string]any{ + "protocolVersion": "2025-03-26", + "capabilities": map[string]any{ + "tools": map[string]any{}, + }, + "clientInfo": map[string]any{ + "name": "test-client", + "version": "1.0.0", + }, + }, + } + jsonBody, _ := json.Marshal(initRequest) req, _ := http.NewRequest(http.MethodPost, server.URL, bytes.NewBuffer(jsonBody)) req.Header.Set("Content-Type", "application/json") resp, err := server.Client().Do(req) if err != nil { - t.Fatalf("Failed to initialize: %v", err) + t.Fatalf("Failed to initialize session: %v", err) } - resp.Body.Close() sessionID := resp.Header.Get(HeaderKeySessionID) if sessionID == "" { t.Fatal("Expected session ID in response header") } + resp.Body.Close() + // Now terminate the session req, _ = http.NewRequest(http.MethodDelete, server.URL, nil) req.Header.Set(HeaderKeySessionID, sessionID) @@ -1865,14 +1904,17 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { server := NewStreamableHTTPServer(mcpServer, WithStateLess(false)) - // Test that the default manager is still used (StatelessSessionIdManager) + // Test that the default manager is still used (StatelessGeneratingSessionIdManager) req, _ := http.NewRequest("POST", "/test", nil) resolved := server.sessionIdManagerResolver.ResolveSessionIdManager(req) - // Verify it's a stateless manager (default behavior) + // Verify it's a generating manager (default behavior) sessionID := resolved.Generate() - if sessionID != "" { - t.Error("Expected stateless manager (empty session ID) by default") + if sessionID == "" { + t.Error("Expected generating manager to generate session ID by default") + } + if !strings.HasPrefix(sessionID, idPrefix) { + t.Error("Expected generating manager to generate session ID with correct prefix") } }) @@ -1935,10 +1977,13 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { t.Error("Expected nil resolver to be replaced with default") } - // Test that the resolved manager works (should be default stateless manager) + // Test that the resolved manager works (should be default generating manager) sessionID := resolved.Generate() - if sessionID != "" { - t.Error("Expected default manager to generate empty session ID (stateless)") + if sessionID == "" { + t.Error("Expected default manager to generate session ID") + } + if !strings.HasPrefix(sessionID, idPrefix) { + t.Error("Expected default manager to generate session ID with correct prefix") } }) @@ -1954,10 +1999,13 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { t.Error("Expected nil manager to be replaced with default") } - // Test that the resolved manager works (should be default stateless manager) + // Test that the resolved manager works (should be default generating manager) sessionID := resolved.Generate() - if sessionID != "" { - t.Error("Expected default manager to generate empty session ID (stateless)") + if sessionID == "" { + t.Error("Expected default manager to generate session ID") + } + if !strings.HasPrefix(sessionID, idPrefix) { + t.Error("Expected default manager to generate session ID with correct prefix") } }) @@ -1976,10 +2024,13 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { t.Error("Expected chained nil options to fall back safely") } - // Verify it uses stateless behavior (default) + // Verify it uses generating behavior (default) sessionID := resolved.Generate() - if sessionID != "" { - t.Error("Expected fallback manager to generate empty session ID (stateless)") + if sessionID == "" { + t.Error("Expected fallback manager to generate session ID") + } + if !strings.HasPrefix(sessionID, idPrefix) { + t.Error("Expected fallback manager to generate session ID with correct prefix") } }) @@ -2027,6 +2078,12 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { if !strings.HasPrefix(sessionID, idPrefix) { t.Error("Expected stateful session ID format") } + + // Test that stateful manager validates session existence (unlike default) + _, err := resolved.Validate("unknown-session-id") + if err == nil { + t.Error("Expected stateful manager to reject unknown session ID") + } }) } From 12e184669db92a60c757867dd9cb0a7164806035 Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Sat, 22 Nov 2025 17:58:24 +0300 Subject: [PATCH 3/5] fix: update NewDefaultSessionIdManagerResolver fallback to StatelessSessionIdManager As requested, updated NewDefaultSessionIdManagerResolver to fall back to StatelessSessionIdManager instead of StatelessGeneratingSessionIdManager when manager is nil. This change affects: - NewDefaultSessionIdManagerResolver(nil) fallback - WithSessionIdManager(nil) fallback - WithSessionIdManagerResolver(nil) fallback Updated corresponding tests to expect stateless behavior: - Generate() returns empty string ("") - Validate() accepts any session ID without error The server default still uses StatelessGeneratingSessionIdManager for backward compatibility, but nil fallbacks now use truly stateless behavior. --- server/streamable_http.go | 6 ++--- server/streamable_http_test.go | 49 ++++++++++++++++------------------ 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/server/streamable_http.go b/server/streamable_http.go index f471cac3..b7d46408 100644 --- a/server/streamable_http.go +++ b/server/streamable_http.go @@ -58,7 +58,7 @@ func WithStateLess(stateLess bool) StreamableHTTPOption { func WithSessionIdManager(manager SessionIdManager) StreamableHTTPOption { return func(s *StreamableHTTPServer) { if manager == nil { - s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&StatelessGeneratingSessionIdManager{}) + s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&StatelessSessionIdManager{}) return } s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(manager) @@ -72,7 +72,7 @@ func WithSessionIdManager(manager SessionIdManager) StreamableHTTPOption { func WithSessionIdManagerResolver(resolver SessionIdManagerResolver) StreamableHTTPOption { return func(s *StreamableHTTPServer) { if resolver == nil { - s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&StatelessGeneratingSessionIdManager{}) + s.sessionIdManagerResolver = NewDefaultSessionIdManagerResolver(&StatelessSessionIdManager{}) return } s.sessionIdManagerResolver = resolver @@ -1254,7 +1254,7 @@ type DefaultSessionIdManagerResolver struct { // NewDefaultSessionIdManagerResolver creates a new DefaultSessionIdManagerResolver with the given SessionIdManager func NewDefaultSessionIdManagerResolver(manager SessionIdManager) *DefaultSessionIdManagerResolver { if manager == nil { - manager = &StatelessGeneratingSessionIdManager{} + manager = &StatelessSessionIdManager{} } return &DefaultSessionIdManagerResolver{manager: manager} } diff --git a/server/streamable_http_test.go b/server/streamable_http_test.go index d2dc11af..78c9b839 100644 --- a/server/streamable_http_test.go +++ b/server/streamable_http_test.go @@ -1819,13 +1819,19 @@ func TestDefaultSessionIdManagerResolver(t *testing.T) { t.Error("Expected resolver to return a non-nil manager") } - // Test that the resolved manager works (generates valid session IDs) + // Test that the resolved manager works (stateless behavior) sessionID := resolved.Generate() - if sessionID == "" { - t.Error("Expected default manager to generate non-empty session ID") + if sessionID != "" { + t.Error("Expected stateless manager to generate empty session ID") } - if !strings.HasPrefix(sessionID, idPrefix) { - t.Error("Expected default manager to generate session ID with correct prefix") + + // Test that validation accepts any session ID (stateless behavior) + isTerminated, err := resolved.Validate("any-session-id") + if err != nil { + t.Errorf("Expected stateless manager to accept any session ID, got error: %v", err) + } + if isTerminated { + t.Error("Expected stateless manager to not terminate sessions") } }) } @@ -1968,7 +1974,7 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { t.Run("WithSessionIdManagerResolver handles nil resolver defensively", func(t *testing.T) { mcpServer := NewMCPServer("test-server", "1.0.0") - // This should not panic and should fall back to default behavior + // This should not panic and should fall back to StatelessSessionIdManager (safe default) server := NewStreamableHTTPServer(mcpServer, WithSessionIdManagerResolver(nil)) req, _ := http.NewRequest("POST", "/test", nil) @@ -1977,20 +1983,17 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { t.Error("Expected nil resolver to be replaced with default") } - // Test that the resolved manager works (should be default generating manager) + // Test that the resolved manager works (should be default stateless manager) sessionID := resolved.Generate() - if sessionID == "" { - t.Error("Expected default manager to generate session ID") - } - if !strings.HasPrefix(sessionID, idPrefix) { - t.Error("Expected default manager to generate session ID with correct prefix") + if sessionID != "" { + t.Error("Expected default stateless manager to generate empty session ID") } }) t.Run("WithSessionIdManager handles nil manager defensively", func(t *testing.T) { mcpServer := NewMCPServer("test-server", "1.0.0") - // This should not panic and should fall back to default behavior + // This should not panic and should fall back to StatelessSessionIdManager (safe default) server := NewStreamableHTTPServer(mcpServer, WithSessionIdManager(nil)) req, _ := http.NewRequest("POST", "/test", nil) @@ -1999,20 +2002,17 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { t.Error("Expected nil manager to be replaced with default") } - // Test that the resolved manager works (should be default generating manager) + // Test that the resolved manager works (should be default stateless manager) sessionID := resolved.Generate() - if sessionID == "" { - t.Error("Expected default manager to generate session ID") - } - if !strings.HasPrefix(sessionID, idPrefix) { - t.Error("Expected default manager to generate session ID with correct prefix") + if sessionID != "" { + t.Error("Expected default stateless manager to generate empty session ID") } }) t.Run("Multiple nil options fall back safely", func(t *testing.T) { mcpServer := NewMCPServer("test-server", "1.0.0") - // Chain multiple nil options - last one should win with safe fallback + // Chain multiple nil options - last one should win with StatelessSessionIdManager fallback server := NewStreamableHTTPServer(mcpServer, WithSessionIdManager(nil), WithSessionIdManagerResolver(nil), @@ -2024,13 +2024,10 @@ func TestSessionIdManagerResolver_Integration(t *testing.T) { t.Error("Expected chained nil options to fall back safely") } - // Verify it uses generating behavior (default) + // Verify it uses stateless behavior (default) sessionID := resolved.Generate() - if sessionID == "" { - t.Error("Expected fallback manager to generate session ID") - } - if !strings.HasPrefix(sessionID, idPrefix) { - t.Error("Expected fallback manager to generate session ID with correct prefix") + if sessionID != "" { + t.Error("Expected fallback stateless manager to generate empty session ID") } }) From 41c58b51d68d158e3c81dc8f4a8175e6708d545d Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Sat, 22 Nov 2025 18:04:49 +0300 Subject: [PATCH 4/5] fix: update tests to use WithStateful(true) for stateful behavior As requested in the CodeRabbit review, updated tests that expect stateful behavior to explicitly use WithStateful(true) instead of relying on the old default behavior. Updated tests: - TestStreamableHTTP_POST_SendAndReceive: expects session IDs in headers and 400 for invalid IDs - TestStreamableHttpResourceGet: expects session ID in header - TestStreamableHTTP_SessionWithLogging: expects session ID in header - TestStreamableHTTP_SendNotificationToSpecificClient: both subtests expect session IDs in headers This ensures tests explicitly opt into stateful mode when needed, while the default remains stateless for multi-instance deployments. --- server/streamable_http_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/streamable_http_test.go b/server/streamable_http_test.go index 78c9b839..9e444c53 100644 --- a/server/streamable_http_test.go +++ b/server/streamable_http_test.go @@ -125,7 +125,7 @@ func TestStreamableHTTP_POST_InvalidContent(t *testing.T) { func TestStreamableHTTP_POST_SendAndReceive(t *testing.T) { mcpServer := NewMCPServer("test-mcp-server", "1.0") addSSETool(mcpServer) - server := NewTestStreamableHTTPServer(mcpServer) + server := NewTestStreamableHTTPServer(mcpServer, WithStateful(true)) var sessionID string t.Run("initialize", func(t *testing.T) { @@ -595,6 +595,7 @@ func TestStreamableHttpResourceGet(t *testing.T) { testServer := NewTestStreamableHTTPServer( s, + WithStateful(true), WithHTTPContextFunc(func(ctx context.Context, r *http.Request) context.Context { session := ClientSessionFromContext(ctx) @@ -1014,7 +1015,7 @@ func TestStreamableHTTP_SessionWithLogging(t *testing.T) { }) mcpServer := NewMCPServer("test", "1.0.0", WithHooks(hooks), WithLogging()) - testServer := NewTestStreamableHTTPServer(mcpServer) + testServer := NewTestStreamableHTTPServer(mcpServer, WithStateful(true)) defer testServer.Close() // obtain a valid session ID first @@ -2100,7 +2101,7 @@ func TestStreamableHTTP_SendNotificationToSpecificClient(t *testing.T) { }) mcpServer := NewMCPServer("test", "1.0.0", WithHooks(hooks)) - testServer := NewTestStreamableHTTPServer(mcpServer) + testServer := NewTestStreamableHTTPServer(mcpServer, WithStateful(true)) defer testServer.Close() // Send initialize request to register session @@ -2171,7 +2172,7 @@ func TestStreamableHTTP_SendNotificationToSpecificClient(t *testing.T) { return mcp.NewToolResultText("notification sent"), nil }) - testServer := NewTestStreamableHTTPServer(mcpServer) + testServer := NewTestStreamableHTTPServer(mcpServer, WithStateful(true)) defer testServer.Close() // Initialize session From 3bc7315aa5154835388d354c0ddd70eb5ba9fb3e Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Sat, 22 Nov 2025 18:16:46 +0300 Subject: [PATCH 5/5] fix: enable coverage on main branch and add artifact retention - Updated coverage job condition to run on push to main branch in addition to PRs - Added 30-day artifact retention to ensure coverage artifacts are available for comparison - This fixes the coverage triggers that were failing due to missing baseline artifacts Now coverage will: 1. Run on main branch pushes to create baseline artifacts 2. Run on PRs to compare against baseline and ensure coverage doesn't decrease 3. Keep artifacts available for 30 days for proper comparison --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a4cd0ac0..1fe659cf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,7 @@ jobs: coverage: runs-on: ubuntu-latest - if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target' + if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target' || (github.event_name == 'push' && github.ref == 'refs/heads/main') permissions: contents: read pull-requests: write @@ -38,6 +38,7 @@ jobs: with: name: code-coverage path: coverage.txt + retention-days: 30 - name: Generate coverage report uses: fgrosse/go-coverage-report@v1.2.0