Skip to content

fix(mcp): reject a server-initiated request as a streamable-HTTP response #1054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kevincodex1 merged 2 commits into from
Sep 15, 2026
+47 −0
Merged

fix(mcp): reject a server-initiated request as a streamable-HTTP response #1054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
086b420
fix(mcp): reject a server-initiated request as a streamable-HTTP resp…
chiliec Sep 13, 2026
3dc67b9
fix(mcp): describe the unexpected frame by method, not as a request
chiliec Sep 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions internal/mcp/network_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@ func (client *networkClient) request(ctx context.Context, method string, params
if err != nil {
return err
}
if message.isRequestOrNotification() {
return fmt.Errorf("MCP %s expected a response from server %s, got method %q instead", method, client.server.Name, message.Method)
}
Comment on lines +219 to +221

@greptile-apps greptile-apps Bot Sep 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Notification mislabeled as request

A method-bearing notification without an ID also satisfies isRequestOrNotification(), but this error always calls the frame a “request.” When a server returns a notification where an HTTP response is expected, callers receive a misleading protocol diagnostic. Describe it as a “request or notification,” or more generally as a method-bearing message.

Suggested change
if message.isRequestOrNotification() {
return fmt.Errorf("MCP %s expected a response from server %s, got %q request", method, client.server.Name, message.Method)
}
if message.isRequestOrNotification() {
return fmt.Errorf("MCP %s expected a response from server %s, got %q request or notification", method, client.server.Name, message.Method)
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@chiliec chiliec Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — a method-bearing frame with no id is a notification, not a request. Reworded to expected a response from server X, got method "roots/list" instead in 3dc67b9 so it stays accurate for both.

if !rpcIDMatches(message.ID, id) {
return fmt.Errorf("MCP %s response id mismatch for server %s", method, client.server.Name)
}
Expand Down
44 changes: 44 additions & 0 deletions internal/mcp/network_client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,50 @@ func TestNonOAuthServerIsUnaffected(t *testing.T) {
}
}

func TestNetworkClientRejectsServerInitiatedRequestAsResponse(t *testing.T) {
// A streamable-HTTP server that answers a tools/call POST with a
// method-bearing frame (a server-initiated request) must surface a protocol
// error rather than an empty, error-free result.
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
message := readHTTPRPCMessage(t, r)
switch message.Method {
case "initialize":
writeHTTPRPCResponse(t, w, message.ID, map[string]any{"protocolVersion": "2024-11-05"})
case "notifications/initialized":
w.WriteHeader(http.StatusAccepted)
case "tools/call":
w.Header().Set("Content-Type", "application/json")
if _, err := w.Write([]byte(`{"jsonrpc":"2.0","id":` + string(mustRaw(message.ID)) + `,"method":"roots/list","params":{}}`)); err != nil {
t.Errorf("write server request: %v", err)
}
default:
writeHTTPRPCResponse(t, w, message.ID, map[string]any{})
}
}))
defer upstream.Close()

client, err := Connect(ctx, Server{Name: "plain", Type: ServerTypeHTTP, URL: upstream.URL})
if err != nil {
t.Fatalf("Connect() error = %v", err)
}
defer func() {
if err := client.Close(); err != nil {
t.Fatalf("Close() error = %v", err)
}
}()

result, err := client.CallTool(ctx, "lookup", map[string]any{"query": "zero"})
if err == nil {
t.Fatalf("CallTool() error = nil, result = %#v, want protocol error", result)
}
if !strings.Contains(err.Error(), `"roots/list"`) {
t.Fatalf("CallTool() error = %q, want it to name the unexpected method", err)
}
}

func TestDecodeSSERPCMessageSkipsNotifications(t *testing.T) {
// A leading server notification (has a method) on the POST's event stream must
// be skipped so the actual response (no method) is returned, instead of the
Expand Down
Loading