task: add _meta field to relevant types as defined in MCP specification #429
Conversation
WalkthroughThe changes introduce an optional Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
mcp/types.go (1)
153-163: Consider input map mutation side effect.The function modifies the input map by deleting the "progressToken" key, which could be unexpected for callers. Consider making a copy of the input map first to avoid side effects.
func NewMetaFromMap(m map[string]any) *Meta { + // Create a copy to avoid modifying the input map + mapCopy := make(map[string]any) + for k, v := range m { + mapCopy[k] = v + } + - progressToken := m["progressToken"] + progressToken := mapCopy["progressToken"] if progressToken != nil { - delete(m, "progressToken") + delete(mapCopy, "progressToken") } return &Meta{ ProgressToken: progressToken, - AdditionalFields: m, + AdditionalFields: mapCopy, } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
mcp/prompts.go(1 hunks)mcp/tools.go(1 hunks)mcp/types.go(11 hunks)mcp/utils.go(3 hunks)server/sse_test.go(1 hunks)
🔇 Additional comments (7)
mcp/prompts.go (1)
46-47: LGTM! Clean implementation following MCP specification.The addition of the
Metafield to thePromptstruct is well-implemented:
- Uses pointer type for optional metadata
- Correct JSON tag
_meta,omitemptyaligns with MCP specification- Proper documentation explaining the field's purpose
- No breaking changes since the field is optional
mcp/tools.go (1)
473-474: LGTM! Consistent with other MCP specification changes.The
Metafield addition to theToolstruct follows the same clean pattern as inPrompt:
- Consistent use of
*Metapointer type- Proper
_meta,omitemptyJSON tagging- Good documentation alignment with MCP specification
- Maintains consistency across the codebase
mcp/utils.go (2)
598-598: Consistent metadata handling across parsing functions.Both
ParseCallToolResultandParseReadResourceResultfollow the same clean pattern asParseGetPromptResult, ensuring consistent metadata handling across the entire codebase.Also applies to: 680-680
532-532: Good refactoring to structured metadata handling.The update to use
NewMetaFromMap(metaMap)improves type safety and consistency with the newMetastruct approach.Verify that the
NewMetaFromMapfunction properly handles edge cases like nil maps and invalid data:#!/bin/bash # Description: Search for NewMetaFromMap function definition and its error handling # Search for the NewMetaFromMap function definition ast-grep --pattern 'func NewMetaFromMap($$$) $$$' # Search for any error handling in NewMetaFromMap rg -A 10 "func NewMetaFromMap"server/sse_test.go (1)
1260-1260: Good test update maintaining consistency with new metadata handling.The test correctly adopts the new
mcp.NewMetaFromMappattern while preserving its original purpose of testing JSON marshaling errors. This ensures test code stays consistent with the production code changes.mcp/types.go (2)
246-246: Breaking change acknowledged for MCP specification compliance.The change from
map[string]anyto*Metais a breaking change but expected for MCP specification compatibility. This aligns with the structured metadata approach being implemented across the codebase.
650-651: Systematic Meta field addition follows consistent pattern.The addition of
Meta *Metafields with"_meta,omitempty"JSON tags across all relevant structs is well-implemented and follows a consistent pattern. The comments are uniform and the implementation aligns with the MCP specification requirements.Also applies to: 676-677, 707-708, 721-722, 855-856, 868-869, 883-884, 916-917, 1050-1051
|
When will this PR be merged? |
Description
This PR adds _meta field to relevant types
Fixes #416
Type of Change
Checklist
MCP Spec Compliance
Summary by CodeRabbit
New Features
Bug Fixes
Tests