fix: do not omitempty ReadOnlyHint in ToolAnnotations (#908)
`ReadOnlyHint` is a `bool` with omitempty, so the zero value (false) is indistinguishable from unset, and **drops out** of marshaled JSON. The current behavior causes an issue with the OpenAI MCP app submission because they require explicit hints. Consumers that explicitly set `ReadOnlyHint: false` on write tools lose the field on the wire. Removing `omitempty` ensures `false` is always serialized, which matches the MCP spec default. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Maciej Kisiel <mkisiel@google.com> Co-authored-by: guglielmoc <guglielmoc@google.com>
This commit is contained in:
parent
bed83e104b
commit
c044cdb734
6 changed files with 107 additions and 2 deletions
|
|
@ -27,6 +27,12 @@ Options listed below were added and will be removed in the 1.9.0 version of the
|
|||
Params), restoring the previous behavior. The default behavior was changed to
|
||||
align with SEP-2164 and the JSON-RPC specification.
|
||||
|
||||
- `hintomitempty` added. If set to `1`, `ToolAnnotations` JSON marshaling
|
||||
will omit `ReadOnlyHint` and `IdempotentHint` when their value is `false`,
|
||||
restoring the previous behavior. The default behavior was changed to always
|
||||
serialize these fields, since their Go types are bare `bool` (not `*bool`)
|
||||
and omitting `false` made it indistinguishable from unset.
|
||||
|
||||
- `allowsessionsinstateless` added. If set to `1`, stateless streamable HTTP
|
||||
servers will read the `Mcp-Session-Id` request header (or generate one via
|
||||
`GetSessionID`), set it on response headers, and accept `DELETE` requests,
|
||||
|
|
|
|||
|
|
@ -63,3 +63,8 @@ v2.
|
|||
- `StreamableHTTPOptions.CrossOriginProtection` should not have been part of
|
||||
the SDK API. Cross-origin protection is a general HTTP concern, not specific
|
||||
to MCP, and can be applied as standard HTTP middleware.
|
||||
|
||||
- `ToolAnnotations` (`mcp/protocol.go`) should have all fields typed as `*bool`
|
||||
for full control to define what is being sent over the wire. Different
|
||||
MCP clients have different requirements, and some of them require all fields
|
||||
to be explicitly set to either `true` or `false`.
|
||||
|
|
|
|||
|
|
@ -26,6 +26,12 @@ Options listed below were added and will be removed in the 1.9.0 version of the
|
|||
Params), restoring the previous behavior. The default behavior was changed to
|
||||
align with SEP-2164 and the JSON-RPC specification.
|
||||
|
||||
- `hintomitempty` added. If set to `1`, `ToolAnnotations` JSON marshaling
|
||||
will omit `ReadOnlyHint` and `IdempotentHint` when their value is `false`,
|
||||
restoring the previous behavior. The default behavior was changed to always
|
||||
serialize these fields, since their Go types are bare `bool` (not `*bool`)
|
||||
and omitting `false` made it indistinguishable from unset.
|
||||
|
||||
- `allowsessionsinstateless` added. If set to `1`, stateless streamable HTTP
|
||||
servers will read the `Mcp-Session-Id` request header (or generate one via
|
||||
`GetSessionID`), set it on response headers, and accept `DELETE` requests,
|
||||
|
|
|
|||
|
|
@ -62,3 +62,8 @@ v2.
|
|||
- `StreamableHTTPOptions.CrossOriginProtection` should not have been part of
|
||||
the SDK API. Cross-origin protection is a general HTTP concern, not specific
|
||||
to MCP, and can be applied as standard HTTP middleware.
|
||||
|
||||
- `ToolAnnotations` (`mcp/protocol.go`) should have all fields typed as `*bool`
|
||||
for full control to define what is being sent over the wire. Different
|
||||
MCP clients have different requirements, and some of them require all fields
|
||||
to be explicitly set to either `true` or `false`.
|
||||
|
|
@ -1346,6 +1346,13 @@ type Tool struct {
|
|||
Icons []Icon `json:"icons,omitempty"`
|
||||
}
|
||||
|
||||
// hintomitempty is a compatibility parameter that restores the pre-1.7.0
|
||||
// behavior of [ToolAnnotations] JSON marshaling, where false-valued bare bool
|
||||
// fields (ReadOnlyHint, IdempotentHint) were omitted from the output.
|
||||
// See the documentation for the mcpgodebug package for instructions on how to
|
||||
// enable it.
|
||||
var hintomitempty = mcpgodebug.Value("hintomitempty")
|
||||
|
||||
// Additional properties describing a Tool to clients.
|
||||
//
|
||||
// NOTE: all properties in ToolAnnotations are hints. They are not
|
||||
|
|
@ -1368,7 +1375,7 @@ type ToolAnnotations struct {
|
|||
// (This property is meaningful only when ReadOnlyHint == false.)
|
||||
//
|
||||
// Default: false
|
||||
IdempotentHint bool `json:"idempotentHint,omitempty"`
|
||||
IdempotentHint bool `json:"idempotentHint"`
|
||||
// If true, this tool may interact with an "open world" of external entities. If
|
||||
// false, the tool's domain of interaction is closed. For example, the world of
|
||||
// a web search tool is open, whereas that of a memory tool is not.
|
||||
|
|
@ -1378,11 +1385,30 @@ type ToolAnnotations struct {
|
|||
// If true, the tool does not modify its environment.
|
||||
//
|
||||
// Default: false
|
||||
ReadOnlyHint bool `json:"readOnlyHint,omitempty"`
|
||||
ReadOnlyHint bool `json:"readOnlyHint"`
|
||||
// A human-readable title for the tool.
|
||||
Title string `json:"title,omitempty"`
|
||||
}
|
||||
|
||||
// MarshalJSON implements [json.Marshaler] for ToolAnnotations.
|
||||
//
|
||||
// To restore the previous behavior where false-valued ReadOnlyHint and
|
||||
// IdempotentHint were omitted, set MCPGODEBUG=hintomitempty=1.
|
||||
func (t ToolAnnotations) MarshalJSON() ([]byte, error) {
|
||||
if hintomitempty == "1" {
|
||||
type compat struct {
|
||||
DestructiveHint *bool `json:"destructiveHint,omitempty"`
|
||||
IdempotentHint bool `json:"idempotentHint,omitempty"`
|
||||
OpenWorldHint *bool `json:"openWorldHint,omitempty"`
|
||||
ReadOnlyHint bool `json:"readOnlyHint,omitempty"`
|
||||
Title string `json:"title,omitempty"`
|
||||
}
|
||||
return json.Marshal(compat(t))
|
||||
}
|
||||
type nomethod ToolAnnotations
|
||||
return json.Marshal(nomethod(t))
|
||||
}
|
||||
|
||||
type ToolListChangedParams struct {
|
||||
// This property is reserved by the protocol to allow clients and servers to
|
||||
// attach additional metadata to their responses.
|
||||
|
|
|
|||
|
|
@ -1194,3 +1194,60 @@ func TestContentUnmarshal(t *testing.T) {
|
|||
var gotpm PromptMessage
|
||||
roundtrip(pm, &gotpm)
|
||||
}
|
||||
|
||||
func TestToolAnnotations_MarshalJSON(t *testing.T) {
|
||||
boolPtr := func(b bool) *bool { return &b }
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
in ToolAnnotations
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "ZeroValue",
|
||||
in: ToolAnnotations{},
|
||||
want: `{"idempotentHint":false,"readOnlyHint":false}`,
|
||||
},
|
||||
{
|
||||
name: "AllFalse",
|
||||
in: ToolAnnotations{
|
||||
DestructiveHint: boolPtr(false),
|
||||
IdempotentHint: false,
|
||||
OpenWorldHint: boolPtr(false),
|
||||
ReadOnlyHint: false,
|
||||
},
|
||||
want: `{"destructiveHint":false,"idempotentHint":false,"openWorldHint":false,"readOnlyHint":false}`,
|
||||
},
|
||||
{
|
||||
name: "AllTrue",
|
||||
in: ToolAnnotations{
|
||||
DestructiveHint: boolPtr(true),
|
||||
IdempotentHint: true,
|
||||
OpenWorldHint: boolPtr(true),
|
||||
ReadOnlyHint: true,
|
||||
Title: "my tool",
|
||||
},
|
||||
want: `{"destructiveHint":true,"idempotentHint":true,"openWorldHint":true,"readOnlyHint":true,"title":"my tool"}`,
|
||||
},
|
||||
{
|
||||
name: "MixedValues",
|
||||
in: ToolAnnotations{
|
||||
ReadOnlyHint: true,
|
||||
Title: "read tool",
|
||||
},
|
||||
want: `{"idempotentHint":false,"readOnlyHint":true,"title":"read tool"}`,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got, err := json.Marshal(tt.in)
|
||||
if err != nil {
|
||||
t.Fatalf("json.Marshal(%v) failed: %v", tt.in, err)
|
||||
}
|
||||
if diff := cmp.Diff(tt.want, string(got)); diff != "" {
|
||||
t.Errorf("json.Marshal() mismatch (-want +got):\n%s", diff)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue