Skip to content

Conversation

jaredly
Copy link

@jaredly jaredly commented Oct 3, 2025

Description

Adds the _meta field to marshaled Tool json. Unmarshalling was already handled.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • MCP spec compatibility implementation
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring (no functional changes)
  • Performance improvement
  • Tests only (no functional changes)
  • Other (please describe):

Checklist

  • My code follows the code style of this project
  • I have performed a self-review of my own code
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the documentation accordingly (didn't seem necessary)

MCP Spec Compliance

  • This PR implements a feature defined in the MCP specification
  • Link to relevant spec section: Link text
  • Implementation follows the specification exactly (the current implementation doesn't do validation on _meta keys, and I didn't change that)

Additional Information

Summary by CodeRabbit

  • New Features

    • JSON responses for tools now include an optional _meta field when metadata is present, providing additional contextual information while remaining backward compatible when metadata is absent.
  • Tests

    • Added tests to ensure the _meta field is included when metadata is provided and omitted when metadata is nil.

@jaredly jaredly changed the title Tool meta Preserve Tool _meta when marshaling to JSON Oct 3, 2025
Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

Walkthrough

Tool.MarshalJSON now conditionally includes a _meta JSON field when the Tool's Meta is non-nil. Two unit tests were added to verify _meta is present when Meta is set and omitted when Meta is nil.

Changes

Cohort / File(s) Summary of Changes
Tool JSON marshaling
mcp/tools.go
Update Tool.MarshalJSON to include a _meta key in the serialized JSON when Tool.Meta is non-nil.
Tests
mcp/tools_test.go
Add TestToolMetaMarshaling and TestToolMetaMarshalingOmitsWhenNil to verify _meta is emitted when present and omitted when Meta is nil.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

type: enhancement, area: mcp spec

Suggested reviewers

  • pottekkat
  • robert-jackson-glean

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly and accurately describes the primary change—adding preservation of the Tool’s _meta field during JSON marshaling—and uses clear, specific language without extraneous details.
Description Check ✅ Passed The PR description follows the repository’s template by providing a concise Description, correctly checked Type of Change entries, a completed Checklist, and an MCP Spec Compliance section with a valid spec link; the optional Additional Information section remains empty but does not detract from completeness.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e58af76 and cc17cf0.

📒 Files selected for processing (1)
  • mcp/tools_test.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • mcp/tools_test.go

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
mcp/tools_test.go (1)

1400-1428: Test correctly verifies _meta marshaling. Consider adding a nil case.

The test appropriately validates that a non-nil Meta field is marshaled as _meta and matches the expected value.

To ensure complete coverage of the omitempty behavior, consider adding a test case that verifies _meta is omitted when Meta is nil:

+func TestToolMetaMarshalingOmitsWhenNil(t *testing.T) {
+	// Marshal a tool without Meta
+	data, err := json.Marshal(Tool{
+		Name:        "test-tool-no-meta",
+		Description: "A test tool without meta data",
+		InputSchema: ToolInputSchema{
+			Type:       "object",
+			Properties: map[string]any{},
+		},
+	})
+	assert.NoError(t, err)
+
+	// Unmarshal to map
+	var result map[string]any
+	err = json.Unmarshal(data, &result)
+	assert.NoError(t, err)
+
+	// Check that _meta field is not present
+	assert.NotContains(t, result, "_meta", "Tool without Meta should not include _meta field")
+}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 61b5d9e and e58af76.

📒 Files selected for processing (2)
  • mcp/tools.go (1 hunks)
  • mcp/tools_test.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
mcp/tools_test.go (2)
mcp/tools.go (4)
  • Tool (557-574)
  • Description (875-879)
  • ToolInputSchema (632-632)
  • Properties (1117-1121)
mcp/types.go (2)
  • Meta (121-133)
  • NewMetaFromMap (156-166)
mcp/tools.go (1)
mcp/types.go (1)
  • Meta (121-133)
🔇 Additional comments (1)
mcp/tools.go (1)

616-619: LGTM!

The conditional marshaling of _meta when Meta is non-nil is correctly implemented and matches the existing pattern in CallToolResult.MarshalJSON (lines 478-480). This ensures spec compliance without breaking backward compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant