Skip to content

Conversation

@jaredly
Copy link
Contributor

@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
@coderabbitai
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.

@ezynda3 ezynda3 merged commit 3458f0b into mark3labs:main Oct 13, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants