mcp: add ClientCapabilities.Roots.Supported

docs/warts.md

@@ -0,0 +1,17 @@
+<!-- Autogenerated by weave; DO NOT EDIT -->
+# "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`.

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.

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`.

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{}

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{},
 			},
 		},

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

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)
+		}
+	}
+}