From 8e8ea04778b452a292b29db2731369637c47be22 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Mon, 27 Oct 2025 22:28:12 +0000 Subject: [PATCH] mcp: add ClientCapabilities.Roots.Supported Address an API oversight (#607) by exposing a Supported field on the ClientCapabilities.Roots. Given our promise of compatibility, we unfortunately can't change the Roots field, so this may be the best we can do to work around this oversight. --- docs/warts.md | 17 ++++++++++ internal/docs/doc.go | 1 + internal/docs/warts.src.md | 16 ++++++++++ mcp/client.go | 1 + mcp/client_test.go | 14 +++++---- mcp/protocol.go | 64 ++++++++++++++++++++++++++++++++++---- mcp/protocol_test.go | 27 ++++++++++++++++ 7 files changed, 128 insertions(+), 12 deletions(-) create mode 100644 docs/warts.md create mode 100644 internal/docs/warts.src.md diff --git a/docs/warts.md b/docs/warts.md new file mode 100644 index 00000000..572439f6 --- /dev/null +++ b/docs/warts.md @@ -0,0 +1,17 @@ + +# "Warts" - API oversights to reconsider for v2 + +This file collects a list of API oversights or rough edges that we've uncovered +post v1.0, both to address them a potential future v2, and to document their +current workarounds. + +- `EventStore.Open` is unnecessary. This was an artifact of an earlier version + of the SDK where event persistence and delivery were combined. + + **Workaround**: `Open` may be implemented as a no-op. + +- `ClientCapabilities.Roots` should have been a distinguished struct pointer. + This was simply an oversight. + + **Workaround**: the `ClientCapabilities.Roots.Supported` field is managed by + custom marshaling of `ClientCapabilities`. diff --git a/internal/docs/doc.go b/internal/docs/doc.go index 7b23ad63..aa2e9b99 100644 --- a/internal/docs/doc.go +++ b/internal/docs/doc.go @@ -7,6 +7,7 @@ //go:generate weave -o ../../docs/protocol.md ./protocol.src.md //go:generate weave -o ../../docs/client.md ./client.src.md //go:generate weave -o ../../docs/server.md ./server.src.md +//go:generate weave -o ../../docs/warts.md ./warts.src.md //go:generate weave -o ../../docs/troubleshooting.md ./troubleshooting.src.md // The doc package generates the documentation at /doc, via go:generate. diff --git a/internal/docs/warts.src.md b/internal/docs/warts.src.md new file mode 100644 index 00000000..961ccdd4 --- /dev/null +++ b/internal/docs/warts.src.md @@ -0,0 +1,16 @@ +# "Warts" - API oversights to reconsider for v2 + +This file collects a list of API oversights or rough edges that we've uncovered +post v1.0, both to address them a potential future v2, and to document their +current workarounds. + +- `EventStore.Open` is unnecessary. This was an artifact of an earlier version + of the SDK where event persistence and delivery were combined. + + **Workaround**: `Open` may be implemented as a no-op. + +- `ClientCapabilities.Roots` should have been a distinguished struct pointer. + This was simply an oversight. + + **Workaround**: the `ClientCapabilities.Roots.Supported` field is managed by + custom marshaling of `ClientCapabilities`. diff --git a/mcp/client.go b/mcp/client.go index d7e3ae5a..0762bd87 100644 --- a/mcp/client.go +++ b/mcp/client.go @@ -117,6 +117,7 @@ type ClientSessionOptions struct{} func (c *Client) capabilities() *ClientCapabilities { caps := &ClientCapabilities{} + caps.Roots.Supported = true caps.Roots.ListChanged = true if c.opts.CreateMessageHandler != nil { caps.Sampling = &SamplingCapabilities{} diff --git a/mcp/client_test.go b/mcp/client_test.go index eaeedc81..3192a3b9 100644 --- a/mcp/client_test.go +++ b/mcp/client_test.go @@ -202,9 +202,10 @@ func TestClientCapabilities(t *testing.T) { name: "With initial capabilities", configureClient: func(s *Client) {}, wantCapabilities: &ClientCapabilities{ - Roots: struct { - ListChanged bool "json:\"listChanged,omitempty\"" - }{ListChanged: true}, + Roots: rootsCapabilities{ + Supported: true, + ListChanged: true, + }, }, }, { @@ -216,9 +217,10 @@ func TestClientCapabilities(t *testing.T) { }, }, wantCapabilities: &ClientCapabilities{ - Roots: struct { - ListChanged bool "json:\"listChanged,omitempty\"" - }{ListChanged: true}, + Roots: rootsCapabilities{ + Supported: true, + ListChanged: true, + }, Sampling: &SamplingCapabilities{}, }, }, diff --git a/mcp/protocol.go b/mcp/protocol.go index 1312dfbd..efac3d79 100644 --- a/mcp/protocol.go +++ b/mcp/protocol.go @@ -183,17 +183,71 @@ func (x *CancelledParams) SetProgressToken(t any) { setProgressToken(x, t) } type ClientCapabilities struct { // Experimental, non-standard capabilities that the client supports. Experimental map[string]any `json:"experimental,omitempty"` - // Present if the client supports listing roots. + + // Roots reports roots capabilities. + // + // Due to an API oversight (#607), roots is a non-pointer. Check the + // Supported field to see if the roots capability is present Roots struct { + // Suppported reports whether roots/list is supported. It works around an + // API oversight: roots capabilities should have been a distinguished type, + // similar to other capabilities. + Supported bool `json:"-"` + // Whether the client supports notifications for changes to the roots list. ListChanged bool `json:"listChanged,omitempty"` - } `json:"roots,omitempty"` - // Present if the client supports sampling from an LLM. + } `json:"roots"` + + // Sampling is present if the client supports sampling from an LLM. Sampling *SamplingCapabilities `json:"sampling,omitempty"` - // Present if the client supports elicitation from the server. + + // Elicitation is present if the client supports elicitation from the server. Elicitation *ElicitationCapabilities `json:"elicitation,omitempty"` } +// clientCapabilitiesV2 is a version of ClientCapabilities fixes +type clientCapabilitiesV2 struct { + Experimental map[string]any `json:"experimental,omitempty"` + Roots *rootsCapabilities `json:"roots,omitempty"` + Sampling *SamplingCapabilities `json:"sampling,omitempty"` + Elicitation *ElicitationCapabilities `json:"elicitation,omitempty"` +} + +// See #607: rootsCapabilities is for internal use, fixing an API oversight. +type rootsCapabilities struct { + Supported bool `json:"-"` + ListChanged bool `json:"listChanged,omitempty"` +} + +func (c ClientCapabilities) MarshalJSON() ([]byte, error) { + c2 := &clientCapabilitiesV2{ + Experimental: c.Experimental, + Sampling: c.Sampling, + Elicitation: c.Elicitation, + } + if c.Roots.Supported { + c2.Roots = &rootsCapabilities{ + ListChanged: c.Roots.ListChanged, + } + } + return json.Marshal(c2) +} + +func (c *ClientCapabilities) UnmarshalJSON(data []byte) error { + var c2 clientCapabilitiesV2 + if err := json.Unmarshal(data, &c2); err != nil { + return err + } + c.Experimental = c2.Experimental + c.Elicitation = c2.Elicitation + c.Sampling = c2.Sampling + if c2.Roots != nil { + c.Roots = *c2.Roots + c.Roots.Supported = true + } + return nil +} + type CompleteParamsArgument struct { // The name of the argument Name string `json:"name"` @@ -1035,8 +1089,6 @@ type ResourceUpdatedNotificationParams struct { func (*ResourceUpdatedNotificationParams) isParams() {} -// TODO(jba): add CompleteRequest and related types. - // A request from the server to elicit additional information from the user via the client. type ElicitParams struct { // This property is reserved by the protocol to allow clients and servers to diff --git a/mcp/protocol_test.go b/mcp/protocol_test.go index 67d021d1..62acc6f4 100644 --- a/mcp/protocol_test.go +++ b/mcp/protocol_test.go @@ -531,3 +531,30 @@ func TestContentUnmarshal(t *testing.T) { var gotpm PromptMessage roundtrip(pm, &gotpm) } + +func TestClientCapabilitiesJSON(t *testing.T) { + tests := []struct { + capabilities ClientCapabilities + want string + }{ + {ClientCapabilities{}, "{}"}, + {ClientCapabilities{Roots: rootsCapabilities{Supported: true}}, `{"roots":{}}`}, + {ClientCapabilities{Roots: rootsCapabilities{Supported: true, ListChanged: true}}, `{"roots":{"listChanged":true}}`}, + } + for _, test := range tests { + gotBytes, err := json.Marshal(test.capabilities) + if err != nil { + t.Fatalf("json.Marshal(%+v) failed: %v", test.capabilities, err) + } + if got := string(gotBytes); got != test.want { + t.Fatalf("json.Marshal(%+v) = %q, want %q", test.capabilities, got, test.want) + } + var caps2 ClientCapabilities + if err := json.Unmarshal(gotBytes, &caps2); err != nil { + t.Fatalf("json.Unmarshal failed: %v", err) + } + if diff := cmp.Diff(test.capabilities, caps2); diff != "" { + t.Errorf("Unmarshal mismatch (-original +unmarshaled):\n%s", diff) + } + } +}