From a5026ea7b727a59b166a06cf7ec4931a50cef427 Mon Sep 17 00:00:00 2001 From: ARPAN MONDAL Date: Sun, 6 Sep 2026 15:36:58 +0530 Subject: [PATCH 01/15] mcp: decode a Base64-wrapped Mcp-Name before comparing it to the body (#1242) The 2026-07-28 transport spec lets a client Base64-wrap a name or resource URI that is not header-safe (`=?base64?...?=`) and requires the server to decode it before comparing. `validateMcpHeaders` decoded `Mcp-Param-*` values that way but compared `Mcp-Name` raw, so a spec-legal encoded name was refused as a header mismatch. This decodes `Mcp-Name` before comparing, mirroring the existing `Mcp-Param-*` path, and rejects an invalid Base64 wrapper with a clear error. Added table cases covering a valid encoded name and an invalid one (red before, green after). Fixes #1234 Co-authored-by: Arpan Mondal Co-authored-by: Guglielmo Colombo --- mcp/streamable_headers.go | 11 ++++++++--- mcp/streamable_headers_test.go | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/mcp/streamable_headers.go b/mcp/streamable_headers.go index 7d06399..77f2415 100644 --- a/mcp/streamable_headers.go +++ b/mcp/streamable_headers.go @@ -378,13 +378,18 @@ func validateMcpHeaders(header http.Header, msg jsonrpc.Message, toolLookup func if nameInHeader == "" { return fmt.Errorf("missing required Mcp-Name header for method %q", msg.Method) } - var ok bool + // A name that is not header-safe may be Base64-wrapped by the client + // (=?base64?...?=), so decode before comparing, as with Mcp-Param-*. + decodedName, ok := decodeHeaderValue(nameInHeader) + if !ok { + return errors.New("header mismatch: Mcp-Name header contains invalid Base64 encoding") + } nameInBody, ok = extractName(msg.Method, msg.Params) if !ok { return fmt.Errorf("failed to extract name from parameters for method %q", msg.Method) } - if nameInHeader != nameInBody { - return fmt.Errorf("header mismatch: Mcp-Name header value '%s' does not match body value '%s'", nameInHeader, nameInBody) + if decodedName != nameInBody { + return fmt.Errorf("header mismatch: Mcp-Name header value '%s' does not match body value '%s'", decodedName, nameInBody) } } diff --git a/mcp/streamable_headers_test.go b/mcp/streamable_headers_test.go index def7ad1..f0862f3 100644 --- a/mcp/streamable_headers_test.go +++ b/mcp/streamable_headers_test.go @@ -6,6 +6,7 @@ package mcp import ( "context" + "encoding/base64" "encoding/json" "fmt" "net/http" @@ -335,6 +336,23 @@ func TestValidateMcpHeaders(t *testing.T) { msg: &jsonrpc.Request{Method: "prompts/get", Params: mustMarshal(&GetPromptParams{Name: "code_review"})}, wantErr: false, }, + { + name: "Base64-encoded Mcp-Name is decoded before comparison", + version: minVersionForStandardHeaders, + methodHeader: "tools/call", + nameHeader: base64Prefix + base64.StdEncoding.EncodeToString([]byte("café-tool")) + base64Suffix, + msg: &jsonrpc.Request{Method: "tools/call", Params: mustMarshal(&CallToolParams{Name: "café-tool"})}, + wantErr: false, + }, + { + name: "Mcp-Name with invalid Base64 encoding", + version: minVersionForStandardHeaders, + methodHeader: "tools/call", + nameHeader: base64Prefix + "not valid base64!" + base64Suffix, + msg: &jsonrpc.Request{Method: "tools/call", Params: mustMarshal(&CallToolParams{Name: "my-tool"})}, + wantErr: true, + wantErrContain: "invalid Base64 encoding", + }, { name: "valid initialize (no name needed)", version: minVersionForStandardHeaders, From f99cdb649b0c61b07341bc31ecbf94b8f960bbef Mon Sep 17 00:00:00 2001 From: akashjainn <80935005+akashjainn@users.noreply.github.com> Date: Mon, 7 Sep 2026 04:16:16 -0400 Subject: [PATCH 02/15] mcp: base64-encode a non-header-safe Mcp-Name (#1246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/modelcontextprotocol/go-sdk/issues/1243 The 2026-07-28 transport spec only SHOULD-constrains names to header-safe characters, and lets a client Base64-wrap one that is not, using the `=?base64?...?=` sentinel. `setStandardHeaders` set `Mcp-Name` raw, while the sibling `Mcp-Param-*` path already ran its values through `encodeHeaderValue`, so a name outside the safe set went out unencoded. This routes `Mcp-Name` through `encodeHeaderValue` as well. #1242 landed the other direction — the server decoding the sentinel before comparing — so the two together close the round trip. Worth noting they interact: after #1242, a name that is literally shaped like the sentinel (`=?base64?abc?=`) is now rejected as invalid Base64, because the server tries to decode a value the client never encoded. The new test covers that case, and it fails on `main` today. `extractName` also covers `resources/read`, so this applies to resource URIs, which is where non-ASCII is most likely to show up in practice. ## Testing `TestMcpNameHeaderEncodingRoundTrip` drives `setStandardHeaders` and then feeds its output to `validateMcpHeaders`, so it pins the client and server halves against each other rather than asserting the encoding in isolation. Cases: an ASCII-safe name (must stay unencoded, no churn for the common path), non-ASCII, a leading space, a control character, and the sentinel-shaped literal above. Verified failing before the change and passing after. `go test ./...` passes. A note on process: [@remote-controlled-man](https://github.com/remote-controlled-man) asked about this issue earlier today and proposed much the same scope. I had the change written before I saw that comment and thought the #1242 interaction was worth surfacing either way, but I'm happy to close this if they'd rather carry it — please just say so. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_011Wf643Lx36V5wdwuuf2EdT --- mcp/streamable_headers.go | 7 ++++- mcp/streamable_headers_test.go | 54 ++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/mcp/streamable_headers.go b/mcp/streamable_headers.go index 77f2415..c2b8dfd 100644 --- a/mcp/streamable_headers.go +++ b/mcp/streamable_headers.go @@ -207,7 +207,12 @@ func setStandardHeaders(ctx context.Context, header http.Header, msg jsonrpc.Mes case *jsonrpc.Request: header.Set(methodHeader, msg.Method) if name, ok := extractName(msg.Method, msg.Params); ok { - header.Set(nameHeader, name) + // Names are only SHOULD-constrained to header-safe characters, so a + // name outside the safe set is Base64-wrapped with the =?base64?...?= + // sentinel, the same rule applied to Mcp-Param-* values. + if encoded, ok := encodeHeaderValue(name); ok { + header.Set(nameHeader, encoded) + } } if msg.Method == "tools/call" { if tool, ok := ctx.Value(toolContextKey).(*Tool); ok && tool != nil { diff --git a/mcp/streamable_headers_test.go b/mcp/streamable_headers_test.go index f0862f3..97af67c 100644 --- a/mcp/streamable_headers_test.go +++ b/mcp/streamable_headers_test.go @@ -1593,3 +1593,57 @@ func TestValidateParamHeaders_NestedArguments(t *testing.T) { t.Error("validateParamHeaders() = nil, want error for mismatched nested value") } } + +// TestMcpNameHeaderEncodingRoundTrip verifies that a name which is not +// header-safe is Base64-wrapped by the client per the 2026-07-28 transport +// spec, and that the server's validation accepts what the client produced. +func TestMcpNameHeaderEncodingRoundTrip(t *testing.T) { + tests := []struct { + name string + msg *jsonrpc.Request + wantHeader string + }{ + { + name: "ascii-safe tool name is sent as-is", + msg: &jsonrpc.Request{Method: "tools/call", Params: mustMarshal(&CallToolParams{Name: "my-tool"})}, + wantHeader: "my-tool", + }, + { + name: "non-ascii tool name is base64 encoded", + msg: &jsonrpc.Request{Method: "tools/call", Params: mustMarshal(&CallToolParams{Name: "café-tool"})}, + wantHeader: encodeBase64("café-tool"), + }, + { + name: "prompt name with a leading space is base64 encoded", + msg: &jsonrpc.Request{Method: "prompts/get", Params: mustMarshal(&GetPromptParams{Name: " leading-space"})}, + wantHeader: encodeBase64(" leading-space"), + }, + { + name: "resource uri with a control character is base64 encoded", + msg: &jsonrpc.Request{Method: "resources/read", Params: mustMarshal(&ReadResourceParams{URI: "file:///a\tb.txt"})}, + wantHeader: encodeBase64("file:///a\tb.txt"), + }, + { + name: "name that looks like the sentinel is base64 encoded", + msg: &jsonrpc.Request{Method: "tools/call", Params: mustMarshal(&CallToolParams{Name: "=?base64?abc?="})}, + wantHeader: encodeBase64("=?base64?abc?="), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + header := http.Header{} + header.Set(protocolVersionHeader, minVersionForStandardHeaders) + setStandardHeaders(context.Background(), header, tt.msg) + + if got := header.Get(nameHeader); got != tt.wantHeader { + t.Errorf("Mcp-Name = %q, want %q", got, tt.wantHeader) + } + + // The server must accept the headers the client just generated. + if err := validateMcpHeaders(header, tt.msg, nil); err != nil { + t.Errorf("validateMcpHeaders() on client-generated headers = %v, want nil", err) + } + }) + } +} From 5bc078a7959a745d1311fefa280b36c5cde19929 Mon Sep 17 00:00:00 2001 From: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> Date: Mon, 7 Sep 2026 05:31:45 -0400 Subject: [PATCH 03/15] auth: strip a terminating slash from the issuer before building metadata URLs (#1245) An issuer identifier written with a terminating slash, like `https://auth.example.com/`, makes authorization server metadata discovery miss every endpoint. `GetAuthServerMetadata` probes three URLs that no conformant server serves, finds nothing, and the caller falls back to the guessed 2025-03-26 endpoints, which are themselves malformed because they are built by appending to the same slash-terminated issuer. `authorizationServerMetadataURLs` in `auth/shared.go` decides whether the issuer has a path component with `baseURL.Path == ""` (line 47), and a trailing-slash issuer has `Path == "/"`, so it takes the path-insertion branch. That branch inserts `strings.TrimLeft(originalPath, "/")` (lines 59 and 62), which strips the leading slash but keeps the trailing one. For `https://auth.example.com/` the probes come out as `/.well-known/oauth-authorization-server/`, `/.well-known/openid-configuration/` and `//.well-known/openid-configuration`. For `https://auth.example.com/tenant1/` the two path-insertion probes come out with a trailing slash. Only the path-appending form was already right, because it uses `strings.Trim`. RFC 8414 section 3.1 settles it: "If the issuer identifier value contains a path component, any terminating "/" MUST be removed before inserting "/.well-known/" and the well-known URI suffix between the host component and the path component." So `https://auth.example.com/` has no path component and gets the two root forms, which is the list the MCP authorization spec gives for issuers without a path component. The change trims slashes off both ends of the issuer path once, uses that to pick between the root and path-insertion branches, and reuses it for all three URLs. An issuer without a terminating slash produces exactly the same list as before. `TestAuthorizationServerMetadataURLs` pins the URL list for all four issuer shapes. `TestGetAuthServerMetadataTrailingSlashIssuer` drives the whole lookup against the existing fake authorization server, serving metadata at the root well-known path. On unmodified main the first fails on both trailing-slash cases and the second fails with `GetAuthServerMetadata() error = Get "/.well-known/openid-configuration": redirect into loopback address "127.0.0.1"`, because `//.well-known/openid-configuration` makes `net/http.ServeMux` issue a canonicalizing redirect that the discovery client's `CheckRedirect` refuses. Both pass with the change. Verification on go1.26.4: `gofmt -l .` clean, `go vet ./...` clean, `staticcheck ./...` clean at v0.6.1 (the version the Test workflow pins), `go test ./auth/...` ok, `go test ./...` ok. I did not file an issue first. The fix is self-contained, confined to one unexported helper, and every issuer shape other than the slash-terminated ones is byte-identical to before. Co-authored-by: Guglielmo Colombo --- auth/shared.go | 14 +++++---- auth/shared_test.go | 70 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/auth/shared.go b/auth/shared.go index 8c5e10c..e58d438 100644 --- a/auth/shared.go +++ b/auth/shared.go @@ -44,7 +44,12 @@ func authorizationServerMetadataURLs(issuerURL string) []string { return nil } - if baseURL.Path == "" { + // RFC 8414, section 3.1 requires any terminating "/" to be removed from the + // issuer identifier before the well-known suffix is inserted, so an issuer + // such as "https://auth.example.com/" has no path component at all. + issuerPath := strings.Trim(baseURL.Path, "/") + + if issuerPath == "" { // "OAuth 2.0 Authorization Server Metadata". baseURL.Path = "/.well-known/oauth-authorization-server" urls = append(urls, baseURL.String()) @@ -54,15 +59,14 @@ func authorizationServerMetadataURLs(issuerURL string) []string { return urls } - originalPath := baseURL.Path // "OAuth 2.0 Authorization Server Metadata with path insertion". - baseURL.Path = "/.well-known/oauth-authorization-server/" + strings.TrimLeft(originalPath, "/") + baseURL.Path = "/.well-known/oauth-authorization-server/" + issuerPath urls = append(urls, baseURL.String()) // "OpenID Connect Discovery 1.0 with path insertion". - baseURL.Path = "/.well-known/openid-configuration/" + strings.TrimLeft(originalPath, "/") + baseURL.Path = "/.well-known/openid-configuration/" + issuerPath urls = append(urls, baseURL.String()) // "OpenID Connect Discovery 1.0 with path appending". - baseURL.Path = "/" + strings.Trim(originalPath, "/") + "/.well-known/openid-configuration" + baseURL.Path = "/" + issuerPath + "/.well-known/openid-configuration" urls = append(urls, baseURL.String()) return urls diff --git a/auth/shared_test.go b/auth/shared_test.go index 16b5054..34e0a35 100644 --- a/auth/shared_test.go +++ b/auth/shared_test.go @@ -6,11 +6,81 @@ package auth import ( "net/http" + "slices" "testing" "github.com/modelcontextprotocol/go-sdk/internal/oauthtest" ) +func TestAuthorizationServerMetadataURLs(t *testing.T) { + tests := []struct { + issuerURL string + want []string + }{ + { + issuerURL: "https://auth.example.com", + want: []string{ + "https://auth.example.com/.well-known/oauth-authorization-server", + "https://auth.example.com/.well-known/openid-configuration", + }, + }, + { + issuerURL: "https://auth.example.com/", + want: []string{ + "https://auth.example.com/.well-known/oauth-authorization-server", + "https://auth.example.com/.well-known/openid-configuration", + }, + }, + { + issuerURL: "https://auth.example.com/tenant1", + want: []string{ + "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", + "https://auth.example.com/.well-known/openid-configuration/tenant1", + "https://auth.example.com/tenant1/.well-known/openid-configuration", + }, + }, + { + issuerURL: "https://auth.example.com/tenant1/", + want: []string{ + "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", + "https://auth.example.com/.well-known/openid-configuration/tenant1", + "https://auth.example.com/tenant1/.well-known/openid-configuration", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.issuerURL, func(t *testing.T) { + if got := authorizationServerMetadataURLs(tt.issuerURL); !slices.Equal(got, tt.want) { + t.Errorf("authorizationServerMetadataURLs(%q) =\n%v\nwant\n%v", tt.issuerURL, got, tt.want) + } + }) + } +} + +// TestGetAuthServerMetadataTrailingSlashIssuer checks that an issuer identifier +// written with a terminating slash still finds metadata served at the root +// well-known path. +func TestGetAuthServerMetadataTrailingSlashIssuer(t *testing.T) { + s := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{ + MetadataEndpointConfig: &oauthtest.MetadataEndpointConfig{ + ServeOAuthInsertedEndpoint: true, + }, + }) + s.Start(t) + + got, err := GetAuthServerMetadata(t.Context(), s.URL()+"/", http.DefaultClient) + if err != nil { + t.Fatalf("GetAuthServerMetadata() error = %v, want nil", err) + } + if got == nil { + t.Fatal("GetAuthServerMetadata() got nil, want metadata") + } + if got.Issuer != s.URL() { + t.Errorf("GetAuthServerMetadata() issuer = %q, want %q", got.Issuer, s.URL()) + } +} + func TestGetAuthServerMetadata(t *testing.T) { tests := []struct { name string From e40f35d137b72bf0e9fd4a186c3d58958c0e6fd7 Mon Sep 17 00:00:00 2001 From: Ronald Date: Mon, 14 Sep 2026 03:52:10 -0400 Subject: [PATCH 04/15] mcp: label resultType on results that bypass the dispatcher (#1226) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1225. See the [issue comment](https://github.com/modelcontextprotocol/go-sdk/issues/1225#issuecomment-5500818546). ## The bug `2026-07-28` requires `resultType` on every result. `CallToolResult`, `GetPromptResult`, and `ReadResourceResult` keep `resultType` in an unexported field. Each of these results can be `complete` or `input_required`, so it cannot embed `completeResultWithType` the way the seven types in #1060 do. Only `handleMultiRoundTripResult` sets that field. It runs inside `Server.callTool`, `getPrompt`, and `readResource`. Some middleware returns a result directly instead of calling `next` — for example, an auth, rate-limit, or entitlement gate. That middleware never reaches `handleMultiRoundTripResult`. The response then ships without the field, and strict clients reject it. ## The fix The choice between `complete` and `input_required` now happens inside `setMultiRoundTripResultType`. Both call sites that need this choice call that function. `setCompleteResultType` now labels results it did not label before, so it is renamed `annotateResultType`. The new function picks a branch with a type switch. Reading matters here. `setResultType` is unexported, so setting `InputRequests` is the only way for middleware to ask for input. If the code always labels results `complete`, the response ships as `{"resultType":"complete","inputRequests":{...}}`. A client that trusts the discriminator then drops the elicitation request. Recomputing the label every time is safe. A result that did reach the dispatcher already carries the same label from that test. This also covers one edge case: a handler that returns `InputRequests` again after the shim's single reinvocation. Before this fix, that case shipped with no `resultType`. Embedding `completeResultWithType` instead is not an option: it labels everything `complete` and overwrites `input_required`, breaking multi-round-trip flows. **Skip nil results.** Middleware can return a typed nil value, which satisfies the `Result` interface. `handle` now checks for that case before it calls the labeling function. Before this change, that same case also panicked, one frame later in `annotateServerInfo` and then `GetMeta`. ## Checks - `TestServerSessionHandle_SetsResultTypeWhenMiddlewareShortCircuits` fails on all three methods before the first change. Its fourth case fails if labeling overwrites a populated `InputRequests`. - `TestServerSessionHandle_ResultTypeGate` records which requests carry `resultType`, including one known gap. See "Separate from this PR" below. - `TestServerSessionHandle_NilResultFromMiddleware` panics without the nil check. - The complete-path cases added to `TestServerSessionHandle_SetsResultTypeOnNewProtocol` also pass before this change. They close a gap in #1060, which added only three cases, and only for `input_required`. - The exported API is unchanged. `go doc -all ./mcp`, with comments stripped, is identical before and after this change. - With the reproduction from the issue, over the streamable transport: the dispatched `tools/call` and the short-circuited `tools/call` both now return `resultType: "complete"`. I could not run `./scripts/conformance.sh`. CONTRIBUTING.md names that script, but the repo only has `client-conformance.sh` and `server-conformance.sh`. Both scripts need GNU `timeout`, and macOS does not include it. ## Also here Ten comments named an exported `ResultType` field and `ResultTypeComplete` / `ResultTypeInputRequired` constants. None of those exist. Two of the comments were `[CallToolResult.ResultType]` doc links, and those links cannot resolve. They now point at `[CallToolResult.NeedsInput]` instead, because an unexported field is also not a valid link target. --------- Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Guglielmo Colombo --- mcp/mrtr.go | 19 +--- mcp/protocol.go | 92 +++++++++------ mcp/server.go | 16 +-- mcp/server_test.go | 271 ++++++++++++++++++++++++++++++++++++++++----- mcp/shared.go | 7 +- 5 files changed, 322 insertions(+), 83 deletions(-) diff --git a/mcp/mrtr.go b/mcp/mrtr.go index 008c0f8..fdf98ec 100644 --- a/mcp/mrtr.go +++ b/mcp/mrtr.go @@ -36,29 +36,20 @@ type multiRoundTripResponse interface { hasContent() bool } -func handleMultiRoundTripResult(ss *ServerSession, logger *slog.Logger, res multiRoundTripResponse) error { +// validateMultiRoundTripResult rejects a result that carries both content and +// input requests. [annotateResultType] labels the result later, in +// [ServerSession.handle], so this function does not set resultType. +func validateMultiRoundTripResult(logger *slog.Logger, res multiRoundTripResponse) error { if res == nil { return nil } - hasInputRequests := res.inputRequests() != nil - - if hasInputRequests && res.hasContent() { + if res.inputRequests() != nil && res.hasContent() { logger.Warn("handler returned both content and inputRequests") return &jsonrpc.Error{ Code: jsonrpc.CodeInternalError, Message: "server bug: result has both content and inputRequests", } } - - if clientSupportsMultiRoundTrip(ss) { - // For older clients the resultType is left unset. Input requests will be handled - // by serverMultiRoundTripMiddleware client calls and handler reinvocation. - if hasInputRequests { - res.setResultType(resultTypeInputRequired) - } else { - res.setResultType(resultTypeComplete) - } - } return nil } diff --git a/mcp/protocol.go b/mcp/protocol.go index 63d7015..be37a18 100644 --- a/mcp/protocol.go +++ b/mcp/protocol.go @@ -19,7 +19,7 @@ type resultType string const ( // resultTypeComplete indicates the result is final. - // This is the default when ResultType is empty. + // This is the default when resultType is empty. resultTypeComplete resultType = "complete" // resultTypeInputRequired indicates the server needs additional client @@ -40,9 +40,20 @@ type completeResultResponse interface { isCompleteResult() } -func setCompleteResultType(res Result) { - if r, ok := res.(completeResultResponse); ok { +// annotateResultType sets the resultType that protocol 2026-07-28 requires on +// every result. +func annotateResultType(res Result) { + switch r := res.(type) { + case completeResultResponse: r.setResultType(resultTypeComplete) + case multiRoundTripResponse: + // These results are complete or input_required, so label them by + // whether the handler asked for more client input. + if r.inputRequests() != nil { + r.setResultType(resultTypeInputRequired) + } else { + r.setResultType(resultTypeComplete) + } } } @@ -226,9 +237,8 @@ type CallToolParams struct { // marshaled to JSON. Arguments any `json:"arguments,omitempty"` - // InputResponses maps input request IDs to responses, provided when - // retrying a call after receiving a result with ResultType - // ResultTypeInputRequired. + // InputResponses maps input request IDs to responses. Set it when the + // client retries a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. // The client must echo this back when retrying. @@ -249,9 +259,8 @@ type CallToolParamsRaw struct { // Arguments (see [AddTool]). Arguments json.RawMessage `json:"arguments,omitempty"` - // InputResponses maps input request IDs to responses, provided when - // retrying a call after receiving a result with ResultType - // ResultTypeInputRequired. + // InputResponses maps input request IDs to responses. Set it when the + // client retries a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. // The client must echo this back when retrying. @@ -305,9 +314,9 @@ type CallToolResult struct { IsError bool `json:"isError,omitempty"` // InputRequests is a map of server-assigned IDs to input requests. - // Populated only when ResultType is ResultTypeInputRequired. + // It is set only when the result needs more client input. // The client must fulfill these and echo the IDs back in InputResponses - // when retrying the call. + // when it retries the call. InputRequests InputRequestMap `json:"inputRequests,omitempty"` // RequestState is an opaque string the client must echo back when @@ -317,10 +326,10 @@ type CallToolResult struct { // Unauthenticated servers must encrypt, sign and verify this value. RequestState string `json:"requestState,omitempty"` - // ResultType indicates whether this result is complete or requires further - // client input. Empty or ResultTypeComplete means the call succeeded - // normally. ResultTypeInputRequired means the client should fulfill the - // InputRequests and retry the call. + // resultType records whether the call finished or needs more client input. + // Empty or resultTypeComplete means it finished. resultTypeInputRequired + // means the client must fulfill InputRequests and retry. + // See [CallToolResult.NeedsInput]. resultType resultType // The error passed to setError, if any. // It is not marshaled, and therefore it is only visible on the server. @@ -348,7 +357,8 @@ func (r *CallToolResult) GetError() error { return r.err } -func (*CallToolResult) isResult() {} +func (*CallToolResult) isResult() {} +func (x *CallToolResult) isNil() bool { return x == nil } func (r *CallToolResult) setResultType(rt resultType) { r.resultType = rt } func (r *CallToolResult) requestState() string { return r.RequestState } @@ -363,7 +373,7 @@ func (r *CallToolResult) hasContent() bool { } // NeedsInput reports whether this result requires further client input. -// This is true when the server returned ResultType "input_required". +// This is true when the server returned a resultType of "input_required". // When NeedsInput returns true, check InputRequests for the set of // requests the server needs fulfilled before retrying the call. // An empty InputRequests with NeedsInput true indicates load-shedding. @@ -655,7 +665,8 @@ type CompleteResult struct { Completion CompletionResultDetails `json:"completion"` } -func (*CompleteResult) isResult() {} +func (*CompleteResult) isResult() {} +func (x *CompleteResult) isNil() bool { return x == nil } // CreateMessageParams holds parameters for a sampling/createMessage request. // @@ -839,6 +850,7 @@ type CreateMessageResult struct { } func (*CreateMessageResult) isResult() {} +func (x *CreateMessageResult) isNil() bool { return x == nil } func (*CreateMessageResult) isInputResponse() {} func (r *CreateMessageResult) UnmarshalJSON(data []byte) error { type result CreateMessageResult // avoid recursion @@ -890,6 +902,7 @@ var createMessageWithToolsResultAllow = map[string]bool{ } func (*CreateMessageWithToolsResult) isResult() {} +func (x *CreateMessageWithToolsResult) isNil() bool { return x == nil } func (*CreateMessageWithToolsResult) isInputResponse() {} // MarshalJSON marshals the result. When Content has a single element, it is @@ -950,9 +963,8 @@ type GetPromptParams struct { // The name of the prompt or prompt template. Name string `json:"name"` - // InputResponses maps input request IDs to responses, provided when - // retrying a call after receiving a result with ResultType - // ResultTypeInputRequired. + // InputResponses maps input request IDs to responses. Set it when the + // client retries a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. RequestState string `json:"requestState,omitempty"` @@ -972,19 +984,20 @@ type GetPromptResult struct { Description string `json:"description,omitempty"` Messages []*PromptMessage `json:"messages"` - // InputRequests is populated when ResultType is ResultTypeInputRequired. + // InputRequests is set when the result needs more client input. // See [CallToolResult.InputRequests]. InputRequests InputRequestMap `json:"inputRequests,omitempty"` // RequestState is the opaque state for multi-round-trip retries. // See [CallToolResult.RequestState]. RequestState string `json:"requestState,omitempty"` - // ResultType indicates whether this result is complete or requires further - // client input. See [CallToolResult.ResultType] for details. + // resultType records whether the call finished or needs more client input. + // See [CallToolResult.NeedsInput]. resultType resultType } -func (*GetPromptResult) isResult() {} +func (*GetPromptResult) isResult() {} +func (x *GetPromptResult) isNil() bool { return x == nil } func (r *GetPromptResult) setResultType(rt resultType) { r.resultType = rt } func (r *GetPromptResult) requestState() string { return r.RequestState } @@ -1094,7 +1107,8 @@ type InitializeResult struct { ServerInfo *Implementation `json:"serverInfo"` } -func (*InitializeResult) isResult() {} +func (*InitializeResult) isResult() {} +func (x *InitializeResult) isNil() bool { return x == nil } type InitializedParams struct { // Meta is reserved by the protocol to allow clients and servers to attach @@ -1137,7 +1151,8 @@ type DiscoverResult struct { Instructions string `json:"instructions,omitempty"` } -func (*DiscoverResult) isResult() {} +func (*DiscoverResult) isResult() {} +func (x *DiscoverResult) isNil() bool { return x == nil } func (x *ListPromptsParams) isParams() {} func (x *ListPromptsParams) isNil() bool { return x == nil } @@ -1207,6 +1222,7 @@ type ListPromptsResult struct { } func (x *ListPromptsResult) isResult() {} +func (x *ListPromptsResult) isNil() bool { return x == nil } func (x *ListPromptsResult) nextCursorPtr() *string { return &x.NextCursor } type ListResourceTemplatesParams struct { @@ -1238,6 +1254,7 @@ type ListResourceTemplatesResult struct { } func (x *ListResourceTemplatesResult) isResult() {} +func (x *ListResourceTemplatesResult) isNil() bool { return x == nil } func (x *ListResourceTemplatesResult) nextCursorPtr() *string { return &x.NextCursor } type ListResourcesParams struct { @@ -1269,6 +1286,7 @@ type ListResourcesResult struct { } func (x *ListResourcesResult) isResult() {} +func (x *ListResourcesResult) isNil() bool { return x == nil } func (x *ListResourcesResult) nextCursorPtr() *string { return &x.NextCursor } // ListRootsParams holds parameters for a roots/list request. @@ -1305,6 +1323,7 @@ type ListRootsResult struct { } func (*ListRootsResult) isResult() {} +func (x *ListRootsResult) isNil() bool { return x == nil } func (*ListRootsResult) isInputResponse() {} type ListToolsParams struct { @@ -1336,6 +1355,7 @@ type ListToolsResult struct { } func (x *ListToolsResult) isResult() {} +func (x *ListToolsResult) isNil() bool { return x == nil } func (x *ListToolsResult) nextCursorPtr() *string { return &x.NextCursor } // The severity of a log message. @@ -1575,9 +1595,8 @@ type ReadResourceParams struct { // the server how to interpret it. URI string `json:"uri"` - // InputResponses maps input request IDs to responses, provided when - // retrying a call after receiving a result with ResultType - // ResultTypeInputRequired. + // InputResponses maps input request IDs to responses. Set it when the + // client retries a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. RequestState string `json:"requestState,omitempty"` @@ -1596,19 +1615,20 @@ type ReadResourceResult struct { Cacheable Contents []*ResourceContents `json:"contents"` - // InputRequests is populated when ResultType is ResultTypeInputRequired. + // InputRequests is set when the result needs more client input. // See [CallToolResult.InputRequests]. InputRequests InputRequestMap `json:"inputRequests,omitempty"` // RequestState is the opaque state for multi-round-trip retries. // See [CallToolResult.RequestState]. RequestState string `json:"requestState,omitempty"` - // ResultType indicates whether this result is complete or requires further - // client input. See [CallToolResult.ResultType] for details. + // resultType records whether the call finished or needs more client input. + // See [CallToolResult.NeedsInput]. resultType resultType } -func (*ReadResourceResult) isResult() {} +func (*ReadResourceResult) isResult() {} +func (x *ReadResourceResult) isNil() bool { return x == nil } func (r *ReadResourceResult) setResultType(rt resultType) { r.resultType = rt } func (r *ReadResourceResult) requestState() string { return r.RequestState } @@ -2114,7 +2134,8 @@ type SubscriptionsListenResult struct { Meta `json:"_meta"` } -func (*SubscriptionsListenResult) isResult() {} +func (*SubscriptionsListenResult) isResult() {} +func (x *SubscriptionsListenResult) isNil() bool { return x == nil } // TODO(jba): add CompleteRequest and related types. @@ -2191,6 +2212,7 @@ type ElicitResult struct { } func (*ElicitResult) isResult() {} +func (x *ElicitResult) isNil() bool { return x == nil } func (*ElicitResult) isInputResponse() {} // ElicitationCompleteParams is sent from the server to the client, informing it that an out-of-band elicitation interaction has completed. diff --git a/mcp/server.go b/mcp/server.go index 5014a4f..183a519 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -910,7 +910,7 @@ func (s *Server) getPrompt(ctx context.Context, req *GetPromptRequest) (*GetProm } res, err := prompt.handler(ctx, req) if err == nil && res != nil { - if err := handleMultiRoundTripResult(req.Session, s.opts.Logger, res); err != nil { + if err := validateMultiRoundTripResult(s.opts.Logger, res); err != nil { return nil, err } } @@ -1012,10 +1012,10 @@ func (s *Server) callTool(ctx context.Context, req *CallToolRequest) (*CallToolR } res, err := st.handler(ctx, req) if err == nil && res != nil { - if err := handleMultiRoundTripResult(req.Session, s.opts.Logger, res); err != nil { + if err := validateMultiRoundTripResult(s.opts.Logger, res); err != nil { return nil, err } - if res.Content == nil && res.resultType != resultTypeInputRequired { + if res.Content == nil && res.InputRequests == nil { res2 := *res res2.Content = []Content{} // avoid "null" res = &res2 @@ -1080,11 +1080,11 @@ func (s *Server) readResource(ctx context.Context, req *ReadResourceRequest) (*R if res == nil { return nil, fmt.Errorf("reading resource %s: read handler returned nil information", uri) } - if err := handleMultiRoundTripResult(req.Session, s.opts.Logger, res); err != nil { + if err := validateMultiRoundTripResult(s.opts.Logger, res); err != nil { return nil, err } s.resolveCacheable(ctx, req, &res.Cacheable) - if res.resultType == resultTypeInputRequired { + if res.InputRequests != nil { return res, nil } if res.Contents == nil { @@ -2039,8 +2039,10 @@ func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any, if err != nil { return nil, err } - if validatedMeta.usesNewProtocol { - setCompleteResultType(res) + // A middleware can return a typed nil value that satisfies the Result + // interface. Annotating that value panics. + if validatedMeta.usesNewProtocol && res != nil && !res.isNil() { + annotateResultType(res) annotateServerInfo(res, ss.server.impl) } return res, nil diff --git a/mcp/server_test.go b/mcp/server_test.go index 11e75e2..77dcb10 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -1399,6 +1399,22 @@ func TestServerSessionHandle_RejectsInitializeOnNewProtocol(t *testing.T) { }) } +// newProtocolParams merges fields into the _meta that opts a request into the +// sessionless protocol (SEP-2575). +func newProtocolParams(fields map[string]any) map[string]any { + params := map[string]any{ + "_meta": map[string]any{ + MetaKeyProtocolVersion: protocolVersion20260728, + MetaKeyClientInfo: map[string]any{"name": "c", "version": "1"}, + MetaKeyClientCapabilities: map[string]any{}, + }, + } + for k, v := range fields { + params[k] = v + } + return params +} + func TestServerSessionHandle_SetsResultTypeOnNewProtocol(t *testing.T) { server := NewServer(testImpl, &ServerOptions{ CompletionHandler: func(context.Context, *CompleteRequest) (*CompleteResult, error) { @@ -1424,19 +1440,19 @@ func TestServerSessionHandle_SetsResultTypeOnNewProtocol(t *testing.T) { InputRequests: InputRequestMap{"confirm": &ElicitParams{Message: "Continue?"}}, }, nil }) - newProtocolParams := func(fields map[string]any) map[string]any { - params := map[string]any{ - "_meta": map[string]any{ - MetaKeyProtocolVersion: protocolVersion20260728, - MetaKeyClientInfo: map[string]any{"name": "c", "version": "1"}, - MetaKeyClientCapabilities: map[string]any{}, - }, - } - for k, v := range fields { - params[k] = v - } - return params - } + AddTool(server, &Tool{Name: "toolComplete"}, func(context.Context, *CallToolRequest, struct{}) (*CallToolResult, any, error) { + return &CallToolResult{Content: []Content{&TextContent{Text: "ok"}}}, nil, nil + }) + server.AddPrompt(&Prompt{Name: "promptComplete"}, func(context.Context, *GetPromptRequest) (*GetPromptResult, error) { + return &GetPromptResult{ + Messages: []*PromptMessage{{Role: "assistant", Content: &TextContent{Text: "ok"}}}, + }, nil + }) + server.AddResource(&Resource{URI: "test://resource-complete", Name: "resourceComplete"}, func(context.Context, *ReadResourceRequest) (*ReadResourceResult, error) { + return &ReadResourceResult{ + Contents: []*ResourceContents{{URI: "test://resource-complete", Text: "ok"}}, + }, nil + }) tests := []struct { name string @@ -1507,40 +1523,243 @@ func TestServerSessionHandle_SetsResultTypeOnNewProtocol(t *testing.T) { params: newProtocolParams(map[string]any{"uri": "test://resource"}), want: resultTypeInputRequired, }, + { + name: "tool complete", + method: methodCallTool, + params: newProtocolParams(map[string]any{"name": "toolComplete", "arguments": map[string]any{}}), + want: resultTypeComplete, + }, + { + name: "prompt complete", + method: methodGetPrompt, + params: newProtocolParams(map[string]any{"name": "promptComplete"}), + want: resultTypeComplete, + }, + { + name: "resource complete", + method: methodReadResource, + params: newProtocolParams(map[string]any{"uri": "test://resource-complete"}), + want: resultTypeComplete, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + if got := resultTypeOf(t, server, nil, tc.method, tc.params); got != string(tc.want) { + t.Errorf("resultType = %q, want %q", got, tc.want) + } + }) + } +} + +// TestServerSessionHandle_SetsResultTypeWhenMiddlewareShortCircuits is a +// regression test for #1225. User middleware wraps outside +// serverMultiRoundTripMiddleware, so a middleware that returns a result +// without calling next bypasses both that shim and the dispatcher. +func TestServerSessionHandle_SetsResultTypeWhenMiddlewareShortCircuits(t *testing.T) { + tests := []struct { + name string + method string + params map[string]any + result Result + want resultType + }{ + { + name: "tools/call", + method: methodCallTool, + params: newProtocolParams(map[string]any{"name": "tool", "arguments": map[string]any{}}), + result: &CallToolResult{Content: []Content{&TextContent{Text: "denied"}}}, + want: resultTypeComplete, + }, + { + name: "prompts/get", + method: methodGetPrompt, + params: newProtocolParams(map[string]any{"name": "prompt"}), + result: &GetPromptResult{ + Messages: []*PromptMessage{{Role: "assistant", Content: &TextContent{Text: "denied"}}}, + }, + want: resultTypeComplete, + }, + { + name: "resources/read", + method: methodReadResource, + params: newProtocolParams(map[string]any{"uri": "test://resource"}), + result: &ReadResourceResult{ + Contents: []*ResourceContents{{URI: "test://resource", Text: "denied"}}, + }, + want: resultTypeComplete, + }, + { + // setResultType is unexported, so populating InputRequests is + // the only way a middleware can ask for input. + name: "tools/call input required", + method: methodCallTool, + params: newProtocolParams(map[string]any{"name": "tool", "arguments": map[string]any{}}), + result: &CallToolResult{ + InputRequests: InputRequestMap{"confirm": &ElicitParams{Message: "Continue?"}}, + }, + want: resultTypeInputRequired, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Nothing is registered: if the middleware fell through to next, + // dispatch would fail rather than return a result. + server := NewServer(testImpl, nil) + server.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { + return func(ctx context.Context, method string, req Request) (Result, error) { + if method == tc.method { + return tc.result, nil + } + return next(ctx, method, req) + } + }) + if got := resultTypeOf(t, server, nil, tc.method, tc.params); got != string(tc.want) { + t.Errorf("resultType = %q, want %q", got, tc.want) + } + }) + } +} + +// TestServerSessionHandle_ResultTypeGate pins when a result carries resultType. +// annotateResultType is the only setter, and handle gates it on the request's +// _meta rather than on the version the session negotiated. So the last case +// below gets no resultType even though the session speaks 2026-07-28. Only a +// client that mixes protocol versions reaches that, so it is recorded rather +// than fixed. +func TestServerSessionHandle_ResultTypeGate(t *testing.T) { + version := func(v string) func(*ServerSessionState) { + return func(s *ServerSessionState) { + s.InitializeParams = &InitializeParams{ProtocolVersion: v} + } + } + callParams := map[string]any{"name": "tool", "arguments": map[string]any{}} + + tests := []struct { + name string + state func(*ServerSessionState) + params map[string]any + want resultType // "" means the field must be absent + }{ + { + name: "legacy session, no _meta", + state: version(protocolVersion20251125), + params: callParams, + }, + { + name: "legacy session, new-protocol _meta", + state: version(protocolVersion20251125), + params: newProtocolParams(callParams), + want: resultTypeComplete, + }, + { + name: "new-protocol session, no _meta", + state: version(protocolVersion20260728), + params: callParams, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + server := NewServer(testImpl, nil) + // Short-circuit so this measures handle's gate, not the + // dispatcher's. + server.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { + return func(context.Context, string, Request) (Result, error) { + return &CallToolResult{Content: []Content{&TextContent{Text: "ok"}}}, nil + } + }) + if got := resultTypeOf(t, server, tc.state, methodCallTool, tc.params); got != string(tc.want) { + t.Errorf("resultType = %q, want %q", got, tc.want) + } + }) + } +} + +// TestServerSessionHandle_NilResultFromMiddleware checks that a middleware +// returning no result does not panic. A typed nil satisfies Result, so the +// annotation helpers would otherwise dereference it. +func TestServerSessionHandle_NilResultFromMiddleware(t *testing.T) { + tests := []struct { + name string + result Result + }{ + {"nil interface", nil}, + {"typed nil", (*CallToolResult)(nil)}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + server := NewServer(testImpl, nil) + server.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { + return func(context.Context, string, Request) (Result, error) { + return tc.result, nil + } + }) ss := &ServerSession{server: server} id, err := jsonrpc.MakeID("test") if err != nil { t.Fatal(err) } - result, err := ss.handle(context.Background(), &jsonrpc.Request{ + res, err := ss.handle(context.Background(), &jsonrpc.Request{ ID: id, - Method: tc.method, - Params: mustMarshal(tc.params), + Method: methodCallTool, + Params: mustMarshal(newProtocolParams(map[string]any{"name": "tool", "arguments": map[string]any{}})), }) if err != nil { - t.Fatal(err) + t.Fatalf("handle: %v", err) } - data, err := json.Marshal(result) + data, err := json.Marshal(res) if err != nil { t.Fatal(err) } - var got struct { - ResultType string `json:"resultType"` - } - if err := json.Unmarshal(data, &got); err != nil { - t.Fatal(err) - } - if got.ResultType != string(tc.want) { - t.Fatalf("resultType = %q, want %q; response = %s", got.ResultType, tc.want, data) + if got := string(data); got != "null" { + t.Errorf("result = %s, want null", got) } }) } } +// resultTypeOf handles one request on a transportless session and reports the +// resultType of the marshaled result, or "" if the field is absent. +func resultTypeOf(t *testing.T, server *Server, state func(*ServerSessionState), method string, params map[string]any) string { + t.Helper() + ss := &ServerSession{server: server} + if state != nil { + ss.updateState(state) + } + id, err := jsonrpc.MakeID("test") + if err != nil { + t.Fatal(err) + } + result, err := ss.handle(context.Background(), &jsonrpc.Request{ + ID: id, + Method: method, + Params: mustMarshal(params), + }) + if err != nil { + t.Fatal(err) + } + data, err := json.Marshal(result) + if err != nil { + t.Fatal(err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + t.Fatal(err) + } + raw, ok := fields["resultType"] + if !ok { + return "" + } + var got string + if err := json.Unmarshal(raw, &got); err != nil { + t.Fatalf("unmarshaling resultType from %s: %v", data, err) + } + return got +} + // TestServerSessionHandle_RejectsRemovedMethodsOnNewProtocol verifies that // the methods removed by SEP-2575 (`initialize`, `notifications/initialized`, // `ping`) all return Method not found when the request opts into the new diff --git a/mcp/shared.go b/mcp/shared.go index 2258752..6637a25 100644 --- a/mcp/shared.go +++ b/mcp/shared.go @@ -809,6 +809,9 @@ type Result interface { // isResult discourages implementation of Result outside of this package. isResult() + // isNil returns true if the underlying value is nil. + isNil() bool + // GetMeta returns metadata from a value. GetMeta() map[string]any // SetMeta sets the metadata on a value. @@ -827,13 +830,15 @@ type ResultBase struct { Meta `json:"_meta,omitempty"` } -func (*ResultBase) isResult() {} +func (*ResultBase) isResult() {} +func (x *ResultBase) isNil() bool { return x == nil } // emptyResult is returned by methods that have no result, like ping. // Those methods cannot return nil, because jsonrpc2 cannot handle nils. type emptyResult struct{} func (*emptyResult) isResult() {} +func (x *emptyResult) isNil() bool { return x == nil } func (*emptyResult) GetMeta() map[string]any { panic("should never be called") } func (*emptyResult) SetMeta(map[string]any) { panic("should never be called") } From 3210c2b34cacd6e80305b8216779bfe6c964d720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Mon, 14 Sep 2026 13:33:30 +0200 Subject: [PATCH 05/15] mcp: carry a cancelled notification's reason as the request's cancel cause (#1255) The canceller read `CancelledParams.Reason` off the wire and dropped it: `Connection.Cancel` cancelled the request's context with a nil cause, so a handler that looked at `context.Cause` saw a bare `context.Canceled` and nothing of what the peer said. The specification asks implementations to log cancellation reasons, and the handler is the one place that can act on one; #1100 had already given the request context a cause, so the reason has a natural place to travel. `Connection.CancelCause` is `Cancel` with a cause. The canceller now cancels with an error that carries the peer's reason and unwraps to `context.Canceled`, so `errors.Is` keeps classifying the cancellation as one while `context.Cause` reads `request cancelled by the peer: `. It also logs the id and the reason at debug level through the logger the connection already carries, which is the SDK's own answer to the SHOULD. No exported API changes: the cause type stays unexported and is read through `context.Cause` and `Error()`. `TestCancellation` now asserts the cause the SDK's own client produces (its context's error, which the client sends as the reason), and `TestCancellationReason` writes the call and the cancelled notification to the connection directly with a reason the client would never send, on a session pinned to 2025-11-25 so the hand-written request needs no `_meta`. Both fail without the change. Fixes #1254 Co-authored-by: Guglielmo Colombo --- internal/jsonrpc2/conn.go | 9 ++++- mcp/mcp_test.go | 71 +++++++++++++++++++++++++++++++++++++-- mcp/transport.go | 29 ++++++++++++++-- 3 files changed, 102 insertions(+), 7 deletions(-) diff --git a/internal/jsonrpc2/conn.go b/internal/jsonrpc2/conn.go index 5849a87..94da6ae 100644 --- a/internal/jsonrpc2/conn.go +++ b/internal/jsonrpc2/conn.go @@ -453,12 +453,19 @@ func (ac *AsyncCall) Await(ctx context.Context, result any) error { // will not cause any messages that have not arrived yet with that ID to be // cancelled. func (c *Connection) Cancel(id ID) { + c.CancelCause(id, nil) +} + +// CancelCause is like [Connection.Cancel], but records cause as the reason +// the Context was cancelled, so that the Handle call can read it back through +// [context.Cause]. A nil cause reads as [context.Canceled]. +func (c *Connection) CancelCause(id ID, cause error) { var req *incomingRequest c.updateInFlight(func(s *inFlightState) { req = s.incomingByID[id] }) if req != nil { - req.cancel(nil) + req.cancel(cause) } } diff --git a/mcp/mcp_test.go b/mcp/mcp_test.go index e1f8c0f..bb76c1c 100644 --- a/mcp/mcp_test.go +++ b/mcp/mcp_test.go @@ -710,13 +710,13 @@ func TestCancellation(t *testing.T) { synctest.Test(t, func(t *testing.T) { var ( start = make(chan struct{}) - cancelled = make(chan struct{}, 1) // don't block the request + cancelled = make(chan error, 1) // don't block the request ) slowTool := func(ctx context.Context, req *CallToolRequest, args any) (*CallToolResult, any, error) { start <- struct{}{} select { case <-ctx.Done(): - cancelled <- struct{}{} + cancelled <- context.Cause(ctx) case <-time.After(5 * time.Second): return nil, nil, nil } @@ -732,7 +732,72 @@ func TestCancellation(t *testing.T) { <-start cancel() - <-cancelled + // The client sends its context's error as the reason of its cancelled + // notification, and the handler reads it back as the cause. + cause := <-cancelled + if !errors.Is(cause, context.Canceled) { + t.Errorf("context.Cause = %v, want it to wrap context.Canceled", cause) + } + if want := "request cancelled by the peer: " + context.Canceled.Error(); cause == nil || cause.Error() != want { + t.Errorf("context.Cause = %v, want %q", cause, want) + } + }) +} + +// TestCancellationReason verifies that the reason a peer gives in its +// cancelled notification reaches the handler as the cause of its context, +// which is what lets a server log it as the specification asks. +// +// The call and the notification are written to the connection directly, so +// the reason is one the SDK's own client would never send; the session is +// pinned to 2025-11-25 so that a request written that way needs no _meta. +func TestCancellationReason(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + var ( + start = make(chan struct{}) + cancelled = make(chan error, 1) // don't block the request + ) + slowTool := func(ctx context.Context, req *CallToolRequest, args any) (*CallToolResult, any, error) { + start <- struct{}{} + select { + case <-ctx.Done(): + cancelled <- context.Cause(ctx) + case <-time.After(5 * time.Second): + cancelled <- nil + } + return nil, nil, nil + } + ctx := context.Background() + ct, st := NewInMemoryTransports() + s := NewServer(testImpl, nil) + AddTool(s, &Tool{Name: "slow", InputSchema: &jsonschema.Schema{Type: "object"}}, slowTool) + ss, err := s.Connect(ctx, st, nil) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = ss.Close() }) + cs, err := NewClient(testImpl, nil).Connect(ctx, ct, &ClientSessionOptions{ProtocolVersion: protocolVersion20251125}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = cs.Close() }) + + call := cs.conn.Call(ctx, methodCallTool, &CallToolParams{Name: "slow"}) + <-start + if err := cs.conn.Notify(ctx, notificationCancelled, &CancelledParams{RequestID: call.ID().Raw(), Reason: "user asked"}); err != nil { + t.Fatal(err) + } + + cause := <-cancelled + if cause == nil { + t.Fatal("the tool ran to completion, want it cancelled") + } + if !errors.Is(cause, context.Canceled) { + t.Errorf("context.Cause = %v, want it to wrap context.Canceled", cause) + } + if got, want := cause.Error(), "request cancelled by the peer: user asked"; got != want { + t.Errorf("context.Cause = %q, want %q", got, want) + } }) } diff --git a/mcp/transport.go b/mcp/transport.go index 8b6cd14..ce841f2 100644 --- a/mcp/transport.go +++ b/mcp/transport.go @@ -207,7 +207,7 @@ func connect[H handler, State any](ctx context.Context, t Transport, b binder[H, reader, writer := jsonrpc2.Reader(mcpConn), jsonrpc2.Writer(mcpConn) var ( h H - preempter canceller + preempter = canceller{logger: logger} ) bind := func(conn *jsonrpc2.Connection) jsonrpc2.Handler { h = b.bind(mcpConn, conn, s, onClose) @@ -252,7 +252,8 @@ type cancellationPropagator interface { // A canceller is a jsonrpc2.Preempter that cancels in-flight requests on MCP // cancelled notifications. type canceller struct { - conn *jsonrpc2.Connection + conn *jsonrpc2.Connection + logger *slog.Logger } // Preempt implements [jsonrpc2.Preempter]. @@ -266,11 +267,33 @@ func (c *canceller) Preempt(ctx context.Context, req *jsonrpc.Request) (result a if err != nil { return nil, err } - go c.conn.Cancel(id) + // The spec says implementations should log cancellation reasons, and + // the handler is the one place that can act on one, so the reason + // travels as the cause of the request's context rather than being + // dropped here. + c.logger.Debug("request cancelled by the peer", "id", id.Raw(), "reason", params.Reason) + go c.conn.CancelCause(id, &peerCancelledError{reason: params.Reason}) } return nil, jsonrpc2.ErrNotHandled } +// A peerCancelledError is the cause a request's context carries once the peer +// has sent a cancelled notification for it: [context.Cause] returns it to the +// handler with the reason the peer gave, and it unwraps to [context.Canceled] +// so that [errors.Is] keeps classifying the cancellation as one. +type peerCancelledError struct { + reason string +} + +func (e *peerCancelledError) Error() string { + if e.reason == "" { + return "request cancelled by the peer" + } + return "request cancelled by the peer: " + e.reason +} + +func (e *peerCancelledError) Unwrap() error { return context.Canceled } + // callSubscriptionsListen issues a "subscriptions/listen" call (SEP-2575) // without awaiting its JSON-RPC response. The call's logical lifetime is the // stream of notifications that follow on the same channel — the empty From a82c86a6cd9d357e56e57cd93abf90f950f59abd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Mon, 14 Sep 2026 17:49:53 +0200 Subject: [PATCH 06/15] mcp: classify the per-request protocol version by membership (#1268) `validateRequestMeta` decided whether a request follows the 2026-07-28 protocol by comparing `_meta.protocolVersion` against `2026-07-28` with `<`. This replaces that comparison with a membership test against `supportedProtocolVersions`, which is the rule `mcp/streamable.go:354` already applies to the `Mcp-Protocol-Version` header. Nothing changes for a version the SDK supports: one predating 2026-07-28 still selects the legacy handshake, and one at 2026-07-28 or later is still validated as the full `_meta` triple. What changes is a version the SDK does not support at all. It now reports `usesNewProtocol`, so it reaches the `CodeUnsupportedProtocolVersion` check in `(*ServerSession).handle` and is answered with the error the versioning page requires. Driving a stdio server with `_meta.protocolVersion` set to `1900-01-01`, the worked example on that page, before: ```json {"jsonrpc":"2.0","id":1,"error":{"code":0,"message":"method \"tools/list\" is invalid during session initialization"}} ``` and after: ```json {"jsonrpc":"2.0","id":1,"error":{"code":-32022,"message":"unsupported protocol version","data":{"supported":["2026-07-28","2025-11-25","2025-06-18","2025-03-26","2024-11-05"],"requested":"1900-01-01"}}} ``` There are two judgement calls in the diff, and I would rather state them than leave them to be found. The first is that an unsupported version returns early with `initializeParams` carrying only that version, and the rest of the `_meta` triple is left unvalidated. The shape of `_meta` is defined by the revision the request names, and that is a revision this SDK does not know, so there is nothing to validate it against; answering "missing `io.modelcontextprotocol/clientCapabilities`" would send the client off to fix the wrong thing while its real problem is the version. The partly populated value cannot escape: `handle` runs the unsupported check before anything reads the capabilities, and `Server.protocolVersions` is always a subset of `supportedProtocolVersions`, so a version that fails the membership test in `validateRequestMeta` always fails it in `handle` too. The second is that an empty `protocolVersion` still means "no version declared" rather than "unsupported version". `mcp/streamable.go` already reads the field that way when it decides whether a request carries a version at all (`metaVersion != ""`), and rejecting the empty string here would put the two layers back into disagreement, in the other direction. The `!ok` and `== ""` cases are therefore handled together. **Behaviour an existing user would see change.** A request whose `_meta.protocolVersion` is a string not in `SupportedProtocolVersions()` is now refused with `-32022`, where before a value sorting below `2026-07-28` was treated as a legacy request: served if the session was already initialized, refused with the session-initialization error if it was not. No published protocol revision is in that set, so a client speaking any released version of MCP sees no difference. A deployment that put a sentinel or placeholder version in `_meta` and relied on it being ignored would see those requests refused, which I believe is the right answer but is a visible change. This is not limited to stdio, and I want to be precise about where the header gate does and does not stand in front of it. The gate at `mcp/streamable.go:354` refuses an unsupported `Mcp-Protocol-Version` only when that version **also** sorts below `2026-07-28`, so an unknown version sorting above it (`2027-01-01`, say) passes the gate and reaches this classification; the deprecated HTTP+SSE transport in `mcp/sse.go` has no header check at all; and stdio, the in-memory transport and any custom transport have none either. The window the old comparison left open is therefore reachable on every transport, and a value sorting above `2026-07-28` with an incomplete `_meta` triple also changes answer, from `-32602` to `-32022`. **The test that pins it** is `TestServerUnknownProtocolVersion_NewProtocol` in `mcp/server_test.go`. It drives `(*ServerSession).handle` with a `tools/list` request whose `_meta.protocolVersion` names a version the SDK does not know, and asserts that the returned error is a `*jsonrpc.Error` with code `CodeUnsupportedProtocolVersion`, `Data.Requested` equal to the version asked for, and `Data.Supported` equal to `SupportedProtocolVersions()`. Four cases: `1900-01-01`, `2025-11-24`, `1.0`, and `1900-01-01` again with the rest of the triple omitted, which is the case that pins the first judgement call above. Before the change all four fail with `handle returned method "tools/list" is invalid during session initialization, want a *jsonrpc.Error`. `TestValidateRequestMeta` in `mcp/shared_test.go` gains four cases at the unit level: two that fail before the change, and two that guard the classifications which must not move, a supported pre-2026-07-28 version and an empty one. `go test ./...` and `gofmt -l .` are both clean. Fixes #1260. --- mcp/server_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++ mcp/shared.go | 39 ++++++++++++++++++++++++------- mcp/shared_test.go | 51 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 8 deletions(-) diff --git a/mcp/server_test.go b/mcp/server_test.go index 77dcb10..1544e29 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -2151,3 +2151,61 @@ func TestServerSupportedProtocolVersions_NewProtocol(t *testing.T) { t.Errorf("UnsupportedProtocolVersionData.Supported mismatch (-want +got):\n%s", diff) } } + +// 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 +// because its version string happens to sort below 2026-07-28. +func TestServerUnknownProtocolVersion_NewProtocol(t *testing.T) { + ctx := context.Background() + // Each case names a version that is not in [supportedProtocolVersions] and + // sorts below 2026-07-28. + tests := []struct { + name string + version string + extraMeta string // further `_meta` entries, each with a leading comma + }{ + // The versioning spec uses 1900-01-01 to illustrate this error. + {"documented example", "1900-01-01", fmt.Sprintf(`,%q:{}`, MetaKeyClientCapabilities)}, + {"unreleased revision", "2025-11-24", fmt.Sprintf(`,%q:{}`, MetaKeyClientCapabilities)}, + {"not a date", "1.0", fmt.Sprintf(`,%q:{}`, MetaKeyClientCapabilities)}, + // The rest of the `_meta` triple is defined by the revision the request + // names, so its absence must not mask the version itself. + {"no clientCapabilities", "1900-01-01", ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + server := NewServer(testImpl, nil) + _, st := NewInMemoryTransports() + ss, err := server.Connect(ctx, st, nil) + if err != nil { + t.Fatalf("server.Connect: %v", err) + } + defer ss.Close() + + params := fmt.Sprintf(`{"_meta":{%q:%q%s}}`, MetaKeyProtocolVersion, tc.version, tc.extraMeta) + _, err = ss.handle(ctx, &jsonrpc.Request{ + ID: jsonrpc2.Int64ID(1), + Method: methodListTools, + Params: json.RawMessage(params), + }) + var jerr *jsonrpc.Error + if !errors.As(err, &jerr) { + t.Fatalf("handle returned %v, want a *jsonrpc.Error", err) + } + if jerr.Code != CodeUnsupportedProtocolVersion { + t.Fatalf("error code = %d, want %d", jerr.Code, CodeUnsupportedProtocolVersion) + } + var data UnsupportedProtocolVersionData + if err := json.Unmarshal(jerr.Data, &data); err != nil { + t.Fatalf("unmarshal error data: %v", err) + } + if data.Requested != tc.version { + t.Errorf("UnsupportedProtocolVersionData.Requested = %q, want %q", data.Requested, tc.version) + } + if diff := cmp.Diff(SupportedProtocolVersions(), data.Supported); diff != "" { + t.Errorf("UnsupportedProtocolVersionData.Supported mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/mcp/shared.go b/mcp/shared.go index 6637a25..75dfeee 100644 --- a/mcp/shared.go +++ b/mcp/shared.go @@ -553,13 +553,19 @@ type validatedMeta struct { // validateRequestMeta inspects a JSON-RPC request to detect whether it follows // the >= 2026-07-28 protocol via the `_meta` field. -// If the request has no _meta, or no protocolVersion in _meta, it returns a non-nil -// validatedMeta with usesNewProtocol set to false, and a nil error. -// If the request has a protocolVersion in _meta it validates the presence of -// clientCapabilities in _meta. If it is missing or invalid, it returns nil and -// a non-nil error. clientInfo is optional; if present but invalid, an error is -// returned. Otherwise, it returns usesNewProtocol set to true and the populated -// initializeParams. +// If the request has no _meta, no protocolVersion in _meta, or a protocolVersion +// naming a supported version that predates 2026-07-28, it returns a non-nil +// validatedMeta with usesNewProtocol set to false, and a nil error: such a request +// belongs to the initialize handshake. +// If the protocolVersion names a version that is not supported at all, it returns +// usesNewProtocol set to true with initializeParams carrying only that version, so +// that the caller answers with [CodeUnsupportedProtocolVersion]. The rest of the +// `_meta` triple is not validated in that case, since the revision defining it is +// unknown. +// Otherwise, it validates the presence of clientCapabilities in _meta. If it is +// missing or invalid, it returns nil and a non-nil error. clientInfo is optional; +// if present but invalid, an error is returned. Otherwise, it returns +// usesNewProtocol set to true and the populated initializeParams. // Notifications always report usesNewProtocol false and never error. func validateRequestMeta(req *jsonrpc.Request) (*validatedMeta, error) { // SEP-2575 defines the `_meta` triple for calls only: NotificationParams @@ -574,9 +580,26 @@ func validateRequestMeta(req *jsonrpc.Request) (*validatedMeta, error) { return &validatedMeta{usesNewProtocol: false, initializeParams: nil}, nil } protocolVersion, ok := meta[MetaKeyProtocolVersion].(string) - if !ok || protocolVersion < protocolVersion20260728 { + // An absent or empty value declares no version at all, which is also how the + // streamable handler reads `_meta.protocolVersion`. + if !ok || protocolVersion == "" { return &validatedMeta{usesNewProtocol: false, initializeParams: nil}, nil } + // Classify by membership rather than by ordering: a version the SDK does not + // know sorts arbitrarily against 2026-07-28, and one that sorts below it would + // be served as a legacy handshake instead of reaching the + // CodeUnsupportedProtocolVersion check in ServerSession.handle. The + // streamable handler already applies the same membership rule to the + // Mcp-Protocol-Version header. + supported := slices.Contains(supportedProtocolVersions, protocolVersion) + if supported && protocolVersion < protocolVersion20260728 { + return &validatedMeta{usesNewProtocol: false, initializeParams: nil}, nil + } + if !supported { + return &validatedMeta{usesNewProtocol: true, initializeParams: &InitializeParams{ + ProtocolVersion: protocolVersion, + }}, nil + } var clientInfo *Implementation if _, present := meta[MetaKeyClientInfo]; present { var ok bool diff --git a/mcp/shared_test.go b/mcp/shared_test.go index 71a071d..d308c87 100644 --- a/mcp/shared_test.go +++ b/mcp/shared_test.go @@ -102,6 +102,57 @@ func TestValidateRequestMeta(t *testing.T) { params: json.RawMessage(`{"_meta": "not an object", "name": "x"}`), wantUsesNew: false, }, + { + name: "supported pre-2026-07-28 version: old protocol", + method: methodCallTool, + params: map[string]any{ + "_meta": map[string]any{ + MetaKeyProtocolVersion: protocolVersion20251125, + }, + "name": "x", + }, + wantUsesNew: false, + }, + { + name: "empty protocolVersion declares nothing: old protocol", + method: methodCallTool, + params: map[string]any{ + "_meta": map[string]any{ + MetaKeyProtocolVersion: "", + }, + "name": "x", + }, + wantUsesNew: false, + }, + { + // An unsupported version sorting below 2026-07-28 is still a + // new-protocol request, so that the caller can answer it with + // CodeUnsupportedProtocolVersion. + name: "unsupported version: new protocol", + method: methodCallTool, + params: map[string]any{ + "_meta": map[string]any{ + MetaKeyProtocolVersion: "1900-01-01", + MetaKeyClientInfo: map[string]any{"name": "c", "version": "1"}, + MetaKeyClientCapabilities: map[string]any{}, + }, + "name": "x", + }, + wantUsesNew: true, + }, + { + // The triple is defined by a revision the SDK does not know, so an + // unsupported version is reported without validating the rest of it. + name: "unsupported version missing clientCapabilities: new protocol", + method: methodCallTool, + params: map[string]any{ + "_meta": map[string]any{ + MetaKeyProtocolVersion: "1900-01-01", + }, + "name": "x", + }, + wantUsesNew: true, + }, { name: "new protocol with logLevel", method: methodCallTool, From 3f43bcf012e45e701bfb91243d439edab44c4238 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Mon, 14 Sep 2026 18:45:48 +0200 Subject: [PATCH 07/15] mcp: add HasParams so middleware can tell whether params are present (#1269) This adds `mcp.HasParams(req Request) bool`, so a receiving middleware can ask whether the message it was handed carried a `params` member. The member is optional for several methods: `serverMethodInfos` sets `missingParamsOK` for `server/discover`, `ping`, the four list methods and the `cancelled`, `initialized` and `roots/list_changed` notifications, so `{"jsonrpc":"2.0","id":1,"method":"tools/list"}` is a complete request. For one of those, `unmarshalParams` returns a nil `Params`, `newRequest` leaves `ServerRequest.Params` at the zero value of `P`, and `GetParams` hands the middleware a non-nil interface holding a typed nil pointer. Comparing that against nil says params are present, and the first method called on it dereferences: `GetMeta` is promoted from the embedded `Meta`. `Params` already declares `isNil` for precisely this case, and it is unexported, so from outside the package the accessor named after the question is the one thing that cannot be reached. `HasParams` is a function over `Request` rather than an exported method on `Params` for two reasons. A middleware holds a `Request`, and this way the caller never has to hold the value that panics. More importantly, `isNil` cannot answer the question for every `Params` value, so exporting it would ship a predicate that is itself unsafe: a params type declared the documented way for a custom method, `struct{ mcp.ParamsBase }`, promotes `isNil` through a field selector, and that dereferences the nil outer pointer before the method body runs. The predicate therefore has to be computed from outside the value, and `reflect.Value.IsNil` is what works for all of them. Three judgement calls, stated so nobody has to find them. First, `getRequestMeta` now goes through `HasParams`. That is a second fix rather than a refactor: `AddReceivingCustomMethod` registers with `missingParamsOK` too, so a client omitting `params` for a custom method made `ServerRequest.ProtocolVersion`, `ClientInfo` and `ClientCapabilities` panic in the promoted `isNil`. They now return `""`, `nil` and `nil`. I could have left it out to keep the diff to one function, but then the SDK would export a predicate that says to ask before touching params while three of its own accessors still panicked on the same value. If you would rather see it as a separate commit, say so and I will split it. Second, I did not add the predicate to the `Request` interface. `Request` is sealed by `isRequest`, so a method would have been a compatible change and would read slightly better at the call site, but a free function leaves the interface alone and serves every `Request` just as well. Third, this reflects once per call of `getRequestMeta`. It is one `reflect.ValueOf` on a value that is already an interface, on a path where `newResult` already reflects per request. Tests, both in `mcp/shared_test.go`. `TestServerRequest_PerRequestAccessors_MissingParams` is the one that fails before the change and passes after: on `main` it panics inside `getRequestMeta` with `invalid memory address or nil pointer dereference` from `(*customParams).isNil`, and here it asserts the three accessors return zero values. `TestHasParams` covers the exported behaviour: it builds exactly what `handleReceive` hands the middleware chain for `tools/list`, `prompts/list`, `resources/list` and `notifications/initialized` with no `params` member, asserts that `GetParams` still returns a non-nil interface so the case under test is still the typed-nil one, asserts `HasParams` reports false, and asserts it reports true when params are present. Behaviour an existing user would see change: only the three accessors above, and only for a custom-method params type built on `ParamsBase` whose `params` member was omitted, where a panic becomes a zero value. Nothing about the wire format changes, no exported signature moves, and no interface gains a method. A middleware that still writes `req.GetParams() != nil` and then dereferences still panics; this gives a caller the way to ask, it does not rescue a caller that does not. Also here: one sentence on the doc comments of both `AddReceivingMiddleware` methods saying a received message need not carry params, since that is where a middleware author is reading, and `examples/server/middleware/main.go` now logs `has_params` with `mcp.HasParams` instead of the comparison that cannot work. The example's printed output is unchanged, because the SDK client always sends a `params` member for the methods it drives. `go test ./...` and `gofmt -l .` are clean. One thing to say plainly, since it is yours to decide and not mine: this adds an exported symbol. `HasParams` is a new name in the public API of `mcp`, and everything else here is doc comments, an example and the three accessors. If you would rather not grow the surface for this, the smaller answer is to keep the `getRequestMeta` fix and the doc-comment sentence and drop the exported function, and a middleware author then has the guarantee written down where they are reading even though they still cannot ask the question in code. I prefer the function because the guarantee alone leaves every middleware writing the same three lines of `reflect`, and the one that does not write them panics on an ordinary `tools/list`, but I will cut it to whichever shape you want. Fixes #1261. Co-authored-by: Guglielmo Colombo --- examples/server/middleware/main.go | 2 +- mcp/client.go | 3 ++ mcp/server.go | 3 ++ mcp/shared.go | 27 ++++++++++++-- mcp/shared_test.go | 56 ++++++++++++++++++++++++++++++ 5 files changed, 88 insertions(+), 3 deletions(-) diff --git a/examples/server/middleware/main.go b/examples/server/middleware/main.go index 4e37471..262d726 100644 --- a/examples/server/middleware/main.go +++ b/examples/server/middleware/main.go @@ -38,7 +38,7 @@ func main() { logger.Info("MCP method started", "method", method, "session_id", req.GetSession().ID(), - "has_params", req.GetParams() != nil, + "has_params", mcp.HasParams(req), ) // Log more for tool calls. if ctr, ok := req.(*mcp.CallToolRequest); ok { diff --git a/mcp/client.go b/mcp/client.go index c11f0c8..1ee13e8 100644 --- a/mcp/client.go +++ b/mcp/client.go @@ -1147,6 +1147,9 @@ func (c *Client) AddSendingMiddleware(middleware ...Middleware) { // // Receiving middleware is called when a request is received. It is useful for tasks // such as authentication, request logging and metrics. +// +// A received message need not carry params: use [HasParams] before inspecting +// [Request.GetParams]. func (c *Client) AddReceivingMiddleware(middleware ...Middleware) { c.mu.Lock() defer c.mu.Unlock() diff --git a/mcp/server.go b/mcp/server.go index 183a519..442d7bc 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -1857,6 +1857,9 @@ func (s *Server) AddSendingMiddleware(middleware ...Middleware) { // // Receiving middleware is called when a request is received. It is useful for tasks // such as authentication, request logging and metrics. +// +// A received message need not carry params: use [HasParams] before inspecting +// [Request.GetParams]. func (s *Server) AddReceivingMiddleware(middleware ...Middleware) { s.mu.Lock() defer s.mu.Unlock() diff --git a/mcp/shared.go b/mcp/shared.go index 75dfeee..cacfa75 100644 --- a/mcp/shared.go +++ b/mcp/shared.go @@ -688,6 +688,30 @@ func (r *ServerRequest[P]) GetParams() Params { return r.Params } func (r *ClientRequest[P]) GetExtra() *RequestExtra { return nil } func (r *ServerRequest[P]) GetExtra() *RequestExtra { return r.Extra } +// HasParams reports whether req carries params. +// +// The "params" member of a message may be omitted for several methods, among +// them the list methods, ping and notifications/initialized. For such a request +// [Request.GetParams] returns a non-nil [Params] holding a nil pointer, so +// comparing its result against nil reports that params are present, while +// calling a method on that result panics. Middleware that inspects params must +// ask this first: +// +// if mcp.HasParams(req) { +// log.Println(method, req.GetParams().GetMeta()) +// } +func HasParams(req Request) bool { + params := req.GetParams() + if params == nil { + return false + } + // isNil cannot answer this for every value: a params type embedding + // ParamsBase promotes isNil through a field selector, which dereferences + // the nil outer pointer before the method body runs. + v := reflect.ValueOf(params) + return v.Kind() != reflect.Pointer || !v.IsNil() +} + // ProtocolVersion returns the protocol version negotiated for this request. // // For requests following the >= 2026-07-28 protocol, the value is read from @@ -749,8 +773,7 @@ func (r *ServerRequest[P]) ClientCapabilities() *ClientCapabilities { // getRequestMeta returns the raw `_meta` map from the request's params, or // nil if the params are absent. func getRequestMeta[P Params](r *ServerRequest[P]) map[string]any { - // In practice P is a pointer type implementing Params. - if any(r.Params) == nil || r.Params.isNil() { + if !HasParams(r) { return nil } return r.Params.GetMeta() diff --git a/mcp/shared_test.go b/mcp/shared_test.go index d308c87..4566768 100644 --- a/mcp/shared_test.go +++ b/mcp/shared_test.go @@ -324,6 +324,62 @@ func TestServerRequest_PerRequestAccessors_Empty(t *testing.T) { } } +func TestHasParams(t *testing.T) { + // {"jsonrpc":"2.0","id":1,"method":"tools/list"} is a complete request: + // serverMethodInfos marks these methods missingParamsOK. Build what + // handleReceive hands the middleware chain for one. + for _, method := range []string{ + methodListTools, + methodListPrompts, + methodListResources, + notificationInitialized, + } { + t.Run(method, func(t *testing.T) { + info := serverMethodInfos[method] + params, err := info.unmarshalParams(nil) + if err != nil { + t.Fatal(err) + } + req := info.newRequest(&ServerSession{}, params, nil) + if req.GetParams() == nil { + t.Fatal("GetParams returned a nil interface, so this test no longer covers the typed-nil case") + } + if HasParams(req) { + t.Error("HasParams = true for a request whose params member was omitted") + } + }) + } + + t.Run("params present", func(t *testing.T) { + info := serverMethodInfos[methodListTools] + params, err := info.unmarshalParams(json.RawMessage(`{"cursor":"c"}`)) + if err != nil { + t.Fatal(err) + } + req := info.newRequest(&ServerSession{}, params, nil) + if !HasParams(req) { + t.Error("HasParams = false for a request carrying params") + } + }) +} + +func TestServerRequest_PerRequestAccessors_MissingParams(t *testing.T) { + // Custom methods also allow the params member to be omitted, and their + // params types embed ParamsBase, whose promoted isNil dereferences the + // typed-nil outer pointer. + type customParams struct{ ParamsBase } + req := &ServerRequest[*customParams]{} + if got := req.ProtocolVersion(); got != "" { + t.Errorf("ProtocolVersion = %q, want empty", got) + } + if got := req.ClientInfo(); got != nil { + t.Errorf("ClientInfo = %+v, want nil", got) + } + if got := req.ClientCapabilities(); got != nil { + t.Errorf("ClientCapabilities = %+v, want nil", got) + } +} + func TestImplementationDescriptionJSON(t *testing.T) { impl := &Implementation{ Name: "greeter", From 0c0eb236257200d913d04e658fd2f2e5cefa7832 Mon Sep 17 00:00:00 2001 From: rishabh Date: Mon, 14 Sep 2026 13:20:25 -0400 Subject: [PATCH 08/15] docs/client: fix the SEP-1034 and SEP-1330 links (#1252) The "Schema defaults and enums" section of docs/client.md links SEP-1034 to https://modelcontextprotocol.io/seps/1034 and SEP-1330 to https://modelcontextprotocol.io/seps/1330, and both of those bare-number URLs are dead. modelcontextprotocol.io serves SEPs at number-plus-slug URLs, which is what every other SEP link in this repo already uses (seps/2575-stateless-mcp, seps/2577-deprecate-roots-sampling-and-logging, seps/2596-feature-lifecycle-and-deprecation-policy, seps/2207-oidc-refresh-token-guidance); these two were the only bare-number ones left. I took the replacement slugs from the site's own sitemap.xml, so the `1034--` double hyphen is what is published rather than a typo here. curl -sIL on the two old URLs returns 404 and 404, and on the two new ones 200 and 200. The edit is in internal/docs/client.src.md with docs/client.md regenerated by `go generate ./internal/docs`, and `go build ./... && go test ./...` pass. Co-authored-by: Guglielmo Colombo --- docs/client.md | 4 ++-- internal/docs/client.src.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/client.md b/docs/client.md index ad71c11..23d57be 100644 --- a/docs/client.md +++ b/docs/client.md @@ -304,7 +304,7 @@ func Example_elicitation() { `ElicitParams.RequestedSchema` is a flat schema of primitive fields, which the client renders as a form. Two field keywords shape that form. -A `Default` ([SEP-1034](https://modelcontextprotocol.io/seps/1034)) prefills a +A `Default` ([SEP-1034](https://modelcontextprotocol.io/seps/1034--support-default-values-for-all-primitive-types-in)) prefills a field. When the user accepts without supplying it, the SDK fills the field in from the schema before the result reaches either side's caller — the client does so after its elicitation handler returns, and `ServerSession.Elicit` does @@ -313,7 +313,7 @@ defaulted field `Required` defeats it: accepted content is validated against the schema before defaults are applied, so an answer that omits the field is rejected rather than defaulted. -An `Enum` ([SEP-1330](https://modelcontextprotocol.io/seps/1330)) restricts a +An `Enum` ([SEP-1330](https://modelcontextprotocol.io/seps/1330-elicitation-enum-schema-improvements-and-standards)) restricts a field to a fixed set of values, which the client renders as a choice. Enums are supported only on `"string"` fields; declaring one on another type is rejected. To label the choices, set the legacy `enumNames` keyword through diff --git a/internal/docs/client.src.md b/internal/docs/client.src.md index fdaef2a..c0f64b4 100644 --- a/internal/docs/client.src.md +++ b/internal/docs/client.src.md @@ -99,7 +99,7 @@ pattern. `ElicitParams.RequestedSchema` is a flat schema of primitive fields, which the client renders as a form. Two field keywords shape that form. -A `Default` ([SEP-1034](https://modelcontextprotocol.io/seps/1034)) prefills a +A `Default` ([SEP-1034](https://modelcontextprotocol.io/seps/1034--support-default-values-for-all-primitive-types-in)) prefills a field. When the user accepts without supplying it, the SDK fills the field in from the schema before the result reaches either side's caller — the client does so after its elicitation handler returns, and `ServerSession.Elicit` does @@ -108,7 +108,7 @@ defaulted field `Required` defeats it: accepted content is validated against the schema before defaults are applied, so an answer that omits the field is rejected rather than defaulted. -An `Enum` ([SEP-1330](https://modelcontextprotocol.io/seps/1330)) restricts a +An `Enum` ([SEP-1330](https://modelcontextprotocol.io/seps/1330-elicitation-enum-schema-improvements-and-standards)) restricts a field to a fixed set of values, which the client renders as a choice. Enums are supported only on `"string"` fields; declaring one on another type is rejected. To label the choices, set the legacy `enumNames` keyword through From c1918e19ad93014def1ef1dcf4a136783b6de9d0 Mon Sep 17 00:00:00 2001 From: jackyyy <142072937+yyydddkkk@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:17:47 +0800 Subject: [PATCH 09/15] mcp: preserve SSE events buffered after the endpoint (#1270) SSEClientTransport.Connect creates one event scanner to read the endpoint event and a second scanner to read subsequent messages. Each scanner owns a buffered reader, so bytes read ahead by the first scanner are discarded when it is abandoned. A message coalesced with the endpoint can therefore disappear while the connection remains open. Reuse the event scanner across the endpoint handshake and the message-reading goroutine. This preserves unread bytes without changing the public API. The regression test exercises Connect and Read with a controlled HTTP response body, covering both a message coalesced with the endpoint and a message arriving after Connect returns. On unmodified main, the separate-message control passes and the coalesced case times out; both pass with the fix. Validation on linux/amd64 with Go 1.26.8: - go test ./... - go test -race ./... - go vet ./... - staticcheck v0.6.1 ./... - gofmt and git diff --check The HTTP response boundary is simulated; no third-party server or proxy was used. This change concerns the legacy SSE transport. The Streamable HTTP conformance suite was not run. --- mcp/sse.go | 6 ++++-- mcp/sse_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/mcp/sse.go b/mcp/sse.go index 0a5bf7f..f5e90b8 100644 --- a/mcp/sse.go +++ b/mcp/sse.go @@ -437,9 +437,11 @@ func (c *SSEClientTransport) Connect(ctx context.Context) (Connection, error) { maxEventSize = DefaultMaxEventSize } + // Reuse the scanner so events buffered after the endpoint are not lost. + events := scanEventsLimited(resp.Body, maxEventSize) msgEndpoint, err := func() (*url.URL, error) { var evt Event - for evt, err = range scanEventsLimited(resp.Body, maxEventSize) { + for evt, err = range events { break } if err != nil { @@ -468,7 +470,7 @@ func (c *SSEClientTransport) Connect(ctx context.Context) (Connection, error) { go func() { defer s.Close() // close the transport when the GET exits - for evt, err := range scanEventsLimited(resp.Body, maxEventSize) { + for evt, err := range events { if err != nil { return } diff --git a/mcp/sse_test.go b/mcp/sse_test.go index de10eb3..d9b7a2b 100644 --- a/mcp/sse_test.go +++ b/mcp/sse_test.go @@ -13,10 +13,13 @@ import ( "net/http" "net/http/httptest" "slices" + "strings" "sync/atomic" "testing" + "time" "github.com/google/go-cmp/cmp" + "github.com/modelcontextprotocol/go-sdk/jsonrpc" ) // TestSSEServerTransport_SupportedVersions verifies that the deprecated @@ -432,3 +435,49 @@ func TestSSEServerTransportMaxRequestBodyBytes(t *testing.T) { }) } } + +func TestSSEClientTransportBufferedEvents(t *testing.T) { + const endpoint = "event: endpoint\ndata: /messages\n\n" + const message = "event: message\ndata: {\"jsonrpc\":\"2.0\",\"method\":\"notifications/tools/list_changed\"}\n\n" + for _, coalesced := range []bool{false, true} { + t.Run(fmt.Sprintf("coalesced=%t", coalesced), func(t *testing.T) { + reader, writer := io.Pipe() + defer reader.Close() + defer writer.Close() + prefix := endpoint + if coalesced { + prefix += message + } + transport := &SSEClientTransport{ + Endpoint: "http://example.com/sse", + HTTPClient: &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": {"text/event-stream"}}, + Body: struct { + io.Reader + io.Closer + }{io.MultiReader(strings.NewReader(prefix), reader), reader}, + }, nil + })}, + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + conn, err := transport.Connect(ctx) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + if !coalesced { + go func() { _, _ = io.WriteString(writer, message) }() + } + msg, err := conn.Read(ctx) + if err != nil { + t.Fatalf("Read: %v", err) + } + if req, ok := msg.(*jsonrpc.Request); !ok || req.Method != "notifications/tools/list_changed" { + t.Fatalf("Read = %v, want tools/list_changed notification", msg) + } + }) + } +} From fbd36cb7870176bfc3e8e423df697cf110d0f927 Mon Sep 17 00:00:00 2001 From: rishabh Date: Tue, 15 Sep 2026 07:44:03 -0400 Subject: [PATCH 10/15] docs/server: fix logging defaults and the SetLoggingLevel method name (#1251) The Logging section of docs/server.md says "For stateful sessions" twice in a row with opposite meanings, so one of the two sentences has to be wrong. The default level of "info" is set in only one place in the SDK, `StreamableHTTPHandler.ephemeralConnectOpts` in mcp/streamable.go, which handles stateless requests; a session created through `Server.Connect` starts with an empty `ServerSessionState.LogLevel`, and `ServerSession.Log` returns early while the level is empty, which is exactly what the first sentence already says. The second sentence now reads "For stateless sessions". The same section also tells readers to call `SetLevel`, but there is no such exported method: the client-side call is `ClientSession.SetLoggingLevel`, and `setLevel` is only the unexported server handler, so the pkg.go.dev link on the page points at an anchor that does not exist. For context, the stateless default arrived in #411 and the paragraph describing it was written later in #497, which is where the duplicated "stateful" came from. Checked with `go doc ./mcp ClientSession` (only `SetLoggingLevel` is exported) and `grep -rn 'LogLevel = "info"' mcp/` (a single hit, mcp/streamable.go:516); docs/server.md was regenerated with `go generate ./internal/docs`, and `go build ./... && go test ./...` pass. --------- Signed-off-by: Rishabh Co-authored-by: Guglielmo Colombo --- docs/server.md | 10 +++++++--- internal/docs/server.src.md | 10 +++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/docs/server.md b/docs/server.md index d7bc314..8973e3a 100644 --- a/docs/server.md +++ b/docs/server.md @@ -1066,8 +1066,12 @@ server produces logs that remain server-side, for use by server maintainers.) **Server-side**: The minimum log level is part of the server state. For stateful sessions, there is no default log level: no log messages will be sent -until the client calls `SetLevel` (see below). -For stateful sessions, the level defaults to "info". +until the client calls `SetLoggingLevel` (see below). +For legacy stateless sessions the level defaults to "info". +For sessionless 2026-07-28 requests there is no server-side default: each request +carries its level in `_meta` (`io.modelcontextprotocol/logLevel`), and nothing is +logged until the client sends one. +(Note: logging is deprecated as of 2026-07-28 / SEP-2577.) [`ServerSession.Log`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ServerSession.Log) is the low-level way for servers to log to clients. It sends a logging notification to the client if the level of the message @@ -1083,7 +1087,7 @@ Servers always report the logging capability. **Client-side**: Set [`ClientOptions.LoggingMessageHandler`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientOptions.LoggingMessageHandler) to receive log messages. -Call [`ClientSession.SetLevel`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientSession.SetLevel) to change the log level for a session. +Call [`ClientSession.SetLoggingLevel`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientSession.SetLoggingLevel) to change the log level for a session. ```go func Example_logging() { diff --git a/internal/docs/server.src.md b/internal/docs/server.src.md index 82b956e..ff66383 100644 --- a/internal/docs/server.src.md +++ b/internal/docs/server.src.md @@ -437,8 +437,12 @@ server produces logs that remain server-side, for use by server maintainers.) **Server-side**: The minimum log level is part of the server state. For stateful sessions, there is no default log level: no log messages will be sent -until the client calls `SetLevel` (see below). -For stateful sessions, the level defaults to "info". +until the client calls `SetLoggingLevel` (see below). +For legacy stateless sessions the level defaults to "info". +For sessionless 2026-07-28 requests there is no server-side default: each request +carries its level in `_meta` (`io.modelcontextprotocol/logLevel`), and nothing is +logged until the client sends one. +(Note: logging is deprecated as of 2026-07-28 / SEP-2577.) [`ServerSession.Log`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ServerSession.Log) is the low-level way for servers to log to clients. It sends a logging notification to the client if the level of the message @@ -454,7 +458,7 @@ Servers always report the logging capability. **Client-side**: Set [`ClientOptions.LoggingMessageHandler`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientOptions.LoggingMessageHandler) to receive log messages. -Call [`ClientSession.SetLevel`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientSession.SetLevel) to change the log level for a session. +Call [`ClientSession.SetLoggingLevel`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientSession.SetLoggingLevel) to change the log level for a session. %include ../../mcp/server_example_test.go logging - From 3785c500b581ad76bc5a74e9f84d8feeb89fef27 Mon Sep 17 00:00:00 2001 From: Luca Candela <73209+CaliLuke@users.noreply.github.com> Date: Wed, 16 Sep 2026 00:22:16 -0700 Subject: [PATCH 11/15] mcp: preserve structured content number precision (#1239) CallToolResult currently decodes structuredContent through an any field, which converts JSON numbers to float64 and silently rounds integers outside the IEEE-754 safe range. This change decodes structuredContent separately with UseNumber, preserving wire numbers as json.Number while retaining the existing decoded object, array, and primitive shapes. The field documentation now describes that representation. Users who depend on the previous float64 behavior can temporarily restore it with MCPGODEBUG=structuredcontentfloat64=1. An end-to-end in-memory client/server regression covers 9007199254740993 from a low-level Server.AddTool result. Additional coverage verifies the compatibility option restores the legacy rounded float64 value. Verification: - gofmt -l . - go test ./... - go vet ./... Fixes #1236 --------- Co-authored-by: Guglielmo Colombo --- docs/mcpgodebug.md | 10 +++++++ internal/docs/mcpgodebug.src.md | 10 +++++++ internal/json/json.go | 9 ++++++ mcp/protocol.go | 22 +++++++++++++-- mcp/protocol_test.go | 20 ++++++++++++- mcp/server_test.go | 50 +++++++++++++++++++++++++++++++-- mcp/streamable_test.go | 10 +++++-- 7 files changed, 124 insertions(+), 7 deletions(-) diff --git a/docs/mcpgodebug.md b/docs/mcpgodebug.md index 57766ce..43a62ce 100644 --- a/docs/mcpgodebug.md +++ b/docs/mcpgodebug.md @@ -18,6 +18,16 @@ ## `MCPGODEBUG` history +### 1.9.0 + +Options listed below were added and will be removed in the 1.11.0 version of the SDK. + +- `structuredcontentfloat64` added. If set to `1`, numbers in + `CallToolResult.StructuredContent` are decoded as `float64`, restoring the + previous behavior. The default behavior decodes numbers as `json.Number` so + that large integers and other exact JSON values do not silently lose + precision. + ### 1.8.0 Options listed below were added and will be removed in the 1.9.0 version of the SDK. diff --git a/internal/docs/mcpgodebug.src.md b/internal/docs/mcpgodebug.src.md index fd0f255..8d1385f 100644 --- a/internal/docs/mcpgodebug.src.md +++ b/internal/docs/mcpgodebug.src.md @@ -17,6 +17,16 @@ ## `MCPGODEBUG` history +### 1.9.0 + +Options listed below were added and will be removed in the 1.11.0 version of the SDK. + +- `structuredcontentfloat64` added. If set to `1`, numbers in + `CallToolResult.StructuredContent` are decoded as `float64`, restoring the + previous behavior. The default behavior decodes numbers as `json.Number` so + that large integers and other exact JSON values do not silently lose + precision. + ### 1.8.0 Options listed below were added and will be removed in the 1.9.0 version of the SDK. diff --git a/internal/json/json.go b/internal/json/json.go index b3fa039..3a0e52b 100644 --- a/internal/json/json.go +++ b/internal/json/json.go @@ -44,6 +44,15 @@ func Unmarshal(data []byte, v any) error { return NewDecoder(bytes.NewReader(data)).Decode(v) } +func UnmarshalUseNumber(data []byte, v any) error { + if err := checkMaxDepth(data, defaultMaxDepth); err != nil { + return err + } + dec := NewDecoder(bytes.NewReader(data)) + dec.dec.UseNumber() + return dec.Decode(v) +} + // checkMaxDepth scans data once and reports [errMaxDepthExceeded] if the // nesting of JSON objects and arrays exceeds maxDepth. It is a lightweight pass which // tracks '{' and '[' against '}' and ']' while skipping over the contents of strings. diff --git a/mcp/protocol.go b/mcp/protocol.go index be37a18..c340e60 100644 --- a/mcp/protocol.go +++ b/mcp/protocol.go @@ -289,6 +289,8 @@ type CallToolResult struct { // result of the tool call. Per SEP-2106, it may marshal to any valid JSON // value (object, array, or primitive) conforming to the tool's // [Tool.OutputSchema]. + // Numbers received from the wire are represented as [json.Number] to preserve + // their exact values. // // When using a [ToolHandlerFor] with structured output, you should not // populate this field. It will be automatically populated with the typed Out @@ -379,6 +381,12 @@ func (r *CallToolResult) hasContent() bool { // An empty InputRequests with NeedsInput true indicates load-shedding. func (r *CallToolResult) NeedsInput() bool { return r.resultType == resultTypeInputRequired } +// structuredcontentfloat64 is a compatibility parameter that restores the +// previous behavior of decoding numbers in [CallToolResult.StructuredContent] +// as float64 values. By default, numbers are decoded as [json.Number] to avoid +// losing precision. The option will be removed in the 1.11.0 version of the SDK. +var structuredcontentfloat64 = mcpgodebug.Value("structuredcontentfloat64") + func (x *CallToolResult) MarshalJSON() ([]byte, error) { type res CallToolResult // avoid recursion type wire struct { @@ -401,12 +409,22 @@ func (x *CallToolResult) UnmarshalJSON(data []byte) error { type res CallToolResult // avoid recursion var wire struct { res - Content []*wireContent `json:"content"` - ResultType resultType `json:"resultType"` + Content []*wireContent `json:"content"` + StructuredContent json.RawMessage `json:"structuredContent"` + ResultType resultType `json:"resultType"` } if err := internaljson.Unmarshal(data, &wire); err != nil { return err } + if len(wire.StructuredContent) > 0 { + unmarshal := internaljson.UnmarshalUseNumber + if structuredcontentfloat64 == "1" { + unmarshal = internaljson.Unmarshal + } + if err := unmarshal(wire.StructuredContent, &wire.res.StructuredContent); err != nil { + return err + } + } var err error if wire.res.Content, err = contentsFromWire(wire.Content, nil); err != nil { return err diff --git a/mcp/protocol_test.go b/mcp/protocol_test.go index fe496de..4f0c345 100644 --- a/mcp/protocol_test.go +++ b/mcp/protocol_test.go @@ -1285,7 +1285,7 @@ func TestContentUnmarshal(t *testing.T) { Meta: Meta{"m": true}, Content: content, IsError: true, - StructuredContent: 3.0, + StructuredContent: json.Number("3"), } var gotf CallToolResult roundtrip(ctrf, &gotf) @@ -1298,6 +1298,24 @@ func TestContentUnmarshal(t *testing.T) { roundtrip(pm, &gotpm) } +func TestCallToolResultStructuredContentFloat64Compatibility(t *testing.T) { + prev := structuredcontentfloat64 + structuredcontentfloat64 = "1" + t.Cleanup(func() { structuredcontentfloat64 = prev }) + + var got CallToolResult + if err := json.Unmarshal([]byte(`{"content":[],"structuredContent":{"id":9007199254740993}}`), &got); err != nil { + t.Fatal(err) + } + structured, ok := got.StructuredContent.(map[string]any) + if !ok { + t.Fatalf("StructuredContent type = %T, want map[string]any", got.StructuredContent) + } + if got := structured["id"]; got != float64(9007199254740992) { + t.Errorf("id = %v (%T), want rounded float64 value", got, got) + } +} + func TestToolAnnotations_MarshalJSON(t *testing.T) { boolPtr := func(b bool) *bool { return &b } diff --git a/mcp/server_test.go b/mcp/server_test.go index 1544e29..2c7bbe0 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -677,7 +677,7 @@ func TestAddToolNonObjectOutputSchema(t *testing.T) { name: "primitive number (map-based schema)", outputSchema: map[string]any{"type": "number"}, content: 42.0, - want: 42.0, + want: json.Number("42"), }, { name: "primitive string (RawMessage schema)", @@ -724,6 +724,52 @@ func TestAddToolNonObjectOutputSchema(t *testing.T) { } } +func TestCallToolStructuredContentPreservesLargeInteger(t *testing.T) { + const want int64 = 9007199254740993 + + server := NewServer(testImpl, nil) + server.AddTool(&Tool{ + Name: "large-integer", + InputSchema: &jsonschema.Schema{Type: "object"}, + }, func(context.Context, *CallToolRequest) (*CallToolResult, error) { + return &CallToolResult{StructuredContent: map[string]any{"id": want}}, nil + }) + + clientTransport, serverTransport := NewInMemoryTransports() + serverSession, err := server.Connect(context.Background(), serverTransport, nil) + if err != nil { + t.Fatal(err) + } + defer serverSession.Close() + + client := NewClient(testImpl, nil) + clientSession, err := client.Connect(context.Background(), clientTransport, nil) + if err != nil { + t.Fatal(err) + } + defer clientSession.Close() + + result, err := clientSession.CallTool(context.Background(), &CallToolParams{Name: "large-integer"}) + if err != nil { + t.Fatal(err) + } + structured, ok := result.StructuredContent.(map[string]any) + if !ok { + t.Fatalf("StructuredContent type = %T, want map[string]any", result.StructuredContent) + } + number, ok := structured["id"].(json.Number) + if !ok { + t.Fatalf("id type = %T, want json.Number", structured["id"]) + } + got, err := number.Int64() + if err != nil { + t.Fatal(err) + } + if got != want { + t.Errorf("id = %d, want %d", got, want) + } +} + // TestAddToolGenericNonObjectOutput verifies SEP-2106 for the generic // AddTool[In, Out] helper: Out may be a slice or primitive whose inferred // JSON Schema has a non-object root. @@ -816,7 +862,7 @@ func TestAddToolGenericNonObjectOutput(t *testing.T) { if res.IsError { t.Fatalf("unexpected tool error: %v", res.Content) } - if diff := cmp.Diff(float64(42), res.StructuredContent); diff != "" { + if diff := cmp.Diff(json.Number("42"), res.StructuredContent); diff != "" { t.Errorf("structured content mismatch (-want +got):\n%s", diff) } }) diff --git a/mcp/streamable_test.go b/mcp/streamable_test.go index ca4d0a7..7af9dff 100644 --- a/mcp/streamable_test.go +++ b/mcp/streamable_test.go @@ -288,8 +288,14 @@ func TestStreamableConcurrentHandling(t *testing.T) { t.Errorf("CallTool failed: %v", err) return } - if got := int(res.StructuredContent.(map[string]any)["Count"].(float64)); got != i { - t.Errorf("got count %d, want %d", got, i) + number, ok := res.StructuredContent.(map[string]any)["Count"].(json.Number) + if !ok { + t.Errorf("count type = %T, want json.Number", res.StructuredContent.(map[string]any)["Count"]) + continue + } + got, err := number.Int64() + if err != nil || got != int64(i) { + t.Errorf("got count %d, %v; want %d", got, err, i) } } }) 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 12/15] 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 From b58aa8d8d8530fdc3c02573a345aa161a466659e Mon Sep 17 00:00:00 2001 From: Guglielmo Colombo Date: Fri, 18 Sep 2026 09:26:01 +0200 Subject: [PATCH 13/15] mcp: ignore notification in stdio batches (#1277) Fixes #1256 --- mcp/transport.go | 2 +- mcp/transport_test.go | 104 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/mcp/transport.go b/mcp/transport.go index ce841f2..08d3b15 100644 --- a/mcp/transport.go +++ b/mcp/transport.go @@ -734,7 +734,7 @@ func (t *ioConn) Read(ctx context.Context) (jsonrpc.Message, error) { if batch { var respBatch *msgBatch // track incoming requests in the batch for _, msg := range msgs { - if req, ok := msg.(*jsonrpc.Request); ok { + if req, ok := msg.(*jsonrpc.Request); ok && req.IsCall() { if respBatch == nil { respBatch = &msgBatch{ unresolved: make(map[jsonrpc2.ID]int), diff --git a/mcp/transport_test.go b/mcp/transport_test.go index d4d7782..a20ee5f 100644 --- a/mcp/transport_test.go +++ b/mcp/transport_test.go @@ -5,6 +5,7 @@ package mcp import ( + "bytes" "context" "errors" "fmt" @@ -171,6 +172,109 @@ func TestIOConnRead_EmptyMethod(t *testing.T) { } } +// TestIOConnReadBatchNotifications is a regression test for go-sdk#1256: +// notifications carried inside a JSON-RPC batch must not be tracked as awaiting +// a response. Only calls belong in a batch's response array, so a batch's +// notifications must neither keep it from completing nor pollute the +// per-connection duplicate-ID tracking. +func TestIOConnReadBatchNotifications(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + // input is one or more newline-separated batch payloads. + input string + // wantCalls records, for each message Read returns in order, whether it + // is a call (true) or a notification (false). Reading every message + // must succeed. + wantCalls []bool + // wantTrackedCalls is the number of in-flight call requests tracked across + // batches awaiting a response once every message has been read, before any + // response is written. + wantTrackedCalls int + // respondIDs are the call IDs to respond to, in order. + respondIDs []int64 + // wantWrites are the batch response payloads expected on the wire, each + // matched as a substring. + wantWrites []string + }{ + { + name: "call and notification is answered", + input: `[{"jsonrpc":"2.0","id":2,"method":"ping"},{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":99}}]`, + wantCalls: []bool{true, false}, + wantTrackedCalls: 1, + respondIDs: []int64{2}, + wantWrites: []string{`[{"jsonrpc":"2.0","id":2,"result":{}}]`}, + }, + { + name: "only notifications keeps the connection open", + input: `[{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":98}},{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":99}}]`, + wantCalls: []bool{false, false}, + wantTrackedCalls: 0, + }, + { + name: "repeated batches holding a notification", + input: `[{"jsonrpc":"2.0","id":2,"method":"ping"},{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":99}}]` + "\n" + + `[{"jsonrpc":"2.0","id":3,"method":"ping"},{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":99}}]`, + wantCalls: []bool{true, false, true, false}, + wantTrackedCalls: 2, + respondIDs: []int64{2, 3}, + wantWrites: []string{ + `[{"jsonrpc":"2.0","id":2,"result":{}}]`, + `[{"jsonrpc":"2.0","id":3,"result":{}}]`, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sink := new(bytes.Buffer) + tr := newIOConn(rwc{ + rc: io.NopCloser(strings.NewReader(tt.input)), + wc: nopCloserWriter{sink}, + }) + t.Cleanup(func() { tr.Close() }) + + for i, wantCall := range tt.wantCalls { + msg, err := tr.Read(ctx) + if err != nil { + t.Fatalf("Read() #%d: unexpected error: %v", i, err) + } + req, ok := msg.(*jsonrpc.Request) + if !ok { + t.Fatalf("Read() #%d = %T, want *jsonrpc.Request", i, msg) + } + if req.IsCall() != wantCall { + t.Fatalf("Read() #%d IsCall() = %t, want %t", i, req.IsCall(), wantCall) + } + } + + // Only calls are tracked as awaiting a response; notifications must + // not linger in the batch bookkeeping. + tr.batchMu.Lock() + gotTrackedCalls := len(tr.batches) + tr.batchMu.Unlock() + if gotTrackedCalls != tt.wantTrackedCalls { + t.Errorf("tracked calls = %d, want %d", gotTrackedCalls, tt.wantTrackedCalls) + } + + for _, id := range tt.respondIDs { + resp := &jsonrpc.Response{ID: jsonrpc2.Int64ID(id), Result: []byte("{}")} + if err := tr.Write(ctx, resp); err != nil { + t.Fatalf("Write(response id=%d): %v", id, err) + } + } + + got := sink.String() + for _, want := range tt.wantWrites { + if !strings.Contains(got, want) { + t.Errorf("batch responses = %q, missing %q", got, want) + } + } + }) + } +} + // TestIOConnFrameCap verifies single inbound frame cap enforcement. func TestIOConnFrameCap(t *testing.T) { tests := []struct { From 3b917b466cc540079b82bcd0fecdc46a2ba13644 Mon Sep 17 00:00:00 2001 From: sravan kilaru Date: Fri, 18 Sep 2026 02:41:05 -0500 Subject: [PATCH 14/15] oauthex: build defaultDiscoveryTransport lazily instead of at import (#1278) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1276 ## What's happening `oauthex` builds `defaultDiscoveryTransport` as a package-level var, so it runs at import time. That path calls `proxyConfigured`, which invokes `http.DefaultTransport`'s proxy func (`http.ProxyFromEnvironment`). net/http reads the proxy env vars only once and caches the result, so this import-time call freezes the proxy config before `main()` can set `HTTPS_PROXY`. New in v1.8.0, which is why it's a regression — just importing `.../mcp` (which pulls in `oauthex`) triggers it, even with no use of the SDK. ## Fix Wrap the setup in `sync.OnceValue` so it runs on first use instead of at import. Still built once and reused. ## Testing (removed) - Ran the issue's repro against this branch via a local `replace`: prints `proxy=http://127.0.0.1:9` now, `proxy=` before. - Added a regression test in `oauthex`. - `go test ./...` passes. Co-authored-by: Guglielmo Colombo --- oauthex/oauth2.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/oauthex/oauth2.go b/oauthex/oauth2.go index caed92f..0e77149 100644 --- a/oauthex/oauth2.go +++ b/oauthex/oauth2.go @@ -16,6 +16,7 @@ import ( "net/netip" "net/url" "strings" + "sync" "syscall" "time" @@ -25,7 +26,15 @@ import ( const maxDiscoveryRedirects = 10 -var defaultDiscoveryTransport = newDiscoveryTransport(http.DefaultTransport) +// Build this on first use, not when the package loads. Building it checks the +// proxy via http.ProxyFromEnvironment, and net/http only reads the proxy +// environment once and then remembers it for the whole run. If that happened +// at import time, it would lock in the proxy setting before the app had a +// chance to set HTTPS_PROXY. sync.OnceValue waits until first use and still +// builds the transport just once. See #1276. +var defaultDiscoveryTransport = sync.OnceValue(func() http.RoundTripper { + return newDiscoveryTransport(http.DefaultTransport) +}) type httpStatusError struct { StatusCode int @@ -103,7 +112,7 @@ func checkHTTPSOrLoopback(addr string) error { } func newDiscoveryClient(c *http.Client) *http.Client { - transport := defaultDiscoveryTransport + transport := defaultDiscoveryTransport() if c != nil && c.Transport != nil { // a caller that has taken over dialing or TLS opts out of the checks if t, ok := c.Transport.(*http.Transport); ok && t.DialContext == nil && t.DialTLSContext == nil { From 4608cda997aad09d9354b2f1305ed04177a23bd2 Mon Sep 17 00:00:00 2001 From: Aircg <64237451+cgair@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:42:05 +0800 Subject: [PATCH 15/15] mcp: retire only the subscriptions a listen registered (#1275) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Narrowed after review — see the discussion below. The earlier revision changed the registry keying; this one changes only the teardown.* ### The problem `subscriptionsListen` registers a session according to what the listen asked for: ```go if allowed.ToolsListChanged { s.toolChangeSubscriptions[req.Session] = requestID } if allowed.PromptsListChanged { s.promptChangeSubscriptions[req.Session] = requestID } if allowed.ResourcesListChanged { s.resourceChangeSubscriptions[req.Session] = requestID } ``` Its deferred cleanup does not: ```go delete(s.toolChangeSubscriptions, req.Session) delete(s.promptChangeSubscriptions, req.Session) delete(s.resourceChangeSubscriptions, req.Session) ``` It runs whether or not this listen ever wrote those registries. A session can hold several listens at once, so the first one to unwind takes the others' registrations with it, and those streams stay open while receiving nothing. The scope is the persistent-session transports, stdio and in-memory. Over streamable HTTP the registries cannot collide: a stateful handler refuses SEP-2575 requests outright with `CodeUnsupportedProtocolVersion`, and a stateless one builds a temporary session per POST and closes it when the request ends. Reachable through this SDK's own client, on stdio: 1. `Client.Connect` opens the auto-listen for list-changed notifications. 2. `ClientSession.Subscribe(uri)` opens a second listen, carrying only a resource subscription. 3. `ClientSession.Unsubscribe(uri)` retires that second listen. 4. Its cleanup deletes `toolChangeSubscriptions[session]`, which belongs to the auto-listen. 5. The auto-listen is still open and never receives `tools/list_changed` again. ### How I pinned it down A diagnostic run over the three states, reading the registries and `ServerSession.listenIDs` directly: ``` after connect toolSub=2 promptSub=2 resSub[r1]=false listenIDs=[{2}] after Subscribe toolSub=2 promptSub=2 resSub[r1]=true listenIDs=[{2} {3}] after Unsubscribe toolSub= promptSub= resSub[r1]=false listenIDs=[{2}] ``` Line 2 rules out the second listen having overwritten the entry: `toolSub` is still 2, listen 3 never wrote that registry. Line 3 rules out the client cancelling the wrong stream: `listenIDs` still holds listen 2, so the auto-listen is alive on the server, parked on `<-ctx.Done()`. What changed is only that its registration is gone, deleted by listen 3's teardown. ### The change Guard each delete with the condition that guarded the registration, so a listen retires what it registered and nothing else. ### Validation Two tests, next to the existing `TestSubscriptionsListen_*` ones and reusing their fixtures. Against `826e653` without the change: ``` --- FAIL: TestSubscriptionsListen_TeardownKeepsOtherListens (5.01s) mcp_test.go:3442: timed out waiting for "tool" --- PASS: TestSubscriptionsListen_TeardownRetiresOwnRegistration (0.01s) ``` The second passes before and after on purpose: it asserts that the only listen on a session does still get retired, so that the guards cannot be tightened into a registry leak. The eight pre-existing `TestSubscriptionsListen_*` tests pass in both runs, and all ten pass with the change. Run on the same commit, before and after: `go test ./...` and `go test -race ./...` ok with no races, `gofmt -l .`, `go vet ./...` and `staticcheck v0.6.1` clean. Also `go test ./...` on Go 1.25.0 and 1.26.8 to match the CI matrix, and `go generate ./internal/readme ./internal/docs` produces no diff. Environment: Intel Core i5-1038NG7, macOS 14. ### Known limitations Two cases are outside this change, both needing two listens on one session that overlap: - Two listens opting in to the same notification type. `allowed.ToolsListChanged` is true for both, so the registration overwrites and this teardown still retires the survivor's entry. - Two listens subscribed to the same URI. `resourceSubscriptions[uri][session]` overwrites the same way, and the deferred `unsubscribe` deletes it. Neither is reachable from this SDK's client — `Subscribe` dedups per URI and there is no exported way to open a second list-changed listen — so both require a non-Go client on stdio. I have tests for them and am happy to open a separate issue if you want them tracked. --- AI assistance was used for reading through the code, the diagnostic and regression tests, and running the verification above. The root cause analysis and the design decisions are mine, and I can explain every line without it. Co-authored-by: Guglielmo Colombo --- mcp/mcp_test.go | 153 ++++++++++++++++++++++++++++++++++++++++++++++++ mcp/server.go | 15 ++++- 2 files changed, 165 insertions(+), 3 deletions(-) diff --git a/mcp/mcp_test.go b/mcp/mcp_test.go index bb76c1c..7b10b30 100644 --- a/mcp/mcp_test.go +++ b/mcp/mcp_test.go @@ -3363,6 +3363,159 @@ func TestSubscriptionsListen_DisconnectScrubsMaps(t *testing.T) { } } +// newSubListenSubscribeServer is newSubListenServer with a resource and the +// handlers that make the server advertise resources.subscribe, so that a +// listen can carry a resource subscription. +func newSubListenSubscribeServer() *Server { + s := NewServer(testImpl, &ServerOptions{ + SubscribeHandler: func(context.Context, *SubscribeRequest) error { return nil }, + UnsubscribeHandler: func(context.Context, *UnsubscribeRequest) error { return nil }, + }) + AddTool(s, &Tool{Name: "t1"}, sayHi) + s.AddPrompt(&Prompt{Name: "p1"}, nil) + s.AddResource(&Resource{Name: "r1", URI: "file:///r1"}, nil) + return s +} + +func waitSubListenEvent(t *testing.T, events chan subListenEvent, kind string) subListenEvent { + t.Helper() + select { + case e := <-events: + if e.kind != kind { + t.Fatalf("got event %q, want %q", e.kind, kind) + } + return e + case <-time.After(5 * time.Second): + t.Fatalf("timed out waiting for %q", kind) + return subListenEvent{} + } +} + +// TestSubscriptionsListen_TeardownKeepsOtherListens verifies that unwinding +// one listen leaves the other listens on the same session registered. +// Unsubscribe tears down the listen that Subscribe opened for the resource; +// the auto-listen opened by Connect must keep delivering list-changed +// notifications. +func TestSubscriptionsListen_TeardownKeepsOtherListens(t *testing.T) { + events := make(chan subListenEvent, 16) + server := newSubListenSubscribeServer() + + ct, st := NewInMemoryTransports() + ss, err := server.Connect(context.Background(), st, nil) + if err != nil { + t.Fatalf("server connect: %v", err) + } + defer ss.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + cs, err := newSubListenClient(events).Connect(ctx, ct, + &ClientSessionOptions{ProtocolVersion: protocolVersion20260728}) + if err != nil { + t.Fatalf("client connect: %v", err) + } + defer cs.Close() + + autoListen := waitSubListenEvent(t, events, "ack") + + if err := cs.Subscribe(ctx, &SubscribeParams{URI: "file:///r1"}); err != nil { + t.Fatalf("Subscribe: %v", err) + } + resourceListen := waitSubListenEvent(t, events, "ack") + if resourceListen.id == autoListen.id { + t.Fatalf("Subscribe reused subscription ID %s", autoListen.id) + } + + if err := cs.Unsubscribe(ctx, &UnsubscribeParams{URI: "file:///r1"}); err != nil { + t.Fatalf("Unsubscribe: %v", err) + } + // Wait for the server to unwind that listen, observed through a registry + // the auto-listen does not appear in. + waitUntil(t, 5*time.Second, "resource listen to unwind", func() bool { + server.mu.Lock() + defer server.mu.Unlock() + _, ok := server.resourceSubscriptions["file:///r1"] + return !ok + }) + + AddTool(server, &Tool{Name: "t2"}, sayHi) + if got := waitSubListenEvent(t, events, "tool"); got.id != autoListen.id { + t.Errorf("tool notification id = %s, want auto-listen %s", got.id, autoListen.id) + } +} + +// TestSubscriptionsListen_TeardownRetiresOwnRegistration verifies that a +// listen still retires what it registered, so that per-listen tracking does +// not turn into a leak. +func TestSubscriptionsListen_TeardownRetiresOwnRegistration(t *testing.T) { + events := make(chan subListenEvent, 8) + server := newSubListenServer() + + ct, st := NewInMemoryTransports() + ss, err := server.Connect(context.Background(), st, nil) + if err != nil { + t.Fatalf("server connect: %v", err) + } + defer ss.Close() + + // A client with no list-changed handlers does not auto-listen, so the + // listen opened below is the session's only one. + c := NewClient(testImpl, nil) + c.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { + return func(ctx context.Context, method string, req Request) (Result, error) { + if method == notificationSubscriptionsAck { + events <- subListenEvent{"ack", ""} + } + return next(ctx, method, req) + } + }) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + cs, err := c.Connect(ctx, ct, &ClientSessionOptions{ProtocolVersion: protocolVersion20260728}) + if err != nil { + t.Fatalf("client connect: %v", err) + } + defer cs.Close() + + listenCtx, cancelListen := context.WithCancel(context.Background()) + defer cancelListen() + if err := cs.subscriptionsListen(listenCtx, &SubscriptionsListenParams{ + Notifications: &NotificationSubscriptions{ToolsListChanged: true, PromptsListChanged: true}, + }); err != nil { + t.Fatalf("listen: %v", err) + } + waitSubListenEvent(t, events, "ack") + + server.mu.Lock() + _, inTool := server.toolChangeSubscriptions[ss] + _, inPrompt := server.promptChangeSubscriptions[ss] + server.mu.Unlock() + if !inTool || !inPrompt { + t.Fatal("listen not registered") + } + + cancelListen() + waitUntil(t, 5*time.Second, "registrations to be retired", func() bool { + server.mu.Lock() + defer server.mu.Unlock() + _, inTool := server.toolChangeSubscriptions[ss] + _, inPrompt := server.promptChangeSubscriptions[ss] + return !inTool && !inPrompt + }) +} + +// waitUntil polls cond until it reports true, failing the test after timeout. +func waitUntil(t *testing.T, timeout time.Duration, what string, cond func() bool) { + t.Helper() + deadline := time.Now().Add(timeout) + for !cond() { + if time.Now().After(deadline) { + t.Fatalf("timed out waiting for %s", what) + } + time.Sleep(10 * time.Millisecond) + } +} + // TestSubscriptionsListen_RespectsServerCapabilities verifies that during // Connect the client only opens a SEP-2575 subscriptions/listen stream for the // change notifications the server advertised during capability negotiation. diff --git a/mcp/server.go b/mcp/server.go index b2f89f4..569abab 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -1276,10 +1276,19 @@ func (s *Server) subscriptionsListen(ctx context.Context, req *SubscriptionsList } s.mu.Unlock() defer func() { + // Retire only what this listen registered above: a session can hold + // several listens at once, and deleting by session alone retires a + // registration belonging to a listen that is still open. s.mu.Lock() - delete(s.toolChangeSubscriptions, req.Session) - delete(s.promptChangeSubscriptions, req.Session) - delete(s.resourceChangeSubscriptions, req.Session) + if allowed.ToolsListChanged { + delete(s.toolChangeSubscriptions, req.Session) + } + if allowed.PromptsListChanged { + delete(s.promptChangeSubscriptions, req.Session) + } + if allowed.ResourcesListChanged { + delete(s.resourceChangeSubscriptions, req.Session) + } s.mu.Unlock() }()