From e0a971f404fa190d12ee9367b3bf38d1de158636 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Sun, 13 Sep 2026 11:07:51 +0200 Subject: [PATCH 1/4] mcp: choose the interaction pattern from the negotiated protocol version initialize is deprecated in 2026-07-28, so negotiatedVersion caps that handshake below it: a client that asks for 2026-07-28 there is answered 2025-11-25. InitializeParams.ProtocolVersion keeps whatever the client asked for, and two capability checks read that value instead of the negotiated one. clientSupportsMultiRoundTrip therefore served such a session an input_required result carrying an inputRequests map, which the version the session actually negotiated does not define, while assertServerInitiatedRequestAllowed refused the elicitation, sampling and roots requests that are the mechanism the session does have. The session was left with neither half of the interaction. The two checks are one decision and have to agree. Fixing only the first turns the silently ignored result into a hard error, because the server-side shim then calls ServerSession.Elicit and the second check refuses it. Both now read ServerSession.protocolVersion, which answers with the negotiated version and falls back to the declared one for a session that ran no initialize: a SEP-2575 session records its version in InitializeParams alone, as does the state synthesized for a stateless request, and for those the declared version is the version the session speaks. --- mcp/mrtr.go | 4 +- mcp/mrtr_test.go | 167 +++++++++++++++++++++++++++++++++++++++++++++++ mcp/server.go | 30 ++++++++- 3 files changed, 196 insertions(+), 5 deletions(-) diff --git a/mcp/mrtr.go b/mcp/mrtr.go index fdf98ece..818cd86f 100644 --- a/mcp/mrtr.go +++ b/mcp/mrtr.go @@ -55,8 +55,8 @@ func validateMultiRoundTripResult(logger *slog.Logger, res multiRoundTripRespons func clientSupportsMultiRoundTrip(ss *ServerSession) bool { protocolVersion := latestProtocolVersion - if iparams := ss.InitializeParams(); iparams != nil { - protocolVersion = iparams.ProtocolVersion + if version := ss.protocolVersion(); version != "" { + protocolVersion = version } return protocolVersion >= protocolVersion20260728 } diff --git a/mcp/mrtr_test.go b/mcp/mrtr_test.go index d9662198..128ddce7 100644 --- a/mcp/mrtr_test.go +++ b/mcp/mrtr_test.go @@ -8,6 +8,7 @@ package mcp import ( "context" + "encoding/json" "fmt" "slices" "strings" @@ -16,6 +17,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2" + "github.com/modelcontextprotocol/go-sdk/jsonrpc" ) func TestMultiRoundTrip_ManualRetry(t *testing.T) { @@ -863,6 +866,170 @@ func TestSetMultiRoundTripRetryParams(t *testing.T) { }) } +func TestClientSupportsMultiRoundTrip(t *testing.T) { + tests := []struct { + name string + state ServerSessionState + want bool + }{ + { + // A session that ran no handshake speaks the new protocol: every + // request carries its own version in _meta (SEP-2575). + name: "no handshake", + want: true, + }, + { + name: "discover, new protocol", + state: ServerSessionState{ + InitializeParams: &InitializeParams{ProtocolVersion: protocolVersion20260728}, + }, + want: true, + }, + { + name: "initialize, legacy version", + state: ServerSessionState{ + InitializeParams: &InitializeParams{ProtocolVersion: protocolVersion20251125}, + NegotiatedProtocolVersion: protocolVersion20251125, + }, + want: false, + }, + { + // initialize is deprecated in protocolVersion20260728, so a client + // asking for it there is negotiated down and must be served the + // legacy interaction whatever it declared. + name: "initialize, negotiated down", + state: ServerSessionState{ + InitializeParams: &InitializeParams{ProtocolVersion: protocolVersion20260728}, + NegotiatedProtocolVersion: protocolVersion20251125, + }, + want: false, + }, + { + name: "stateless request, legacy version", + state: ServerSessionState{ + InitializeParams: &InitializeParams{ProtocolVersion: protocolVersion20250618}, + }, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ss := &ServerSession{state: tt.state} + if got := clientSupportsMultiRoundTrip(ss); got != tt.want { + t.Errorf("clientSupportsMultiRoundTrip() = %t, want %t", got, tt.want) + } + }) + } +} + +// TestMultiRoundTrip_NegotiatedDownFromNewProtocol drives a client that asks +// for protocolVersion20260728 in the deprecated initialize handshake and is +// answered with protocolVersion20251125. The session speaks the negotiated +// version, so the server must fulfill the handler's input request itself with +// elicitation/create, rather than returning an input-required result that the +// negotiated version does not define. +func TestMultiRoundTrip_NegotiatedDownFromNewProtocol(t *testing.T) { + ctx := context.Background() + + srv := NewServer(testImpl, nil) + srv.AddTool( + &Tool{Name: "act", InputSchema: &jsonschema.Schema{Type: "object"}}, + func(ctx context.Context, req *CallToolRequest) (*CallToolResult, error) { + if len(req.Params.InputResponses) == 0 { + return &CallToolResult{ + InputRequests: InputRequestMap{"confirm": &ElicitParams{Message: "OK?"}}, + RequestState: "state-1", + }, nil + } + return &CallToolResult{Content: []Content{&TextContent{Text: "confirmed"}}}, nil + }, + ) + + ct, st := NewInMemoryTransports() + ss, err := srv.Connect(ctx, st, nil) + if err != nil { + t.Fatalf("server.Connect() error = %v", err) + } + defer ss.Close() + + conn, err := ct.Connect(ctx) + if err != nil { + t.Fatalf("transport.Connect() error = %v", err) + } + defer conn.Close() + + write := func(msg jsonrpc.Message, err error) { + t.Helper() + if err != nil { + t.Fatalf("building message: %v", err) + } + if err := conn.Write(ctx, msg); err != nil { + t.Fatalf("conn.Write() error = %v", err) + } + } + read := func() jsonrpc.Message { + t.Helper() + msg, err := conn.Read(ctx) + if err != nil { + t.Fatalf("conn.Read() error = %v", err) + } + return msg + } + + write(jsonrpc2.NewCall(jsonrpc2.Int64ID(1), methodInitialize, &InitializeParams{ + ProtocolVersion: protocolVersion20260728, + ClientInfo: testImpl, + Capabilities: &ClientCapabilities{Elicitation: &ElicitationCapabilities{}}, + })) + initResp, ok := read().(*jsonrpc2.Response) + if !ok { + t.Fatalf("initialize: got %T, want *jsonrpc2.Response", initResp) + } + if initResp.Error != nil { + t.Fatalf("initialize failed: %v", initResp.Error) + } + var initRes InitializeResult + if err := json.Unmarshal(initResp.Result, &initRes); err != nil { + t.Fatalf("unmarshalling initialize result: %v", err) + } + if initRes.ProtocolVersion != protocolVersion20251125 { + t.Fatalf("negotiated protocol version = %q, want %q", initRes.ProtocolVersion, protocolVersion20251125) + } + write(jsonrpc2.NewNotification(notificationInitialized, &InitializedParams{})) + + write(jsonrpc2.NewCall(jsonrpc2.Int64ID(2), methodCallTool, &CallToolParams{Name: "act"})) + msg := read() + elicitReq, ok := msg.(*jsonrpc2.Request) + if !ok { + resp := msg.(*jsonrpc2.Response) + t.Fatalf("tools/call was answered without an %q request: result = %s, error = %v", + methodElicit, resp.Result, resp.Error) + } + if elicitReq.Method != methodElicit { + t.Fatalf("server request method = %q, want %q", elicitReq.Method, methodElicit) + } + write(jsonrpc2.NewResponse(elicitReq.ID, &ElicitResult{Action: "accept"}, nil)) + + callResp, ok := read().(*jsonrpc2.Response) + if !ok { + t.Fatalf("tools/call: got %T, want *jsonrpc2.Response", callResp) + } + if callResp.Error != nil { + t.Fatalf("tools/call failed: %v", callResp.Error) + } + var callRes CallToolResult + if err := json.Unmarshal(callResp.Result, &callRes); err != nil { + t.Fatalf("unmarshalling tools/call result: %v", err) + } + if len(callRes.Content) != 1 { + t.Fatalf("len(result.Content) = %d, want 1", len(callRes.Content)) + } + if got := callRes.Content[0].(*TextContent).Text; got != "confirmed" { + t.Errorf("result text = %q, want %q", got, "confirmed") + } +} + func mustConnect(t *testing.T, s *Server, clientOpts *ClientOptions) *ClientSession { t.Helper() diff --git a/mcp/server.go b/mcp/server.go index e7d7558a..fd2e8077 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -1645,12 +1645,11 @@ func (ss *ServerSession) ID() string { // in an [InputRequiredResult] returned from a handler for one of the multi // round-trip methods (`tools/call`, `prompts/get`, `resources/read`). func (ss *ServerSession) assertServerInitiatedRequestAllowed(method string) error { - if iparams := ss.InitializeParams(); iparams != nil && - iparams.ProtocolVersion >= protocolVersion20260728 { + if version := ss.protocolVersion(); version >= protocolVersion20260728 { return fmt.Errorf( "%q cannot be sent while serving a request on protocol version %s: "+ "return an InputRequests map instead (multi round-trip requests, SEP-2322)", - method, iparams.ProtocolVersion) + method, version) } return nil } @@ -2118,6 +2117,31 @@ func (ss *ServerSession) InitializeParams() *InitializeParams { return ss.state.InitializeParams } +// protocolVersion returns the protocol version the session speaks: the version +// negotiated by 'initialize', or the version the client declared when the +// session never ran that handshake (SEP-2575 sessions, and the synthesized +// state of a stateless request). +// +// The two differ for a client that asks for protocolVersion20260728 in +// 'initialize': that method is deprecated in protocolVersion20260728, so +// [negotiatedVersion] answers with an older version while +// [InitializeParams.ProtocolVersion] keeps what the client asked for. A +// capability decision must use the negotiated version, which is the one both +// sides agreed to speak. +// +// It returns "" when the session has recorded neither version. +func (ss *ServerSession) protocolVersion() string { + ss.mu.Lock() + defer ss.mu.Unlock() + if v := ss.state.NegotiatedProtocolVersion; v != "" { + return v + } + if ss.state.InitializeParams != nil { + return ss.state.InitializeParams.ProtocolVersion + } + return "" +} + func (ss *ServerSession) initialize(ctx context.Context, params *InitializeParams) (*InitializeResult, error) { if params == nil { return nil, fmt.Errorf("%w: \"params\" must be be provided", jsonrpc2.ErrInvalidParams) From 6759c1685673521b0c287735506c02ce40804af2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Mon, 14 Sep 2026 20:15:40 +0200 Subject: [PATCH 2/4] mcp: classify notification delivery by the negotiated protocol version notifySessions and ResourceUpdated read InitializeParams.ProtocolVersion to decide which delivery mechanism a session gets, so a client negotiated down from 2026-07-28 by the deprecated initialize handshake was counted a new-protocol session. It was not notified on the shared session channel, and it could not have opened the subscriptions/listen stream the other branch delivers on, because that method does not exist in the version it negotiated. ResourceUpdated went further and stamped the per-session subscription id into the notification's _meta, which that version does not define. Both now go through ServerSession.speaksLegacyProtocol, which reads the version the session speaks and reports legacy only for a version older than 2026-07-28. clientSupportsMultiRoundTrip is its negation, so the three places that choose an interaction pattern make one decision. A session that has recorded no version at all changes group: it was legacy through isNil() and is now new-protocol, which is what SEP-2575 says a session without an initialize handshake is, and what clientSupportsMultiRoundTrip already assumed. Server.handle records the declared version on the first call a new-protocol client makes, so this leaves only a session that has issued no call yet. --- mcp/mrtr.go | 6 +- mcp/server.go | 21 ++++- mcp/server_test.go | 203 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 223 insertions(+), 7 deletions(-) diff --git a/mcp/mrtr.go b/mcp/mrtr.go index 818cd86f..32711f58 100644 --- a/mcp/mrtr.go +++ b/mcp/mrtr.go @@ -54,11 +54,7 @@ func validateMultiRoundTripResult(logger *slog.Logger, res multiRoundTripRespons } func clientSupportsMultiRoundTrip(ss *ServerSession) bool { - protocolVersion := latestProtocolVersion - if version := ss.protocolVersion(); version != "" { - protocolVersion = version - } - return protocolVersion >= protocolVersion20260728 + return !ss.speaksLegacyProtocol() } func clientMultiRoundTripMiddleware() Middleware { diff --git a/mcp/server.go b/mcp/server.go index fd2e8077..c864a718 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -785,7 +785,7 @@ func (s *Server) notifySessions(n string) { // shared session channel without opt-in; collect them while we hold the lock. var legacySessions []*ServerSession for _, sess := range s.sessions { - if sess.InitializeParams().isNil() || sess.InitializeParams().ProtocolVersion < protocolVersion20260728 { + if sess.speaksLegacyProtocol() { legacySessions = append(legacySessions, sess) } } @@ -1206,7 +1206,7 @@ func (s *Server) ResourceUpdated(ctx context.Context, params *ResourceUpdatedNot var legacySessions []*ServerSession newSessions := make(map[*ServerSession]jsonrpc.ID) for sess, reqID := range subscribedSessions { - if sess.InitializeParams().isNil() || sess.InitializeParams().ProtocolVersion < protocolVersion20260728 { + if sess.speaksLegacyProtocol() { legacySessions = append(legacySessions, sess) } else { newSessions[sess] = reqID @@ -2142,6 +2142,23 @@ func (ss *ServerSession) protocolVersion() string { return "" } +// speaksLegacyProtocol reports whether the session speaks a protocol version +// older than protocolVersion20260728, and so is served the interaction +// patterns that version defines: server-initiated requests while a request is +// being served, and list-changed and resource-updated notifications on the +// shared session channel rather than through subscriptions/listen. +// +// A session that has recorded no version at all is not legacy. SEP-2575 is +// where a session without an 'initialize' handshake comes from, so the absence +// of a version is read as the current protocol rather than as the oldest one; +// [Server.handle] records the declared version on the first call a +// new-protocol client makes, so this only covers a session that has issued no +// call yet. +func (ss *ServerSession) speaksLegacyProtocol() bool { + version := ss.protocolVersion() + return version != "" && version < protocolVersion20260728 +} + func (ss *ServerSession) initialize(ctx context.Context, params *InitializeParams) (*InitializeResult, error) { if params == nil { return nil, fmt.Errorf("%w: \"params\" must be be provided", jsonrpc2.ErrInvalidParams) diff --git a/mcp/server_test.go b/mcp/server_test.go index 64e7de0d..f5b3e6ca 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -2692,3 +2692,206 @@ func TestServerUnknownProtocolVersion_NewProtocol(t *testing.T) { }) } } + +// rawSession is a JSON-RPC connection to a server, driven message by message +// so a test can run a handshake the SDK's own client does not offer. +type rawSession struct { + write func(jsonrpc.Message, error) + read func() jsonrpc.Message +} + +// connectNegotiatedDown runs the deprecated initialize handshake declaring +// protocolVersion20260728 and asserts the server answers protocolVersion20251125. +// +// The session it leaves behind is the one the notification paths used to +// misclassify: InitializeParams carries the version the client asked for, +// which is the new protocol, while the session speaks the older version the +// handshake settled on. +func connectNegotiatedDown(t *testing.T, ctx context.Context, srv *Server) *rawSession { + t.Helper() + + ct, st := NewInMemoryTransports() + ss, err := srv.Connect(ctx, st, nil) + if err != nil { + t.Fatalf("server.Connect() error = %v", err) + } + t.Cleanup(func() { ss.Close() }) + + conn, err := ct.Connect(ctx) + if err != nil { + t.Fatalf("transport.Connect() error = %v", err) + } + t.Cleanup(func() { conn.Close() }) + + sess := &rawSession{ + write: func(msg jsonrpc.Message, err error) { + t.Helper() + if err != nil { + t.Fatalf("building message: %v", err) + } + if err := conn.Write(ctx, msg); err != nil { + t.Fatalf("conn.Write() error = %v", err) + } + }, + read: func() jsonrpc.Message { + t.Helper() + msg, err := conn.Read(ctx) + if err != nil { + t.Fatalf("conn.Read() error = %v", err) + } + return msg + }, + } + + sess.write(jsonrpc2.NewCall(jsonrpc2.Int64ID(1), methodInitialize, &InitializeParams{ + ProtocolVersion: protocolVersion20260728, + ClientInfo: testImpl, + Capabilities: &ClientCapabilities{}, + })) + initResp, ok := sess.read().(*jsonrpc2.Response) + if !ok { + t.Fatalf("initialize was not answered with a response") + } + if initResp.Error != nil { + t.Fatalf("initialize failed: %v", initResp.Error) + } + var initRes InitializeResult + if err := json.Unmarshal(initResp.Result, &initRes); err != nil { + t.Fatalf("unmarshalling initialize result: %v", err) + } + if initRes.ProtocolVersion != protocolVersion20251125 { + t.Fatalf("negotiated protocol version = %q, want %q", initRes.ProtocolVersion, protocolVersion20251125) + } + sess.write(jsonrpc2.NewNotification(notificationInitialized, &InitializedParams{})) + return sess +} + +// TestNotifySessions_NegotiatedDownFromNewProtocol asserts that a session +// negotiated down from protocolVersion20260728 is counted a legacy subscriber +// and receives a list-changed notification on the shared session channel. +// +// Reading the declared version instead left it in neither group: it was not +// notified on the session channel, and it could not have opened the +// subscriptions/listen stream the other branch delivers on, because that +// method does not exist in the version it negotiated. +func TestNotifySessions_NegotiatedDownFromNewProtocol(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + srv := NewServer(testImpl, nil) + sess := connectNegotiatedDown(t, ctx, srv) + + srv.AddTool(&Tool{Name: "act", InputSchema: &jsonschema.Schema{Type: "object"}}, nil) + + msg := sess.read() + note, ok := msg.(*jsonrpc2.Request) + if !ok { + t.Fatalf("got %T, want the %q notification", msg, notificationToolListChanged) + } + if note.Method != notificationToolListChanged { + t.Fatalf("notification method = %q, want %q", note.Method, notificationToolListChanged) + } +} + +// TestResourceUpdated_NegotiatedDownFromNewProtocol asserts the same +// classification for resource subscriptions, where getting it wrong has a +// second consequence: a session sorted into the new-protocol group is sent the +// notification with its per-session subscription id in _meta, which the +// version it negotiated does not define. +func TestResourceUpdated_NegotiatedDownFromNewProtocol(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + const uri = "test://resource" + srv := NewServer(testImpl, &ServerOptions{ + SubscribeHandler: func(context.Context, *SubscribeRequest) error { return nil }, + UnsubscribeHandler: func(context.Context, *UnsubscribeRequest) error { return nil }, + }) + srv.AddResource(&Resource{URI: uri, Name: "res"}, + func(context.Context, *ReadResourceRequest) (*ReadResourceResult, error) { + return &ReadResourceResult{Contents: []*ResourceContents{{URI: uri, Text: "data"}}}, nil + }) + + sess := connectNegotiatedDown(t, ctx, srv) + + sess.write(jsonrpc2.NewCall(jsonrpc2.Int64ID(2), methodSubscribe, &SubscribeParams{URI: uri})) + subResp, ok := sess.read().(*jsonrpc2.Response) + if !ok { + t.Fatalf("subscribe was not answered with a response") + } + if subResp.Error != nil { + t.Fatalf("subscribe failed: %v", subResp.Error) + } + + if err := srv.ResourceUpdated(ctx, &ResourceUpdatedNotificationParams{URI: uri}); err != nil { + t.Fatalf("ResourceUpdated() error = %v", err) + } + + msg := sess.read() + note, ok := msg.(*jsonrpc2.Request) + if !ok { + t.Fatalf("got %T, want the %q notification", msg, notificationResourceUpdated) + } + if note.Method != notificationResourceUpdated { + t.Fatalf("notification method = %q, want %q", note.Method, notificationResourceUpdated) + } + var params ResourceUpdatedNotificationParams + if err := json.Unmarshal(note.Params, ¶ms); err != nil { + t.Fatalf("unmarshalling notification params: %v", err) + } + if params.URI != uri { + t.Errorf("notification URI = %q, want %q", params.URI, uri) + } + if _, ok := params.GetMeta()[MetaKeySubscriptionID]; ok { + t.Errorf("notification carries %q, which protocol version %s does not define", + MetaKeySubscriptionID, protocolVersion20251125) + } +} + +// TestSpeaksLegacyProtocol_NoHandshakeIsNotLegacy pins the one session the +// classification change moves: one that has recorded no protocol version at +// all. SEP-2575 is where a session without an initialize handshake comes from, +// so the absence of a version reads as the current protocol; Server.handle +// records the declared version on the first call such a client makes, which +// leaves only a session that has issued no call yet. +func TestSpeaksLegacyProtocol_NoHandshakeIsNotLegacy(t *testing.T) { + tests := []struct { + name string + state ServerSessionState + want bool + }{ + {name: "no handshake", want: false}, + { + name: "discover, new protocol", + state: ServerSessionState{ + InitializeParams: &InitializeParams{ProtocolVersion: protocolVersion20260728}, + }, + want: false, + }, + { + name: "initialize, legacy version", + state: ServerSessionState{ + InitializeParams: &InitializeParams{ProtocolVersion: protocolVersion20251125}, + NegotiatedProtocolVersion: protocolVersion20251125, + }, + want: true, + }, + { + name: "initialize, negotiated down", + state: ServerSessionState{ + InitializeParams: &InitializeParams{ProtocolVersion: protocolVersion20260728}, + NegotiatedProtocolVersion: protocolVersion20251125, + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ss := &ServerSession{state: tt.state} + if got := ss.speaksLegacyProtocol(); got != tt.want { + t.Errorf("speaksLegacyProtocol() = %t, want %t", got, tt.want) + } + }) + } +} From b6f3c55528524041b61b88e038821d79982a4e85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Tue, 15 Sep 2026 16:54:34 +0200 Subject: [PATCH 3/4] mcp: rename the predicate to negotiatedLegacyProtocol The decision it reports is about the version the session negotiated, not about what a client asked for, which is the whole point of the change, so the name says negotiated. --- mcp/mrtr.go | 2 +- mcp/server.go | 10 +++++----- mcp/server_test.go | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/mcp/mrtr.go b/mcp/mrtr.go index 32711f58..7d358c8c 100644 --- a/mcp/mrtr.go +++ b/mcp/mrtr.go @@ -54,7 +54,7 @@ func validateMultiRoundTripResult(logger *slog.Logger, res multiRoundTripRespons } func clientSupportsMultiRoundTrip(ss *ServerSession) bool { - return !ss.speaksLegacyProtocol() + return !ss.negotiatedLegacyProtocol() } func clientMultiRoundTripMiddleware() Middleware { diff --git a/mcp/server.go b/mcp/server.go index c864a718..0c260ccf 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -785,7 +785,7 @@ func (s *Server) notifySessions(n string) { // shared session channel without opt-in; collect them while we hold the lock. var legacySessions []*ServerSession for _, sess := range s.sessions { - if sess.speaksLegacyProtocol() { + if sess.negotiatedLegacyProtocol() { legacySessions = append(legacySessions, sess) } } @@ -1206,7 +1206,7 @@ func (s *Server) ResourceUpdated(ctx context.Context, params *ResourceUpdatedNot var legacySessions []*ServerSession newSessions := make(map[*ServerSession]jsonrpc.ID) for sess, reqID := range subscribedSessions { - if sess.speaksLegacyProtocol() { + if sess.negotiatedLegacyProtocol() { legacySessions = append(legacySessions, sess) } else { newSessions[sess] = reqID @@ -2142,8 +2142,8 @@ func (ss *ServerSession) protocolVersion() string { return "" } -// speaksLegacyProtocol reports whether the session speaks a protocol version -// older than protocolVersion20260728, and so is served the interaction +// negotiatedLegacyProtocol reports whether the version this session negotiated +// is older than protocolVersion20260728, and so is served the interaction // patterns that version defines: server-initiated requests while a request is // being served, and list-changed and resource-updated notifications on the // shared session channel rather than through subscriptions/listen. @@ -2154,7 +2154,7 @@ func (ss *ServerSession) protocolVersion() string { // [Server.handle] records the declared version on the first call a // new-protocol client makes, so this only covers a session that has issued no // call yet. -func (ss *ServerSession) speaksLegacyProtocol() bool { +func (ss *ServerSession) negotiatedLegacyProtocol() bool { version := ss.protocolVersion() return version != "" && version < protocolVersion20260728 } diff --git a/mcp/server_test.go b/mcp/server_test.go index f5b3e6ca..48505c8c 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -2889,8 +2889,8 @@ func TestSpeaksLegacyProtocol_NoHandshakeIsNotLegacy(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ss := &ServerSession{state: tt.state} - if got := ss.speaksLegacyProtocol(); got != tt.want { - t.Errorf("speaksLegacyProtocol() = %t, want %t", got, tt.want) + if got := ss.negotiatedLegacyProtocol(); got != tt.want { + t.Errorf("negotiatedLegacyProtocol() = %t, want %t", got, tt.want) } }) } From da8f87c9ceaf04d31e68f5973608580e8d72449f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Sun, 27 Sep 2026 23:55:19 +0200 Subject: [PATCH 4/4] mcp: restate protocolVersion after #1274 and shorten comments #1274 records NegotiatedProtocolVersion on server/discover, on the first SEP-2575 call and on the synthesized state of a streamable request, so the fallback to the declared version in protocolVersion now covers only a caller-supplied ServerSessionState and state saved by an older release. The doc comment said otherwise. The comments this change adds are cut to at most three lines each; the reasoning they carried is in the pull request description. --- mcp/mrtr_test.go | 9 +++------ mcp/server.go | 31 ++++++------------------------- mcp/server_test.go | 36 +++++++++++------------------------- 3 files changed, 20 insertions(+), 56 deletions(-) diff --git a/mcp/mrtr_test.go b/mcp/mrtr_test.go index 128ddce7..82b372c3 100644 --- a/mcp/mrtr_test.go +++ b/mcp/mrtr_test.go @@ -923,12 +923,9 @@ func TestClientSupportsMultiRoundTrip(t *testing.T) { } } -// TestMultiRoundTrip_NegotiatedDownFromNewProtocol drives a client that asks -// for protocolVersion20260728 in the deprecated initialize handshake and is -// answered with protocolVersion20251125. The session speaks the negotiated -// version, so the server must fulfill the handler's input request itself with -// elicitation/create, rather than returning an input-required result that the -// negotiated version does not define. +// TestMultiRoundTrip_NegotiatedDownFromNewProtocol asserts that a session +// negotiated down to protocolVersion20251125 gets elicitation/create, not an +// input-required result that its version does not define. func TestMultiRoundTrip_NegotiatedDownFromNewProtocol(t *testing.T) { ctx := context.Background() diff --git a/mcp/server.go b/mcp/server.go index 0c260ccf..c1cd27e3 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -2117,19 +2117,9 @@ func (ss *ServerSession) InitializeParams() *InitializeParams { return ss.state.InitializeParams } -// protocolVersion returns the protocol version the session speaks: the version -// negotiated by 'initialize', or the version the client declared when the -// session never ran that handshake (SEP-2575 sessions, and the synthesized -// state of a stateless request). -// -// The two differ for a client that asks for protocolVersion20260728 in -// 'initialize': that method is deprecated in protocolVersion20260728, so -// [negotiatedVersion] answers with an older version while -// [InitializeParams.ProtocolVersion] keeps what the client asked for. A -// capability decision must use the negotiated version, which is the one both -// sides agreed to speak. -// -// It returns "" when the session has recorded neither version. +// protocolVersion returns the version the session speaks: the negotiated one, +// or the declared one when the state recorded none (a caller-supplied State, or +// state saved by an older release). It returns "" when neither is known. func (ss *ServerSession) protocolVersion() string { ss.mu.Lock() defer ss.mu.Unlock() @@ -2142,18 +2132,9 @@ func (ss *ServerSession) protocolVersion() string { return "" } -// negotiatedLegacyProtocol reports whether the version this session negotiated -// is older than protocolVersion20260728, and so is served the interaction -// patterns that version defines: server-initiated requests while a request is -// being served, and list-changed and resource-updated notifications on the -// shared session channel rather than through subscriptions/listen. -// -// A session that has recorded no version at all is not legacy. SEP-2575 is -// where a session without an 'initialize' handshake comes from, so the absence -// of a version is read as the current protocol rather than as the oldest one; -// [Server.handle] records the declared version on the first call a -// new-protocol client makes, so this only covers a session that has issued no -// call yet. +// negotiatedLegacyProtocol reports whether the session speaks a version older +// than protocolVersion20260728. A session with no recorded version is not +// legacy: without an 'initialize' handshake it is a SEP-2575 session. func (ss *ServerSession) negotiatedLegacyProtocol() bool { version := ss.protocolVersion() return version != "" && version < protocolVersion20260728 diff --git a/mcp/server_test.go b/mcp/server_test.go index 48505c8c..b935fc24 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -2700,13 +2700,9 @@ type rawSession struct { read func() jsonrpc.Message } -// connectNegotiatedDown runs the deprecated initialize handshake declaring -// protocolVersion20260728 and asserts the server answers protocolVersion20251125. -// -// The session it leaves behind is the one the notification paths used to -// misclassify: InitializeParams carries the version the client asked for, -// which is the new protocol, while the session speaks the older version the -// handshake settled on. +// connectNegotiatedDown runs initialize declaring protocolVersion20260728 and +// asserts it settles on protocolVersion20251125, so InitializeParams and the +// negotiated version disagree. func connectNegotiatedDown(t *testing.T, ctx context.Context, srv *Server) *rawSession { t.Helper() @@ -2767,13 +2763,8 @@ func connectNegotiatedDown(t *testing.T, ctx context.Context, srv *Server) *rawS } // TestNotifySessions_NegotiatedDownFromNewProtocol asserts that a session -// negotiated down from protocolVersion20260728 is counted a legacy subscriber -// and receives a list-changed notification on the shared session channel. -// -// Reading the declared version instead left it in neither group: it was not -// notified on the session channel, and it could not have opened the -// subscriptions/listen stream the other branch delivers on, because that -// method does not exist in the version it negotiated. +// negotiated down from protocolVersion20260728 receives a list-changed +// notification on the shared session channel. func TestNotifySessions_NegotiatedDownFromNewProtocol(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() @@ -2793,11 +2784,9 @@ func TestNotifySessions_NegotiatedDownFromNewProtocol(t *testing.T) { } } -// TestResourceUpdated_NegotiatedDownFromNewProtocol asserts the same -// classification for resource subscriptions, where getting it wrong has a -// second consequence: a session sorted into the new-protocol group is sent the -// notification with its per-session subscription id in _meta, which the -// version it negotiated does not define. +// TestResourceUpdated_NegotiatedDownFromNewProtocol asserts the same for a +// resource update, which must also carry no subscription id in _meta, since +// the negotiated version does not define one. func TestResourceUpdated_NegotiatedDownFromNewProtocol(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() @@ -2848,12 +2837,9 @@ func TestResourceUpdated_NegotiatedDownFromNewProtocol(t *testing.T) { } } -// TestSpeaksLegacyProtocol_NoHandshakeIsNotLegacy pins the one session the -// classification change moves: one that has recorded no protocol version at -// all. SEP-2575 is where a session without an initialize handshake comes from, -// so the absence of a version reads as the current protocol; Server.handle -// records the declared version on the first call such a client makes, which -// leaves only a session that has issued no call yet. +// TestSpeaksLegacyProtocol_NoHandshakeIsNotLegacy pins that a session with no +// recorded protocol version reads as the current protocol, since a session +// without an initialize handshake is a SEP-2575 session. func TestSpeaksLegacyProtocol_NoHandshakeIsNotLegacy(t *testing.T) { tests := []struct { name string