Skip to content

Commit 984f4b2

Browse files
committed
fix(mcp): skip SSE frames that carry a method member
Treat method presence the same on stdio and SSE so empty or null method never completes a pending client request.
1 parent caa4cfc commit 984f4b2

4 files changed

Lines changed: 66 additions & 2 deletions

File tree

‎internal/mcp/client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,7 @@ func (client *Client) readLoop() {
590590
}
591591
// A message with a Method is a server-initiated request or notification.
592592
// It must never be routed as a response to a pending client request.
593-
if message.methodPresent || message.Method != "" {
593+
if message.isRequestOrNotification() {
594594
if message.ID != nil && jsonRPCIDEchoable(message.ID) {
595595
client.enqueueCourtesy(rpcMessage{
596596
ID: message.ID,

‎internal/mcp/network_client.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,9 @@ func (client *remoteSSEClient) deliverEventMessage(value string) error {
539539
if err := decoder.Decode(&message); err != nil {
540540
return fmt.Errorf("decode MCP SSE stream message: %w", err)
541541
}
542+
if message.isRequestOrNotification() {
543+
return nil
544+
}
542545
key := rpcResponseKey(message.ID)
543546
if key == "" {
544547
return nil
@@ -633,7 +636,7 @@ func decodeSSERPCMessage(reader io.Reader) (rpcMessage, error) {
633636
// those — the response has no method — and keep scanning. Previously the
634637
// first message event was returned unconditionally, so a leading
635638
// notification surfaced to the caller as an id mismatch and failed the call.
636-
if candidate.Method != "" {
639+
if candidate.isRequestOrNotification() {
637640
return true
638641
}
639642
decoded = candidate

‎internal/mcp/network_client_test.go‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,3 +339,60 @@ func TestDecodeSSERPCMessageSkipsNotifications(t *testing.T) {
339339
t.Fatalf("expected a result payload, got %#v", msg)
340340
}
341341
}
342+
343+
func TestDecodeSSERPCMessageSkipsEmptyAndNullMethod(t *testing.T) {
344+
stream := "event: message\n" +
345+
`data: {"jsonrpc":"2.0","id":7,"method":""}` + "\n\n" +
346+
"event: message\n" +
347+
`data: {"jsonrpc":"2.0","id":7,"method":null}` + "\n\n" +
348+
"event: message\n" +
349+
`data: {"jsonrpc":"2.0","id":7,"result":{"ok":true}}` + "\n\n"
350+
351+
msg, err := decodeSSERPCMessage(strings.NewReader(stream))
352+
if err != nil {
353+
t.Fatalf("decodeSSERPCMessage: %v", err)
354+
}
355+
if msg.isRequestOrNotification() {
356+
t.Fatalf("empty/null method must not be treated as the response, got method %q present=%v", msg.Method, msg.methodPresent)
357+
}
358+
if !rpcIDMatches(msg.ID, 7) {
359+
t.Fatalf("expected response id 7, got %#v", msg.ID)
360+
}
361+
}
362+
363+
func TestDeliverEventMessageSkipsMethodPresence(t *testing.T) {
364+
client := &remoteSSEClient{pending: map[string]chan ssePendingResponse{}}
365+
key := rpcResponseKey(1)
366+
pending := make(chan ssePendingResponse, 1)
367+
client.pending[key] = pending
368+
369+
if err := client.deliverEventMessage(`{"jsonrpc":"2.0","id":1,"method":""}`); err != nil {
370+
t.Fatalf("deliverEventMessage empty method: %v", err)
371+
}
372+
if err := client.deliverEventMessage(`{"jsonrpc":"2.0","id":1,"method":null}`); err != nil {
373+
t.Fatalf("deliverEventMessage null method: %v", err)
374+
}
375+
select {
376+
case got := <-pending:
377+
t.Fatalf("method presence must not complete pending, got %#v", got)
378+
default:
379+
}
380+
if _, ok := client.pending[key]; !ok {
381+
t.Fatal("deliverEventMessage must not delete pending for a request/notification")
382+
}
383+
384+
if err := client.deliverEventMessage(`{"jsonrpc":"2.0","id":1,"result":{"ok":true}}`); err != nil {
385+
t.Fatalf("deliverEventMessage response: %v", err)
386+
}
387+
select {
388+
case got := <-pending:
389+
if got.err != nil {
390+
t.Fatalf("true response: %v", got.err)
391+
}
392+
if got.message.isRequestOrNotification() {
393+
t.Fatal("true response must not carry a method")
394+
}
395+
default:
396+
t.Fatal("true response must complete pending")
397+
}
398+
}

‎internal/mcp/protocol.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ type rpcMessage struct {
2727
methodPresent bool `json:"-"`
2828
}
2929

30+
func (m rpcMessage) isRequestOrNotification() bool {
31+
return m.methodPresent || m.Method != ""
32+
}
33+
3034
func (m *rpcMessage) UnmarshalJSON(data []byte) error {
3135
var probe map[string]json.RawMessage
3236
if err := json.Unmarshal(data, &probe); err != nil {

0 commit comments

Comments
 (0)