feat: Add Structured Tool Output Support #451
Conversation
|
""" WalkthroughThis change introduces structured tool output support by adding output schema definitions and validation to the tool system. It includes new fields for output schemas and structured content, schema generation from Go types, runtime validation, helper functions for structured results, and comprehensive tests for output schema enforcement and client/server behavior. Changes
Possibly related issues
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used🧠 Learnings (4)📓 Common learningsserver/server_test.go (7)mcp/tools.go (8)mcp/tools_test.go (6)🔇 Additional comments (26)
✨ 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 review |
✅ Actions performedReview triggered.
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 321 🏁 Script executed: Length of output: 435 @davidleitw I can see you're working on PR #451 "feat: Add Structured Tool Output Support" which is currently in draft status. It looks like your message got cut off with just "@" at the end. Here are your options to get a CodeRabbit review: Option 1: Trigger a review right now (for this draft) Option 2: Convert from draft to ready for review Option 3: Configure CodeRabbit to review drafts by default # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
reviews:
review_status: false # Disable status messages
drafts: true # Enable reviews for draft PRsWhat would you like to do? If you want me to review the current changes, just reply with |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
mcp/tools.go (4)
1219-1244: Add validation for existing schema type.Same issue as WithOutputBoolean - needs to validate existing schema type.
1246-1271: Add validation for existing schema type.Same issue as WithOutputBoolean - needs to validate existing schema type.
1273-1299: Add validation for existing schema type.Same issue as WithOutputBoolean - needs to validate existing schema type.
1301-1326: Add validation for existing schema type.Same issue as WithOutputBoolean - needs to validate existing schema type.
🧹 Nitpick comments (4)
mcp/utils.go (1)
356-406: Well-implemented structured content helpers with proper backwards compatibility.These functions correctly implement the MCP protocol guideline that tools returning structured content should also provide functionally equivalent unstructured content. The error handling is robust and follows established patterns.
Consider improving the error message format for consistency:
- jsonText = fmt.Sprintf("Error serializing structured content: %v", err) + jsonText = fmt.Sprintf("error serializing structured content: %v", err)This follows Go convention of starting error messages with lowercase letters.
mcp/tools.go (3)
522-547: Consider making the validator key unique per tool.Using a hardcoded key "output-schema-validator" could potentially cause conflicts if the compiler instance is shared across multiple tools.
Consider using a unique key per tool:
- const validatorKey = "output-schema-validator" + validatorKey := fmt.Sprintf("output-schema-validator-%s", t.Name)
549-566: Enhance error message with schema details.The error message could be more helpful by including the expected schema type.
if result.StructuredContent == nil { - return fmt.Errorf("tool %s has output schema but structuredContent is nil", t.Name) + return fmt.Errorf("tool %s expects output of type %s but structuredContent is nil", t.Name, t.OutputSchema.Type) }
570-602: Consider adjusting the initial map capacity.The map could potentially need up to 6 fields, but the current capacity might be slightly low for optimal performance.
This is a minor optimization - the current implementation is functionally correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
go.mod(1 hunks)mcp/tools.go(8 hunks)mcp/tools_test.go(1 hunks)mcp/types_test.go(1 hunks)mcp/utils.go(1 hunks)server/server.go(1 hunks)server/server_test.go(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
server/server_test.go (3)
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 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.
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.
mcp/tools.go (3)
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.
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.
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.
🧬 Code Graph Analysis (5)
server/server.go (2)
mcp/tools.go (1)
Tool(477-495)mcp/utils.go (1)
NewToolResultErrorFromErr(326-339)
mcp/utils.go (2)
mcp/tools.go (1)
CallToolResult(39-48)mcp/types.go (3)
Content(827-829)TextContent(833-838)TextContent(840-840)
mcp/types_test.go (1)
mcp/tools.go (6)
Tool(477-495)Title(787-791)Description(771-775)ToolSchema(604-608)Properties(1013-1017)ToolAnnotation(627-638)
mcp/tools_test.go (3)
mcp/tools.go (19)
NewTool(655-683)WithOutputString(1248-1271)Required(779-783)WithString(947-965)Description(771-775)DefaultString(799-803)Properties(1013-1017)WithOutputNumber(1221-1244)Min(859-863)Max(851-855)DefaultNumber(843-847)WithOutputBoolean(1194-1217)DefaultBool(879-883)WithOutputArray(1303-1326)WithStringItems(1101-1113)WithNumberItems(1141-1153)WithOutputObject(1275-1299)ToolSchema(604-608)WithOutputSchema(1186-1190)mcp/utils.go (3)
NewStructuredToolResult(360-381)NewStructuredToolError(385-406)NewToolResultText(245-254)mcp/types.go (3)
Content(827-829)TextContent(833-838)TextContent(840-840)
mcp/tools.go (1)
mcp/types.go (1)
Annotations(805-818)
🔇 Additional comments (21)
go.mod (1)
7-7: Verify the dependency version is current and secure.The added JSON schema validation library supports the new structured output functionality. Please ensure this version is up-to-date and free from known vulnerabilities.
#!/bin/bash # Check for the latest version and any security advisories for the jsonschema library echo "Checking latest version..." go list -m -versions github.com/santhosh-tekuri/jsonschema | tail -1 echo "Checking for known vulnerabilities..." go list -json -m github.com/santhosh-tekuri/[email protected] | jq -r '.Version' # Check GitHub releases for this repository curl -s https://api.github.com/repos/santhosh-tekuri/jsonschema/releases/latest | jq -r '.tag_name'server/server.go (1)
1037-1039: I couldn’t locate theValidateStructuredOutputmethod with the initial AST query. Let’s search the entire repo using a text search:#!/bin/bash # Search for any occurrences of ValidateStructuredOutput in the codebase rg "ValidateStructuredOutput" -n .mcp/types_test.go (1)
72-115: Comprehensive test coverage for Tool serialization.The test thoroughly covers JSON serialization/deserialization of the Tool struct, including the new Title field and nested annotations. The test structure follows best practices and provides good coverage of the functionality.
server/server_test.go (3)
1113-1113: LGTM! Type rename aligns with breaking change.The change from
mcp.ToolInputSchematomcp.ToolSchemacorrectly implements the breaking change mentioned in the PR description, unifying input and output schemas under a single type.
2025-2092: Excellent test coverage for successful structured output validation.This test thoroughly validates the success path for structured output schema validation. The test setup correctly uses the new output schema builders and verifies that:
- Valid structured content passes validation
- The
IsErrorflag is properly set to false- Structured content is preserved and accessible in the response
2094-2161: Well-designed test for structured output validation failure.This test effectively validates the error path when structured output doesn't match the defined schema. The test correctly:
- Uses the same tool schema as the success test for consistency
- Provides invalid structured content (missing required fields)
- Verifies that
IsErroris set to true- Checks that the error message indicates schema validation failure
mcp/tools_test.go (7)
716-732: Good coverage of HasOutputSchema method.These tests properly validate the
HasOutputSchemamethod behavior for both empty and defined output schemas. The test cases are clear and focused.
734-792: Comprehensive JSON marshaling tests for output schema.These tests thoroughly validate the JSON serialization behavior:
- Correctly tests that
outputSchemais included when defined- Verifies that
outputSchemais omitted when empty- Provides detailed validation of schema structure and properties
794-928: Excellent coverage of output schema builder functions.These tests thoroughly validate all the
WithOutput*functions:
- Proper schema type setting and property creation
- Correct handling of required fields and descriptions
- Validation of PropertyOptions like defaults, constraints, and nested structures
- Good use of assertions to verify schema details
930-982: Solid tests for structured result helper functions.These tests effectively validate the
NewStructuredToolResultandNewStructuredToolErrorfunctions:
- Proper
IsErrorflag setting (false for success, true for error)- Structured content preservation
- Consistent JSON text content generation
- Good verification of dual content representation
984-1072: Comprehensive CallToolResult marshaling tests.These tests thoroughly validate JSON serialization of
CallToolResult:
- Proper handling of structured content in JSON
- Correct text content representation as JSON string
- Appropriate
isErrorfield behavior (omitted when false, present when true)- Good verification of round-trip consistency
1074-1119: Great integration test for multiple output properties.This test effectively validates the combination of multiple output property types:
- Tests integration of string, number, boolean, array, and object properties
- Verifies correct required field handling across property types
- Provides good validation of complex schema JSON structure
- Serves as an excellent integration test for the output schema system
1121-1156: Complete coverage of ValidateStructuredOutput method.These tests thoroughly validate all scenarios for structured output validation:
- Tools without output schema (no validation performed)
- Tools with schema but missing structured content (error case)
- Valid structured content (validation passes)
- Invalid structured content (validation fails with appropriate error)
The test cases are well-designed and cover the complete validation logic flow.
mcp/tools.go (8)
4-11: Import additions look good.The new imports are appropriate for the structured output validation functionality.
42-43: LGTM! Structured content field properly added.The
StructuredContentfield is correctly typed asanywith appropriate JSON tagging for backward compatibility.
480-490: Tool struct enhancements are well-designed.The additions properly support output schema validation with performance optimization through cached compilation. The breaking change to unify schema types is clearly documented.
502-509: Title retrieval logic is well-implemented.The precedence order (direct field → annotations → empty) provides good flexibility and backward compatibility.
604-625: Schema type rename and marshaling logic are well-implemented.The unified
ToolSchematype and its custom marshaling properly handle edge cases while maintaining clean JSON output.
658-668: Tool constructor properly initializes new fields.The output schema initialization follows the same pattern as input schema for consistency.
710-716: WithTitle option correctly implemented.Follows the established pattern for tool configuration options.
1184-1190: WithOutputSchema implementation is correct.This function properly sets the entire output schema structure.
davidleitw
force-pushed
the
structured_output
branch
from
58ef0d3 to
8c30311
Compare
mcp/tools.go
Outdated
| @@ -487,14 +499,83 @@ func (t Tool) GetName() string { | |||
| return t.Name | |||
| } | |||
|
|
|||
| // GetTitle returns the display title for the tool. | |||
There was a problem hiding this comment.
I don't see the need for this. There should only be one way to set the title. The MCP spec defines it as an annotation so it makes more sense for it to stay an annotation.
Also this is unreleated and totally out of scope for the PR.
There was a problem hiding this comment.
Sorry about that — the changes in these commits actually belong to #431. I accidentally included them in this draft PR by mistake. I’ll remove them shortly. Apologies for the confusion!
There was a problem hiding this comment.
Ok, hmm, even in the other PR, I'm not sure we need this. It is already supported through tool annotations right?
There was a problem hiding this comment.
The PR is associated with the following task: #418
As I understand it, objects like Prompt and Resource do not have a ToolAnnotation.Title field. Instead, their annotations are implemented using:
type Annotations struct {
// Describes who the intended customer of this object or data is.
//
// It can include multiple entries to indicate content useful for multiple
// audiences (e.g., ["user", "assistant"]).
Audience []Role `json:"audience,omitempty"`
// Describes how important this data is for operating the server.
//
// A value of 1 means "most important," and indicates that the data is
// effectively required, while 0 means "least important," and indicates that
// the data is entirely optional.
Priority float64 `json:"priority,omitempty"`
}
// Annotated is the base for objects that include optional annotations for the
// client. The client can use annotations to inform how objects are used or
// displayed.
type Annotated struct {
Annotations *Annotations `json:"annotations,omitempty"`
}Given this, it seems our intention is to provide a consistent way to access a DisplayName (or similar property) across different object types—such as tools, prompts, and resources—so that this name can be presented in UI or any human-facing context.
This aligns with what the MCP documentation describes, where it encourages tools to optionally include a human-readable name for display purposes:
title: Optional human-readable name of the tool for display purposes.
It seems that the changes in our PR are consistent with the intent of the MCP documentation and serve a useful purpose in standardizing how human-readable names are exposed across object types.
There was a problem hiding this comment.
Ok, can you remove the unrelated changes from this PR?
There was a problem hiding this comment.
Sure, I'm on it — I'll remove the unrelated changes right away.
There was a problem hiding this comment.
Hi @pottekkat
I’ve removed the unrelated code and verified all tests pass, Thx
mcp/tools.go
Outdated
| func WithOutputNumber(name string, opts ...PropertyOption) ToolOption { | ||
| return func(t *Tool) { | ||
| // Initialize output schema if not set | ||
| if t.OutputSchema.Type == "" { | ||
| t.OutputSchema.Type = "object" | ||
| } | ||
|
|
||
| schema := map[string]any{ | ||
| "type": "number", | ||
| } | ||
|
|
||
| for _, opt := range opts { | ||
| opt(schema) | ||
| } | ||
|
|
||
| // Remove required from property schema and add to OutputSchema.required | ||
| if required, ok := schema["required"].(bool); ok && required { | ||
| delete(schema, "required") | ||
| t.OutputSchema.Required = append(t.OutputSchema.Required, name) | ||
| } | ||
|
|
||
| t.OutputSchema.Properties[name] = schema | ||
| } | ||
| } |
There was a problem hiding this comment.
Do we need a separate schema generation function for each datatype because of backwards compatibility? Like can we reuse the same methods we use for input schema?
There was a problem hiding this comment.
That's a good point. The initial approach was to create explicit WithOutput... functions to mirror the existing With... API for consistency, which led to the code duplication you pointed out.
After thinking through the best way to resolve this, we can refactor the implementation to eliminate the repetition while keeping the public API stable.
Here are the two main approaches:
Option 1: Internal Helper Function (Recommended)
We can create a single, private applyProperty helper that does all the heavy lifting. To make it type-safe, we can define constants for the property types instead of using raw strings. The public functions would just become one-line calls to it. This solves the duplication problem without any breaking changes.
// Define a type for safety and clarity
type PropertyType string
const (
PropertyTypeString PropertyType = "string"
PropertyTypeNumber PropertyType = "number"
PropertyTypeBoolean PropertyType = "boolean"
PropertyTypeObject PropertyType = "object"
PropertyTypeArray PropertyType = "array"
)
// The private helper does all the work, now with a typed propertyType
func applyProperty(target *ToolSchema, name string, propertyType PropertyType, opts ...PropertyOption) {
if target.Type == "" {
target.Type = "object"
}
if target.Properties == nil {
target.Properties = make(map[string]any)
}
schema := map[string]any{"type": propertyType}
for _, opt := range opts {
opt(schema)
}
if required, ok := schema["required"].(bool); ok && required {
delete(schema, "required")
target.Required = append(target.Required, name)
}
target.Properties[name] = schema
}
// Public APIs just become simple wrappers
func WithNumber(name string, opts ...PropertyOption) ToolOption {
return func(t *Tool) {
applyProperty(&t.InputSchema, name, PropertyTypeNumber, opts...)
}
}
func WithOutputNumber(name string, opts ...PropertyOption) ToolOption {
return func(t *Tool) {
applyProperty(&t.OutputSchema, name, PropertyTypeNumber, opts...)
}
}Option 2: Parameterized Functions
Alternatively, we could merge the functions and add a target parameter to specify input or output. This would be a much cleaner API, but it's a major breaking change that would require every user to update their tool definitions.
type SchemaTarget int
const (
InputSchema SchemaTarget = iota
OutputSchema
)
// One function to rule them all
func WithNumber(name string, target SchemaTarget, opts ...PropertyOption) ToolOption {
// ... implementation that switches on 'target'
}Conclusion
Option 1 (the internal helper) is the clear path forward. It's the pragmatic choice that gives us the maintainability win of a single internal logic source without forcing a painful, breaking migration on our users.
Option 2 would be the textbook answer for a new API, but the cost of the breaking change is too high for an established one.
My preference is for Option 1, as it gives us the maintainability win without the breaking change. What are your thoughts?
There was a problem hiding this comment.
Come one man, I do not want to talk to an AI. I'm ok with AI assistance but this whole PR and proposal is AI generated. We are spending our free time to maintain this and this is very inconsiderate.
There was a problem hiding this comment.
I admit that I used AI assistance for the summary part, but the core code was written by me. I just wanted to make sure my explanation was clear and avoid any potential misunderstandings, so I asked AI to help organize it. I’m really sorry if that caused any inconvenience.
As for the applyProperty idea, I genuinely think it's a good approach. Even though we still have two sets of ToolOptions like WithNumber and WithOutputNumber, the overall code avoids duplication by handling everything through a unified applyProperty function.
There was a problem hiding this comment.
Yeah but it makes it hard to review. We already augment the reviews with AI and when we try to review, we want to have a discussion and think through the problem. We are moving much faster than most open source projects even with the number of PRs. I'm sure the AI answer is correct, but it takes away the meaningful discussion.
There was a problem hiding this comment.
Got it, I understand. In future discussions, I’ll respond directly based on my own understanding. I reflected on it just now and realized that the AI did generate a lot of unnecessary extra information.
I've put together some examples to help you quickly understand what this PR does and how to use the new output schema feature. These should give you a good sense of the changes without having to dig through all the diff details. Usage ExamplesThis PR adds Output Schema support to MCP tools. Basically, you can now define what your tools should return, and the server will validate it automatically. Here's how it works: Example 1: Basic Weather Tool with Output SchemaSay you want a weather tool that always returns temperature and condition in a predictable format: // Define a tool with structured output
weatherTool := mcp.NewTool(
"get_weather",
mcp.WithDescription("Get current weather for a location"),
// Input schema
mcp.WithString("location", mcp.Description("City name"), mcp.Required()),
// Output schema - NEW FEATURE
mcp.WithOutputString("condition", mcp.Description("Weather condition"), mcp.Required()),
mcp.WithOutputNumber("temperature", mcp.Description("Temperature in Celsius"), mcp.Required()),
mcp.WithOutputNumber("humidity", mcp.Description("Humidity percentage")),
)
// Tool handler implementation
func handleWeather(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
location := request.Arguments["location"].(string)
// Business logic here...
weatherData := map[string]any{
"condition": "Sunny",
"temperature": 25.5,
"humidity": 60,
}
// Return structured data - automatically validated against output schema
return mcp.NewStructuredToolResult(weatherData), nil
}The Example 2: Complex Output with Nested ObjectsNeed something more complex? You can nest objects and arrays too: // Tool with complex output structure
userTool := mcp.NewTool(
"get_user_profile",
mcp.WithDescription("Fetch user profile data"),
mcp.WithString("user_id", mcp.Required()),
// Complex output schema
mcp.WithOutputString("user_id", mcp.Required()),
mcp.WithOutputString("name", mcp.Required()),
mcp.WithOutputObject("preferences", mcp.Description("User preferences"),
mcp.Properties(map[string]any{
"theme": map[string]any{"type": "string"},
"language": map[string]any{"type": "string"},
"notifications": map[string]any{"type": "boolean"},
}),
),
mcp.WithOutputArray("tags", mcp.Description("User tags"), mcp.WithStringItems()),
)Example 3: Error Handling with Structured OutputEven when things go wrong, you can still return structured data that matches your schema: func handleUserProfile(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
userID := request.Arguments["user_id"].(string)
user, err := fetchUser(userID)
if err != nil {
// Return structured error that matches output schema
errorData := map[string]any{
"user_id": userID,
"name": "",
"preferences": map[string]any{},
"tags": []string{},
}
return mcp.NewStructuredToolError(errorData), nil
}
// Return successful structured data
return mcp.NewStructuredToolResult(user), nil
} |
davidleitw
force-pushed
the
structured_output
branch
from
8c30311 to
d49b6b7
Compare
|
@davidleitw How does the structured errors work? Does the spec require errors to also be returned in this format? |
|
I will work on implementing this as a way to test it since I already require this for the MCP server I'm building. Looking at the examples, it feels intuitive and easy to use and does not break any existing code. If I find any issues with using it, I will report. |
|
We might need to reiterate on the design. There are better ways to do it without duplicating logic or mimicking the current tool schema creation flow. Based on internal discussion, @davidleitw and I will come up with more intuitive/Go-idiomatic ways to implement this. |
|
I've reviewed the implementations of the other official MCP SDKs. I'll also summarize that along with the discussion I had with @pottekkat yesterday and post it here shortly. |
TL;DR: Structured Output Interface Changes for go-mcp
Still organizing the details — more to come soon. |
|
I've grouped the remaining unimplemented parts into two topics, summarizing how they're described in the MCP docs, how the SDK interprets them, and related implementation details:
One design consideration (not part of the protocol): should mcp-go provide WithOutputXXX methods for Output Schema, similar to Input Schema? After discussing with @pottekkat, we think a more idiomatic Go approach might be better. TL;DR and conclusions are already provided above. First, I’d like to focus on discussing the implementation of Structured Content in relation to how it's defined in the MCP documentation. The main goal is to examine how the behavior is described in the spec and how it's interpreted and implemented in the Python SDK. In the Structured Content section, it mentions:
However, what I’ve observed is that most official SDKs do not strictly enforce consistency between the two. Instead, the behavior depends on the form of the tool’s return value. In the Python SDK, when the MCP Server receives a CallToolResult, it processes the result using the logic found at the following location: tool_call The Python MCP Server automatically determines how to handle tool results based on their return type. # output normalization
unstructured_content: UnstructuredContent
maybe_structured_content: StructuredContent | None
if isinstance(results, tuple) and len(results) == 2:
# tool returned both structured and unstructured content
unstructured_content, maybe_structured_content = cast(CombinationContent, results)
elif isinstance(results, dict):
# tool returned structured content only
maybe_structured_content = cast(StructuredContent, results)
unstructured_content = [types.TextContent(type="text", text=json.dumps(results, indent=2))]
elif hasattr(results, "__iter__"):
# tool returned unstructured content only
unstructured_content = cast(UnstructuredContent, results)
maybe_structured_content = None
else:
return self._make_error_result(f"Unexpected return type from tool: {type(results).__name__}")
# output validation
if tool and tool.outputSchema is not None:
if maybe_structured_content is None:
return self._make_error_result(
"Output validation error: outputSchema defined but no structured output returned"
)
else:
try:
jsonschema.validate(instance=maybe_structured_content, schema=tool.outputSchema)
except jsonschema.ValidationError as e:
return self._make_error_result(f"Output validation error: {e.message}")In the combination mode test at tests/server/test_lowlevel_output_validation.py, the tool function returns like this: async def call_tool_handler(name: str, arguments: dict[str, Any]) -> tuple[list[TextContent], dict[str, Any]]:
if name == "process":
content = [TextContent(type="text", text="Processing complete")]
data = {"result": "success", "count": 10}
return (content, data)This tuple return triggers the first branch isinstance(results, tuple), treating "Processing complete" as the content, and {"result": "success", "count": 10} as the structured content, allowing the two to be completely different. When the tool directly returns a dict, like in the example at tests/server/test_lowlevel_output_validation.py:get_info async def call_tool_handler(name: str, arguments: dict[str, Any]) -> dict[str, Any]:
if name == "get_info":
return {"status": "ok", "data": {"value": 42}}It falls into the second branch isinstance(results, dict), and the system sets:
After the above logic is processed, it proceeds to the server-side Output Schema validation step. This code highlights the key behavioral differences between tools with and without an Output Schema:
If the tool returns only content (e.g., the test case in tests/server/test_lowlevel_output_validation.py:214-240), since maybe_structured_content = None, it will immediately trigger the first error branch.If structured content is present but does not conform to the schema, a JSON Schema validation error will be raised. I originally thought that in Python, this kind of code meant that even when an error occurs, it would still go through the Output Schema validation process. But after reviewing some test cases, I realized that it actually doesn’t happen at all. The reason is simple: async def handler(req: types.CallToolRequest):
try:
# ... tool invocation logic ...
# ... output validation logic ...
return types.ServerResult(types.CallToolResult(...))
except Exception as e:
return self._make_error_result(str(e)) # directly returns, skips all validationKey Insight:
So the actual behavior is:
def error_tool_fn() -> None:
raise ValueError("Test error")That’s why tool functions that raise exceptions end up producing a result with isError = true in the tests. Now, let's look at how the TypeScript SDK handles this. To be honest, the behavior is pretty similar, but TypeScript has a special feature: you can manually set I took a look at the TypeScript SDK's source code, and its approach is quite interesting. The whole flow goes something like this:
The biggest difference from the Python SDK is in the error handling. Python's approach is more direct; it wraps the entire process in a try-catch block. As soon as an error occurs, it immediately returns an error result, and none of the subsequent logic runs. TypeScript's approach is more granular, distinguishing between different types of errors:
# Python's approach: exit immediately
try:
# All logic is wrapped in here
return types.ServerResult(types.CallToolResult(...))
except Exception as e:
return self._make_error_result(str(e)) # Return directly, nothing else runs// TypeScript's approach: convert the error but continue the flow
try {
result = await Promise.resolve(cb(args, extra));
} catch (error) {
result = {
content: [{ type: "text", text: error.message }],
isError: true, // Set a flag
};
}
// Continue execution, but check the flag
if (tool.outputSchema && !result.isError) {
// Skip validation if there's an error
}For example, you can manually control errors like this: // Handling a business logic error yourself
if (user.permission < REQUIRED_LEVEL) {
return {
content: [{ type: "text", text: "Permission denied" }],
structuredContent: { errorCode: 403, message: "Permission denied" },
isError: true // Set this manually to skip schema validation
};
}Or you can just throw an exception and let the system handle it automatically: // Throw an exception, and the system will automatically wrap it as an isError: true result
if (database.isDown()) {
throw new Error("Database connection failed");
}I think this design is a great reference for our Another benefit of this design is that you can still provide structured content even when there's an error (though it won't be validated). This could be really useful in some scenarios, like if you want to return structured error information like an error code or reason, perhaps by adding fields like In summary, both the Python and TypeScript SDKs achieve the same core goal: when a tool execution fails, output schema validation is skipped. This aligns perfectly with the spirit of the MCP specification, as error results are primarily intended to help the LLM understand what went wrong and attempt self-correction The key difference lies in their implementation style and philosophy:
Put simply, one relies on implicit exception handling, while the other uses explicit flag checking for control Reflecting this conclusion, we propose the following concrete design directions for mcp-go:
|
davidleitw
force-pushed
the
structured_output
branch
from
7eb80e8 to
ae446b4
Compare
|
Based on the TLDR, the following changes were made:
New tests added:
Example usage: type WeatherData struct {
Temperature float64 `json:"temperature" jsonschema:"description=The temperature in Celsius."`
Conditions string `json:"conditions" jsonschema:"description=Weather conditions (e.g. Cloudy)."`
}
// Register the tool with output schema
tool := mcp.NewTool("get_weather", mcp.WithOutputType[WeatherData]())
server.AddTool(tool, func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
// Return structured content using the helper
return mcp.NewToolResultStructured(WeatherData{
Temperature: 23.5,
Conditions: "Cloudy",
}), nil
})This example shows how to define a tool with a structured output schema and return structured content using the new API. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
mcp/tools.go (2)
686-691: Undocumented addition of WithDescription helper.This helper function is a useful addition but wasn't mentioned in the PR description. Consider documenting this in the PR summary.
1189-1222: Schema generation with compatibility workaround.The implementation correctly handles schema generation and compilation. The Draft-07 version override is a necessary workaround for validator compatibility - this is well-documented in the code.
Consider documenting this Draft-07 limitation in the package documentation or README to help users understand the schema version constraints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
client/http_test.go(2 hunks)go.mod(1 hunks)mcp/tools.go(7 hunks)mcp/tools_test.go(1 hunks)mcp/utils.go(1 hunks)server/server.go(1 hunks)server/server_test.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- server/server.go
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
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.
go.mod (3)
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.
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.
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.
client/http_test.go (5)
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 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.
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.
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.
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
mcp/tools_test.go (5)
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.
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.
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
server/server_test.go (6)
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.
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.
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
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.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
mcp/utils.go (6)
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.
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.
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
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.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
mcp/tools.go (7)
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.
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.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
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.
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
Learnt from: lariel-fernandes
PR: mark3labs/mcp-go#428
File: www/docs/pages/servers/prompts.mdx:218-234
Timestamp: 2025-06-20T20:39:51.870Z
Learning: In the mcp-go library, the GetPromptParams.Arguments field is of type map[string]string, not map[string]interface{}, so direct string access without type assertions is safe and correct.
🧬 Code Graph Analysis (1)
mcp/utils.go (2)
mcp/types.go (1)
Content(827-829)mcp/tools.go (1)
CallToolResult(40-49)
🔇 Additional comments (27)
go.mod (1)
7-8: Dependencies are current and vulnerability-free
- github.com/invopop/jsonschema v0.13.0 matches the latest stable release
- github.com/santhosh-tekuri/jsonschema v1.2.4 matches the latest stable release
- No security advisories found in the GitHub Vulnerability Database for either package
No further action required.
client/http_test.go (1)
117-176: Well-structured integration test for output schema support.The test effectively validates the end-to-end functionality of output schema serialization and retrieval. The test structure is clear and follows good testing practices:
- Proper setup with server and client initialization
- Uses realistic test data with struct tags for schema generation
- Verifies both the presence and content of the output schema
- Includes proper cleanup with
defer httpServer.Close()mcp/utils.go (4)
356-367: Good error handling in JSON marshaling helper.The
marshalToContentfunction properly handles JSON marshaling errors by returning an error message as text content, ensuring the function never fails catastrophically.
369-377: Clean generic API for structured tool results.The
NewToolResultStructuredfunction provides a clean, type-safe way to create tool results with structured content while maintaining backwards compatibility through automatic JSON content generation.
379-386: Flexible helper for mixed content scenarios.The
NewToolResultWithStructuredfunction appropriately handles cases where developers need both custom content and structured data, providing flexibility for complex tool responses.
388-405: Consistent error handling variants.The error-specific helper functions (
NewToolResultErrorStructuredandNewToolResultErrorWithStructured) maintain the same clean API pattern while properly setting theIsErrorflag, ensuring consistency with the MCP specification.mcp/tools_test.go (5)
716-731: Good basic functionality testing.The tests for
HasOutputSchemaproperly validate both empty and populated schema scenarios with clear assertions.
734-777: Thorough JSON marshaling validation.The test comprehensively validates that output schemas are correctly included in marshaled JSON, checking both the presence of the schema and its internal structure.
829-867: Excellent struct-based schema generation test.The
TestWithOutputTypefunction effectively validates the new generic schema generation feature using Go structs with jsonschema tags, confirming the schema is properly generated and included.
870-897: Comprehensive structured result testing.The test validates that
NewToolResultStructuredcorrectly creates structured content, sets appropriate flags, and generates proper JSON fallback content for backwards compatibility.
965-1021: Thorough validation logic testing.The validation tests comprehensively cover the key scenarios:
- Tools without output schema skip validation
- Error results skip validation regardless of schema presence
- Tools with output schema require structured content
- Structured content is validated against the compiled schema
This properly reflects the MCP specification requirements.
server/server_test.go (8)
4-4: LGTM!The
bytesimport is appropriately added to support JSON schema formatting in the new test functions.
1114-1114: Type rename aligns with breaking change.The change from
mcp.ToolInputSchematomcp.ToolSchemacorrectly reflects the documented breaking change in this PR.
2026-2101: Well-structured test for successful validation.The test comprehensively validates that tools with properly structured output pass validation. Good coverage of both the text content and structured content in the response.
2102-2178: Comprehensive validation failure test.The test effectively validates that schema violations are caught and properly reported with
IsError=trueand descriptive error messages.
2180-2258: Excellent test coverage with important limitation documented.The test thoroughly validates the
WithOutputTypeAPI and properly documents the jsonschema parser limitation about commas in descriptions. The schema generation verification is particularly valuable.
2260-2315: Effective schema mismatch testing.Good approach using two different structs to simulate schema validation failure. The test correctly verifies that missing required properties are caught.
2317-2371: Critical edge case well tested.Excellent test validating that error responses (
IsError=true) correctly bypass schema validation, aligning with MCP specification behavior.
2373-2416: Validates schema registration and storage.Good test ensuring that output schemas generated from Go types are properly stored in the server's tool registry with descriptions intact.
mcp/tools.go (8)
3-13: LGTM!The jsonschema imports are appropriately chosen -
invopop/jsonschemafor generation andsanthosh-tekuri/jsonschemafor validation.
43-44: Structured content field properly added.The
StructuredContentfield is correctly typed asanywithomitemptytag for backward compatibility.
484-495: Tool struct properly extended for output schemas.Good design choices:
OutputSchemaasjson.RawMessageprovides flexibility- Internal validation fields correctly hidden with
json:"-"- Type rename aligns with the breaking change
502-556: Well-designed validation architecture.The validation implementation demonstrates good design patterns:
- Lazy compilation of validators for efficiency
- Clear separation between public and private methods
- Proper nil checks and error handling
561-584: JSON marshaling correctly updated.The marshaling logic properly handles the new
OutputSchemafield with appropriate nil checking.
590-611: Type rename and marshaling logic are correct.The
ToolSchematype properly replacesToolInputSchemawith clean JSON marshaling that omits empty fields.
1171-1187: Clean generic API for output schema definition.Excellent use of Go generics to provide a type-safe way to define output schemas. The graceful error handling allows tools to work even if schema compilation fails.
1224-1262: Comprehensive validation logic with proper error handling.The validation implementation correctly:
- Skips validation for error responses (MCP spec compliance)
- Handles various structured content types
- Provides clear error messages
davidleitw
force-pushed
the
structured_output
branch
from
ae446b4 to
43b576e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
server/server_test.go (1)
2373-2416: Thorough verification of schema storage in the server registry!The test properly verifies that output schemas with descriptions are correctly stored in the server's tool registry.
Consider adding an assertion to verify that
HasOutputSchema()returns true for the registered tool:registeredTool, ok := server.tools[tool.Name] require.True(t, ok, "Tool should be registered on the server") require.NotNil(t, registeredTool.Tool.OutputSchema, "OutputSchema should not be nil") +assert.True(t, registeredTool.Tool.HasOutputSchema(), "HasOutputSchema should return true")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
client/http_test.go(3 hunks)go.mod(1 hunks)mcp/tools.go(7 hunks)mcp/tools_test.go(1 hunks)mcp/utils.go(1 hunks)server/server.go(1 hunks)server/server_test.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- go.mod
- server/server.go
- client/http_test.go
- mcp/utils.go
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
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.
mcp/tools_test.go (6)
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 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.
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.
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
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.
mcp/tools.go (7)
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.
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.
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: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
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.
Learnt from: lariel-fernandes
PR: mark3labs/mcp-go#428
File: www/docs/pages/servers/prompts.mdx:218-234
Timestamp: 2025-06-20T20:39:51.870Z
Learning: In the mcp-go library, the GetPromptParams.Arguments field is of type map[string]string, not map[string]interface{}, so direct string access without type assertions is safe and correct.
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
server/server_test.go (6)
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 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.
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
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.
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.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
🔇 Additional comments (17)
mcp/tools_test.go (6)
716-732: LGTM!The tests for
HasOutputSchemamethod are concise and cover both empty and defined schema cases correctly.
734-777: LGTM!Comprehensive test that verifies JSON marshaling includes the output schema with all its properties and required fields.
816-827: Good cleanup of deprecated tests.The comments clearly indicate the migration path to the new generic functions
NewToolResultStructured[T]()andNewToolResultErrorStructured[T]().
829-867: Excellent test for struct-based schema generation!The test properly demonstrates the use of
WithOutputType[T]()with jsonschema tags and verifies that the generated schema includes all expected properties and descriptions.
870-962: Comprehensive test coverage for new helper functions!All four helper function tests properly verify:
- Structured content assignment
- JSON marshaling to text content
- IsError flag handling
- Content preservation when custom content is provided
996-1021: Excellent edge case coverage for output validation!The test properly verifies the conditional validation logic:
- No validation when output schema is not defined
- Validation skipped for error results (IsError = true)
- Validation error when structured content is missing but required
server/server_test.go (5)
1114-1114: Correct type name update.This fix aligns with the breaking change that renames
ToolInputSchematoToolSchemafor unified schema handling.
2027-2101: Well-structured integration test for successful validation!The test properly verifies that the server accepts valid structured content matching the defined output schema.
2103-2178: Good test coverage for validation failure scenarios!The test correctly verifies that the server sets IsError=true and provides an appropriate error message when structured content fails validation.
2180-2258: Excellent tests with helpful documentation!Both tests properly verify WithOutputType validation behavior. The comments about jsonschema tag limitations (no comma support) are particularly valuable for future maintainers.
The success test thoroughly verifies that descriptions from struct tags are preserved in the generated schema, while the failure test cleverly uses different struct types to simulate validation errors.
Also applies to: 2260-2315
2317-2371: Important test for error response handling!This test correctly verifies that schema validation is bypassed when IsError=true, preserving the original error content. This behavior aligns with the MCP specification.
mcp/tools.go (6)
4-4: LGTM - Necessary imports for output schema functionality.The added imports support JSON schema generation from Go structs and schema validation, which are core to the structured output feature.
Also applies to: 11-12
43-44: LGTM - Proper addition of structured content field.The
StructuredContentfield correctly implements the MCP specification for structured tool outputs with appropriate JSON marshaling behavior.
502-506: LGTM - Correct output schema detection.The nil check is appropriate for
json.RawMessagetype and correctly identifies when an output schema is defined.
1224-1262: Well-implemented validation logic with comprehensive type handling.The method correctly:
- Skips validation when
IsErroris true (per MCP spec)- Handles various Go types appropriately for JSON schema validation
- Uses JSON marshaling round-trip for struct conversion, ensuring compatibility
The validation logic aligns well with the MCP specification requirements.
580-583: LGTM - Proper JSON marshaling of output schema.The output schema is correctly included in the tool's JSON representation when defined.
1198-1202: Appropriate workaround for validator compatibility.The manual override to JSON Schema Draft-07 is well-documented and necessary for compatibility with the
santhosh-tekuri/jsonschemavalidator library.
mcp/tools.go
Outdated
| // WithOutputType sets the output schema for the Tool using Go generics and struct tags. | ||
| // This replaces the builder pattern with a cleaner interface based on struct definitions. | ||
| func WithOutputType[T any]() ToolOption { | ||
| return func(t *Tool) { | ||
| var zero T | ||
| validator, schemaBytes, err := compileOutputSchema(zero) | ||
| if err != nil { | ||
| // Skip setting output schema if compilation fails | ||
| // This allows the tool to work without validation | ||
| return | ||
| } | ||
|
|
||
| t.OutputSchema = schemaBytes | ||
| t.outputValidator = validator | ||
| t.outputType = reflect.TypeOf(zero) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for schema compilation failures.
The function silently ignores compilation errors, which could hide important issues during development. Consider logging the error or providing a way for developers to detect when schema compilation fails.
func WithOutputType[T any]() ToolOption {
return func(t *Tool) {
var zero T
validator, schemaBytes, err := compileOutputSchema(zero)
if err != nil {
- // Skip setting output schema if compilation fails
- // This allows the tool to work without validation
+ // Log compilation failure for debugging
+ // Consider: panic, log.Printf, or return error-aware ToolOption
return
}
// ... rest of function
}
}🤖 Prompt for AI Agents
In mcp/tools.go around lines 1171 to 1187, the WithOutputType function currently
ignores errors from compileOutputSchema, which can hide important issues. Modify
the function to log the error or return it so developers can detect schema
compilation failures. For example, add error logging inside the if err != nil
block or change the function signature to return an error and propagate it
accordingly.
aa0c7b8 to
025783b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
client/http_test.go(3 hunks)go.mod(1 hunks)mcp/tools.go(7 hunks)mcp/tools_test.go(1 hunks)mcp/utils.go(1 hunks)server/server.go(1 hunks)server/server_test.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- go.mod
- server/server.go
- client/http_test.go
- mcp/utils.go
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
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.
mcp/tools.go (7)
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.
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.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
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.
Learnt from: lariel-fernandes
PR: mark3labs/mcp-go#428
File: www/docs/pages/servers/prompts.mdx:218-234
Timestamp: 2025-06-20T20:39:51.870Z
Learning: In the mcp-go library, the GetPromptParams.Arguments field is of type map[string]string, not map[string]interface{}, so direct string access without type assertions is safe and correct.
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
mcp/tools_test.go (6)
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.
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: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
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.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
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.
server/server_test.go (6)
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 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.
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
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.
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.
Learnt from: davidleitw
PR: mark3labs/mcp-go#451
File: mcp/tools.go:1192-1217
Timestamp: 2025-06-26T09:38:18.629Z
Learning: In mcp-go project, the maintainer prefers keeping builder pattern APIs simple without excessive validation for edge cases. The WithOutput* functions are designed to assume correct usage rather than defensive programming, following the principle of API simplicity over comprehensive validation.
🧬 Code Graph Analysis (2)
mcp/tools.go (2)
mcp/types.go (1)
Annotations(805-818)mcp/utils.go (1)
ToBoolPtr(868-870)
mcp/tools_test.go (3)
mcp/tools.go (7)
NewTool(662-688)WithOutputSchema(1183-1187)WithDescription(709-713)WithString(944-962)Required(776-780)WithOutputType(1195-1214)CallToolResult(41-50)mcp/utils.go (7)
NewToolResultStructured(371-377)NewTextContent(199-204)NewImageContent(208-214)NewToolResultWithStructured(380-386)NewToolResultErrorStructured(390-396)NewToolResultErrorWithStructured(399-405)NewToolResultText(245-254)mcp/types.go (3)
Content(827-829)TextContent(833-838)TextContent(840-840)
🔇 Additional comments (26)
server/server_test.go (8)
4-4: LGTM: Import addition supports test functionality.The
bytesimport is appropriately added to support JSON pretty printing in the new structured output validation tests.
114-114: LGTM: Type rename aligns with API unification.The change from
mcp.ToolInputSchematomcp.ToolSchemacorrectly implements the breaking change mentioned in the PR to unify input and output schema handling.
2030-2101: LGTM: Comprehensive success case validation test.This test thoroughly validates the structured output validation success path:
- Properly defines output schema with required fields
- Creates tool with
WithOutputSchema- Verifies successful validation when structured content matches schema
- Correctly asserts
IsErroris false and structured content is preservedThe test design aligns with MCP specification requirements for structured output validation.
2106-2178: LGTM: Thorough validation failure test coverage.This test effectively verifies validation failure scenarios:
- Uses same schema as success test for consistency
- Returns structured content that violates required fields
- Properly asserts
IsErroris true when validation fails- Verifies error message contains validation failure details
The test correctly demonstrates server behavior when tool output doesn't match the defined schema.
2182-2258: LGTM: Excellent test of the new WithOutputType API.This test comprehensively validates the new generic
WithOutputType[T]()functionality:
- Demonstrates Go struct-based schema definition with jsonschema tags
- Verifies generated schema includes field descriptions
- Tests successful validation with
NewToolResultStructuredhelper- Properly validates structured content preservation
The test effectively showcases the idiomatic Go approach for output schema definition mentioned in the PR objectives.
2262-2315: LGTM: Validation failure test with WithOutputType.This test properly verifies validation failure when using
WithOutputType:
- Uses separate types to simulate schema mismatch (missing required field)
- Correctly expects validation failure and
IsErrortrue- Verifies specific error message about missing properties
The test design effectively demonstrates validation enforcement with the new type-based API.
2319-2371: LGTM: Critical test for error response validation skip.This test validates a crucial MCP specification requirement:
- Verifies that when
IsErroris true, schema validation is skipped- Uses intentionally invalid structured content that would normally fail validation
- Confirms original error content is preserved without validation interference
- Uses
NewToolResultErrorStructuredhelper appropriatelyThis behavior is essential for proper error handling in the MCP protocol.
2376-2416: LGTM: Schema generation and storage verification.This test ensures proper integration of output schemas into the server's tool registry:
- Verifies
WithOutputTypegenerates schema with descriptions from struct tags- Confirms schema is properly stored in server's internal tool registry
- Validates that generated schema contains expected field descriptions
- Includes helpful debug output for manual verification
The test provides confidence that the schema generation and storage mechanisms work correctly.
mcp/tools.go (12)
4-14: LGTM! Appropriate imports for structured output functionality.The new imports are well-chosen for the structured output schema feature:
bytesfor JSON schema resource handlingsyncfor thread-safe validator initializationinvopop/jsonschemafor Go struct to JSON schema generationsanthosh-tekuri/jsonschemafor JSON schema validation
44-45: Well-designed structured content field.The addition of
StructuredContentfield toCallToolResultproperly supports structured tool outputs while maintaining backward compatibility through theomitemptyJSON tag.
478-483: Excellent thread-safe design for validator state.The
validatorStatestruct withsync.Onceensures thread-safe initialization of validators while storing any initialization errors for consistent error reporting across goroutines.
492-504: Well-structured Tool enhancement for output schemas.The new fields properly support structured outputs:
OutputSchemafor the JSON schema definitionoutputTypefor Go type informationvalidatorStatefor thread-safe validator managementThe pointer to
validatorStatecorrectly avoids copy issues during concurrent access.
511-515: Correct output schema detection logic.The
HasOutputSchemamethod properly checks fornilOutputSchema, which correctly detects all valid output schemas regardless of their JSON Schema type (object, string, array, etc.).
525-557: Excellent thread-safe validator initialization.The
ensureOutputSchemaValidatormethod usessync.Oncecorrectly to ensure the validator is compiled exactly once. The error handling stores initialization errors in the shared state for consistent error reporting across goroutines.
559-576: Appropriate validation entry point with proper checks.The
ValidateStructuredOutputmethod correctly:
- Skips validation when no output schema is defined
- Validates that structured content is provided when a schema exists
- Delegates to the thread-safe validator initialization
601-604: Proper JSON marshaling with backward compatibility.The marshaling correctly includes the
outputSchemafield only when defined, maintaining backward compatibility while exposing the schema when present.
1181-1187: Simple and effective raw schema configuration.The
WithOutputSchemafunction provides a straightforward way to set output schemas using raw JSON, maintaining consistency with the existing API patterns.
1195-1214: Well-designed generic schema generation.The
WithOutputType[T]()function elegantly replaces the builder pattern with a cleaner struct-based approach. The silent error handling on compilation failure aligns with the project's API design philosophy of simplicity over defensive programming.
1216-1249: Robust schema compilation with compatibility override.The
compileOutputSchemafunction properly:
- Uses the invopop/jsonschema reflector for Go struct analysis
- Overrides schema version to Draft-07 for validator compatibility
- Provides comprehensive error handling for marshal/compile failures
1251-1293: Comprehensive validation logic with proper skip conditions.The
ValidateOutputmethod correctly implements the MCP specification requirements:
- Skips validation when
IsErroris true (per TypeScript SDK pattern)- Skips validation when no output schema is defined
- Handles struct-to-JSON conversion for validation
- Provides clear error messages for validation failures
mcp/tools_test.go (6)
716-731: Comprehensive output schema detection tests.These tests properly verify the
HasOutputSchemamethod behavior for both empty tools and tools with defined schemas, ensuring correct detection logic.
733-777: Thorough JSON marshaling verification.This test comprehensively validates that tools with output schemas correctly include the schema in their JSON representation, with proper structure validation.
828-867: Excellent struct-based schema generation test.The test validates the
WithOutputTypefunction using a realistic struct with JSON schema annotations, confirming proper schema generation and JSON marshaling.
869-962: Comprehensive structured result helper tests.These tests thoroughly validate the new generic helper functions (
NewToolResultStructured,NewToolResultWithStructured, etc.), ensuring proper content marshaling and error flag handling.
1023-1073: Excellent thread safety validation.This test properly verifies that
ensureOutputSchemaValidatoris thread-safe by running concurrent goroutines and ensuring consistent validator initialization without race conditions.
1123-1179: Robust error consistency testing.This test validates that error handling during validator initialization is consistent across concurrent access, ensuring that all goroutines receive the same error state. This is crucial for maintaining predictable behavior in multi-threaded environments.
davidleitw
force-pushed
the
structured_output
branch
from
025783b to
440bc9e
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This commit replaces the builder-style `WithOutput*` functions with a single, type-safe generic function, `WithOutputType[T any]`. Developers can now define a tool's output schema using a standard Go struct with `json` and `jsonschema` tags, simplifying the API and improving developer experience. Key changes: - Adds `invopop/jsonschema` for schema generation from structs. - Improves server-side validation to correctly skip validation on tool errors. - Adds new generic helper functions (`NewToolResultStructured`, etc.) for creating structured results. - Updates and adds tests to cover the new API and validation logic.
davidleitw
force-pushed
the
structured_output
branch
from
440bc9e to
f5cb4bd
Compare
|
Will this feature(Structured Tool Output) be released soon? |
|
Hi @ezynda3 Would you be available to briefly review this when you have time? If there's anything that needs to be revised, I'm happy to make improvements. Thank you In principle, my implementation follows the requirements outlined in the MCP spec as closely as possible — for example, the server side enforces output schema validation. |
|
I also hope this feature can be released. I've been waiting for a few weeks😂 |
|
Nice community |
Description
This PR introduces support for structured tool outputs, a feature defined in the Model Context Protocol (MCP) specification.
The key changes include:
outputSchemafield to theTooldefinition to describe the structure of the tool's output.structuredContentfield to theCallToolResultto carry the structured data.structuredContentagainst the tool'soutputSchema.WithOutput*helper functions for programmatic and type-safe schema definition.NewStructuredToolResultandNewStructuredToolErrorhelpers for creating structured results with backward-compatible JSON text content.This implementation covers the server-side requirements (MUST validate). The optional client-side validation (SHOULD validate) is left open for discussion.
Type of Change
Checklist
MCP Spec Compliance
Additional Information
Breaking Change
The exported type
mcp.ToolInputSchemahas been renamed tomcp.ToolSchemato handle both input and output schemas uniformly.Any code that directly depends on this type will need to be updated.
For example,
mcp.ToolInputSchema{...}should be changed tomcp.ToolSchema{...}.Code that creates tools using the
With...()helper functions is not affected.Design Decisions
ValidateStructuredOutput) is implemented as a method on theToolobject. This aligns with the existing design pattern where theToolobject already contains complex logic (MarshalJSON,GetTitle).github.com/santhosh-tekuri/jsonschemalibrary was chosen for its performance, comprehensive spec support, and good standing in the Go ecosystem.NewStructuredToolResult) automatically serialize the structured content into a JSON string for thetextfield, ensuring backward compatibility as recommended by the MCP spec.Client-side Implementation Considerations
Per the MCP spec, a client SHOULD validate the structured output it receives. This implementation is deferred to a future PR to discuss the best approach.
The primary challenge is that the client does not inherently have the
outputSchemafor a given tool. A potential solution is to have the client fetch all tool schemas via aListToolscall upon initialization (or on the first tool call) and cache them locally. SubsequentCallToolresponses could then be validated against this cache.This approach introduces several design questions:
ClientOptionseems like a good approach.Feedback on this proposed direction and any alternative solutions would be appreciated.
Summary by CodeRabbit
New Features
Bug Fixes
Tests