*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>
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>
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>
### 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>
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.
## 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>
## Description
This PR introduces support for custom (non-standard) JSON-RPC methods in
the MCP Go SDK.
This feature enables servers to handle arbitrary JSON-RPC methods and
clients to invoke them, all while maintaining full type safety and
integrating seamlessly with the SDK's existing middleware and session
management.
- Server-Side Registration: Added AddReceivingCustomMethod to allow
servers to register handlers for custom methods. Incoming requests are
automatically unmarshaled into user-defined parameter structs.
- Client-Side Calling: Added AddSendingCustomMethod which returns a
strongly-typed caller function that clients can use to invoke custom
methods and receive strongly-typed results.
- Helper Structs: Provided ParamsBase and ResultBase for users to embed
in their custom structs. This allows them to easily satisfy the SDK's
internal Params and Result interfaces.
Fixes#954
This PR adds the protocol version `2026-07-28` to the list of supported
protocol versions and fixes legacy tests hardcoding the legacy protocol
version for those use cases.
## Summary
Part 3 of the SEP-2575 ("Make MCP Stateless") implementation. Builds on
PR #975 which landed the client-side discover/fallback flow and the
per-request `_meta` plumbing. This PR adds the **server-side**
counterparts:
- `server/discover` handler (replaces the
stub),ProtocolVersionSupporter`
- transport hook for per-transport version filtering,
- transport-agnostic version validation and method-removal enforcement
in `ServerSession.handle`,
- HTTP-level wire compliance for SEP-2575 error codes/status codes,
- bug fix in `internal/jsonrpc2`,`processResult` with an `MCPGODEBUG`
opt-out for backward compatibility.
Implements the client half of the SEP-2575 / `2026-06-30` protocol. This
CL is client-only; server-side `server/discover` is still a
`MethodNotFound` stub.
### Changes
- **`server/discover` probe in `Client.Connect()`** (`client.go`). When
the caller opts into `protocolVersion >= 2026-06-30`, the client probes
the server with `server/discover` before falling back to the legacy
`initialize`
handshake.
- **Per-request `_meta` injection** (`shared.go`, `injectRequestMeta`).
For`>= 2026-06-30` sessions every outgoing request gets the SEP-2575
triple (`protocolVersion`, `clientInfo`, `clientCapabilities`).
- **Server-side fix**: `ServerSession.handle` no longer pre-populates
`state.InitializeParams` from a `server/discover` request's `_meta`, so
a subsequent fallback `initialize` is not rejected as a duplicate.
### Fallback rules
Client falls back to the legacy `initialize` handshake when discover
returns:
- `MethodNotFound` (-32601),
- `UnsupportedProtocolVersionError` (-32004),
- an HTTP `Bad Request` (substring-matched on the error message, to
interop
with pre-2026-06-30 servers).
Fixes#966
mcp: add configurable keepalive failure threshold
Introduce `KeepAliveFailureThreshold` option in both `ClientOptions` and
`ServerOptions` to control how many consecutive keepalive ping failures
are tolerated before closing the session.
This aligns with the MCP spec's guidance that "multiple failed pings MAY
trigger a connection reset," allowing operators to tune resilience
against transient network hiccups without immediately tearing down
otherwise healthy sessions.
A threshold of 0 or 1 (the default) closes on the first failure,
preserving existing behavior. Higher values let isolated misses pass
while still closing the session once consecutive failures reach the
threshold. A successful ping resets the counter.
Tolerated failures are logged at WARN level; the final failure that
closes the session is logged at ERROR level.
This is rework of #979.
### Context
Design (`design/mrtr.md`) and implementation proposal for Multi
Round-Trip Requests (MRTR) per
[SEP-2322](de6d76fba3/seps/2322-MRTR.md).
### Changes
- Introduce `InputRequest` and `InputResponse` sealed interfaces with
corresponding `InputRequestMap` and `InputResponseMap` types for custom
map JSON codec implementation.
- Extend `CallToolParams`, `GetPromptParams`, and `ReadResourceParams`
with `InputResponses`, `RequestState` and an unexported `resultType`
("complete" or "input_required") for retry round-trips. The type is set
by the SDK based on input requests presence.
- Add client-side MRTR middleware enabled by default that automatically
fulfills input requests and retries.
- Added MRTROptions on ClientOptions to allow disabling the middleware
and configuring the number of retries.
- Add server-side MRTR middleware for backward compatibility:
transparently bridges input requests to direct
Elicit/CreateMessage/ListRoots calls for older clients
## Description
This PR lays the foundational server-side groundwork for the `>=
2026-06-30` sessionless and stateless feature introduced by
[SEP-2575](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2575)
and SEP-2567, tracked in
[design/stateless.md](614460a225/design/stateless.md).
### SEP-2575: Stateless MCP
* **Per-request protocol detection in `ServerSession.handle()`**
Unmarshal `_meta` from the raw JSON-RPC params determines whether each
request follows the new protocol.
* **Per-request typed accessors on `ServerRequest[P]`**
```go
func (r *ServerRequest[P]) ProtocolVersion() string
func (r *ServerRequest[P]) ClientInfo() *Implementation
func (r *ServerRequest[P]) ClientCapabilities() *ClientCapabilities
```
* **Reject client->server `initialize`, `initialized` and `ping` for
new-protocol requests**
* **Per-request `_meta` field name constants**
Three constants for the wire-protocol field names
(`MetaKeyProtocolVersion`, `MetaKeyClientInfo`,
`MetaKeyClientCapabilities`)
* **Stop synthesizing fake `InitializeParams` for new-protocol
requests**
Fixes: #966
## Summary
- reject a second `initialize` request on an already initialized server
session
- keep the original `ServerSession.InitializeParams()` instead of
replacing it with later client parameters
- add a raw JSON-RPC regression test for the duplicate-initialize path
Fixes#961.
## To verify
- `go test ./mcp -run TestServerRejectsDuplicateInitialize -count=1`
- `go test ./mcp -count=1`
- `go test ./...`
---------
Co-authored-by: guglielmoc <guglielmoc@google.com>
## Bug
`mcp.AddTool` crashes the server with SIGSEGV when input/output struct
tags trigger a nil schema.
## Root Cause
In `mcp/server.go`, the function `setSchema` calls
`internalSchema.Resolve(...)` without checking if `internalSchema` is
nil. If the schema inference (e.g., via `jsonschema.ForType`) or a
provided schema results in a nil value, this dereference causes a panic.
## Fix
Added nil checks for `internalSchema` before calling `Resolve` in two
locations within `setSchema`:
1. After schema generation via `jsonschema.ForType`.
2. After processing a provided schema (which might be nil or fail to
unmarshal).
If `internalSchema` is nil, the function now returns a descriptive error
instead of crashing.
## Verification
- `go test ./mcp/...` passes.
- The fix prevents the crash by handling the nil case gracefully.
Closes#916.
---------
Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
## Summary
Two error sites in the MCP runtime were previously unobservable:
- **`mcp/shared.go` `startKeepalive`** — when a keepalive ping failed,
the session was closed silently with no indication of why.
- **`mcp/transport.go` `connect`** — `jsonrpc2.OnInternalError` printed
via the package-level `log.Printf`, bypassing the `*slog.Logger`
configured on `ServerOptions`/`ClientOptions`.
Both sites now report through the existing `*slog.Logger` that `Server`
and `Client` already guarantee non-nil via `ensureLogger`. The logger is
threaded into the unexported `startKeepalive` and `connect` helpers as a
parameter — **no new public API surface**.
## Context
Per @jba's [suggestion on
#865](https://github.com/modelcontextprotocol/go-sdk/pull/865#issuecomment-4156166474):
> Maybe we can just use the logger in all the places we currently drop
the error. Then it's still reported, but we don't have to add any API.
This PR is that smaller alternative. It addresses the concrete
observability gap in #218 without expanding `ServerOptions`. PR #865
(the `ErrorHandler` callback proposal) remains open pending community
input on whether a structured callback API is also wanted on top of
logging.
`notifySessions` already logs at `Warn` level on delivery errors and is
intentionally left untouched.
## Test plan
- [x] New regression test `TestKeepAliveFailure_Logged` asserts the
keepalive failure produces a log line on the configured logger.
- [x] Verified the test fails when the new `logger.Error(...)` call is
removed (sanity check that it pins the behavior).
- [x] `go test ./mcp/... -count=1` passes.
- [x] `go vet ./...` clean.
- [x] `go build ./...` clean.
Refs #218
---------
Co-authored-by: Maciej Kisiel <mkisiel@google.com>
This PR fixes a race condition where `ServerSession.keepaliveCancel` was
accessed in `Close` concurrently with its initialization in
`ServerSession.startKeepalive`. It also removes a now redundant test.
The fix moves keepalive initialization into `ServerSession.Connect`
similar to the implementation in `ClientSession.Connect`. This is safe
because the MCP spec allows keepalive pings before the initialization
messages. From the MCP
[spec](https://modelcontextprotocol.io/specification/2025-11-25/basic/lifecycle)
(emphasis added):
> The client SHOULD NOT send requests **other than pings** before the
server has responded to the initialize request.
The PR includes a reproducer test case, which fails with the following
error without this fix:
```
> go test -count=10 -race -run ^TestStreamableStatelessKeepaliveRace$ github.com/modelcontextprotocol/go-sdk/mcp 8s
==================
WARNING: DATA RACE
Write at 0x00c00007fbf0 by goroutine 1095:
github.com/modelcontextprotocol/go-sdk/mcp.startKeepalive()
/Users/benjamin/Documents/go-sdk/mcp/shared.go:590 +0x58
github.com/modelcontextprotocol/go-sdk/mcp.(*ServerSession).startKeepalive()
/Users/benjamin/Documents/go-sdk/mcp/server.go:1526 +0x130
github.com/modelcontextprotocol/go-sdk/mcp.(*ServerSession).initialized()
/Users/benjamin/Documents/go-sdk/mcp/server.go:1059 +0xf8
github.com/modelcontextprotocol/go-sdk/mcp.init.serverSessionMethod[go.shape.*uint8,go.shape.interface { GetMeta() map[string]interface {}; SetMeta(map[string]interface {}); github.com/modelcontextprotocol/go-sdk/mcp.isResult() }].func27()
/Users/benjamin/Documents/go-sdk/mcp/shared.go:332 +0x98
github.com/modelcontextprotocol/go-sdk/mcp.newServerMethodInfo[go.shape.*github.com/modelcontextprotocol/go-sdk/mcp.InitializedParams,go.shape.interface { GetMeta() map[string]interface {}; SetMeta(map[string]interface {}); github.com/modelcontextprotocol/go-sdk/mcp.isResult() },go.shape.struct { github.com/modelcontextprotocol/go-sdk/mcp.Meta "json:\"_meta,omitempty\"" }].func2()
/Users/benjamin/Documents/go-sdk/mcp/shared.go:272 +0x84
github.com/modelcontextprotocol/go-sdk/mcp.defaultReceivingMethodHandler[go.shape.*uint8]()
/Users/benjamin/Documents/go-sdk/mcp/shared.go:152 +0xe8
github.com/modelcontextprotocol/go-sdk/mcp.defaultReceivingMethodHandler[*github.com/modelcontextprotocol/go-sdk/mcp.ServerSession]()
/Users/benjamin/Documents/go-sdk/mcp/shared.go:146 +0x60
github.com/modelcontextprotocol/go-sdk/mcp.handleReceive[go.shape.*uint8]()
/Users/benjamin/Documents/go-sdk/mcp/shared.go:169 +0x228
github.com/modelcontextprotocol/go-sdk/mcp.(*ServerSession).handle()
/Users/benjamin/Documents/go-sdk/mcp/server.go:1445 +0x370
github.com/modelcontextprotocol/go-sdk/mcp.connect[go.shape.*uint8,go.shape.*uint8].func1.1()
/Users/benjamin/Documents/go-sdk/mcp/transport.go:170 +0x5c
github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2.HandlerFunc.Handle()
/Users/benjamin/Documents/go-sdk/internal/jsonrpc2/jsonrpc2.go:84 +0x48
github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2.(*Connection).handleAsync.func3()
/Users/benjamin/Documents/go-sdk/internal/jsonrpc2/conn.go:717 +0xd8
Previous read at 0x00c00007fbf0 by goroutine 1089:
github.com/modelcontextprotocol/go-sdk/mcp.(*ServerSession).Close()
/Users/benjamin/Documents/go-sdk/mcp/server.go:1502 +0x34
github.com/modelcontextprotocol/go-sdk/mcp.(*StreamableHTTPHandler).ServeHTTP.deferwrap1()
/Users/benjamin/Documents/go-sdk/mcp/streamable.go:519 +0x34
runtime.deferreturn()
/opt/homebrew/Cellar/go/1.25.4/libexec/src/runtime/panic.go:589 +0x5c
github.com/modelcontextprotocol/go-sdk/mcp.TestStreamableStatelessKeepaliveRace.mustNotPanic.func2()
/Users/benjamin/Documents/go-sdk/mcp/streamable_test.go:2156 +0x94
net/http.HandlerFunc.ServeHTTP()
/opt/homebrew/Cellar/go/1.25.4/libexec/src/net/http/server.go:2322 +0x48
net/http.serverHandler.ServeHTTP()
/opt/homebrew/Cellar/go/1.25.4/libexec/src/net/http/server.go:3340 +0x270
net/http.(*conn).serve()
/opt/homebrew/Cellar/go/1.25.4/libexec/src/net/http/server.go:2109 +0x9b0
net/http.(*Server).Serve.gowrap3()
/opt/homebrew/Cellar/go/1.25.4/libexec/src/net/http/server.go:3493 +0x48
Goroutine 1095 (running) created at:
github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2.(*Connection).handleAsync()
/Users/benjamin/Documents/go-sdk/internal/jsonrpc2/conn.go:715 +0x334
github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2.(*Connection).acceptRequest.func2.gowrap1()
/Users/benjamin/Documents/go-sdk/internal/jsonrpc2/conn.go:676 +0x34
Goroutine 1089 (running) created at:
net/http.(*Server).Serve()
/opt/homebrew/Cellar/go/1.25.4/libexec/src/net/http/server.go:3493 +0x5dc
net/http/httptest.(*Server).goServe.func1()
/opt/homebrew/Cellar/go/1.25.4/libexec/src/net/http/httptest/server.go:311 +0x98
==================
--- FAIL: TestStreamableStatelessKeepaliveRace (0.14s)
testing.go:1617: race detected during execution of test
FAIL
FAIL github.com/modelcontextprotocol/go-sdk/mcp 1.727s
FAIL
```
---------
Co-authored-by: Maciej Kisiel <mkisiel@google.com>
This implements [SEP-1303](https://github.com/modelcontextprotocol/modelcontextprotocol/issues/1303).
Input validation errors (missing required fields, wrong types) during
tool calls
are now returned as `CallToolResult` with `IsError=true` instead of
JSON-RPC
`-32602` errors. This allows LLMs to see the validation error and
self-correct,
per the MCP spec.
The existing pattern for handler-returned errors (lines 347-350 in
server.go)
already converts regular errors to tool results with `IsError=true`.
This change
applies the same pattern to argument schema validation and unmarshal
errors,
resolving the `TODO(#450)` in the code.
Thanks to @iwyrkore for filing the issue and proposing the approach, and
@findleyr for the cross-SDK analysis in the discussion.
Fixes#450
Co-authored-by: Maciej Kisiel <mkisiel@google.com>
1. (non-test) Don't queue notifications if there are no connected
sessions.
2. Change `reconnectInitialDelay` to be atomic, as it is modified in
tests.
3. Protect map access with mutex in `TestServerTransportCleanup`.
As reported in
https://github.com/modelcontextprotocol/go-sdk/issues/805, case
insensitive unmarshaling can pose security issues. This change adjusts
all non-test and non-example unmarshaling to use a case sensitive mode.
This introduces a new external dependency
`github.com/segmentio/encoding`.
Summary
Add support for tool use within sampling requests, as described in the
MCP 2025-11-25 spec's sampling.tools capability.
New types
Content types:
- ToolUseContent — represents a tool invocation request from the
assistant
- ToolResultContent — represents the result of a tool invocation, with
nested []Content for the result blocks
Capability types:
- SamplingCapabilities gains Tools and Context sub-fields
- SamplingToolsCapabilities, SamplingContextCapabilities
Tool-enabled sampling (parallel tool calls):
- CreateMessageWithToolsParams with SamplingMessageV2 (supports array
content)
- CreateMessageWithToolsResult (supports array content)
- ToolChoice — controls tool invocation mode (auto, required, none)
- ServerSession.CreateMessageWithTools
- ClientOptions.CreateMessageWithToolsHandler
Design
Following the TypeScript SDK's pattern, tool-enabled sampling uses
separate types from basic sampling to avoid breaking the existing API.
The basic CreateMessage/CreateMessageResult path
is unchanged.
Both paths share the same wire method (sampling/createMessage) and go
through the method info table. The table uses the broader
CreateMessageWithToolsResult type internally;
CreateMessage downconverts (erroring if multiple content blocks are
returned). This is documented as a rough edge to unify in v2.
Setting CreateMessageWithToolsHandler automatically infers the
sampling.tools capability. It is a panic to set both
CreateMessageHandler and CreateMessageWithToolsHandler.
References
-
https://modelcontextprotocol.io/specification/2025-11-25/client/sampling
- TypeScript SDK: CreateMessageResultWithToolsSchema /
CreateMessageRequestParamsWithTools
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
