Skip to content

fix: keep empty properties in tool's InputSchema #596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: keep empty properties in tool's InputSchema #596

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb94274
Keep empty properties
hhxiao Sep 19, 2025
a80f289
Merge branch 'mark3labs:main' into main
hhxiao Nov 13, 2025
051ec94
Update tools.go
hhxiao Nov 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions mcp/tools.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,8 +629,8 @@ type ToolArgumentsSchema struct {
Required []string `json:"required,omitempty"`
}

type ToolInputSchema ToolArgumentsSchema // For retro-compatibility
type ToolOutputSchema ToolArgumentsSchema
type ToolInputSchema = ToolArgumentsSchema // For retro-compatibility
type ToolOutputSchema = ToolArgumentsSchema
Comment on lines +632 to +633
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Type alias change doesn't resolve the OpenAI properties field issue.

Converting these to type aliases is a reasonable refactor for code consistency, but it doesn't fix the stated bug. The OpenAI 400 error occurs because the MarshalJSON method (lines 645-647) omits the properties field when nil.

As noted in the previous review, you must update the MarshalJSON implementation to always include the properties field for object-type schemas, regardless of whether Properties is nil or an empty map.

The PR description states this change "ensures empty properties are preserved," but the type alias change alone doesn't accomplish this. You still need to modify MarshalJSON as suggested in the previous review.

🤖 Prompt for AI Agents 
In mcp/tools.go around lines 632-647 (aliases at 632-633 and MarshalJSON
implementation at ~645-647), the type alias refactor didn't address the OpenAI
400 error because MarshalJSON omits the properties field when it's nil; update
MarshalJSON to always include a properties field for schemas whose Type ==
"object" even if Properties is nil by ensuring the encoded JSON contains
"properties": {} (or an empty map) instead of omitting it—e.g., build the output
map/struct explicitly and if Type == "object" set Properties to an empty map
when nil (or remove omitempty for that field) so the properties key is always
present in the serialized JSON.

Copy link
Contributor Author

@hhxiao hhxiao Nov 14, 2025

Choose a reason for hiding this comment

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

according to https://github.com/mark3labs/mcp-go/blob/main/mcp/tools.go#L689, properties are always initialized to empty map, this ensures the marshalled json always contain properties field

Copy link
Contributor

@coderabbitai coderabbitai bot Nov 14, 2025

Choose a reason for hiding this comment

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

@hhxiao You're absolutely right, and I apologize for missing this crucial detail!

The type alias change IS essential to fixing the issue. Here's why:

Before this PR:

  • type ToolInputSchema ToolArgumentsSchema created a distinct type that strips all methods from ToolArgumentsSchema
  • This means ToolInputSchema did NOT use the custom MarshalJSON method

After this PR:

  • type ToolInputSchema = ToolArgumentsSchema creates (binds) a new identifier to the same type
  • Now ToolInputSchema DOES use the MarshalJSON method from ToolArgumentsSchema

How it all works together:

  1. Line 689: Properties: make(map[string]any) initializes to an empty map (not nil)
  2. Lines 645-647: MarshalJSON includes properties when tis.Properties != nil
  3. Type alias ensures ToolInputSchema uses this MarshalJSON
  4. Result: "properties": {} is always included in the JSON output

This elegantly solves the OpenAI 400 error for tools created with NewTool. My apologies for not recognizing the significance of the type alias change in relation to method inheritance!

🐰

🧠 Learnings used 
Learnt from: xinwo
Repo: mark3labs/mcp-go PR: 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
Repo: mark3labs/mcp-go PR: 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
Repo: mark3labs/mcp-go PR: 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: lariel-fernandes
Repo: mark3labs/mcp-go PR: 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: floatingIce91
Repo: mark3labs/mcp-go PR: 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: ezynda3
Repo: mark3labs/mcp-go PR: 461
File: server/sampling.go:22-26
Timestamp: 2025-06-30T07:13:17.052Z
Learning: In the mark3labs/mcp-go project, the MCPServer.capabilities field is a struct value (serverCapabilities), not a pointer, so it cannot be nil and doesn't require nil checking. Only pointer fields within the capabilities struct should be checked for nil.


// MarshalJSON implements the json.Marshaler interface for ToolInputSchema.
func (tis ToolArgumentsSchema) MarshalJSON() ([]byte, error) {
Expand Down
Loading