diff --git a/auth/shared.go b/auth/shared.go index e58d438..8c5e10c 100644 --- a/auth/shared.go +++ b/auth/shared.go @@ -44,12 +44,7 @@ func authorizationServerMetadataURLs(issuerURL string) []string { return nil } - // RFC 8414, section 3.1 requires any terminating "/" to be removed from the - // issuer identifier before the well-known suffix is inserted, so an issuer - // such as "https://auth.example.com/" has no path component at all. - issuerPath := strings.Trim(baseURL.Path, "/") - - if issuerPath == "" { + if baseURL.Path == "" { // "OAuth 2.0 Authorization Server Metadata". baseURL.Path = "/.well-known/oauth-authorization-server" urls = append(urls, baseURL.String()) @@ -59,14 +54,15 @@ func authorizationServerMetadataURLs(issuerURL string) []string { return urls } + originalPath := baseURL.Path // "OAuth 2.0 Authorization Server Metadata with path insertion". - baseURL.Path = "/.well-known/oauth-authorization-server/" + issuerPath + baseURL.Path = "/.well-known/oauth-authorization-server/" + strings.TrimLeft(originalPath, "/") urls = append(urls, baseURL.String()) // "OpenID Connect Discovery 1.0 with path insertion". - baseURL.Path = "/.well-known/openid-configuration/" + issuerPath + baseURL.Path = "/.well-known/openid-configuration/" + strings.TrimLeft(originalPath, "/") urls = append(urls, baseURL.String()) // "OpenID Connect Discovery 1.0 with path appending". - baseURL.Path = "/" + issuerPath + "/.well-known/openid-configuration" + baseURL.Path = "/" + strings.Trim(originalPath, "/") + "/.well-known/openid-configuration" urls = append(urls, baseURL.String()) return urls diff --git a/auth/shared_test.go b/auth/shared_test.go index 34e0a35..16b5054 100644 --- a/auth/shared_test.go +++ b/auth/shared_test.go @@ -6,81 +6,11 @@ package auth import ( "net/http" - "slices" "testing" "github.com/modelcontextprotocol/go-sdk/internal/oauthtest" ) -func TestAuthorizationServerMetadataURLs(t *testing.T) { - tests := []struct { - issuerURL string - want []string - }{ - { - issuerURL: "https://auth.example.com", - want: []string{ - "https://auth.example.com/.well-known/oauth-authorization-server", - "https://auth.example.com/.well-known/openid-configuration", - }, - }, - { - issuerURL: "https://auth.example.com/", - want: []string{ - "https://auth.example.com/.well-known/oauth-authorization-server", - "https://auth.example.com/.well-known/openid-configuration", - }, - }, - { - issuerURL: "https://auth.example.com/tenant1", - want: []string{ - "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", - "https://auth.example.com/.well-known/openid-configuration/tenant1", - "https://auth.example.com/tenant1/.well-known/openid-configuration", - }, - }, - { - issuerURL: "https://auth.example.com/tenant1/", - want: []string{ - "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", - "https://auth.example.com/.well-known/openid-configuration/tenant1", - "https://auth.example.com/tenant1/.well-known/openid-configuration", - }, - }, - } - - for _, tt := range tests { - t.Run(tt.issuerURL, func(t *testing.T) { - if got := authorizationServerMetadataURLs(tt.issuerURL); !slices.Equal(got, tt.want) { - t.Errorf("authorizationServerMetadataURLs(%q) =\n%v\nwant\n%v", tt.issuerURL, got, tt.want) - } - }) - } -} - -// TestGetAuthServerMetadataTrailingSlashIssuer checks that an issuer identifier -// written with a terminating slash still finds metadata served at the root -// well-known path. -func TestGetAuthServerMetadataTrailingSlashIssuer(t *testing.T) { - s := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{ - MetadataEndpointConfig: &oauthtest.MetadataEndpointConfig{ - ServeOAuthInsertedEndpoint: true, - }, - }) - s.Start(t) - - got, err := GetAuthServerMetadata(t.Context(), s.URL()+"/", http.DefaultClient) - if err != nil { - t.Fatalf("GetAuthServerMetadata() error = %v, want nil", err) - } - if got == nil { - t.Fatal("GetAuthServerMetadata() got nil, want metadata") - } - if got.Issuer != s.URL() { - t.Errorf("GetAuthServerMetadata() issuer = %q, want %q", got.Issuer, s.URL()) - } -} - func TestGetAuthServerMetadata(t *testing.T) { tests := []struct { name string diff --git a/docs/client.md b/docs/client.md index 23d57be..ad71c11 100644 --- a/docs/client.md +++ b/docs/client.md @@ -304,7 +304,7 @@ func Example_elicitation() { `ElicitParams.RequestedSchema` is a flat schema of primitive fields, which the client renders as a form. Two field keywords shape that form. -A `Default` ([SEP-1034](https://modelcontextprotocol.io/seps/1034--support-default-values-for-all-primitive-types-in)) prefills a +A `Default` ([SEP-1034](https://modelcontextprotocol.io/seps/1034)) prefills a field. When the user accepts without supplying it, the SDK fills the field in from the schema before the result reaches either side's caller — the client does so after its elicitation handler returns, and `ServerSession.Elicit` does @@ -313,7 +313,7 @@ defaulted field `Required` defeats it: accepted content is validated against the schema before defaults are applied, so an answer that omits the field is rejected rather than defaulted. -An `Enum` ([SEP-1330](https://modelcontextprotocol.io/seps/1330-elicitation-enum-schema-improvements-and-standards)) restricts a +An `Enum` ([SEP-1330](https://modelcontextprotocol.io/seps/1330)) restricts a field to a fixed set of values, which the client renders as a choice. Enums are supported only on `"string"` fields; declaring one on another type is rejected. To label the choices, set the legacy `enumNames` keyword through diff --git a/docs/mcpgodebug.md b/docs/mcpgodebug.md index 43a62ce..57766ce 100644 --- a/docs/mcpgodebug.md +++ b/docs/mcpgodebug.md @@ -18,16 +18,6 @@ ## `MCPGODEBUG` history -### 1.9.0 - -Options listed below were added and will be removed in the 1.11.0 version of the SDK. - -- `structuredcontentfloat64` added. If set to `1`, numbers in - `CallToolResult.StructuredContent` are decoded as `float64`, restoring the - previous behavior. The default behavior decodes numbers as `json.Number` so - that large integers and other exact JSON values do not silently lose - precision. - ### 1.8.0 Options listed below were added and will be removed in the 1.9.0 version of the SDK. diff --git a/docs/server.md b/docs/server.md index 8973e3a..d7bc314 100644 --- a/docs/server.md +++ b/docs/server.md @@ -1066,12 +1066,8 @@ server produces logs that remain server-side, for use by server maintainers.) **Server-side**: The minimum log level is part of the server state. For stateful sessions, there is no default log level: no log messages will be sent -until the client calls `SetLoggingLevel` (see below). -For legacy stateless sessions the level defaults to "info". -For sessionless 2026-07-28 requests there is no server-side default: each request -carries its level in `_meta` (`io.modelcontextprotocol/logLevel`), and nothing is -logged until the client sends one. -(Note: logging is deprecated as of 2026-07-28 / SEP-2577.) +until the client calls `SetLevel` (see below). +For stateful sessions, the level defaults to "info". [`ServerSession.Log`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ServerSession.Log) is the low-level way for servers to log to clients. It sends a logging notification to the client if the level of the message @@ -1087,7 +1083,7 @@ Servers always report the logging capability. **Client-side**: Set [`ClientOptions.LoggingMessageHandler`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientOptions.LoggingMessageHandler) to receive log messages. -Call [`ClientSession.SetLoggingLevel`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientSession.SetLoggingLevel) to change the log level for a session. +Call [`ClientSession.SetLevel`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientSession.SetLevel) to change the log level for a session. ```go func Example_logging() { diff --git a/examples/server/middleware/main.go b/examples/server/middleware/main.go index 262d726..4e37471 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", mcp.HasParams(req), + "has_params", req.GetParams() != nil, ) // Log more for tool calls. if ctr, ok := req.(*mcp.CallToolRequest); ok { diff --git a/internal/docs/client.src.md b/internal/docs/client.src.md index c0f64b4..fdaef2a 100644 --- a/internal/docs/client.src.md +++ b/internal/docs/client.src.md @@ -99,7 +99,7 @@ pattern. `ElicitParams.RequestedSchema` is a flat schema of primitive fields, which the client renders as a form. Two field keywords shape that form. -A `Default` ([SEP-1034](https://modelcontextprotocol.io/seps/1034--support-default-values-for-all-primitive-types-in)) prefills a +A `Default` ([SEP-1034](https://modelcontextprotocol.io/seps/1034)) prefills a field. When the user accepts without supplying it, the SDK fills the field in from the schema before the result reaches either side's caller — the client does so after its elicitation handler returns, and `ServerSession.Elicit` does @@ -108,7 +108,7 @@ defaulted field `Required` defeats it: accepted content is validated against the schema before defaults are applied, so an answer that omits the field is rejected rather than defaulted. -An `Enum` ([SEP-1330](https://modelcontextprotocol.io/seps/1330-elicitation-enum-schema-improvements-and-standards)) restricts a +An `Enum` ([SEP-1330](https://modelcontextprotocol.io/seps/1330)) restricts a field to a fixed set of values, which the client renders as a choice. Enums are supported only on `"string"` fields; declaring one on another type is rejected. To label the choices, set the legacy `enumNames` keyword through diff --git a/internal/docs/mcpgodebug.src.md b/internal/docs/mcpgodebug.src.md index 8d1385f..fd0f255 100644 --- a/internal/docs/mcpgodebug.src.md +++ b/internal/docs/mcpgodebug.src.md @@ -17,16 +17,6 @@ ## `MCPGODEBUG` history -### 1.9.0 - -Options listed below were added and will be removed in the 1.11.0 version of the SDK. - -- `structuredcontentfloat64` added. If set to `1`, numbers in - `CallToolResult.StructuredContent` are decoded as `float64`, restoring the - previous behavior. The default behavior decodes numbers as `json.Number` so - that large integers and other exact JSON values do not silently lose - precision. - ### 1.8.0 Options listed below were added and will be removed in the 1.9.0 version of the SDK. diff --git a/internal/docs/server.src.md b/internal/docs/server.src.md index ff66383..82b956e 100644 --- a/internal/docs/server.src.md +++ b/internal/docs/server.src.md @@ -437,12 +437,8 @@ server produces logs that remain server-side, for use by server maintainers.) **Server-side**: The minimum log level is part of the server state. For stateful sessions, there is no default log level: no log messages will be sent -until the client calls `SetLoggingLevel` (see below). -For legacy stateless sessions the level defaults to "info". -For sessionless 2026-07-28 requests there is no server-side default: each request -carries its level in `_meta` (`io.modelcontextprotocol/logLevel`), and nothing is -logged until the client sends one. -(Note: logging is deprecated as of 2026-07-28 / SEP-2577.) +until the client calls `SetLevel` (see below). +For stateful sessions, the level defaults to "info". [`ServerSession.Log`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ServerSession.Log) is the low-level way for servers to log to clients. It sends a logging notification to the client if the level of the message @@ -458,7 +454,7 @@ Servers always report the logging capability. **Client-side**: Set [`ClientOptions.LoggingMessageHandler`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientOptions.LoggingMessageHandler) to receive log messages. -Call [`ClientSession.SetLoggingLevel`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientSession.SetLoggingLevel) to change the log level for a session. +Call [`ClientSession.SetLevel`](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk/mcp#ClientSession.SetLevel) to change the log level for a session. %include ../../mcp/server_example_test.go logging - diff --git a/internal/json/json.go b/internal/json/json.go index 3a0e52b..b3fa039 100644 --- a/internal/json/json.go +++ b/internal/json/json.go @@ -44,15 +44,6 @@ func Unmarshal(data []byte, v any) error { return NewDecoder(bytes.NewReader(data)).Decode(v) } -func UnmarshalUseNumber(data []byte, v any) error { - if err := checkMaxDepth(data, defaultMaxDepth); err != nil { - return err - } - dec := NewDecoder(bytes.NewReader(data)) - dec.dec.UseNumber() - return dec.Decode(v) -} - // checkMaxDepth scans data once and reports [errMaxDepthExceeded] if the // nesting of JSON objects and arrays exceeds maxDepth. It is a lightweight pass which // tracks '{' and '[' against '}' and ']' while skipping over the contents of strings. diff --git a/internal/jsonrpc2/conn.go b/internal/jsonrpc2/conn.go index 94da6ae..5849a87 100644 --- a/internal/jsonrpc2/conn.go +++ b/internal/jsonrpc2/conn.go @@ -453,19 +453,12 @@ func (ac *AsyncCall) Await(ctx context.Context, result any) error { // will not cause any messages that have not arrived yet with that ID to be // cancelled. func (c *Connection) Cancel(id ID) { - c.CancelCause(id, nil) -} - -// CancelCause is like [Connection.Cancel], but records cause as the reason -// the Context was cancelled, so that the Handle call can read it back through -// [context.Cause]. A nil cause reads as [context.Canceled]. -func (c *Connection) CancelCause(id ID, cause error) { var req *incomingRequest c.updateInFlight(func(s *inFlightState) { req = s.incomingByID[id] }) if req != nil { - req.cancel(cause) + req.cancel(nil) } } diff --git a/mcp/client.go b/mcp/client.go index 1ee13e8..c11f0c8 100644 --- a/mcp/client.go +++ b/mcp/client.go @@ -1147,9 +1147,6 @@ 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/mcp_test.go b/mcp/mcp_test.go index 7b10b30..e1f8c0f 100644 --- a/mcp/mcp_test.go +++ b/mcp/mcp_test.go @@ -710,13 +710,13 @@ func TestCancellation(t *testing.T) { synctest.Test(t, func(t *testing.T) { var ( start = make(chan struct{}) - cancelled = make(chan error, 1) // don't block the request + cancelled = make(chan struct{}, 1) // don't block the request ) slowTool := func(ctx context.Context, req *CallToolRequest, args any) (*CallToolResult, any, error) { start <- struct{}{} select { case <-ctx.Done(): - cancelled <- context.Cause(ctx) + cancelled <- struct{}{} case <-time.After(5 * time.Second): return nil, nil, nil } @@ -732,72 +732,7 @@ func TestCancellation(t *testing.T) { <-start cancel() - // The client sends its context's error as the reason of its cancelled - // notification, and the handler reads it back as the cause. - cause := <-cancelled - if !errors.Is(cause, context.Canceled) { - t.Errorf("context.Cause = %v, want it to wrap context.Canceled", cause) - } - if want := "request cancelled by the peer: " + context.Canceled.Error(); cause == nil || cause.Error() != want { - t.Errorf("context.Cause = %v, want %q", cause, want) - } - }) -} - -// TestCancellationReason verifies that the reason a peer gives in its -// cancelled notification reaches the handler as the cause of its context, -// which is what lets a server log it as the specification asks. -// -// The call and the notification are written to the connection directly, so -// the reason is one the SDK's own client would never send; the session is -// pinned to 2025-11-25 so that a request written that way needs no _meta. -func TestCancellationReason(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - var ( - start = make(chan struct{}) - cancelled = make(chan error, 1) // don't block the request - ) - slowTool := func(ctx context.Context, req *CallToolRequest, args any) (*CallToolResult, any, error) { - start <- struct{}{} - select { - case <-ctx.Done(): - cancelled <- context.Cause(ctx) - case <-time.After(5 * time.Second): - cancelled <- nil - } - return nil, nil, nil - } - ctx := context.Background() - ct, st := NewInMemoryTransports() - s := NewServer(testImpl, nil) - AddTool(s, &Tool{Name: "slow", InputSchema: &jsonschema.Schema{Type: "object"}}, slowTool) - ss, err := s.Connect(ctx, st, nil) - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = ss.Close() }) - cs, err := NewClient(testImpl, nil).Connect(ctx, ct, &ClientSessionOptions{ProtocolVersion: protocolVersion20251125}) - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = cs.Close() }) - - call := cs.conn.Call(ctx, methodCallTool, &CallToolParams{Name: "slow"}) - <-start - if err := cs.conn.Notify(ctx, notificationCancelled, &CancelledParams{RequestID: call.ID().Raw(), Reason: "user asked"}); err != nil { - t.Fatal(err) - } - - cause := <-cancelled - if cause == nil { - t.Fatal("the tool ran to completion, want it cancelled") - } - if !errors.Is(cause, context.Canceled) { - t.Errorf("context.Cause = %v, want it to wrap context.Canceled", cause) - } - if got, want := cause.Error(), "request cancelled by the peer: user asked"; got != want { - t.Errorf("context.Cause = %q, want %q", got, want) - } + <-cancelled }) } @@ -3363,159 +3298,6 @@ func TestSubscriptionsListen_DisconnectScrubsMaps(t *testing.T) { } } -// newSubListenSubscribeServer is newSubListenServer with a resource and the -// handlers that make the server advertise resources.subscribe, so that a -// listen can carry a resource subscription. -func newSubListenSubscribeServer() *Server { - s := NewServer(testImpl, &ServerOptions{ - SubscribeHandler: func(context.Context, *SubscribeRequest) error { return nil }, - UnsubscribeHandler: func(context.Context, *UnsubscribeRequest) error { return nil }, - }) - AddTool(s, &Tool{Name: "t1"}, sayHi) - s.AddPrompt(&Prompt{Name: "p1"}, nil) - s.AddResource(&Resource{Name: "r1", URI: "file:///r1"}, nil) - return s -} - -func waitSubListenEvent(t *testing.T, events chan subListenEvent, kind string) subListenEvent { - t.Helper() - select { - case e := <-events: - if e.kind != kind { - t.Fatalf("got event %q, want %q", e.kind, kind) - } - return e - case <-time.After(5 * time.Second): - t.Fatalf("timed out waiting for %q", kind) - return subListenEvent{} - } -} - -// TestSubscriptionsListen_TeardownKeepsOtherListens verifies that unwinding -// one listen leaves the other listens on the same session registered. -// Unsubscribe tears down the listen that Subscribe opened for the resource; -// the auto-listen opened by Connect must keep delivering list-changed -// notifications. -func TestSubscriptionsListen_TeardownKeepsOtherListens(t *testing.T) { - events := make(chan subListenEvent, 16) - server := newSubListenSubscribeServer() - - ct, st := NewInMemoryTransports() - ss, err := server.Connect(context.Background(), st, nil) - if err != nil { - t.Fatalf("server connect: %v", err) - } - defer ss.Close() - - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - cs, err := newSubListenClient(events).Connect(ctx, ct, - &ClientSessionOptions{ProtocolVersion: protocolVersion20260728}) - if err != nil { - t.Fatalf("client connect: %v", err) - } - defer cs.Close() - - autoListen := waitSubListenEvent(t, events, "ack") - - if err := cs.Subscribe(ctx, &SubscribeParams{URI: "file:///r1"}); err != nil { - t.Fatalf("Subscribe: %v", err) - } - resourceListen := waitSubListenEvent(t, events, "ack") - if resourceListen.id == autoListen.id { - t.Fatalf("Subscribe reused subscription ID %s", autoListen.id) - } - - if err := cs.Unsubscribe(ctx, &UnsubscribeParams{URI: "file:///r1"}); err != nil { - t.Fatalf("Unsubscribe: %v", err) - } - // Wait for the server to unwind that listen, observed through a registry - // the auto-listen does not appear in. - waitUntil(t, 5*time.Second, "resource listen to unwind", func() bool { - server.mu.Lock() - defer server.mu.Unlock() - _, ok := server.resourceSubscriptions["file:///r1"] - return !ok - }) - - AddTool(server, &Tool{Name: "t2"}, sayHi) - if got := waitSubListenEvent(t, events, "tool"); got.id != autoListen.id { - t.Errorf("tool notification id = %s, want auto-listen %s", got.id, autoListen.id) - } -} - -// TestSubscriptionsListen_TeardownRetiresOwnRegistration verifies that a -// listen still retires what it registered, so that per-listen tracking does -// not turn into a leak. -func TestSubscriptionsListen_TeardownRetiresOwnRegistration(t *testing.T) { - events := make(chan subListenEvent, 8) - server := newSubListenServer() - - ct, st := NewInMemoryTransports() - ss, err := server.Connect(context.Background(), st, nil) - if err != nil { - t.Fatalf("server connect: %v", err) - } - defer ss.Close() - - // A client with no list-changed handlers does not auto-listen, so the - // listen opened below is the session's only one. - c := NewClient(testImpl, nil) - c.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { - return func(ctx context.Context, method string, req Request) (Result, error) { - if method == notificationSubscriptionsAck { - events <- subListenEvent{"ack", ""} - } - return next(ctx, method, req) - } - }) - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - cs, err := c.Connect(ctx, ct, &ClientSessionOptions{ProtocolVersion: protocolVersion20260728}) - if err != nil { - t.Fatalf("client connect: %v", err) - } - defer cs.Close() - - listenCtx, cancelListen := context.WithCancel(context.Background()) - defer cancelListen() - if err := cs.subscriptionsListen(listenCtx, &SubscriptionsListenParams{ - Notifications: &NotificationSubscriptions{ToolsListChanged: true, PromptsListChanged: true}, - }); err != nil { - t.Fatalf("listen: %v", err) - } - waitSubListenEvent(t, events, "ack") - - server.mu.Lock() - _, inTool := server.toolChangeSubscriptions[ss] - _, inPrompt := server.promptChangeSubscriptions[ss] - server.mu.Unlock() - if !inTool || !inPrompt { - t.Fatal("listen not registered") - } - - cancelListen() - waitUntil(t, 5*time.Second, "registrations to be retired", func() bool { - server.mu.Lock() - defer server.mu.Unlock() - _, inTool := server.toolChangeSubscriptions[ss] - _, inPrompt := server.promptChangeSubscriptions[ss] - return !inTool && !inPrompt - }) -} - -// waitUntil polls cond until it reports true, failing the test after timeout. -func waitUntil(t *testing.T, timeout time.Duration, what string, cond func() bool) { - t.Helper() - deadline := time.Now().Add(timeout) - for !cond() { - if time.Now().After(deadline) { - t.Fatalf("timed out waiting for %s", what) - } - time.Sleep(10 * time.Millisecond) - } -} - // TestSubscriptionsListen_RespectsServerCapabilities verifies that during // Connect the client only opens a SEP-2575 subscriptions/listen stream for the // change notifications the server advertised during capability negotiation. diff --git a/mcp/mrtr.go b/mcp/mrtr.go index fdf98ec..008c0f8 100644 --- a/mcp/mrtr.go +++ b/mcp/mrtr.go @@ -36,20 +36,29 @@ type multiRoundTripResponse interface { hasContent() bool } -// validateMultiRoundTripResult rejects a result that carries both content and -// input requests. [annotateResultType] labels the result later, in -// [ServerSession.handle], so this function does not set resultType. -func validateMultiRoundTripResult(logger *slog.Logger, res multiRoundTripResponse) error { +func handleMultiRoundTripResult(ss *ServerSession, logger *slog.Logger, res multiRoundTripResponse) error { if res == nil { return nil } - if res.inputRequests() != nil && res.hasContent() { + hasInputRequests := res.inputRequests() != nil + + if hasInputRequests && res.hasContent() { logger.Warn("handler returned both content and inputRequests") return &jsonrpc.Error{ Code: jsonrpc.CodeInternalError, Message: "server bug: result has both content and inputRequests", } } + + if clientSupportsMultiRoundTrip(ss) { + // For older clients the resultType is left unset. Input requests will be handled + // by serverMultiRoundTripMiddleware client calls and handler reinvocation. + if hasInputRequests { + res.setResultType(resultTypeInputRequired) + } else { + res.setResultType(resultTypeComplete) + } + } return nil } diff --git a/mcp/protocol.go b/mcp/protocol.go index c340e60..63d7015 100644 --- a/mcp/protocol.go +++ b/mcp/protocol.go @@ -19,7 +19,7 @@ type resultType string const ( // resultTypeComplete indicates the result is final. - // This is the default when resultType is empty. + // This is the default when ResultType is empty. resultTypeComplete resultType = "complete" // resultTypeInputRequired indicates the server needs additional client @@ -40,20 +40,9 @@ type completeResultResponse interface { isCompleteResult() } -// annotateResultType sets the resultType that protocol 2026-07-28 requires on -// every result. -func annotateResultType(res Result) { - switch r := res.(type) { - case completeResultResponse: +func setCompleteResultType(res Result) { + if r, ok := res.(completeResultResponse); ok { r.setResultType(resultTypeComplete) - case multiRoundTripResponse: - // These results are complete or input_required, so label them by - // whether the handler asked for more client input. - if r.inputRequests() != nil { - r.setResultType(resultTypeInputRequired) - } else { - r.setResultType(resultTypeComplete) - } } } @@ -237,8 +226,9 @@ type CallToolParams struct { // marshaled to JSON. Arguments any `json:"arguments,omitempty"` - // InputResponses maps input request IDs to responses. Set it when the - // client retries a call that asked for input. See [CallToolResult.NeedsInput]. + // InputResponses maps input request IDs to responses, provided when + // retrying a call after receiving a result with ResultType + // ResultTypeInputRequired. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. // The client must echo this back when retrying. @@ -259,8 +249,9 @@ type CallToolParamsRaw struct { // Arguments (see [AddTool]). Arguments json.RawMessage `json:"arguments,omitempty"` - // InputResponses maps input request IDs to responses. Set it when the - // client retries a call that asked for input. See [CallToolResult.NeedsInput]. + // InputResponses maps input request IDs to responses, provided when + // retrying a call after receiving a result with ResultType + // ResultTypeInputRequired. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. // The client must echo this back when retrying. @@ -289,8 +280,6 @@ type CallToolResult struct { // result of the tool call. Per SEP-2106, it may marshal to any valid JSON // value (object, array, or primitive) conforming to the tool's // [Tool.OutputSchema]. - // Numbers received from the wire are represented as [json.Number] to preserve - // their exact values. // // When using a [ToolHandlerFor] with structured output, you should not // populate this field. It will be automatically populated with the typed Out @@ -316,9 +305,9 @@ type CallToolResult struct { IsError bool `json:"isError,omitempty"` // InputRequests is a map of server-assigned IDs to input requests. - // It is set only when the result needs more client input. + // Populated only when ResultType is ResultTypeInputRequired. // The client must fulfill these and echo the IDs back in InputResponses - // when it retries the call. + // when retrying the call. InputRequests InputRequestMap `json:"inputRequests,omitempty"` // RequestState is an opaque string the client must echo back when @@ -328,10 +317,10 @@ type CallToolResult struct { // Unauthenticated servers must encrypt, sign and verify this value. RequestState string `json:"requestState,omitempty"` - // resultType records whether the call finished or needs more client input. - // Empty or resultTypeComplete means it finished. resultTypeInputRequired - // means the client must fulfill InputRequests and retry. - // See [CallToolResult.NeedsInput]. + // ResultType indicates whether this result is complete or requires further + // client input. Empty or ResultTypeComplete means the call succeeded + // normally. ResultTypeInputRequired means the client should fulfill the + // InputRequests and retry the call. resultType resultType // The error passed to setError, if any. // It is not marshaled, and therefore it is only visible on the server. @@ -359,8 +348,7 @@ func (r *CallToolResult) GetError() error { return r.err } -func (*CallToolResult) isResult() {} -func (x *CallToolResult) isNil() bool { return x == nil } +func (*CallToolResult) isResult() {} func (r *CallToolResult) setResultType(rt resultType) { r.resultType = rt } func (r *CallToolResult) requestState() string { return r.RequestState } @@ -375,18 +363,12 @@ func (r *CallToolResult) hasContent() bool { } // NeedsInput reports whether this result requires further client input. -// This is true when the server returned a resultType of "input_required". +// This is true when the server returned ResultType "input_required". // When NeedsInput returns true, check InputRequests for the set of // requests the server needs fulfilled before retrying the call. // An empty InputRequests with NeedsInput true indicates load-shedding. func (r *CallToolResult) NeedsInput() bool { return r.resultType == resultTypeInputRequired } -// structuredcontentfloat64 is a compatibility parameter that restores the -// previous behavior of decoding numbers in [CallToolResult.StructuredContent] -// as float64 values. By default, numbers are decoded as [json.Number] to avoid -// losing precision. The option will be removed in the 1.11.0 version of the SDK. -var structuredcontentfloat64 = mcpgodebug.Value("structuredcontentfloat64") - func (x *CallToolResult) MarshalJSON() ([]byte, error) { type res CallToolResult // avoid recursion type wire struct { @@ -409,22 +391,12 @@ func (x *CallToolResult) UnmarshalJSON(data []byte) error { type res CallToolResult // avoid recursion var wire struct { res - Content []*wireContent `json:"content"` - StructuredContent json.RawMessage `json:"structuredContent"` - ResultType resultType `json:"resultType"` + Content []*wireContent `json:"content"` + ResultType resultType `json:"resultType"` } if err := internaljson.Unmarshal(data, &wire); err != nil { return err } - if len(wire.StructuredContent) > 0 { - unmarshal := internaljson.UnmarshalUseNumber - if structuredcontentfloat64 == "1" { - unmarshal = internaljson.Unmarshal - } - if err := unmarshal(wire.StructuredContent, &wire.res.StructuredContent); err != nil { - return err - } - } var err error if wire.res.Content, err = contentsFromWire(wire.Content, nil); err != nil { return err @@ -683,8 +655,7 @@ type CompleteResult struct { Completion CompletionResultDetails `json:"completion"` } -func (*CompleteResult) isResult() {} -func (x *CompleteResult) isNil() bool { return x == nil } +func (*CompleteResult) isResult() {} // CreateMessageParams holds parameters for a sampling/createMessage request. // @@ -868,7 +839,6 @@ type CreateMessageResult struct { } func (*CreateMessageResult) isResult() {} -func (x *CreateMessageResult) isNil() bool { return x == nil } func (*CreateMessageResult) isInputResponse() {} func (r *CreateMessageResult) UnmarshalJSON(data []byte) error { type result CreateMessageResult // avoid recursion @@ -920,7 +890,6 @@ var createMessageWithToolsResultAllow = map[string]bool{ } func (*CreateMessageWithToolsResult) isResult() {} -func (x *CreateMessageWithToolsResult) isNil() bool { return x == nil } func (*CreateMessageWithToolsResult) isInputResponse() {} // MarshalJSON marshals the result. When Content has a single element, it is @@ -981,8 +950,9 @@ type GetPromptParams struct { // The name of the prompt or prompt template. Name string `json:"name"` - // InputResponses maps input request IDs to responses. Set it when the - // client retries a call that asked for input. See [CallToolResult.NeedsInput]. + // InputResponses maps input request IDs to responses, provided when + // retrying a call after receiving a result with ResultType + // ResultTypeInputRequired. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. RequestState string `json:"requestState,omitempty"` @@ -1002,20 +972,19 @@ type GetPromptResult struct { Description string `json:"description,omitempty"` Messages []*PromptMessage `json:"messages"` - // InputRequests is set when the result needs more client input. + // InputRequests is populated when ResultType is ResultTypeInputRequired. // See [CallToolResult.InputRequests]. InputRequests InputRequestMap `json:"inputRequests,omitempty"` // RequestState is the opaque state for multi-round-trip retries. // See [CallToolResult.RequestState]. RequestState string `json:"requestState,omitempty"` - // resultType records whether the call finished or needs more client input. - // See [CallToolResult.NeedsInput]. + // ResultType indicates whether this result is complete or requires further + // client input. See [CallToolResult.ResultType] for details. resultType resultType } -func (*GetPromptResult) isResult() {} -func (x *GetPromptResult) isNil() bool { return x == nil } +func (*GetPromptResult) isResult() {} func (r *GetPromptResult) setResultType(rt resultType) { r.resultType = rt } func (r *GetPromptResult) requestState() string { return r.RequestState } @@ -1125,8 +1094,7 @@ type InitializeResult struct { ServerInfo *Implementation `json:"serverInfo"` } -func (*InitializeResult) isResult() {} -func (x *InitializeResult) isNil() bool { return x == nil } +func (*InitializeResult) isResult() {} type InitializedParams struct { // Meta is reserved by the protocol to allow clients and servers to attach @@ -1169,8 +1137,7 @@ type DiscoverResult struct { Instructions string `json:"instructions,omitempty"` } -func (*DiscoverResult) isResult() {} -func (x *DiscoverResult) isNil() bool { return x == nil } +func (*DiscoverResult) isResult() {} func (x *ListPromptsParams) isParams() {} func (x *ListPromptsParams) isNil() bool { return x == nil } @@ -1240,7 +1207,6 @@ type ListPromptsResult struct { } func (x *ListPromptsResult) isResult() {} -func (x *ListPromptsResult) isNil() bool { return x == nil } func (x *ListPromptsResult) nextCursorPtr() *string { return &x.NextCursor } type ListResourceTemplatesParams struct { @@ -1272,7 +1238,6 @@ type ListResourceTemplatesResult struct { } func (x *ListResourceTemplatesResult) isResult() {} -func (x *ListResourceTemplatesResult) isNil() bool { return x == nil } func (x *ListResourceTemplatesResult) nextCursorPtr() *string { return &x.NextCursor } type ListResourcesParams struct { @@ -1304,7 +1269,6 @@ type ListResourcesResult struct { } func (x *ListResourcesResult) isResult() {} -func (x *ListResourcesResult) isNil() bool { return x == nil } func (x *ListResourcesResult) nextCursorPtr() *string { return &x.NextCursor } // ListRootsParams holds parameters for a roots/list request. @@ -1341,7 +1305,6 @@ type ListRootsResult struct { } func (*ListRootsResult) isResult() {} -func (x *ListRootsResult) isNil() bool { return x == nil } func (*ListRootsResult) isInputResponse() {} type ListToolsParams struct { @@ -1373,7 +1336,6 @@ type ListToolsResult struct { } func (x *ListToolsResult) isResult() {} -func (x *ListToolsResult) isNil() bool { return x == nil } func (x *ListToolsResult) nextCursorPtr() *string { return &x.NextCursor } // The severity of a log message. @@ -1613,8 +1575,9 @@ type ReadResourceParams struct { // the server how to interpret it. URI string `json:"uri"` - // InputResponses maps input request IDs to responses. Set it when the - // client retries a call that asked for input. See [CallToolResult.NeedsInput]. + // InputResponses maps input request IDs to responses, provided when + // retrying a call after receiving a result with ResultType + // ResultTypeInputRequired. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. RequestState string `json:"requestState,omitempty"` @@ -1633,20 +1596,19 @@ type ReadResourceResult struct { Cacheable Contents []*ResourceContents `json:"contents"` - // InputRequests is set when the result needs more client input. + // InputRequests is populated when ResultType is ResultTypeInputRequired. // See [CallToolResult.InputRequests]. InputRequests InputRequestMap `json:"inputRequests,omitempty"` // RequestState is the opaque state for multi-round-trip retries. // See [CallToolResult.RequestState]. RequestState string `json:"requestState,omitempty"` - // resultType records whether the call finished or needs more client input. - // See [CallToolResult.NeedsInput]. + // ResultType indicates whether this result is complete or requires further + // client input. See [CallToolResult.ResultType] for details. resultType resultType } -func (*ReadResourceResult) isResult() {} -func (x *ReadResourceResult) isNil() bool { return x == nil } +func (*ReadResourceResult) isResult() {} func (r *ReadResourceResult) setResultType(rt resultType) { r.resultType = rt } func (r *ReadResourceResult) requestState() string { return r.RequestState } @@ -2152,8 +2114,7 @@ type SubscriptionsListenResult struct { Meta `json:"_meta"` } -func (*SubscriptionsListenResult) isResult() {} -func (x *SubscriptionsListenResult) isNil() bool { return x == nil } +func (*SubscriptionsListenResult) isResult() {} // TODO(jba): add CompleteRequest and related types. @@ -2230,7 +2191,6 @@ type ElicitResult struct { } func (*ElicitResult) isResult() {} -func (x *ElicitResult) isNil() bool { return x == nil } func (*ElicitResult) isInputResponse() {} // ElicitationCompleteParams is sent from the server to the client, informing it that an out-of-band elicitation interaction has completed. diff --git a/mcp/protocol_test.go b/mcp/protocol_test.go index 4f0c345..fe496de 100644 --- a/mcp/protocol_test.go +++ b/mcp/protocol_test.go @@ -1285,7 +1285,7 @@ func TestContentUnmarshal(t *testing.T) { Meta: Meta{"m": true}, Content: content, IsError: true, - StructuredContent: json.Number("3"), + StructuredContent: 3.0, } var gotf CallToolResult roundtrip(ctrf, &gotf) @@ -1298,24 +1298,6 @@ func TestContentUnmarshal(t *testing.T) { roundtrip(pm, &gotpm) } -func TestCallToolResultStructuredContentFloat64Compatibility(t *testing.T) { - prev := structuredcontentfloat64 - structuredcontentfloat64 = "1" - t.Cleanup(func() { structuredcontentfloat64 = prev }) - - var got CallToolResult - if err := json.Unmarshal([]byte(`{"content":[],"structuredContent":{"id":9007199254740993}}`), &got); err != nil { - t.Fatal(err) - } - structured, ok := got.StructuredContent.(map[string]any) - if !ok { - t.Fatalf("StructuredContent type = %T, want map[string]any", got.StructuredContent) - } - if got := structured["id"]; got != float64(9007199254740992) { - t.Errorf("id = %v (%T), want rounded float64 value", got, got) - } -} - func TestToolAnnotations_MarshalJSON(t *testing.T) { boolPtr := func(b bool) *bool { return &b } diff --git a/mcp/server.go b/mcp/server.go index 569abab..5014a4f 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -910,7 +910,7 @@ func (s *Server) getPrompt(ctx context.Context, req *GetPromptRequest) (*GetProm } res, err := prompt.handler(ctx, req) if err == nil && res != nil { - if err := validateMultiRoundTripResult(s.opts.Logger, res); err != nil { + if err := handleMultiRoundTripResult(req.Session, s.opts.Logger, res); err != nil { return nil, err } } @@ -1012,10 +1012,10 @@ func (s *Server) callTool(ctx context.Context, req *CallToolRequest) (*CallToolR } res, err := st.handler(ctx, req) if err == nil && res != nil { - if err := validateMultiRoundTripResult(s.opts.Logger, res); err != nil { + if err := handleMultiRoundTripResult(req.Session, s.opts.Logger, res); err != nil { return nil, err } - if res.Content == nil && res.InputRequests == nil { + if res.Content == nil && res.resultType != resultTypeInputRequired { res2 := *res res2.Content = []Content{} // avoid "null" res = &res2 @@ -1080,11 +1080,11 @@ func (s *Server) readResource(ctx context.Context, req *ReadResourceRequest) (*R if res == nil { return nil, fmt.Errorf("reading resource %s: read handler returned nil information", uri) } - if err := validateMultiRoundTripResult(s.opts.Logger, res); err != nil { + if err := handleMultiRoundTripResult(req.Session, s.opts.Logger, res); err != nil { return nil, err } s.resolveCacheable(ctx, req, &res.Cacheable) - if res.InputRequests != nil { + if res.resultType == resultTypeInputRequired { return res, nil } if res.Contents == nil { @@ -1276,19 +1276,10 @@ func (s *Server) subscriptionsListen(ctx context.Context, req *SubscriptionsList } s.mu.Unlock() defer func() { - // Retire only what this listen registered above: a session can hold - // several listens at once, and deleting by session alone retires a - // registration belonging to a listen that is still open. s.mu.Lock() - if allowed.ToolsListChanged { - delete(s.toolChangeSubscriptions, req.Session) - } - if allowed.PromptsListChanged { - delete(s.promptChangeSubscriptions, req.Session) - } - if allowed.ResourcesListChanged { - delete(s.resourceChangeSubscriptions, req.Session) - } + delete(s.toolChangeSubscriptions, req.Session) + delete(s.promptChangeSubscriptions, req.Session) + delete(s.resourceChangeSubscriptions, req.Session) s.mu.Unlock() }() @@ -1866,9 +1857,6 @@ 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() @@ -1998,6 +1986,10 @@ func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any, } } default: + if !initialized && !validatedMeta.usesNewProtocol && req.IsCall() { + ss.server.opts.Logger.Error("method invalid during initialization", "method", req.Method) + return nil, fmt.Errorf("method %q is invalid during session initialization", req.Method) + } if !initialized && validatedMeta.usesNewProtocol && validatedMeta.initializeParams != nil { ss.updateState(func(state *ServerSessionState) { state.InitializeParams = validatedMeta.initializeParams @@ -2005,16 +1997,6 @@ func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any, } } - // In legacy protocol versions, a client cannot send requests other than - // pings before the server has responded to the initialize request. A - // new-protocol request is exempt: a SEP-2575 session has no 'initialize' - // to wait for, and the request itself says which protocol it speaks. - if !initialized && !validatedMeta.usesNewProtocol && req.IsCall() && - req.Method != methodInitialize && req.Method != methodPing { - ss.server.opts.Logger.Error("method invalid during initialization", "method", req.Method) - return nil, fmt.Errorf("method %q is invalid during session initialization", req.Method) - } - // modelcontextprotocol/go-sdk#26: handle calls asynchronously, and // notifications synchronously, except for 'initialize' which shouldn't be // asynchronous to other @@ -2057,10 +2039,8 @@ func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any, if err != nil { return nil, err } - // A middleware can return a typed nil value that satisfies the Result - // interface. Annotating that value panics. - if validatedMeta.usesNewProtocol && res != nil && !res.isNil() { - annotateResultType(res) + if validatedMeta.usesNewProtocol { + setCompleteResultType(res) annotateServerInfo(res, ss.server.impl) } return res, nil diff --git a/mcp/server_test.go b/mcp/server_test.go index 4d09cbc..11e75e2 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -677,7 +677,7 @@ func TestAddToolNonObjectOutputSchema(t *testing.T) { name: "primitive number (map-based schema)", outputSchema: map[string]any{"type": "number"}, content: 42.0, - want: json.Number("42"), + want: 42.0, }, { name: "primitive string (RawMessage schema)", @@ -724,52 +724,6 @@ func TestAddToolNonObjectOutputSchema(t *testing.T) { } } -func TestCallToolStructuredContentPreservesLargeInteger(t *testing.T) { - const want int64 = 9007199254740993 - - server := NewServer(testImpl, nil) - server.AddTool(&Tool{ - Name: "large-integer", - InputSchema: &jsonschema.Schema{Type: "object"}, - }, func(context.Context, *CallToolRequest) (*CallToolResult, error) { - return &CallToolResult{StructuredContent: map[string]any{"id": want}}, nil - }) - - clientTransport, serverTransport := NewInMemoryTransports() - serverSession, err := server.Connect(context.Background(), serverTransport, nil) - if err != nil { - t.Fatal(err) - } - defer serverSession.Close() - - client := NewClient(testImpl, nil) - clientSession, err := client.Connect(context.Background(), clientTransport, nil) - if err != nil { - t.Fatal(err) - } - defer clientSession.Close() - - result, err := clientSession.CallTool(context.Background(), &CallToolParams{Name: "large-integer"}) - if err != nil { - t.Fatal(err) - } - structured, ok := result.StructuredContent.(map[string]any) - if !ok { - t.Fatalf("StructuredContent type = %T, want map[string]any", result.StructuredContent) - } - number, ok := structured["id"].(json.Number) - if !ok { - t.Fatalf("id type = %T, want json.Number", structured["id"]) - } - got, err := number.Int64() - if err != nil { - t.Fatal(err) - } - if got != want { - t.Errorf("id = %d, want %d", got, want) - } -} - // TestAddToolGenericNonObjectOutput verifies SEP-2106 for the generic // AddTool[In, Out] helper: Out may be a slice or primitive whose inferred // JSON Schema has a non-object root. @@ -862,7 +816,7 @@ func TestAddToolGenericNonObjectOutput(t *testing.T) { if res.IsError { t.Fatalf("unexpected tool error: %v", res.Content) } - if diff := cmp.Diff(json.Number("42"), res.StructuredContent); diff != "" { + if diff := cmp.Diff(float64(42), res.StructuredContent); diff != "" { t.Errorf("structured content mismatch (-want +got):\n%s", diff) } }) @@ -1445,22 +1399,6 @@ func TestServerSessionHandle_RejectsInitializeOnNewProtocol(t *testing.T) { }) } -// newProtocolParams merges fields into the _meta that opts a request into the -// sessionless protocol (SEP-2575). -func newProtocolParams(fields map[string]any) map[string]any { - params := map[string]any{ - "_meta": map[string]any{ - MetaKeyProtocolVersion: protocolVersion20260728, - MetaKeyClientInfo: map[string]any{"name": "c", "version": "1"}, - MetaKeyClientCapabilities: map[string]any{}, - }, - } - for k, v := range fields { - params[k] = v - } - return params -} - func TestServerSessionHandle_SetsResultTypeOnNewProtocol(t *testing.T) { server := NewServer(testImpl, &ServerOptions{ CompletionHandler: func(context.Context, *CompleteRequest) (*CompleteResult, error) { @@ -1486,19 +1424,19 @@ func TestServerSessionHandle_SetsResultTypeOnNewProtocol(t *testing.T) { InputRequests: InputRequestMap{"confirm": &ElicitParams{Message: "Continue?"}}, }, nil }) - AddTool(server, &Tool{Name: "toolComplete"}, func(context.Context, *CallToolRequest, struct{}) (*CallToolResult, any, error) { - return &CallToolResult{Content: []Content{&TextContent{Text: "ok"}}}, nil, nil - }) - server.AddPrompt(&Prompt{Name: "promptComplete"}, func(context.Context, *GetPromptRequest) (*GetPromptResult, error) { - return &GetPromptResult{ - Messages: []*PromptMessage{{Role: "assistant", Content: &TextContent{Text: "ok"}}}, - }, nil - }) - server.AddResource(&Resource{URI: "test://resource-complete", Name: "resourceComplete"}, func(context.Context, *ReadResourceRequest) (*ReadResourceResult, error) { - return &ReadResourceResult{ - Contents: []*ResourceContents{{URI: "test://resource-complete", Text: "ok"}}, - }, nil - }) + newProtocolParams := func(fields map[string]any) map[string]any { + params := map[string]any{ + "_meta": map[string]any{ + MetaKeyProtocolVersion: protocolVersion20260728, + MetaKeyClientInfo: map[string]any{"name": "c", "version": "1"}, + MetaKeyClientCapabilities: map[string]any{}, + }, + } + for k, v := range fields { + params[k] = v + } + return params + } tests := []struct { name string @@ -1569,243 +1507,40 @@ func TestServerSessionHandle_SetsResultTypeOnNewProtocol(t *testing.T) { params: newProtocolParams(map[string]any{"uri": "test://resource"}), want: resultTypeInputRequired, }, - { - name: "tool complete", - method: methodCallTool, - params: newProtocolParams(map[string]any{"name": "toolComplete", "arguments": map[string]any{}}), - want: resultTypeComplete, - }, - { - name: "prompt complete", - method: methodGetPrompt, - params: newProtocolParams(map[string]any{"name": "promptComplete"}), - want: resultTypeComplete, - }, - { - name: "resource complete", - method: methodReadResource, - params: newProtocolParams(map[string]any{"uri": "test://resource-complete"}), - want: resultTypeComplete, - }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - if got := resultTypeOf(t, server, nil, tc.method, tc.params); got != string(tc.want) { - t.Errorf("resultType = %q, want %q", got, tc.want) - } - }) - } -} - -// TestServerSessionHandle_SetsResultTypeWhenMiddlewareShortCircuits is a -// regression test for #1225. User middleware wraps outside -// serverMultiRoundTripMiddleware, so a middleware that returns a result -// without calling next bypasses both that shim and the dispatcher. -func TestServerSessionHandle_SetsResultTypeWhenMiddlewareShortCircuits(t *testing.T) { - tests := []struct { - name string - method string - params map[string]any - result Result - want resultType - }{ - { - name: "tools/call", - method: methodCallTool, - params: newProtocolParams(map[string]any{"name": "tool", "arguments": map[string]any{}}), - result: &CallToolResult{Content: []Content{&TextContent{Text: "denied"}}}, - want: resultTypeComplete, - }, - { - name: "prompts/get", - method: methodGetPrompt, - params: newProtocolParams(map[string]any{"name": "prompt"}), - result: &GetPromptResult{ - Messages: []*PromptMessage{{Role: "assistant", Content: &TextContent{Text: "denied"}}}, - }, - want: resultTypeComplete, - }, - { - name: "resources/read", - method: methodReadResource, - params: newProtocolParams(map[string]any{"uri": "test://resource"}), - result: &ReadResourceResult{ - Contents: []*ResourceContents{{URI: "test://resource", Text: "denied"}}, - }, - want: resultTypeComplete, - }, - { - // setResultType is unexported, so populating InputRequests is - // the only way a middleware can ask for input. - name: "tools/call input required", - method: methodCallTool, - params: newProtocolParams(map[string]any{"name": "tool", "arguments": map[string]any{}}), - result: &CallToolResult{ - InputRequests: InputRequestMap{"confirm": &ElicitParams{Message: "Continue?"}}, - }, - want: resultTypeInputRequired, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - // Nothing is registered: if the middleware fell through to next, - // dispatch would fail rather than return a result. - server := NewServer(testImpl, nil) - server.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { - return func(ctx context.Context, method string, req Request) (Result, error) { - if method == tc.method { - return tc.result, nil - } - return next(ctx, method, req) - } - }) - if got := resultTypeOf(t, server, nil, tc.method, tc.params); got != string(tc.want) { - t.Errorf("resultType = %q, want %q", got, tc.want) - } - }) - } -} - -// TestServerSessionHandle_ResultTypeGate pins when a result carries resultType. -// annotateResultType is the only setter, and handle gates it on the request's -// _meta rather than on the version the session negotiated. So the last case -// below gets no resultType even though the session speaks 2026-07-28. Only a -// client that mixes protocol versions reaches that, so it is recorded rather -// than fixed. -func TestServerSessionHandle_ResultTypeGate(t *testing.T) { - version := func(v string) func(*ServerSessionState) { - return func(s *ServerSessionState) { - s.InitializeParams = &InitializeParams{ProtocolVersion: v} - } - } - callParams := map[string]any{"name": "tool", "arguments": map[string]any{}} - - tests := []struct { - name string - state func(*ServerSessionState) - params map[string]any - want resultType // "" means the field must be absent - }{ - { - name: "legacy session, no _meta", - state: version(protocolVersion20251125), - params: callParams, - }, - { - name: "legacy session, new-protocol _meta", - state: version(protocolVersion20251125), - params: newProtocolParams(callParams), - want: resultTypeComplete, - }, - { - name: "new-protocol session, no _meta", - state: version(protocolVersion20260728), - params: callParams, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - server := NewServer(testImpl, nil) - // Short-circuit so this measures handle's gate, not the - // dispatcher's. - server.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { - return func(context.Context, string, Request) (Result, error) { - return &CallToolResult{Content: []Content{&TextContent{Text: "ok"}}}, nil - } - }) - if got := resultTypeOf(t, server, tc.state, methodCallTool, tc.params); got != string(tc.want) { - t.Errorf("resultType = %q, want %q", got, tc.want) - } - }) - } -} - -// TestServerSessionHandle_NilResultFromMiddleware checks that a middleware -// returning no result does not panic. A typed nil satisfies Result, so the -// annotation helpers would otherwise dereference it. -func TestServerSessionHandle_NilResultFromMiddleware(t *testing.T) { - tests := []struct { - name string - result Result - }{ - {"nil interface", nil}, - {"typed nil", (*CallToolResult)(nil)}, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - server := NewServer(testImpl, nil) - server.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { - return func(context.Context, string, Request) (Result, error) { - return tc.result, nil - } - }) ss := &ServerSession{server: server} id, err := jsonrpc.MakeID("test") if err != nil { t.Fatal(err) } - res, err := ss.handle(context.Background(), &jsonrpc.Request{ + result, err := ss.handle(context.Background(), &jsonrpc.Request{ ID: id, - Method: methodCallTool, - Params: mustMarshal(newProtocolParams(map[string]any{"name": "tool", "arguments": map[string]any{}})), + Method: tc.method, + Params: mustMarshal(tc.params), }) - if err != nil { - t.Fatalf("handle: %v", err) - } - data, err := json.Marshal(res) if err != nil { t.Fatal(err) } - if got := string(data); got != "null" { - t.Errorf("result = %s, want null", got) + data, err := json.Marshal(result) + if err != nil { + t.Fatal(err) + } + var got struct { + ResultType string `json:"resultType"` + } + if err := json.Unmarshal(data, &got); err != nil { + t.Fatal(err) + } + if got.ResultType != string(tc.want) { + t.Fatalf("resultType = %q, want %q; response = %s", got.ResultType, tc.want, data) } }) } } -// resultTypeOf handles one request on a transportless session and reports the -// resultType of the marshaled result, or "" if the field is absent. -func resultTypeOf(t *testing.T, server *Server, state func(*ServerSessionState), method string, params map[string]any) string { - t.Helper() - ss := &ServerSession{server: server} - if state != nil { - ss.updateState(state) - } - id, err := jsonrpc.MakeID("test") - if err != nil { - t.Fatal(err) - } - result, err := ss.handle(context.Background(), &jsonrpc.Request{ - ID: id, - Method: method, - Params: mustMarshal(params), - }) - if err != nil { - t.Fatal(err) - } - data, err := json.Marshal(result) - if err != nil { - t.Fatal(err) - } - var fields map[string]json.RawMessage - if err := json.Unmarshal(data, &fields); err != nil { - t.Fatal(err) - } - raw, ok := fields["resultType"] - if !ok { - return "" - } - var got string - if err := json.Unmarshal(raw, &got); err != nil { - t.Fatalf("unmarshaling resultType from %s: %v", data, err) - } - return got -} - // TestServerSessionHandle_RejectsRemovedMethodsOnNewProtocol verifies that // the methods removed by SEP-2575 (`initialize`, `notifications/initialized`, // `ping`) all return Method not found when the request opts into the new @@ -2197,217 +1932,3 @@ func TestServerSupportedProtocolVersions_NewProtocol(t *testing.T) { t.Errorf("UnsupportedProtocolVersionData.Supported mismatch (-want +got):\n%s", diff) } } - -// TestServerHandle_LegacyCallBeforeInitialize asserts that a call carrying no -// new-protocol `_meta` is refused on a session that has not completed -// 'initialize', whichever branch of the method switch it lands in. The -// handshake itself is what ends that state, and the lifecycle spec exempts -// pings, so those two are served. -// -// The gate used to sit in the default branch, so 'logging/setLevel', -// 'resources/subscribe' and 'resources/unsubscribe', which share a case that -// only refuses new-protocol requests, were served before the handshake. For -// subscribe that registered a session that never handshook as a subscriber, -// so the subtest also asserts that the refusal leaves no subscriber and runs -// no handler. -func TestServerHandle_LegacyCallBeforeInitialize(t *testing.T) { - ctx := context.Background() - const uri = "test://resource" - - tests := []struct { - name string - method string - params string - served bool - }{ - { - name: "initialize", - method: methodInitialize, - params: fmt.Sprintf(`{"protocolVersion":%q,"capabilities":{},"clientInfo":{"name":"c","version":"1"}}`, protocolVersion20251125), - served: true, - }, - {name: "ping", method: methodPing, params: `{}`, served: true}, - {name: "tools/list", method: methodListTools, params: `{}`}, - {name: "logging/setLevel", method: methodSetLevel, params: `{"level":"info"}`}, - {name: "resources/subscribe", method: methodSubscribe, params: fmt.Sprintf(`{"uri":%q}`, uri)}, - {name: "resources/unsubscribe", method: methodUnsubscribe, params: fmt.Sprintf(`{"uri":%q}`, uri)}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - var handlerRan bool - server := NewServer(testImpl, &ServerOptions{ - SubscribeHandler: func(context.Context, *SubscribeRequest) error { - handlerRan = true - return nil - }, - UnsubscribeHandler: func(context.Context, *UnsubscribeRequest) error { - handlerRan = true - return nil - }, - }) - _, st := NewInMemoryTransports() - ss, err := server.Connect(ctx, st, nil) - if err != nil { - t.Fatalf("server.Connect: %v", err) - } - defer ss.Close() - - res, err := ss.handle(ctx, &jsonrpc.Request{ - ID: jsonrpc2.Int64ID(1), - Method: tc.method, - Params: json.RawMessage(tc.params), - }) - if tc.served { - if err != nil { - t.Fatalf("handle(%q) before initialize returned %v, want it served", tc.method, err) - } - if res == nil { - t.Fatalf("handle(%q) before initialize returned no result", tc.method) - } - if tc.method == methodInitialize && ss.InitializeParams() == nil { - t.Errorf("initialize was answered without recording InitializeParams") - } - return - } - if err == nil { - t.Fatalf("handle(%q) served the call before initialize, want a refusal", tc.method) - } - if want := fmt.Sprintf("method %q is invalid during session initialization", tc.method); err.Error() != want { - t.Errorf("handle(%q) error = %q, want %q", tc.method, err.Error(), want) - } - if handlerRan { - t.Errorf("handle(%q) ran the server's handler for a call it refused", tc.method) - } - server.mu.Lock() - _, subscribed := server.resourceSubscriptions[uri][ss] - server.mu.Unlock() - if subscribed { - t.Errorf("handle(%q) left the session subscribed to %q", tc.method, uri) - } - }) - } -} - -// TestServerHandle_NewProtocolCallWithoutInitialize asserts that the gate -// stays off the SEP-2575 path. A fresh session that never sends 'initialize' -// is served a new-protocol 'tools/list', and the methods the gate now covers -// are answered, when a new-protocol request names them, with the error the -// new protocol already gave them (the method does not exist there), and not -// with the initialization refusal. -func TestServerHandle_NewProtocolCallWithoutInitialize(t *testing.T) { - ctx := context.Background() - const uri = "test://resource" - - tests := []struct { - name string - method string - params map[string]any - served bool - }{ - {name: "tools/list", method: methodListTools, params: newProtocolParams(nil), served: true}, - {name: "logging/setLevel", method: methodSetLevel, params: newProtocolParams(map[string]any{"level": "info"})}, - {name: "resources/subscribe", method: methodSubscribe, params: newProtocolParams(map[string]any{"uri": uri})}, - {name: "resources/unsubscribe", method: methodUnsubscribe, params: newProtocolParams(map[string]any{"uri": uri})}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - server := NewServer(testImpl, &ServerOptions{ - SubscribeHandler: func(context.Context, *SubscribeRequest) error { return nil }, - UnsubscribeHandler: func(context.Context, *UnsubscribeRequest) error { return nil }, - }) - _, st := NewInMemoryTransports() - ss, err := server.Connect(ctx, st, nil) - if err != nil { - t.Fatalf("server.Connect: %v", err) - } - defer ss.Close() - - params, err := json.Marshal(tc.params) - if err != nil { - t.Fatalf("marshalling params: %v", err) - } - res, err := ss.handle(ctx, &jsonrpc.Request{ - ID: jsonrpc2.Int64ID(1), - Method: tc.method, - Params: params, - }) - if tc.served { - if err != nil { - t.Fatalf("handle(%q) on a new-protocol session returned %v, want it served", tc.method, err) - } - if res == nil { - t.Fatalf("handle(%q) on a new-protocol session returned no result", tc.method) - } - return - } - var jerr *jsonrpc.Error - if !errors.As(err, &jerr) { - t.Fatalf("handle(%q) returned %v, want a *jsonrpc.Error", tc.method, err) - } - if jerr.Code != jsonrpc.CodeMethodNotFound { - t.Errorf("handle(%q) error code = %d, want %d (removed in the new protocol)", tc.method, jerr.Code, jsonrpc.CodeMethodNotFound) - } - if strings.Contains(err.Error(), "invalid during session initialization") { - t.Errorf("handle(%q) applied the initialization gate to a new-protocol request: %v", tc.method, err) - } - }) - } -} - -// TestServerUnknownProtocolVersion_NewProtocol verifies that a request whose -// `_meta.protocolVersion` names a version the SDK does not know is rejected -// with [CodeUnsupportedProtocolVersion], and not served as a legacy handshake -// because its version string happens to sort below 2026-07-28. -func TestServerUnknownProtocolVersion_NewProtocol(t *testing.T) { - ctx := context.Background() - // Each case names a version that is not in [supportedProtocolVersions] and - // sorts below 2026-07-28. - tests := []struct { - name string - version string - extraMeta string // further `_meta` entries, each with a leading comma - }{ - // The versioning spec uses 1900-01-01 to illustrate this error. - {"documented example", "1900-01-01", fmt.Sprintf(`,%q:{}`, MetaKeyClientCapabilities)}, - {"unreleased revision", "2025-11-24", fmt.Sprintf(`,%q:{}`, MetaKeyClientCapabilities)}, - {"not a date", "1.0", fmt.Sprintf(`,%q:{}`, MetaKeyClientCapabilities)}, - // The rest of the `_meta` triple is defined by the revision the request - // names, so its absence must not mask the version itself. - {"no clientCapabilities", "1900-01-01", ""}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - server := NewServer(testImpl, nil) - _, st := NewInMemoryTransports() - ss, err := server.Connect(ctx, st, nil) - if err != nil { - t.Fatalf("server.Connect: %v", err) - } - defer ss.Close() - - params := fmt.Sprintf(`{"_meta":{%q:%q%s}}`, MetaKeyProtocolVersion, tc.version, tc.extraMeta) - _, err = ss.handle(ctx, &jsonrpc.Request{ - ID: jsonrpc2.Int64ID(1), - Method: methodListTools, - Params: json.RawMessage(params), - }) - var jerr *jsonrpc.Error - if !errors.As(err, &jerr) { - t.Fatalf("handle returned %v, want a *jsonrpc.Error", err) - } - if jerr.Code != CodeUnsupportedProtocolVersion { - t.Fatalf("error code = %d, want %d", jerr.Code, CodeUnsupportedProtocolVersion) - } - var data UnsupportedProtocolVersionData - if err := json.Unmarshal(jerr.Data, &data); err != nil { - t.Fatalf("unmarshal error data: %v", err) - } - if data.Requested != tc.version { - t.Errorf("UnsupportedProtocolVersionData.Requested = %q, want %q", data.Requested, tc.version) - } - if diff := cmp.Diff(SupportedProtocolVersions(), data.Supported); diff != "" { - t.Errorf("UnsupportedProtocolVersionData.Supported mismatch (-want +got):\n%s", diff) - } - }) - } -} diff --git a/mcp/shared.go b/mcp/shared.go index cacfa75..2258752 100644 --- a/mcp/shared.go +++ b/mcp/shared.go @@ -553,19 +553,13 @@ type validatedMeta struct { // validateRequestMeta inspects a JSON-RPC request to detect whether it follows // the >= 2026-07-28 protocol via the `_meta` field. -// If the request has no _meta, no protocolVersion in _meta, or a protocolVersion -// naming a supported version that predates 2026-07-28, it returns a non-nil -// validatedMeta with usesNewProtocol set to false, and a nil error: such a request -// belongs to the initialize handshake. -// If the protocolVersion names a version that is not supported at all, it returns -// usesNewProtocol set to true with initializeParams carrying only that version, so -// that the caller answers with [CodeUnsupportedProtocolVersion]. The rest of the -// `_meta` triple is not validated in that case, since the revision defining it is -// unknown. -// Otherwise, it validates the presence of clientCapabilities in _meta. If it is -// missing or invalid, it returns nil and a non-nil error. clientInfo is optional; -// if present but invalid, an error is returned. Otherwise, it returns -// usesNewProtocol set to true and the populated initializeParams. +// If the request has no _meta, or no protocolVersion in _meta, it returns a non-nil +// validatedMeta with usesNewProtocol set to false, and a nil error. +// If the request has a protocolVersion in _meta it validates the presence of +// clientCapabilities in _meta. If it is missing or invalid, it returns nil and +// a non-nil error. clientInfo is optional; if present but invalid, an error is +// returned. Otherwise, it returns usesNewProtocol set to true and the populated +// initializeParams. // Notifications always report usesNewProtocol false and never error. func validateRequestMeta(req *jsonrpc.Request) (*validatedMeta, error) { // SEP-2575 defines the `_meta` triple for calls only: NotificationParams @@ -580,26 +574,9 @@ func validateRequestMeta(req *jsonrpc.Request) (*validatedMeta, error) { return &validatedMeta{usesNewProtocol: false, initializeParams: nil}, nil } protocolVersion, ok := meta[MetaKeyProtocolVersion].(string) - // An absent or empty value declares no version at all, which is also how the - // streamable handler reads `_meta.protocolVersion`. - if !ok || protocolVersion == "" { + if !ok || protocolVersion < protocolVersion20260728 { return &validatedMeta{usesNewProtocol: false, initializeParams: nil}, nil } - // Classify by membership rather than by ordering: a version the SDK does not - // know sorts arbitrarily against 2026-07-28, and one that sorts below it would - // be served as a legacy handshake instead of reaching the - // CodeUnsupportedProtocolVersion check in ServerSession.handle. The - // streamable handler already applies the same membership rule to the - // Mcp-Protocol-Version header. - supported := slices.Contains(supportedProtocolVersions, protocolVersion) - if supported && protocolVersion < protocolVersion20260728 { - return &validatedMeta{usesNewProtocol: false, initializeParams: nil}, nil - } - if !supported { - return &validatedMeta{usesNewProtocol: true, initializeParams: &InitializeParams{ - ProtocolVersion: protocolVersion, - }}, nil - } var clientInfo *Implementation if _, present := meta[MetaKeyClientInfo]; present { var ok bool @@ -688,30 +665,6 @@ 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 @@ -773,7 +726,8 @@ 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 { - if !HasParams(r) { + // In practice P is a pointer type implementing Params. + if any(r.Params) == nil || r.Params.isNil() { return nil } return r.Params.GetMeta() @@ -855,9 +809,6 @@ type Result interface { // isResult discourages implementation of Result outside of this package. isResult() - // isNil returns true if the underlying value is nil. - isNil() bool - // GetMeta returns metadata from a value. GetMeta() map[string]any // SetMeta sets the metadata on a value. @@ -876,15 +827,13 @@ type ResultBase struct { Meta `json:"_meta,omitempty"` } -func (*ResultBase) isResult() {} -func (x *ResultBase) isNil() bool { return x == nil } +func (*ResultBase) isResult() {} // emptyResult is returned by methods that have no result, like ping. // Those methods cannot return nil, because jsonrpc2 cannot handle nils. type emptyResult struct{} func (*emptyResult) isResult() {} -func (x *emptyResult) isNil() bool { return x == nil } func (*emptyResult) GetMeta() map[string]any { panic("should never be called") } func (*emptyResult) SetMeta(map[string]any) { panic("should never be called") } diff --git a/mcp/shared_test.go b/mcp/shared_test.go index 4566768..71a071d 100644 --- a/mcp/shared_test.go +++ b/mcp/shared_test.go @@ -102,57 +102,6 @@ func TestValidateRequestMeta(t *testing.T) { params: json.RawMessage(`{"_meta": "not an object", "name": "x"}`), wantUsesNew: false, }, - { - name: "supported pre-2026-07-28 version: old protocol", - method: methodCallTool, - params: map[string]any{ - "_meta": map[string]any{ - MetaKeyProtocolVersion: protocolVersion20251125, - }, - "name": "x", - }, - wantUsesNew: false, - }, - { - name: "empty protocolVersion declares nothing: old protocol", - method: methodCallTool, - params: map[string]any{ - "_meta": map[string]any{ - MetaKeyProtocolVersion: "", - }, - "name": "x", - }, - wantUsesNew: false, - }, - { - // An unsupported version sorting below 2026-07-28 is still a - // new-protocol request, so that the caller can answer it with - // CodeUnsupportedProtocolVersion. - name: "unsupported version: new protocol", - method: methodCallTool, - params: map[string]any{ - "_meta": map[string]any{ - MetaKeyProtocolVersion: "1900-01-01", - MetaKeyClientInfo: map[string]any{"name": "c", "version": "1"}, - MetaKeyClientCapabilities: map[string]any{}, - }, - "name": "x", - }, - wantUsesNew: true, - }, - { - // The triple is defined by a revision the SDK does not know, so an - // unsupported version is reported without validating the rest of it. - name: "unsupported version missing clientCapabilities: new protocol", - method: methodCallTool, - params: map[string]any{ - "_meta": map[string]any{ - MetaKeyProtocolVersion: "1900-01-01", - }, - "name": "x", - }, - wantUsesNew: true, - }, { name: "new protocol with logLevel", method: methodCallTool, @@ -324,62 +273,6 @@ 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", diff --git a/mcp/sse.go b/mcp/sse.go index f5e90b8..0a5bf7f 100644 --- a/mcp/sse.go +++ b/mcp/sse.go @@ -437,11 +437,9 @@ func (c *SSEClientTransport) Connect(ctx context.Context) (Connection, error) { maxEventSize = DefaultMaxEventSize } - // Reuse the scanner so events buffered after the endpoint are not lost. - events := scanEventsLimited(resp.Body, maxEventSize) msgEndpoint, err := func() (*url.URL, error) { var evt Event - for evt, err = range events { + for evt, err = range scanEventsLimited(resp.Body, maxEventSize) { break } if err != nil { @@ -470,7 +468,7 @@ func (c *SSEClientTransport) Connect(ctx context.Context) (Connection, error) { go func() { defer s.Close() // close the transport when the GET exits - for evt, err := range events { + for evt, err := range scanEventsLimited(resp.Body, maxEventSize) { if err != nil { return } diff --git a/mcp/sse_test.go b/mcp/sse_test.go index d9b7a2b..de10eb3 100644 --- a/mcp/sse_test.go +++ b/mcp/sse_test.go @@ -13,13 +13,10 @@ import ( "net/http" "net/http/httptest" "slices" - "strings" "sync/atomic" "testing" - "time" "github.com/google/go-cmp/cmp" - "github.com/modelcontextprotocol/go-sdk/jsonrpc" ) // TestSSEServerTransport_SupportedVersions verifies that the deprecated @@ -435,49 +432,3 @@ func TestSSEServerTransportMaxRequestBodyBytes(t *testing.T) { }) } } - -func TestSSEClientTransportBufferedEvents(t *testing.T) { - const endpoint = "event: endpoint\ndata: /messages\n\n" - const message = "event: message\ndata: {\"jsonrpc\":\"2.0\",\"method\":\"notifications/tools/list_changed\"}\n\n" - for _, coalesced := range []bool{false, true} { - t.Run(fmt.Sprintf("coalesced=%t", coalesced), func(t *testing.T) { - reader, writer := io.Pipe() - defer reader.Close() - defer writer.Close() - prefix := endpoint - if coalesced { - prefix += message - } - transport := &SSEClientTransport{ - Endpoint: "http://example.com/sse", - HTTPClient: &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { - return &http.Response{ - StatusCode: http.StatusOK, - Header: http.Header{"Content-Type": {"text/event-stream"}}, - Body: struct { - io.Reader - io.Closer - }{io.MultiReader(strings.NewReader(prefix), reader), reader}, - }, nil - })}, - } - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - conn, err := transport.Connect(ctx) - if err != nil { - t.Fatal(err) - } - defer conn.Close() - if !coalesced { - go func() { _, _ = io.WriteString(writer, message) }() - } - msg, err := conn.Read(ctx) - if err != nil { - t.Fatalf("Read: %v", err) - } - if req, ok := msg.(*jsonrpc.Request); !ok || req.Method != "notifications/tools/list_changed" { - t.Fatalf("Read = %v, want tools/list_changed notification", msg) - } - }) - } -} diff --git a/mcp/streamable_headers.go b/mcp/streamable_headers.go index c2b8dfd..7d06399 100644 --- a/mcp/streamable_headers.go +++ b/mcp/streamable_headers.go @@ -207,12 +207,7 @@ func setStandardHeaders(ctx context.Context, header http.Header, msg jsonrpc.Mes case *jsonrpc.Request: header.Set(methodHeader, msg.Method) if name, ok := extractName(msg.Method, msg.Params); ok { - // Names are only SHOULD-constrained to header-safe characters, so a - // name outside the safe set is Base64-wrapped with the =?base64?...?= - // sentinel, the same rule applied to Mcp-Param-* values. - if encoded, ok := encodeHeaderValue(name); ok { - header.Set(nameHeader, encoded) - } + header.Set(nameHeader, name) } if msg.Method == "tools/call" { if tool, ok := ctx.Value(toolContextKey).(*Tool); ok && tool != nil { @@ -383,18 +378,13 @@ func validateMcpHeaders(header http.Header, msg jsonrpc.Message, toolLookup func if nameInHeader == "" { return fmt.Errorf("missing required Mcp-Name header for method %q", msg.Method) } - // A name that is not header-safe may be Base64-wrapped by the client - // (=?base64?...?=), so decode before comparing, as with Mcp-Param-*. - decodedName, ok := decodeHeaderValue(nameInHeader) - if !ok { - return errors.New("header mismatch: Mcp-Name header contains invalid Base64 encoding") - } + var ok bool nameInBody, ok = extractName(msg.Method, msg.Params) if !ok { return fmt.Errorf("failed to extract name from parameters for method %q", msg.Method) } - if decodedName != nameInBody { - return fmt.Errorf("header mismatch: Mcp-Name header value '%s' does not match body value '%s'", decodedName, nameInBody) + if nameInHeader != nameInBody { + return fmt.Errorf("header mismatch: Mcp-Name header value '%s' does not match body value '%s'", nameInHeader, nameInBody) } } diff --git a/mcp/streamable_headers_test.go b/mcp/streamable_headers_test.go index 97af67c..def7ad1 100644 --- a/mcp/streamable_headers_test.go +++ b/mcp/streamable_headers_test.go @@ -6,7 +6,6 @@ package mcp import ( "context" - "encoding/base64" "encoding/json" "fmt" "net/http" @@ -336,23 +335,6 @@ func TestValidateMcpHeaders(t *testing.T) { msg: &jsonrpc.Request{Method: "prompts/get", Params: mustMarshal(&GetPromptParams{Name: "code_review"})}, wantErr: false, }, - { - name: "Base64-encoded Mcp-Name is decoded before comparison", - version: minVersionForStandardHeaders, - methodHeader: "tools/call", - nameHeader: base64Prefix + base64.StdEncoding.EncodeToString([]byte("café-tool")) + base64Suffix, - msg: &jsonrpc.Request{Method: "tools/call", Params: mustMarshal(&CallToolParams{Name: "café-tool"})}, - wantErr: false, - }, - { - name: "Mcp-Name with invalid Base64 encoding", - version: minVersionForStandardHeaders, - methodHeader: "tools/call", - nameHeader: base64Prefix + "not valid base64!" + base64Suffix, - msg: &jsonrpc.Request{Method: "tools/call", Params: mustMarshal(&CallToolParams{Name: "my-tool"})}, - wantErr: true, - wantErrContain: "invalid Base64 encoding", - }, { name: "valid initialize (no name needed)", version: minVersionForStandardHeaders, @@ -1593,57 +1575,3 @@ func TestValidateParamHeaders_NestedArguments(t *testing.T) { t.Error("validateParamHeaders() = nil, want error for mismatched nested value") } } - -// TestMcpNameHeaderEncodingRoundTrip verifies that a name which is not -// header-safe is Base64-wrapped by the client per the 2026-07-28 transport -// spec, and that the server's validation accepts what the client produced. -func TestMcpNameHeaderEncodingRoundTrip(t *testing.T) { - tests := []struct { - name string - msg *jsonrpc.Request - wantHeader string - }{ - { - name: "ascii-safe tool name is sent as-is", - msg: &jsonrpc.Request{Method: "tools/call", Params: mustMarshal(&CallToolParams{Name: "my-tool"})}, - wantHeader: "my-tool", - }, - { - name: "non-ascii tool name is base64 encoded", - msg: &jsonrpc.Request{Method: "tools/call", Params: mustMarshal(&CallToolParams{Name: "café-tool"})}, - wantHeader: encodeBase64("café-tool"), - }, - { - name: "prompt name with a leading space is base64 encoded", - msg: &jsonrpc.Request{Method: "prompts/get", Params: mustMarshal(&GetPromptParams{Name: " leading-space"})}, - wantHeader: encodeBase64(" leading-space"), - }, - { - name: "resource uri with a control character is base64 encoded", - msg: &jsonrpc.Request{Method: "resources/read", Params: mustMarshal(&ReadResourceParams{URI: "file:///a\tb.txt"})}, - wantHeader: encodeBase64("file:///a\tb.txt"), - }, - { - name: "name that looks like the sentinel is base64 encoded", - msg: &jsonrpc.Request{Method: "tools/call", Params: mustMarshal(&CallToolParams{Name: "=?base64?abc?="})}, - wantHeader: encodeBase64("=?base64?abc?="), - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - header := http.Header{} - header.Set(protocolVersionHeader, minVersionForStandardHeaders) - setStandardHeaders(context.Background(), header, tt.msg) - - if got := header.Get(nameHeader); got != tt.wantHeader { - t.Errorf("Mcp-Name = %q, want %q", got, tt.wantHeader) - } - - // The server must accept the headers the client just generated. - if err := validateMcpHeaders(header, tt.msg, nil); err != nil { - t.Errorf("validateMcpHeaders() on client-generated headers = %v, want nil", err) - } - }) - } -} diff --git a/mcp/streamable_test.go b/mcp/streamable_test.go index 7af9dff..ca4d0a7 100644 --- a/mcp/streamable_test.go +++ b/mcp/streamable_test.go @@ -288,14 +288,8 @@ func TestStreamableConcurrentHandling(t *testing.T) { t.Errorf("CallTool failed: %v", err) return } - number, ok := res.StructuredContent.(map[string]any)["Count"].(json.Number) - if !ok { - t.Errorf("count type = %T, want json.Number", res.StructuredContent.(map[string]any)["Count"]) - continue - } - got, err := number.Int64() - if err != nil || got != int64(i) { - t.Errorf("got count %d, %v; want %d", got, err, i) + if got := int(res.StructuredContent.(map[string]any)["Count"].(float64)); got != i { + t.Errorf("got count %d, want %d", got, i) } } }) diff --git a/mcp/transport.go b/mcp/transport.go index 08d3b15..8b6cd14 100644 --- a/mcp/transport.go +++ b/mcp/transport.go @@ -207,7 +207,7 @@ func connect[H handler, State any](ctx context.Context, t Transport, b binder[H, reader, writer := jsonrpc2.Reader(mcpConn), jsonrpc2.Writer(mcpConn) var ( h H - preempter = canceller{logger: logger} + preempter canceller ) bind := func(conn *jsonrpc2.Connection) jsonrpc2.Handler { h = b.bind(mcpConn, conn, s, onClose) @@ -252,8 +252,7 @@ type cancellationPropagator interface { // A canceller is a jsonrpc2.Preempter that cancels in-flight requests on MCP // cancelled notifications. type canceller struct { - conn *jsonrpc2.Connection - logger *slog.Logger + conn *jsonrpc2.Connection } // Preempt implements [jsonrpc2.Preempter]. @@ -267,33 +266,11 @@ func (c *canceller) Preempt(ctx context.Context, req *jsonrpc.Request) (result a if err != nil { return nil, err } - // The spec says implementations should log cancellation reasons, and - // the handler is the one place that can act on one, so the reason - // travels as the cause of the request's context rather than being - // dropped here. - c.logger.Debug("request cancelled by the peer", "id", id.Raw(), "reason", params.Reason) - go c.conn.CancelCause(id, &peerCancelledError{reason: params.Reason}) + go c.conn.Cancel(id) } return nil, jsonrpc2.ErrNotHandled } -// A peerCancelledError is the cause a request's context carries once the peer -// has sent a cancelled notification for it: [context.Cause] returns it to the -// handler with the reason the peer gave, and it unwraps to [context.Canceled] -// so that [errors.Is] keeps classifying the cancellation as one. -type peerCancelledError struct { - reason string -} - -func (e *peerCancelledError) Error() string { - if e.reason == "" { - return "request cancelled by the peer" - } - return "request cancelled by the peer: " + e.reason -} - -func (e *peerCancelledError) Unwrap() error { return context.Canceled } - // callSubscriptionsListen issues a "subscriptions/listen" call (SEP-2575) // without awaiting its JSON-RPC response. The call's logical lifetime is the // stream of notifications that follow on the same channel — the empty @@ -734,7 +711,7 @@ func (t *ioConn) Read(ctx context.Context) (jsonrpc.Message, error) { if batch { var respBatch *msgBatch // track incoming requests in the batch for _, msg := range msgs { - if req, ok := msg.(*jsonrpc.Request); ok && req.IsCall() { + if req, ok := msg.(*jsonrpc.Request); ok { if respBatch == nil { respBatch = &msgBatch{ unresolved: make(map[jsonrpc2.ID]int), diff --git a/mcp/transport_test.go b/mcp/transport_test.go index a20ee5f..d4d7782 100644 --- a/mcp/transport_test.go +++ b/mcp/transport_test.go @@ -5,7 +5,6 @@ package mcp import ( - "bytes" "context" "errors" "fmt" @@ -172,109 +171,6 @@ func TestIOConnRead_EmptyMethod(t *testing.T) { } } -// TestIOConnReadBatchNotifications is a regression test for go-sdk#1256: -// notifications carried inside a JSON-RPC batch must not be tracked as awaiting -// a response. Only calls belong in a batch's response array, so a batch's -// notifications must neither keep it from completing nor pollute the -// per-connection duplicate-ID tracking. -func TestIOConnReadBatchNotifications(t *testing.T) { - ctx := context.Background() - - tests := []struct { - name string - // input is one or more newline-separated batch payloads. - input string - // wantCalls records, for each message Read returns in order, whether it - // is a call (true) or a notification (false). Reading every message - // must succeed. - wantCalls []bool - // wantTrackedCalls is the number of in-flight call requests tracked across - // batches awaiting a response once every message has been read, before any - // response is written. - wantTrackedCalls int - // respondIDs are the call IDs to respond to, in order. - respondIDs []int64 - // wantWrites are the batch response payloads expected on the wire, each - // matched as a substring. - wantWrites []string - }{ - { - name: "call and notification is answered", - input: `[{"jsonrpc":"2.0","id":2,"method":"ping"},{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":99}}]`, - wantCalls: []bool{true, false}, - wantTrackedCalls: 1, - respondIDs: []int64{2}, - wantWrites: []string{`[{"jsonrpc":"2.0","id":2,"result":{}}]`}, - }, - { - name: "only notifications keeps the connection open", - input: `[{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":98}},{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":99}}]`, - wantCalls: []bool{false, false}, - wantTrackedCalls: 0, - }, - { - name: "repeated batches holding a notification", - input: `[{"jsonrpc":"2.0","id":2,"method":"ping"},{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":99}}]` + "\n" + - `[{"jsonrpc":"2.0","id":3,"method":"ping"},{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":99}}]`, - wantCalls: []bool{true, false, true, false}, - wantTrackedCalls: 2, - respondIDs: []int64{2, 3}, - wantWrites: []string{ - `[{"jsonrpc":"2.0","id":2,"result":{}}]`, - `[{"jsonrpc":"2.0","id":3,"result":{}}]`, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - sink := new(bytes.Buffer) - tr := newIOConn(rwc{ - rc: io.NopCloser(strings.NewReader(tt.input)), - wc: nopCloserWriter{sink}, - }) - t.Cleanup(func() { tr.Close() }) - - for i, wantCall := range tt.wantCalls { - msg, err := tr.Read(ctx) - if err != nil { - t.Fatalf("Read() #%d: unexpected error: %v", i, err) - } - req, ok := msg.(*jsonrpc.Request) - if !ok { - t.Fatalf("Read() #%d = %T, want *jsonrpc.Request", i, msg) - } - if req.IsCall() != wantCall { - t.Fatalf("Read() #%d IsCall() = %t, want %t", i, req.IsCall(), wantCall) - } - } - - // Only calls are tracked as awaiting a response; notifications must - // not linger in the batch bookkeeping. - tr.batchMu.Lock() - gotTrackedCalls := len(tr.batches) - tr.batchMu.Unlock() - if gotTrackedCalls != tt.wantTrackedCalls { - t.Errorf("tracked calls = %d, want %d", gotTrackedCalls, tt.wantTrackedCalls) - } - - for _, id := range tt.respondIDs { - resp := &jsonrpc.Response{ID: jsonrpc2.Int64ID(id), Result: []byte("{}")} - if err := tr.Write(ctx, resp); err != nil { - t.Fatalf("Write(response id=%d): %v", id, err) - } - } - - got := sink.String() - for _, want := range tt.wantWrites { - if !strings.Contains(got, want) { - t.Errorf("batch responses = %q, missing %q", got, want) - } - } - }) - } -} - // TestIOConnFrameCap verifies single inbound frame cap enforcement. func TestIOConnFrameCap(t *testing.T) { tests := []struct { diff --git a/oauthex/oauth2.go b/oauthex/oauth2.go index 0e77149..caed92f 100644 --- a/oauthex/oauth2.go +++ b/oauthex/oauth2.go @@ -16,7 +16,6 @@ import ( "net/netip" "net/url" "strings" - "sync" "syscall" "time" @@ -26,15 +25,7 @@ import ( const maxDiscoveryRedirects = 10 -// Build this on first use, not when the package loads. Building it checks the -// proxy via http.ProxyFromEnvironment, and net/http only reads the proxy -// environment once and then remembers it for the whole run. If that happened -// at import time, it would lock in the proxy setting before the app had a -// chance to set HTTPS_PROXY. sync.OnceValue waits until first use and still -// builds the transport just once. See #1276. -var defaultDiscoveryTransport = sync.OnceValue(func() http.RoundTripper { - return newDiscoveryTransport(http.DefaultTransport) -}) +var defaultDiscoveryTransport = newDiscoveryTransport(http.DefaultTransport) type httpStatusError struct { StatusCode int @@ -112,7 +103,7 @@ func checkHTTPSOrLoopback(addr string) error { } func newDiscoveryClient(c *http.Client) *http.Client { - transport := defaultDiscoveryTransport() + transport := defaultDiscoveryTransport if c != nil && c.Transport != nil { // a caller that has taken over dialing or TLS opts out of the checks if t, ok := c.Transport.(*http.Transport); ok && t.DialContext == nil && t.DialTLSContext == nil {