Skip to content

mcp: choose the interaction pattern from the negotiated protocol version #1266

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

Open
jmrplens wants to merge 6 commits into
base: main
Choose a base branch
from
+381 −10
Open

mcp: choose the interaction pattern from the negotiated protocol version #1266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e0a971f
mcp: choose the interaction pattern from the negotiated protocol version
jmrplens Sep 13, 2026
6759c16
mcp: classify notification delivery by the negotiated protocol version
jmrplens Sep 14, 2026
b6f3c55
mcp: rename the predicate to negotiatedLegacyProtocol
jmrplens Sep 15, 2026
da8f87c
mcp: restate protocolVersion after #1274 and shorten comments
jmrplens Sep 27, 2026
85f855f
Merge branch 'main' into jmrp-mrtr-uses-the-negotiated-version
guglielmo-san Sep 29, 2026
bb0e7c5
Merge branch 'main' into jmrp-mrtr-uses-the-negotiated-version
jmrplens Sep 29, 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
6 changes: 1 addition & 5 deletions mcp/mrtr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,7 @@ func validateMultiRoundTripResult(logger *slog.Logger, res multiRoundTripRespons
}

func clientSupportsMultiRoundTrip(ss *ServerSession) bool {
protocolVersion := latestProtocolVersion
if iparams := ss.InitializeParams(); iparams != nil {
protocolVersion = iparams.ProtocolVersion
}
return protocolVersion >= protocolVersion20260728
return !ss.negotiatedLegacyProtocol()
}

func clientMultiRoundTripMiddleware() Middleware {
Expand Down
164 changes: 164 additions & 0 deletions mcp/mrtr_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package mcp

import (
"context"
"encoding/json"
"fmt"
"slices"
"strings"
Expand All @@ -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) {
Expand Down Expand Up @@ -863,6 +866,167 @@ 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 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()

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()

Expand Down
32 changes: 27 additions & 5 deletions mcp/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,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.negotiatedLegacyProtocol() {
legacySessions = append(legacySessions, sess)
}
}
Expand Down Expand Up @@ -1237,7 +1237,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.negotiatedLegacyProtocol() {
legacySessions = append(legacySessions, sess)
} else {
newSessions[sess] = reqID
Expand Down Expand Up @@ -1676,12 +1676,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
}
Expand Down Expand Up @@ -2149,6 +2148,29 @@ func (ss *ServerSession) InitializeParams() *InitializeParams {
return ss.state.InitializeParams
}

// 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()
if v := ss.state.NegotiatedProtocolVersion; v != "" {
return v
Comment on lines +2156 to +2158

@guglielmo-san guglielmo-san Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what do you think about setting the ss.state.NegotiatedProtocolVersion also in case of new protocol version?

@jmrplens jmrplens Sep 15, 2026 •
edited
Loading

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.

(ignore this) Parts of this are factually wrong; see the correction below in this thread.

I think it is the right direction and that it should not be this PR, because doing it properly changes behaviour rather than tidying a field.

Four places record a version today, and only one of them negotiates:

  • initialize records both, and NegotiatedProtocolVersion is the result of negotiatedVersion(params.ProtocolVersion, ...).
  • handle records InitializeParams from validatedMeta.initializeParams on the first new-protocol call. That is the version the client declared in _meta.
  • server/discover records InitializeParams with the version it was asked about.
  • streamable.go synthesizes InitializeParams from the MCP-Protocol-Version header, for old-protocol requests with no handshake.

So writing the declared version into NegotiatedProtocolVersion at the other three would put a value in a field whose name says it was agreed by both sides, when nobody agreed anything. That is the same confusion this PR is about, one field further in.

Running it through negotiatedVersion first would make it true, and then the fallback in protocolVersion() could go. What stops me proposing it here is that negotiation has a second half: initialize tells the client what it got, in InitializeResult.ProtocolVersion. None of the other three has a handshake response to say it in. A client that declared a version we downgrade would be served the older behaviour and never be told, which is worse than the current state, where the declared version is at least recorded as declared.

What I would suggest instead, as its own change: decide what those paths do when the declared version is one the server does not support at all. initialize answers with the closest supported version. The _meta path currently accepts whatever arrives. Refusing the call with an error is the honest equivalent of a downgrade the client can see, and once that is settled, recording the negotiated version at all four places is a mechanical follow-up and the fallback disappears.

If you would rather have the field written at all four now and treat the "declared but unsupported" question later, say so and I will push it here. My preference is to leave protocolVersion() preferring the negotiated value and falling back to the declared one, which reads as what it is: one place negotiates, the others record what they were told.

@jmrplens jmrplens Sep 15, 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.

Two corrections to my answer above, both in your favour.

I wrote that "the _meta path currently accepts whatever arrives". It does not. ServerSession.handle refuses a _meta version outside ss.server.protocolVersions with CodeUnsupportedProtocolVersion before anything is recorded, and #1268 routes an unrecognised string into that same check. The streamable header path answers an unsupported legacy header with 400 as the transport spec requires, and server/discover is a new-protocol call behind the same check, whose refusal carries Supported, which is how a SEP-2575 client learns the list. So the "decide what an unsupported declared version does" step I proposed as a prerequisite is already done everywhere it applies. initialize is the one path that downgrades instead of refusing, and the lifecycle spec requires exactly that.

Which means your suggestion is smaller than I made it sound: recording the negotiated version at the other three places is available now, and I was wrong to put a prerequisite in front of it.

The second correction is to my own conclusion. I implied that once the field is set everywhere, the fallback in protocolVersion() could go. It cannot. ServerSessionOptions.State is exported, so a caller may supply InitializeParams with no negotiated version, and state persisted before #1199 has none either. The fallback is what reads those correctly and should stay.

One thing that turned up while checking, which is an argument for doing this rather than a detail of it: ioConn.sessionUpdated reads only NegotiatedProtocolVersion, so over stdio a SEP-2575 session is currently treated as 2025-03-26 and accepts JSON-RPC batches that 2025-06-18 and later forbid. Recording the version on the _meta path fixes that as a side effect; the streamable handler already refuses them from the header.

@jmrplens jmrplens Sep 15, 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.

This is implemented and open as #1274, with #1272 as the issue behind it. It is based on main rather than on this branch, so it does not wait on this one.

Since the question was raised here, the offer stands the other way round too: if you would rather have it inside this pull request, say so and I will fold it in and close #1274. It is one commit.

Either way there is a small follow-up between the two, which I would rather name than leave for a rebase to surface. The doc comment protocolVersion() carries on this branch says a SEP-2575 session and a synthesized streamable state hold their version in InitializeParams alone. That stops being true once #1274 lands, on either path. The fallback itself stays, for a caller-supplied ServerSessionOptions.State and for state persisted before #1199; only the sentence needs a touch.

@jmrplens jmrplens Sep 27, 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.

#1274 has merged, so I rebased this branch onto main and restated the doc comment of protocolVersion() in da8f87c: SEP-2575 and synthesized sessions now record NegotiatedProtocolVersion too, so the fallback to the declared version only covers a caller-supplied ServerSessionOptions.State and state persisted before #1199. I also cut every comment this PR adds to three lines, and updated the description to match.

}
if ss.state.InitializeParams != nil {
return ss.state.InitializeParams.ProtocolVersion
}
return ""
}

// 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
}

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)
Expand Down
Loading
Loading