Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion mcp/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,9 @@ func (t Tool) MarshalJSON() ([]byte, error) {
m["outputSchema"] = t.OutputSchema
}

m["annotations"] = t.Annotations
if t.Annotations.HasAny() {
m["annotations"] = t.Annotations
}

if t.DeferLoading {
m["defer_loading"] = t.DeferLoading
Expand Down Expand Up @@ -777,6 +779,15 @@ type ToolAnnotation struct {
OpenWorldHint *bool `json:"openWorldHint,omitempty"`
}

// HasAny reports whether any annotation field was explicitly set.
func (a ToolAnnotation) HasAny() bool {
return a.Title != "" ||
a.ReadOnlyHint != nil ||
a.DestructiveHint != nil ||
a.IdempotentHint != nil ||
a.OpenWorldHint != nil
}

// ToolOption is a function that configures a Tool.
// It provides a flexible way to set various properties of a Tool using the functional options pattern.
type ToolOption func(*Tool)
Expand Down Expand Up @@ -944,6 +955,15 @@ func WithRawOutputSchema(schema json.RawMessage) ToolOption {
}
}

// WithoutDefaultAnnotations clears the default annotation hints initialized by
// NewTool so the annotations field is omitted from JSON unless later options
// set annotation values explicitly.
func WithoutDefaultAnnotations() ToolOption {
return func(t *Tool) {
t.Annotations = ToolAnnotation{}
}
Comment on lines +961 to +964

@coderabbitai coderabbitai Bot Jun 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Don't clear an explicitly set annotation title here.

This resets the whole ToolAnnotation, so NewTool("test", WithTitleAnnotation("My Tool"), WithoutDefaultAnnotations()) drops the user-supplied title and MarshalJSON omits annotations entirely. WithoutDefaultAnnotations() should only strip the default hint pointers, not unrelated explicit metadata.

Suggested fix
 func WithoutDefaultAnnotations() ToolOption {
 	return func(t *Tool) {
-		t.Annotations = ToolAnnotation{}
+		title := t.Annotations.Title
+		t.Annotations = ToolAnnotation{
+			Title: title,
+		}
 	}
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func WithoutDefaultAnnotations() ToolOption {
return func(t *Tool) {
t.Annotations = ToolAnnotation{}
}
func WithoutDefaultAnnotations() ToolOption {
return func(t *Tool) {
title := t.Annotations.Title
t.Annotations = ToolAnnotation{
Title: title,
}
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mcp/tools.go` around lines 961 - 964, WithoutDefaultAnnotations currently
resets the entire ToolAnnotation on Tool, which clears explicitly set metadata
like the title and can make MarshalJSON omit annotations entirely. Update
WithoutDefaultAnnotations() so it only removes the default hint pointer fields
while preserving any user-supplied annotations such as the title set by
WithTitleAnnotation, and keep the behavior localized to the ToolAnnotation
handling in that option.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai was this fixed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ezynda3 No—the current implementation still resets the entire ToolAnnotation, so it clears a title supplied by an earlier option. For example:

NewTool("test",
    WithTitleAnnotation("My Tool"),
    WithoutDefaultAnnotations(),
)

will omit annotations rather than retain {"title":"My Tool"}. WithoutDefaultAnnotations() should preserve t.Annotations.Title while clearing only the default hint pointers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@syf2211 pleas have a look

}

// WithToolAnnotation adds optional hints about the Tool.
func WithToolAnnotation(annotation ToolAnnotation) ToolOption {
return func(t *Tool) {
Expand Down
90 changes: 90 additions & 0 deletions mcp/tools_additional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,96 @@ func TestToolAnnotations(t *testing.T) {
})
}

func TestToolAnnotationsMarshalJSON(t *testing.T) {
rawSchema := json.RawMessage(`{"type":"object","properties":{}}`)

tests := []struct {
name string
tool Tool
wantAnnotations bool
wantContains string
wantNotContains string
checkAnnotations func(t *testing.T, annotations map[string]any)
}{
{
name: "NewTool includes default annotations",
tool: NewTool("test", WithDescription("desc")),
wantAnnotations: true,
checkAnnotations: func(t *testing.T, annotations map[string]any) {
t.Helper()
assert.Equal(t, false, annotations["readOnlyHint"])
assert.Equal(t, true, annotations["destructiveHint"])
},
},
{
name: "WithoutDefaultAnnotations omits annotations field",
tool: NewTool("test",
WithoutDefaultAnnotations(),
WithDescription("desc"),
),
wantAnnotations: false,
wantNotContains: `"annotations"`,
},
{
name: "WithoutDefaultAnnotations with explicit hint includes annotations",
tool: NewTool("test",
WithoutDefaultAnnotations(),
WithReadOnlyHintAnnotation(true),
),
wantAnnotations: true,
checkAnnotations: func(t *testing.T, annotations map[string]any) {
t.Helper()
assert.Equal(t, true, annotations["readOnlyHint"])
assert.NotContains(t, annotations, "destructiveHint")
},
},
{
name: "NewToolWithRawSchema omits annotations when unset",
tool: NewToolWithRawSchema("raw-tool", "Raw tool", rawSchema),
wantAnnotations: false,
wantNotContains: `"annotations"`,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
data, err := json.Marshal(tt.tool)
require.NoError(t, err)

if tt.wantContains != "" {
assert.Contains(t, string(data), tt.wantContains)
}
if tt.wantNotContains != "" {
assert.NotContains(t, string(data), tt.wantNotContains)
}

var parsed map[string]any
require.NoError(t, json.Unmarshal(data, &parsed))

annotations, ok := parsed["annotations"].(map[string]any)
if tt.wantAnnotations {
require.True(t, ok)
if tt.checkAnnotations != nil {
tt.checkAnnotations(t, annotations)
}
return
}

_, hasAnnotations := parsed["annotations"]
assert.False(t, hasAnnotations)
})
}

t.Run("zero-value ToolAnnotation HasAny is false", func(t *testing.T) {
assert.False(t, ToolAnnotation{}.HasAny())
})

t.Run("ToolAnnotation HasAny detects explicit fields", func(t *testing.T) {
assert.True(t, ToolAnnotation{Title: "x"}.HasAny())
assert.True(t, ToolAnnotation{ReadOnlyHint: ToBoolPtr(false)}.HasAny())
})
}

// Test Tool with both InputSchema and OutputSchema

func TestToolWithBothSchemas(t *testing.T) {
Expand Down
Loading