tool "properties" and "required" fields missing if set them as null, AdditionalProperties can't be masharl

[yuehaii]

Description

tool "properties" and "required" fields missing cause LLM (qwen, openai) failure if set them as null.

the pr fix below issues:
1. 'Required' is not included in mashal content even set it as []string{}. Some LLM, like qwen and openai require this field.
2. Mashal 'Tool' dose not trigger its sub fields InputSchema and OutputSchema' MarshalJSON and UnmarshalJSON function call. The same thing happens even marsh its sub fields InputSchema and OutputSchema directly.

Fixes #712

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• MCP spec compatibility implementation
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Code refactoring (no functional changes)
• Performance improvement
• Tests only (no functional changes)
• Other (please describe):

Checklist

• My code follows the code style of this project
• I have performed a self-review of my own code
• I have added tests that prove my fix is effective or that my feature works
• I have updated the documentation accordingly

Additional Information

Code to reproduce the issue:

	return mcp.Tool{
		Name:        ListInteractName,
		Description: "get the kubernetes deployments list. you may want to know the deployment list before call 'deployment-logger' tool",
		InputSchema: mcp.ToolInputSchema{
			Type: "object",
		}}

....

	toolsRequest := mcp.ListToolsRequest{}
	toolsResult, err := a.client.ListTools(a.ctx, toolsRequest)
	for _, tool := range toolsResult.Tools {
		mshT, _ := json.Marshal(tool)
		fmt.Printf("tool %s marshal to %s\n", tool.Name, string(mshT))
	}

Summary by CodeRabbit

• Bug Fixes

  • Improved tool schema JSON serialization to reliably include required fields and ensure empty properties/arrays are serialized as empty objects/arrays in exported tool definitions.
  • Minor comment typo corrected.

• Tests

  • Added tests validating schema marshaling for empty, nil, and populated properties and required fields.

[coderabbitai]

Walkthrough

Adds custom JSON MarshalJSON/UnmarshalJSON implementations for ToolInputSchema, ToolOutputSchema, and ToolArgumentsSchema (with helpers) to control serialization of properties/required/additionalProperties; adds tests for empty/nil serialization cases; fixes a comment spelling typo.

Changes

Cohort / File(s)	Summary
Spelling Fix client/client.go	Corrected comment spelling: "lastest" → "latest" in Initialize.
Custom JSON Serialization mcp/tools.go	Added MarshalJSON/UnmarshalJSON for ToolInputSchema, ToolOutputSchema, ToolArgumentsSchema; introduced helpers toolArgumentsSchemaMarshalJSON and toolArgumentsSchemaUnmarshalJSON; ensure properties serializes as {} when nil and required as [] when empty; preserve additionalProperties handling.
Serialization Tests mcp/tools_test.go	Added tests TestToolInputSchema_MarshalWithEmptyPropertiesAndRequired and TestToolOutputSchema_MarshalWithEmptyPropertiesAndRequired covering nil/empty/populated properties and required serialization scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related issues

• ToolInputSchema doesn't inherit MarshalJSON from ToolArgumentsSchema #694 — Implements missing MarshalJSON/UnmarshalJSON for tool schemas referenced by the issue.
• bug: MCP-Go Library Tool Schema Serialization Bug #671 — Both modify JSON (un)marshaling for ToolInputSchema/ToolOutputSchema, addressing related serialization bugs.

Possibly related PRs

• fix(tool): Return the OutputSchema from the tool definition #571 — Strong overlap: modifies JSON (un)marshalling for ToolInputSchema/ToolOutputSchema/ToolArgumentsSchema in mcp/tools.go.
• fix: marshal ToolInputSchema.Properties to {} when len=0 #225 — Directly related: changes ToolInputSchema marshaling to emit {} for empty/nil properties and [] for required.
• Add AdditionalProperties to ToolInputSchema #678 — Relevant: adjusts ToolArgumentsSchema marshal/unmarshal behavior around additionalProperties.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title contains grammatical errors and unclear phrasing ('masharl' instead of 'marshal') and doesn't clearly describe the primary change being made.	Revise the title to be clear and grammatically correct, such as 'Fix JSON marshaling for tool properties and required fields' or 'Ensure required and properties fields are included in tool JSON serialization'.
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR addresses all requirements from issue #712: ensuring 'required' fields are included in JSON even when empty, and preventing omission of 'properties' and 'required' when explicitly set to empty values.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing JSON marshaling/unmarshaling for ToolInputSchema and ToolOutputSchema; the spelling fix in client.go is a minor incidental correction with negligible scope impact.
Description check	✅ Passed	PR description covers all essential template sections: description of the issue, type of change (Bug fix), checklist items completed, and reproduction code provided.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@mcp/tools.go`:
- Around line 698-701: The GoDoc comment for the UnmarshalJSON method is
incorrect: it references "ToolOutputSchema" but the method is defined on
ToolArgumentsSchema. Update the comment to accurately describe the receiver by
changing the comment text to reference ToolArgumentsSchema (i.e., the
UnmarshalJSON method on ToolArgumentsSchema that calls
toolArgumentsSchemaUnmarshalJSON), ensuring the comment matches the function
name UnmarshalJSON and the helper toolArgumentsSchemaUnmarshalJSON.

🧹 Nitpick comments (1)

mcp/tools.go (1)

703-731: Minor typo and LGTM on the logic.

The implementation correctly ensures properties is serialized as {} when nil and required is serialized as [] when empty, addressing the LLM compatibility issues described in the PR.

📝 Fix typo in comment

-// It handls the fields stored in ToolArgumentsSchema when json.Marshaler been called
+// toolArgumentsSchemaMarshalJSON handles the fields stored in ToolArgumentsSchema when json.Marshaler is called

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@mcp/tools.go`:
- Around line 723-728: The inline comment incorrectly mentions "Properties"
while the code handles tis.Required; update the comment above the block that
marshals the "required" field to accurately describe that it marshals
tis.Required to an empty slice ([]) rather than nil when its length is zero,
referencing tis.Required and the "required" map key so the comment matches the
code's intent.