From 1c456eaacd1601f7506734771b194633e39ca63f Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Wed, 23 Jul 2025 05:33:26 -0400 Subject: [PATCH 1/2] mcp: panic if AddTool is given a nil InputSchema The spec requires an input schema for every tool. --- mcp/mcp_test.go | 2 +- mcp/server.go | 4 +--- mcp/server_test.go | 6 ++++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/mcp/mcp_test.go b/mcp/mcp_test.go index 032181a1..3c222eb4 100644 --- a/mcp/mcp_test.go +++ b/mcp/mcp_test.go @@ -659,7 +659,7 @@ func TestCancellation(t *testing.T) { return nil, nil } _, cs := basicConnection(t, func(s *Server) { - s.AddTool(&Tool{Name: "slow"}, slowRequest) + s.AddTool(&Tool{Name: "slow", InputSchema: &jsonschema.Schema{}}, slowRequest) }) defer cs.Close() diff --git a/mcp/server.go b/mcp/server.go index 75e66dbc..c91a9723 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -12,7 +12,6 @@ import ( "encoding/json" "fmt" "iter" - "log" "maps" "net/url" "path/filepath" @@ -137,8 +136,7 @@ func (s *Server) RemovePrompts(names ...string) { func (s *Server) AddTool(t *Tool, h ToolHandler) { // TODO(jba): This is a breaking behavior change. Add before v0.2.0? if t.InputSchema == nil { - log.Printf("mcp: tool %q has a nil input schema. This will panic in a future release.", t.Name) - // panic(fmt.Sprintf("adding tool %q: nil input schema", t.Name)) + panic(fmt.Sprintf("adding tool %q: nil input schema", t.Name)) } if err := addToolErr(s, t, h); err != nil { panic(err) diff --git a/mcp/server_test.go b/mcp/server_test.go index bb539772..9a36c962 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/modelcontextprotocol/go-sdk/jsonschema" ) type testItem struct { @@ -230,6 +231,7 @@ func TestServerPaginateVariousPageSizes(t *testing.T) { } func TestServerCapabilities(t *testing.T) { + tool := &Tool{Name: "t", InputSchema: &jsonschema.Schema{}} testCases := []struct { name string configureServer func(s *Server) @@ -299,7 +301,7 @@ func TestServerCapabilities(t *testing.T) { { name: "With tools", configureServer: func(s *Server) { - s.AddTool(&Tool{Name: "t"}, nil) + s.AddTool(tool, nil) }, wantCapabilities: &serverCapabilities{ Completions: &completionCapabilities{}, @@ -313,7 +315,7 @@ func TestServerCapabilities(t *testing.T) { s.AddPrompt(&Prompt{Name: "p"}, nil) s.AddResource(&Resource{URI: "file:///r"}, nil) s.AddResourceTemplate(&ResourceTemplate{URITemplate: "file:///rt"}, nil) - s.AddTool(&Tool{Name: "t"}, nil) + s.AddTool(tool, nil) }, serverOpts: ServerOptions{ SubscribeHandler: func(ctx context.Context, sp *SubscribeParams) error { From d28233011a1d8d33134c392f7cc59adf2c703dc2 Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Wed, 23 Jul 2025 14:58:56 -0400 Subject: [PATCH 2/2] mcp: panic if AddTool ... add comment --- mcp/server.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/mcp/server.go b/mcp/server.go index c91a9723..2a2b4b64 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -131,11 +131,17 @@ func (s *Server) RemovePrompts(names ...string) { } // AddTool adds a [Tool] to the server, or replaces one with the same name. -// The tool's input schema must be non-nil. // The Tool argument must not be modified after this call. +// +// The tool's input schema must be non-nil. For a tool that takes no input, +// or one where any input is valid, set [Tool.InputSchema] to the empty schema, +// &jsonschema.Schema{}. func (s *Server) AddTool(t *Tool, h ToolHandler) { - // TODO(jba): This is a breaking behavior change. Add before v0.2.0? if t.InputSchema == nil { + // This prevents the tool author from forgetting to write a schema where + // one should be provided. If we papered over this by supplying the empty + // schema, then every input would be validated and the problem wouldn't be + // discovered until runtime, when the LLM sent bad data. panic(fmt.Sprintf("adding tool %q: nil input schema", t.Name)) } if err := addToolErr(s, t, h); err != nil {