Skip to content

Commit e5f9e1d

Browse files
authored
fix: use server-issued session ID for HTTP backend tools/list (#1559)
## Problem HTTP backends that implement the Streamable HTTP MCP transport (spec 2025-03-26) issue a specific `Mcp-Session-Id` during `initialize` and require that exact session ID on all subsequent requests. When `tools/list` arrived with a different session ID, they returned **HTTP 400**, causing the gateway to register zero tools for the backend — even though the health check reported the server as `running`. Root cause: `registerToolsFromBackend` injected a fabricated `"gateway-init-{serverID}"` session ID into the request context. In `sendHTTPRequest`, the context session ID has higher priority than `conn.httpSessionID` (the real session ID captured from `initializeHTTPSession()`), so the server-issued ID was silently overridden on every startup-time `tools/list` call. Reported in: github/gh-aw#18712 (Datadog MCP server affected) ## Fix Remove the fabricated context session ID from `registerToolsFromBackend` and pass `context.Background()` directly. `sendHTTPRequest` already falls back to `conn.httpSessionID` when no context session ID is present, so it now correctly uses the server-issued session ID. ```go // Before ctx := context.WithValue(context.Background(), SessionIDContextKey, fmt.Sprintf("gateway-init-%s", serverID)) result, err := conn.SendRequestWithServerID(ctx, "tools/list", nil, serverID) // After result, err := conn.SendRequestWithServerID(context.Background(), "tools/list", nil, serverID) ``` ## Tests `TestHTTPBackendInitialization` is rewritten as a regression test for this exact scenario: - Mock server issues a specific `Mcp-Session-Id` during `initialize` - Mock server rejects `tools/list` with HTTP 400 if the session ID doesn't match - Test asserts the gateway uses the server-issued session ID (not a fabricated one) All existing tests continue to pass.
2 parents 8d1a49e + 983706d commit e5f9e1d

File tree

2 files changed

+51
-57
lines changed

2 files changed

+51
-57
lines changed

internal/server/unified.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -275,12 +275,8 @@ func (us *UnifiedServer) registerToolsFromBackend(serverID string) error {
275275
return fmt.Errorf("failed to connect: %w", err)
276276
}
277277

278-
// Create a context with session ID for HTTP backends
279-
// HTTP backends may require Mcp-Session-Id header even during initialization
280-
ctx := context.WithValue(context.Background(), SessionIDContextKey, fmt.Sprintf("gateway-init-%s", serverID))
281-
282278
// List tools from backend
283-
result, err := conn.SendRequestWithServerID(ctx, "tools/list", nil, serverID)
279+
result, err := conn.SendRequestWithServerID(context.Background(), "tools/list", nil, serverID)
284280
if err != nil {
285281
return fmt.Errorf("failed to list tools: %w", err)
286282
}

internal/server/unified_http_backend_test.go

Lines changed: 50 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -15,89 +15,87 @@ import (
1515
"github.com/github/gh-aw-mcpg/internal/mcp"
1616
)
1717

18-
// TestHTTPBackendInitialization tests that HTTP backends receive session ID during initialization
18+
// TestHTTPBackendInitialization tests that HTTP backends use the session ID issued by the
19+
// server during initialize (not a locally-fabricated one) when calling tools/list.
20+
// This is a regression test for https://github.com/github/gh-aw/issues/18712 where
21+
// gateway-issued fake session IDs overrode the real server-issued session ID, causing
22+
// HTTP 400 on tools/list from strict backends like Datadog.
1923
func TestHTTPBackendInitialization(t *testing.T) {
20-
// Track whether the session ID header was received
21-
var receivedSessionID string
22-
var requestMethod string
24+
const serverSessionID = "server-issued-session-42"
25+
var toolsListSessionID string
2326

24-
// Create a mock HTTP MCP server that requires Mcp-Session-Id header
27+
// Create a mock HTTP MCP server that:
28+
// 1. Issues a specific session ID during initialize
29+
// 2. Requires that exact session ID for subsequent requests
2530
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
26-
receivedSessionID = r.Header.Get("Mcp-Session-Id")
27-
28-
// Parse the JSON-RPC request to get the method
2931
var req struct {
3032
Method string `json:"method"`
3133
}
3234
json.NewDecoder(r.Body).Decode(&req)
33-
requestMethod = req.Method
3435

35-
// Simulate a backend that requires Mcp-Session-Id header
36-
if receivedSessionID == "" {
37-
w.WriteHeader(http.StatusBadRequest)
38-
response := map[string]interface{}{
36+
switch req.Method {
37+
case "initialize":
38+
// Issue a specific session ID (as Datadog and other Streamable HTTP servers do)
39+
w.Header().Set("Mcp-Session-Id", serverSessionID)
40+
w.Header().Set("Content-Type", "application/json")
41+
json.NewEncoder(w).Encode(map[string]interface{}{
3942
"jsonrpc": "2.0",
40-
"error": map[string]interface{}{
41-
"code": -32600,
42-
"message": "Invalid Request: Missing Mcp-Session-Id header",
43+
"id": 1,
44+
"result": map[string]interface{}{
45+
"protocolVersion": "2024-11-05",
46+
"capabilities": map[string]interface{}{},
47+
"serverInfo": map[string]interface{}{"name": "test-server", "version": "1.0.0"},
4348
},
44-
"id": 1,
49+
})
50+
case "tools/list":
51+
toolsListSessionID = r.Header.Get("Mcp-Session-Id")
52+
// Reject requests with wrong or missing session ID (as strict backends do)
53+
if toolsListSessionID != serverSessionID {
54+
w.WriteHeader(http.StatusBadRequest)
55+
json.NewEncoder(w).Encode(map[string]interface{}{
56+
"jsonrpc": "2.0",
57+
"error": map[string]interface{}{"code": -32603, "message": "Invalid session ID"},
58+
"id": 1,
59+
})
60+
return
4561
}
46-
json.NewEncoder(w).Encode(response)
47-
return
48-
}
49-
50-
// Return a successful tools/list response
51-
response := map[string]interface{}{
52-
"jsonrpc": "2.0",
53-
"id": 1,
54-
"result": map[string]interface{}{
55-
"tools": []map[string]interface{}{
56-
{
57-
"name": "test_tool",
58-
"description": "A test tool",
59-
"inputSchema": map[string]interface{}{
60-
"type": "object",
61-
},
62+
w.Header().Set("Content-Type", "application/json")
63+
json.NewEncoder(w).Encode(map[string]interface{}{
64+
"jsonrpc": "2.0",
65+
"id": 1,
66+
"result": map[string]interface{}{
67+
"tools": []map[string]interface{}{
68+
{"name": "test_tool", "description": "A test tool", "inputSchema": map[string]interface{}{"type": "object"}},
6269
},
6370
},
64-
},
71+
})
6572
}
66-
w.Header().Set("Content-Type", "application/json")
67-
json.NewEncoder(w).Encode(response)
6873
}))
6974
defer mockServer.Close()
7075

71-
// Create config with HTTP backend
76+
// Use a custom header to force plain JSON-RPC transport (avoids SDK transport timeouts in tests)
7277
cfg := &config.Config{
7378
Servers: map[string]*config.ServerConfig{
7479
"http-backend": {
7580
Type: "http",
7681
URL: mockServer.URL,
77-
Headers: map[string]string{},
82+
Headers: map[string]string{"X-Auth": "test"},
7883
},
7984
},
8085
}
8186

82-
// Create unified server - this should call tools/list during initialization
87+
// Create unified server - this calls tools/list during initialization
8388
ctx := context.Background()
8489
us, err := NewUnified(ctx, cfg)
85-
require.NoError(t, err, "Failed to create unified server")
90+
require.NoError(t, err, "Failed to create unified server: gateway must use server-issued session ID for tools/list")
8691
defer us.Close()
8792

88-
// Verify that the session ID was sent
89-
assert.False(t, receivedSessionID == "", "Expected Mcp-Session-Id header to be sent during initialization, but it was empty")
90-
91-
// Verify the session ID follows the gateway-init pattern
92-
expectedPrefix := "gateway-init-"
93-
if len(receivedSessionID) < len(expectedPrefix) || receivedSessionID[:len(expectedPrefix)] != expectedPrefix {
94-
t.Errorf("Expected session ID to start with '%s', got '%s'", expectedPrefix, receivedSessionID)
95-
}
96-
97-
// Verify it was a tools/list request
98-
assert.Equal(t, "tools/list", requestMethod, "method 'tools/list', got '%s'")
93+
// The session ID used for tools/list must be the one issued by the server during initialize,
94+
// not a locally-fabricated "gateway-init-*" value.
95+
assert.Equal(t, serverSessionID, toolsListSessionID,
96+
"tools/list must use the session ID issued by the server during initialize, not a fabricated one")
9997

100-
t.Logf("Successfully initialized HTTP backend with session ID: %s", receivedSessionID)
98+
t.Logf("Correctly used server-issued session ID for tools/list: %s", toolsListSessionID)
10199
}
102100

103101
// TestHTTPBackendInitializationWithSessionIDRequirement tests the exact error scenario from the problem statement

0 commit comments

Comments
 (0)