-
Notifications
You must be signed in to change notification settings - Fork 702
feat: add NewToolResultJSON #550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a new generic helper NewToolResultJSON[T any](data T) in package mcp that marshals the provided data to JSON, returns a CallToolResult with TextContent containing the JSON string and StructuredContent set to the original data, and returns an error on marshal failure. Docs updated to use the new return form. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
mcp/utils.go (1)
256-258
: Tighten doc comment phrasingGrammar and clarity nit: “a JSON content” is slightly awkward; also explicitly calling out “JSON-encoded text content” improves precision.
-// NewToolResultJSON creates a new CallToolResult with a JSON content. -// This function panics if the JSON marshalling fails. +// NewToolResultJSON creates a CallToolResult with JSON-encoded text content, +// and sets structuredContent to the original value.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
mcp/utils.go
(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: xinwo
PR: mark3labs/mcp-go#35
File: mcp/tools.go:107-137
Timestamp: 2025-03-04T06:59:43.882Z
Learning: Tool responses from the MCP server shouldn't contain RawInputSchema, which is why the UnmarshalJSON method for the Tool struct is implemented to handle only the structured InputSchema format.
Learnt from: xinwo
PR: mark3labs/mcp-go#35
File: mcp/tools.go:0-0
Timestamp: 2025-03-04T07:00:57.111Z
Learning: The Tool struct in mark3labs/mcp-go handles both InputSchema and RawInputSchema formats. When unmarshaling JSON, it first tries to parse into a structured ToolInputSchema format, and if that fails, it falls back to using the raw schema format, providing symmetry with the MarshalJSON method.
📚 Learning: 2025-03-04T06:59:43.882Z
Learnt from: xinwo
PR: mark3labs/mcp-go#35
File: mcp/tools.go:107-137
Timestamp: 2025-03-04T06:59:43.882Z
Learning: Tool responses from the MCP server shouldn't contain RawInputSchema, which is why the UnmarshalJSON method for the Tool struct is implemented to handle only the structured InputSchema format.
Applied to files:
mcp/utils.go
📚 Learning: 2025-03-04T07:00:57.111Z
Learnt from: xinwo
PR: mark3labs/mcp-go#35
File: mcp/tools.go:0-0
Timestamp: 2025-03-04T07:00:57.111Z
Learning: The Tool struct in mark3labs/mcp-go handles both InputSchema and RawInputSchema formats. When unmarshaling JSON, it first tries to parse into a structured ToolInputSchema format, and if that fails, it falls back to using the raw schema format, providing symmetry with the MarshalJSON method.
Applied to files:
mcp/utils.go
📚 Learning: 2025-03-04T07:00:57.111Z
Learnt from: xinwo
PR: mark3labs/mcp-go#35
File: mcp/tools.go:0-0
Timestamp: 2025-03-04T07:00:57.111Z
Learning: The Tool struct in the mark3labs/mcp-go project should handle both InputSchema and RawInputSchema consistently between MarshalJSON and UnmarshalJSON methods, even though the tools response from MCP server typically doesn't contain rawInputSchema.
Applied to files:
mcp/utils.go
🧬 Code Graph Analysis (1)
mcp/utils.go (3)
mcp/tools.go (1)
CallToolResult
(40-51)mcp/types.go (3)
Content
(885-887)TextContent
(891-898)TextContent
(900-900)mcp/consts.go (1)
ContentTypeText
(4-4)
9480382
to
c31fa3d
Compare
c31fa3d
to
f7124ac
Compare
There was a problem hiding this 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 (3)
www/docs/pages/servers/advanced.mdx (2)
133-133
: Return updated to use NewToolResultJSON — correct and idiomaticThis matches the new signature that returns (*mcp.CallToolResult, error). Returning the helper directly (without appending ", nil") is the right approach.
For consistency across this page, consider updating earlier examples that hand-roll JSON:
- Lines 77–83 (in handleCalculateTyped)
- Lines 288–294 (in the session-aware tool)
to use mcp.NewToolResultJSON(...) as well. It reduces boilerplate and ensures consistent behavior on JSON marshal errors.
133-133
: Optional: add a brief note about error propagationSince NewToolResultJSON can fail on marshal, this example now propagates that error up (good). If you want to show explicit error handling in docs, you could expand this example to demonstrate how to wrap or transform the error in a domain-specific way. Optional.
www/docs/pages/transports/http.mdx (1)
497-497
: Return updated to use NewToolResultJSON — correctThis aligns with the helper’s new two-value return and simplifies the example.
Elsewhere in this file, responses are built with fmt.Sprintf JSON strings (e.g., lines 109–111, 140–142, 188–190). Consider switching those to NewToolResultJSON with typed structs or maps to avoid brittle string construction and ensure valid JSON.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
mcp/utils.go
(1 hunks)www/docs/pages/servers/advanced.mdx
(2 hunks)www/docs/pages/transports/http.mdx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- mcp/utils.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-23T11:10:42.948Z
Learnt from: floatingIce91
PR: mark3labs/mcp-go#401
File: server/server.go:1082-1092
Timestamp: 2025-06-23T11:10:42.948Z
Learning: In Go MCP server, ServerTool.Tool field is only used for tool listing and indexing, not for tool execution or middleware. During handleToolCall, only the Handler field is used, so dynamic tools don't need the Tool field populated.
Applied to files:
www/docs/pages/servers/advanced.mdx
📚 Learning: 2025-03-04T06:59:43.882Z
Learnt from: xinwo
PR: mark3labs/mcp-go#35
File: mcp/tools.go:107-137
Timestamp: 2025-03-04T06:59:43.882Z
Learning: Tool responses from the MCP server shouldn't contain RawInputSchema, which is why the UnmarshalJSON method for the Tool struct is implemented to handle only the structured InputSchema format.
Applied to files:
www/docs/pages/servers/advanced.mdx
🔇 Additional comments (3)
www/docs/pages/servers/advanced.mdx (2)
965-965
: Docs formatting change is fineThe “Next Steps” bullet tweak is non-functional and looks good.
133-133
: No lingering oldNewToolResultJSON(...), nil
usages
A repo-wide search located only two call sites—both correctly using the new signature—and no instances of the old, nil
pattern remain:
- www/docs/pages/transports/http.mdx:497
- www/docs/pages/servers/advanced.mdx:133
All clear.
www/docs/pages/transports/http.mdx (1)
696-696
: Docs formatting change is fineThe “Next Steps” bullet formatting update is OK.
Description
This PR adds a new util
NewToolResultJSON
. It is mentioned in https://mcp-go.dev/servers/advanced but the function doesn't really exist.Type of Change
Checklist
MCP Spec Compliance
Additional Information
Summary by CodeRabbit
New Features
Documentation