*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=<nil> promptSub=<nil> 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 <guglielmoc@google.com>
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 <guglielmoc@google.com>
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 <guglielmoc@google.com>
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.
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 <guglielmoc@google.com>
`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.
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:
<reason>`. 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 <guglielmoc@google.com>
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) <noreply@anthropic.com>
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
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
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 <arpanmondal@Arpans-MacBook-Pro.local>
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
update sse I/O code to limit the amount of memory a single
non-terminated event can take
---------
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
added for consistency with the streamable transport because sse is not
officially deprecated in the SDK
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
The doc comments on `ToolCapabilities.ListChanged` and
`ResourceCapabilities.ListChanged` say the flag reports whether **the
client** supports list-change notifications. Both types only ever appear
inside `ServerCapabilities`, though, and the spec defines the field the
other way around: whether **this server** supports notifications for
changes to the list. `PromptCapabilities` in the same file already words
it correctly.
The "client" wording looks like it was copied from `RootCapabilities`,
the one listChanged capability that genuinely belongs to the client.
Docs only, no behavior change.
Fixes#1144.
`clientMultiRoundTripMiddleware` calls
`setMultiRoundTripRetryParams(req, ...)`, which writes `InputResponses`
and `RequestState` through the caller's params pointer and never clears
them. A `*CallToolParams` (or `GetPromptParams` / `ReadResourceParams`)
struct reused across calls then carries the first call's answers into
the next call: a server gating on `len(req.Params.InputResponses) == 0`
— the idiom in this repo's own tests and examples — treats the fresh
call as already answered, so the client's `ElicitationHandler` never
fires and an elicitation-gated (e.g. destructive) tool runs without
confirmation.
This change makes the retry loop carry the fulfilled responses and
echoed state on a shallow copy of the params
(`multiRoundTripRetryRequest`) instead of mutating the original request.
Caller-owned params are now left untouched by `CallTool`, `GetPrompt`,
and `ReadResource`. The server-side shim
(`serverMultiRoundTripMiddleware`) is unchanged: the params it mutates
are SDK-owned, decoded from the wire.
Tests: `TestMultiRoundTrip_AutoRetryDoesNotMutateCallerParams`
(CallTool: params unchanged after the call, and reusing the same struct
fulfills the input requests again instead of silently replaying the
first call's answers — it failed with `elicitations after reuse = 1,
want 2` before the fix), plus GetPrompt/ReadResource analogues asserting
params are left clean. All three fail on main and pass with this change;
`go test ./...` and `go vet ./...` are clean.
Note on observable behavior: callers can no longer read the fulfilled
responses off their own params struct after a call returns. That side
effect was undocumented, and relying on it is exactly what makes the
replay hazard above possible.
---------
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
## What does this PR do?
Preserve JSON-RPC batch response arrays when a batch contains a single
request.
## Why is this needed?
A single-request legacy batch must be returned as a one-element JSON
array, but the JSON response path currently unwraps it into a single
JSON object.
## Changes
- Preserve the original batch state on the logical stream
- Keep single-request batch responses as arrays
- Add a regression test for legacy initialization batches
## Testing
- `go test ./mcp -count=1`
- `go test ./... -count=1`
- `git diff --check`
Fixes#1211
### Problem
`ServerSession` tracks the request IDs of in-flight
`subscriptions/listen` streams so `Close` can cancel the parked handlers
and avoid the jsonrpc2 drain deadlock. The ID is appended when the
listen starts, but it is never removed when the listen handler returns.
Every completed listen stream (peer cancel, unsubscribe, stream break)
therefore leaves one stale entry in the session for its entire lifetime.
On a long-lived session where a client repeatedly subscribes and
unsubscribes, the slice grows without bound: one leaked entry per cycle.
`Close` calls `jsonrpc2.Connection.Cancel` on each recorded ID, and a
stale ID is a no-op there today, but the bookkeeping is meant to hold
only in-flight listens and currently never shrinks.
### Change
Drop the request ID from the session's bookkeeping when the listen
handler returns, matching the lifecycle of the in-flight handler that
owns it.
### Tests
Three tests reproduce the leak, failing on `main` and passing here:
- `TestListenPrunedAfterSingleCompletion`
- `TestListenIDsDoNotAccumulate`
- `TestListenIDsLeakViaPublicSubscribeUnsubscribe`
The third exercises the leak through the public
`Subscribe`/`Unsubscribe` API. `go test ./mcp/` passes.
---------
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
When connecting to remote stateless servers (such as GitHub's MCP
server), optional streams like subscriptions/listen may be rejected by
middleware with a plain-text HTTP 404 before reaching a JSON-RPC
handler.
Previously, checkResponse misclassified these 404s as lost stateful
sessions (ErrSessionMissing) and tore down the transport connection
because the error didn't wrap jsonrpc2.ErrRejected.
This change aligns with the MCP 2026-07-28 Streamable HTTP
specification:
1. Conditionally check for an active session ID before returning
ErrSessionMissing, since a 404 on a stateless connection cannot mean a
session was lost.
2. Wrap subscriptions/listen failures with ErrRejected (matching
server/discover) so the jsonrpc2 layer doesn't permanently break the
transport when encountering non-compliant plain-text 404s.
---
I have manually tested this change against GitHub's hosted MCP server
and confirmed that it fixes
https://github.com/github/github-mcp-server/issues/3129
Close issued a synchronous session-termination DELETE on the connection
context and cancelled that context only after `Do` returned. A reachable
but silent peer could stall `Close()` until the TCP timeout.
Give the DELETE its own short timeout.
RED on current main: `TestCloseHangsOnBlackHoledDelete` failed with
`Close() blocked on a black-holing DELETE`.
GREEN: `go test ./mcp/ -count=1 -timeout 120s -run
'TestStreamableClient|TestCloseHangs|TestCallCancellation_FastReturn'`
Fixes#1183
Made with [Cursor](https://cursor.com)
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
## Summary
`MemoryEventStore.After` guarded only the lower bound of the requested
index:
```go
start := index + 1
if dl.first > start { // too old → ErrEventsPurged
return nil, fmt.Errorf(... ErrEventsPurged)
}
return slices.Clone(dl.data[start-dl.first:]), nil
```
There was no *upper*-bound guard, so an `index` greater than the highest
stored event made the final slice expression panic with `slice bounds
out of range`. The boundary `start-dl.first == len(dl.data)` (resume
from the last event seen) is a valid empty replay, so only
strictly-greater indices faulted.
This is reachable from untrusted input: `parseEventID` accepts any
non-negative index from the client-supplied `Last-Event-ID` header, and
`serveGET` passes it straight to `eventStore.After`. Direct callers of
the exported `EventStore.After` API crash outright.
Fixes#1131
## Change
Return an empty replay for an index at or beyond the latest stored event
— there is nothing after it — consistent with the already-valid `index
== last` case. `ErrEventsPurged` would be misleading here, since those
events were never sent rather than purged.
## Testing
Extended `TestMemoryEventStoreAfter` with indices past the end of a
stream (`3`, `100`, and one past a second stream), asserting an empty
replay with no error.
Verified the new cases **panic on the pre-fix code** (`panic: runtime
error: slice bounds out of range [3:2]`) and **pass with the fix**. `go
vet ./mcp/` and the full `go test ./mcp/` suite pass.
---------
Co-authored-by: Yaroslav Shevchuk <yarolegovich@gmail.com>
setMCPHeaders resolved Mcp-Protocol-Version from the request context
before
c.initializedResult. #1107 established the opposite order and said so in
its
description; the code it replaced also consulted the context last. The
two
branches were transposed by the switch-to-if rewrite in #1115, whose
subject
was an unrelated nil-pointer check.
The context branch serves the pre-negotiation server/discover probe,
which
sets its own value in Client.Connect. Once a version has been negotiated
it is
the authoritative one for that connection, so initializedResult should
win.
_meta.protocolVersion from the outgoing message stays on top, so #1109
stays
fixed.
The existing table built its request with http.NewRequest, so
protocolVersionFromContext returned "" in every case and the two
orderings
were indistinguishable, which is why the transposition went unnoticed.
Adds a
ctxVersion dimension and three cases pinning the precedence.
Fixes#1162
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
Fixes a request-scoping bug in the new-protocol logging path.
`ServerSession.handle` decoded
`_meta[io.modelcontextprotocol/logLevel]`, but stored it in
`ServerSessionState.LogLevel`. Concurrent new-protocol requests on the
same session could therefore clear or inherit another request's log
threshold before the original handler emitted logs.
This keeps the new-protocol log level on the handler context instead.
`ServerSession.Log` and `LoggingHandler.Enabled` read that context value
first, while legacy `logging/setLevel` behavior still uses session
state.
Tests:
- `go test ./mcp -run
TestServerLogLevelDoesNotLeakBetweenNewProtocolRequests -count=1`
- `go test -race ./mcp -run
TestServerLogLevelDoesNotLeakBetweenNewProtocolRequests -count=1`
- `go test ./mcp -run TestEndToEnd -count=1`
- `go test ./mcp -run TestValidateRequestMeta -count=1`
- `go test ./mcp -run
TestStreamableStateless_NewProtocolSession_NoFakeInit -count=1`
- `go test ./... -skip
TestSSELocalhostProtection|TestStreamableLocalhostProtection`
A full `go test ./...` run in my local environment still fails the two
localhost protection cases for `0.0.0.0 via localhost rejects evil.com`;
those same two tests also fail when run directly and are unrelated to
this logging change.
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
### What
`readResource` dereferenced the handler's result before the `if res ==
nil`
check that was meant to guard it. A `ResourceHandler` returning `(nil,
nil)`
therefore panicked instead of returning an error:
- modern clients: `handleMultiRoundTripResult` calls `res.setResultType`
on the
nil result, and
- older clients: `res.setDefaultCacheableValues()` runs on the nil
result.
Either way the `if res == nil` guard a few lines below was dead code.
### Why it matters
Request handlers run in their own goroutine and nothing recovers, so the
panic
does not just fail the one request. It terminates the whole server
process and
every concurrent session.
The sibling handlers already avoid this: `callTool` and `getPrompt` both
guard
their result with `if err == nil && res != nil` before touching it, so a
`(nil, nil)` return flows up and becomes a normal error response.
### Change
Move the nil check in `readResource` ahead of
`handleMultiRoundTripResult` and
`setDefaultCacheableValues`, so a nil result returns the existing
descriptive
error (`read handler returned nil information`) instead of panicking.
Added a
regression test that drives a resource handler returning `(nil, nil)`
through
the server and asserts a clean error.
`protcol` should be `protocol`.
The comment sits above the `Mcp-Protocol-Version` header lookup, and the
surrounding code uses `protocolVersion` and
`protocolVersionFromContext`, so this is the only place in the package
where the word is misspelled.
## Summary
`NewSSEHandler` sessions were advertising `2026-07-28` in
`server/discover` `supportedVersions`. That revision only defines stdio
and Streamable HTTP, not the deprecated HTTP+SSE binding, so
Modern-first clients could negotiate a revision the transport cannot
honour.
## Changes
- Implement `ProtocolVersionSupporter` on `SSEServerTransport` so
`filterSupportedVersions` excludes `>= 2026-07-28` (matching the
streamable transport's version gate pattern).
- In `Server.discover`, only set session `InitializeParams` when the
requested version is actually supported by the transport. Without this,
a discover probe that returns only legacy versions still marked the
session initialized, and the client's fallback `initialize` failed with
`duplicate "initialize" received`.
- Tests: unit coverage for filtered SSE versions, plus an end-to-end
Modern client → SSE fallback-to-initialize path.
Fixes#1112
## Test plan
- [x] `go test ./...`
- [x] `go test ./mcp/ -run
'TestSSE|TestInMemory_E2E_Discover|TestStreamableStateful_AcceptsDiscover|TestStreamableHTTP_E2E_Discover|TestClientConnectDiscover'`
Assistance: human-reviewed AI assistance used for implementation.
---------
Co-authored-by: arimu1 <19286898+arimu1@users.noreply.github.com>
## Summary
`ClientSessionOptions.protocolVersion` was unexported and documented as
test-only, so external consumers of `Client.Connect` could not request a
specific protocol revision.
This exports the field as `ProtocolVersion` with the same semantics:
- empty → request the latest supported version (unchanged default)
- non-empty → send that version in initialize/discover
- existing negotiate-down path is unchanged
## Usage
```go
cs, err := client.Connect(ctx, transport, &mcp.ClientSessionOptions{
ProtocolVersion: "2025-11-25",
})
```
## Testing
- `go test ./mcp/ -count=1` (pass)
Fixes#1113
Co-authored-by: arimu1 <19286898+arimu1@users.noreply.github.com>
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
Fixes#1119.
`ClientSession.ListTools` post-processes the server response through
`filterValidTools`, which has two problems on malformed input:
1. **Nil-pointer panic on `"tools":[null]`.** A `nil` `*Tool` element
reaches `validateParamHeaderAnnotations(tool)`, which dereferences
`tool.InputSchema`, crashing the client on untrusted server input.
2. **`"tools":null` is normalized into a non-nil empty slice.**
`make([]*Tool, 0, len(tools))` is always non-nil, so a nil `Tools` slice
from a malformed response is silently laundered into valid-looking data.
Reproduction:
```go
filterValidTools(nil, []*Tool{nil}) // panics: nil pointer dereference
filterValidTools(nil, nil) // returns non-nil slice (len 0), want nil
```
Fix (`mcp/streamable_headers.go`):
- Skip (log and exclude) nil tool elements instead of dereferencing
them, so a malformed `"tools":[null]` can no longer crash the client.
This is consistent with the function’s existing contract of excluding
invalid tools.
- Return early with `nil` for a nil input slice so a `"tools":null`
response is preserved as nil rather than normalized into an empty slice.
Added `TestFilterValidToolsNilHandling` covering both defects plus the
mixed valid/nil case. `go test ./mcp/` passes; `gofmt` and `go vet` are
clean.
Note on scope: this keeps the change minimal and within
`filterValidTools`’s existing signature. If maintainers would rather
surface a hard protocol error for a malformed `"tools":null` /
`"tools":[null]` response at the `ListTools` level (rather than
tolerating and filtering it), I’m happy to follow up with that larger
change.
`writeEvent` built the SSE frame with `fmt.Fprintf(&b, "data: %s\n\n",
string(evt.Data))`, which copies the payload into a fresh string and
then again into the buffer, growing it via repeated `bytes.growSlice`
doublings. For large MCP responses (e.g. ~162KB `tools/list`) under a
burst of concurrent writes this ~3x amplifies allocations and peak heap.
Write the payload directly into a pre-grown buffer instead. Output
framing is byte-identical. Benchmarks (162KB payload): 517KB->172KB per
op, 9->4 allocs/op, ~3x faster.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Yaroslav <yarolegovich@gmail.com>
`streamableClientConn.setMCPHeaders` sets `Mcp-Protocol-Version` only
from `c.initializedResult`, which is populated exclusively through the
unexported `clientConnection.sessionUpdated` hook. `Client.Connect`
reaches that hook via a type assertion on the `Connection` returned by
the `Transport`:
```go
if hc, ok := cs.mcpConn.(clientConnection); ok {
hc.sessionUpdated(cs.state)
}
```
That assertion silently fails whenever the `Connection` is wrapped by
another type that exposes only the exported `Connection` interface.
`c.initializedResult` then stays `nil` for the life of the session, and
every `SEP-2575` (>= `2026-07-28`) request sent through that connection
is missing the header. Servers that validate the header per spec reject
the request with `400 Bad Request`, closing the connection.
Every SEP-2575 request already carries its own `_meta.protocolVersion`
field via `injectRequestMeta`. `setMCPHeaders` now reads the protocol
version from the outgoing message first, falling back to
`c.initializedResult` and then the request context.
This is currently breaking the GitHub MCP with the 1.7-pre releases of
this module.
Fixes#1109
Co-authored-by: eu.anthropic.claude-sonnet-5 via Crush <charmcrush@users.noreply.github.com>
## Summary
- Add `resultType: "complete"` to non-MRTR server result responses for
2026-07-28 requests.
- Cover `server/discover`, list methods, and completion responses, and
update expected conformance/example output.
## Why
Fixes#1032. The 2026-07-28 schema requires `resultType` on result
objects, but only the MRTR-capable results were setting it.
## Validation
- `go test ./mcp -run
'Test(ServerSessionHandle|CompleteResult|ClientConnectDiscover|InMemory_E2E_Discover)'
-count=1` — passed\n- `go test ./mcp -count=1` — passed\n- `go test
./...` — passed\n- `go vet ./...` — passed
---------
Co-authored-by: ychampion <ychampion@users.noreply.github.com>
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
