go-sdk
go-sdk copied to clipboard
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.
I'm not sure it was an oversight. Our client always supports roots, even if the user doesn't add any.
I assume that servers using the Go SDK should support any client that correctly implements the spec (including clients that lack support of roots)
@jba I'm guessing that the origin of the mistake was that this type was internal only, for use by the client. In that case, it was acceptable to have inlined structs. But when capabilities were exported it became an API oversight, because we need to be able to represent whether the roots capability is present on the client.
How about this option? I'm not totally sure it would work.
Add a Roots2 field, and move the json:roots tag to that field. Its type is a pointer. Change our client to populate Roots2.
At least this removes the asymmetry between Roots and the other capabilities. Servers have to know to check Roots2, but is this so different from them having to know to check Supported?
Technically speaking any change to struct tags is a breaking API change, no? I realized that I removed omitempty in this CL, and probably can't even do that.
Technically speaking any change to struct tags is a breaking API change, no?
Excellent point. The spec says (Italics mine):
Two struct types are identical if they have the same sequence of fields, and if corresponding pairs of fields have the same names, identical types, and identical tags...
Technically speaking any change to struct tags is a breaking API change, no? I realized that I removed omitempty in this CL, and probably can't even do that.
Adding the MarshalJSON and UnmarshalJSON methods is just as likely to break other people's usage of the struct as changing the json tags. Both change the behavior of the json package.
I don't see any public functions where the ClientCapabilities struct is used as an input or output so the odds of someone outside the SDK being affected are very low. The only possibility that I see is if client middleware is processing the params of the initialize method.
Adding a new field to a struct is almost always safe. (ref)
Adding RootsV2 and deprecating the old field is technically the right way to go here but I don't think anyone would notice if you made the current field a pointer.