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] 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",