From 826e653c270433bbaee6d5d46a771a66f9e15d98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Thu, 17 Sep 2026 10:36:23 +0200 Subject: [PATCH] mcp: refuse a call on a legacy session before initialize (#1273) Closes #1271. `Server.handle` refuses a call made before `initialize` on a legacy session, and that refusal lives in the `default` branch of its method switch, so any method with a `case` of its own skips it. Three are served on a session that never handshook: `logging/setLevel`, `resources/subscribe` and `resources/unsubscribe`. `resources/subscribe` is the one that reaches state rather than merely answering: it starts a watcher and registers a subscriber, so a client that subscribes before `initialize` is delivered `notifications/resources/updated` for the lifetime of a session the server never agreed to. The check now runs after the switch, for every call carrying no new-protocol `_meta`, with `initialize` and `ping` exempt. That is the same condition the `default` branch already applied and the same error it produced; the change is that the three methods above stop skipping it. `ping` stays allowed because the lifecycle page exempts it. `server/discover` is deliberately left alone: a legacy request there already receives method-not-found, which tells the client more than an initialization error would. The two notification cases stay outside, since the gate covers calls and a notification carries no refusal. The SEP-2575 path is untouched. A session without an `initialize` handshake is exactly what that proposal is for, so the exemption is the request's own `_meta.protocolVersion` rather than anything about the session, and a new-protocol client is served as it is today. Tests: `TestServerHandle_LegacyCallBeforeInitialize` drives each affected method on a fresh session and asserts the refusal, that `resources/subscribe` runs no handler and leaves no subscriber, and that `initialize` and `ping` are served. `TestServerHandle_NewProtocolCallWithoutInitialize` asserts a new-protocol session with no handshake still gets `tools/list`, and that the three methods keep their existing method-not-found rather than the initialization refusal. Without the change, the setLevel, subscribe and unsubscribe rows fail. `go test ./mcp/ -count=1` is green, `gofmt -l` is empty and `go vet ./mcp/` is clean. This does not depend on #1266 and rebases onto it cleanly, verified with the suite green on the combination. One decision worth your view: the refusal is a plain error, which on the wire carries code 0, exactly as the `default` branch produced before this change. The specification prescribes no code for it. If you would rather it were `-32600` or `-32603`, say so and I will change it here. --------- Co-authored-by: Guglielmo Colombo --- mcp/server.go | 14 ++-- mcp/server_test.go | 156 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 4 deletions(-) diff --git a/mcp/server.go b/mcp/server.go index 442d7bc..b2f89f4 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -1989,10 +1989,6 @@ func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any, } } default: - if !initialized && !validatedMeta.usesNewProtocol && req.IsCall() { - ss.server.opts.Logger.Error("method invalid during initialization", "method", req.Method) - return nil, fmt.Errorf("method %q is invalid during session initialization", req.Method) - } if !initialized && validatedMeta.usesNewProtocol && validatedMeta.initializeParams != nil { ss.updateState(func(state *ServerSessionState) { state.InitializeParams = validatedMeta.initializeParams @@ -2000,6 +1996,16 @@ func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any, } } + // In legacy protocol versions, a client cannot send requests other than + // pings before the server has responded to the initialize request. A + // new-protocol request is exempt: a SEP-2575 session has no 'initialize' + // to wait for, and the request itself says which protocol it speaks. + if !initialized && !validatedMeta.usesNewProtocol && req.IsCall() && + req.Method != methodInitialize && req.Method != methodPing { + ss.server.opts.Logger.Error("method invalid during initialization", "method", req.Method) + return nil, fmt.Errorf("method %q is invalid during session initialization", req.Method) + } + // modelcontextprotocol/go-sdk#26: handle calls asynchronously, and // notifications synchronously, except for 'initialize' which shouldn't be // asynchronous to other diff --git a/mcp/server_test.go b/mcp/server_test.go index 2c7bbe0..4d09cbc 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -2198,6 +2198,162 @@ func TestServerSupportedProtocolVersions_NewProtocol(t *testing.T) { } } +// TestServerHandle_LegacyCallBeforeInitialize asserts that a call carrying no +// new-protocol `_meta` is refused on a session that has not completed +// 'initialize', whichever branch of the method switch it lands in. The +// handshake itself is what ends that state, and the lifecycle spec exempts +// pings, so those two are served. +// +// The gate used to sit in the default branch, so 'logging/setLevel', +// 'resources/subscribe' and 'resources/unsubscribe', which share a case that +// only refuses new-protocol requests, were served before the handshake. For +// subscribe that registered a session that never handshook as a subscriber, +// so the subtest also asserts that the refusal leaves no subscriber and runs +// no handler. +func TestServerHandle_LegacyCallBeforeInitialize(t *testing.T) { + ctx := context.Background() + const uri = "test://resource" + + tests := []struct { + name string + method string + params string + served bool + }{ + { + name: "initialize", + method: methodInitialize, + params: fmt.Sprintf(`{"protocolVersion":%q,"capabilities":{},"clientInfo":{"name":"c","version":"1"}}`, protocolVersion20251125), + served: true, + }, + {name: "ping", method: methodPing, params: `{}`, served: true}, + {name: "tools/list", method: methodListTools, params: `{}`}, + {name: "logging/setLevel", method: methodSetLevel, params: `{"level":"info"}`}, + {name: "resources/subscribe", method: methodSubscribe, params: fmt.Sprintf(`{"uri":%q}`, uri)}, + {name: "resources/unsubscribe", method: methodUnsubscribe, params: fmt.Sprintf(`{"uri":%q}`, uri)}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var handlerRan bool + server := NewServer(testImpl, &ServerOptions{ + SubscribeHandler: func(context.Context, *SubscribeRequest) error { + handlerRan = true + return nil + }, + UnsubscribeHandler: func(context.Context, *UnsubscribeRequest) error { + handlerRan = true + return nil + }, + }) + _, st := NewInMemoryTransports() + ss, err := server.Connect(ctx, st, nil) + if err != nil { + t.Fatalf("server.Connect: %v", err) + } + defer ss.Close() + + res, err := ss.handle(ctx, &jsonrpc.Request{ + ID: jsonrpc2.Int64ID(1), + Method: tc.method, + Params: json.RawMessage(tc.params), + }) + if tc.served { + if err != nil { + t.Fatalf("handle(%q) before initialize returned %v, want it served", tc.method, err) + } + if res == nil { + t.Fatalf("handle(%q) before initialize returned no result", tc.method) + } + if tc.method == methodInitialize && ss.InitializeParams() == nil { + t.Errorf("initialize was answered without recording InitializeParams") + } + return + } + if err == nil { + t.Fatalf("handle(%q) served the call before initialize, want a refusal", tc.method) + } + if want := fmt.Sprintf("method %q is invalid during session initialization", tc.method); err.Error() != want { + t.Errorf("handle(%q) error = %q, want %q", tc.method, err.Error(), want) + } + if handlerRan { + t.Errorf("handle(%q) ran the server's handler for a call it refused", tc.method) + } + server.mu.Lock() + _, subscribed := server.resourceSubscriptions[uri][ss] + server.mu.Unlock() + if subscribed { + t.Errorf("handle(%q) left the session subscribed to %q", tc.method, uri) + } + }) + } +} + +// TestServerHandle_NewProtocolCallWithoutInitialize asserts that the gate +// stays off the SEP-2575 path. A fresh session that never sends 'initialize' +// is served a new-protocol 'tools/list', and the methods the gate now covers +// are answered, when a new-protocol request names them, with the error the +// new protocol already gave them (the method does not exist there), and not +// with the initialization refusal. +func TestServerHandle_NewProtocolCallWithoutInitialize(t *testing.T) { + ctx := context.Background() + const uri = "test://resource" + + tests := []struct { + name string + method string + params map[string]any + served bool + }{ + {name: "tools/list", method: methodListTools, params: newProtocolParams(nil), served: true}, + {name: "logging/setLevel", method: methodSetLevel, params: newProtocolParams(map[string]any{"level": "info"})}, + {name: "resources/subscribe", method: methodSubscribe, params: newProtocolParams(map[string]any{"uri": uri})}, + {name: "resources/unsubscribe", method: methodUnsubscribe, params: newProtocolParams(map[string]any{"uri": uri})}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + server := NewServer(testImpl, &ServerOptions{ + SubscribeHandler: func(context.Context, *SubscribeRequest) error { return nil }, + UnsubscribeHandler: func(context.Context, *UnsubscribeRequest) error { return nil }, + }) + _, st := NewInMemoryTransports() + ss, err := server.Connect(ctx, st, nil) + if err != nil { + t.Fatalf("server.Connect: %v", err) + } + defer ss.Close() + + params, err := json.Marshal(tc.params) + if err != nil { + t.Fatalf("marshalling params: %v", err) + } + res, err := ss.handle(ctx, &jsonrpc.Request{ + ID: jsonrpc2.Int64ID(1), + Method: tc.method, + Params: params, + }) + if tc.served { + if err != nil { + t.Fatalf("handle(%q) on a new-protocol session returned %v, want it served", tc.method, err) + } + if res == nil { + t.Fatalf("handle(%q) on a new-protocol session returned no result", tc.method) + } + return + } + var jerr *jsonrpc.Error + if !errors.As(err, &jerr) { + t.Fatalf("handle(%q) returned %v, want a *jsonrpc.Error", tc.method, err) + } + if jerr.Code != jsonrpc.CodeMethodNotFound { + t.Errorf("handle(%q) error code = %d, want %d (removed in the new protocol)", tc.method, jerr.Code, jsonrpc.CodeMethodNotFound) + } + if strings.Contains(err.Error(), "invalid during session initialization") { + t.Errorf("handle(%q) applied the initialization gate to a new-protocol request: %v", tc.method, err) + } + }) + } +} + // TestServerUnknownProtocolVersion_NewProtocol verifies that a request whose // `_meta.protocolVersion` names a version the SDK does not know is rejected // with [CodeUnsupportedProtocolVersion], and not served as a legacy handshake