Add AdditionalProperties to ToolInputSchema#678
Conversation
- Add AdditionalProperties field to ToolArgumentsSchema struct - Update MarshalJSON to include additionalProperties in output - Add WithSchemaAdditionalProperties ToolOption function - Add comprehensive tests for marshal/unmarshal and ToolOption Fixes mark3labs#672
WalkthroughThis PR adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
mcp/tools_test.go (1)
1756-1767: LGTM! ToolOption behaves correctly.The test validates that
WithSchemaAdditionalPropertiesproperly sets the field on a tool's InputSchema and that it appears correctly in the JSON output.Optional: Consider expanding test coverage
While the marshal/unmarshal tests already cover various
additionalPropertiesvalues, you could optionally expand this test to verify the ToolOption works withtrueand schema objects as well:func TestWithSchemaAdditionalProperties(t *testing.T) { - tool := NewTool( - "strict-tool", - WithSchemaAdditionalProperties(false), - ) - - assert.Equal(t, false, tool.InputSchema.AdditionalProperties) - - data, err := json.Marshal(tool) - assert.NoError(t, err) - assert.Contains(t, string(data), `"additionalProperties":false`) + tests := []struct { + name string + value any + expected string + }{ + {"false", false, `"additionalProperties":false`}, + {"true", true, `"additionalProperties":true`}, + {"schema", map[string]any{"type": "string"}, `"additionalProperties":{"type":"string"}`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tool := NewTool("test-tool", WithSchemaAdditionalProperties(tt.value)) + assert.Equal(t, tt.value, tool.InputSchema.AdditionalProperties) + data, err := json.Marshal(tool) + assert.NoError(t, err) + assert.Contains(t, string(data), tt.expected) + }) + } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mcp/tools.gomcp/tools_test.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing
Files:
mcp/tools_test.gomcp/tools.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Testing: use testify/assert and testify/require
Write table-driven tests using a tests := []struct{ name, ... } pattern
Go test files must end with _test.go
Files:
mcp/tools_test.go
🧠 Learnings (5)
📓 Common learnings
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: 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.
📚 Learning: 2025-03-04T07:00:57.111Z
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.
Applied to files:
mcp/tools_test.gomcp/tools.go
📚 Learning: 2025-03-04T07:00:57.111Z
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.
Applied to files:
mcp/tools_test.gomcp/tools.go
📚 Learning: 2025-03-04T06:59:43.882Z
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.
Applied to files:
mcp/tools_test.gomcp/tools.go
📚 Learning: 2025-10-13T09:35:20.180Z
Learnt from: CR
Repo: mark3labs/mcp-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-13T09:35:20.180Z
Learning: Applies to **/*_test.go : Write table-driven tests using a tests := []struct{ name, ... } pattern
Applied to files:
mcp/tools_test.go
🔇 Additional comments (7)
mcp/tools_test.go (4)
1697-1713: LGTM! Proper test for boolean additionalProperties.The test correctly verifies that
additionalProperties: falseis unmarshaled as a boolean value and stored in the schema.
1715-1732: LGTM! Correctly tests schema-object additionalProperties.The test validates that
additionalPropertiescan be a schema object (per JSON Schema spec), and properly asserts the parsed structure.
1734-1743: LGTM! Validates marshal behavior for additionalProperties.The test correctly verifies that setting
AdditionalProperties: falseresults in the expected JSON output.
1745-1754: LGTM! Confirms backward compatibility with omitempty.This test is important for ensuring that tools without
AdditionalPropertiesset won't have the field in their JSON output, maintaining backward compatibility.mcp/tools.go (3)
638-642: LGTM! Proper struct field definition.The
AdditionalProperties anyfield is correctly defined with:
- Appropriate type (
any) to support boolean and schema-object values per JSON Schema specomitemptytag for backward compatibility- Follows Go naming conventions
The field integrates cleanly with the existing
UnmarshalJSONmethod via theAliaspattern (lines 673-696).
666-668: LGTM! Correct marshaling logic.The conditional inclusion of
additionalPropertiesin the JSON output is consistent with theomitemptytag and follows the same pattern as other optional fields in this method.
927-934: LGTM! Well-designed ToolOption function.The
WithSchemaAdditionalPropertiesfunction:
- Follows Go naming conventions and coding guidelines
- Has a clear GoDoc comment explaining usage
- Correctly sets the field on the tool's InputSchema
- Naming avoids conflict with the existing
AdditionalPropertiesPropertyOptionThe implementation properly supports the feature objectives for controlling additional properties on tool input schemas.
2 participants
Summary
Add support for
additionalPropertiesfield in theToolInputSchema(aliasToolArgumentsSchema) struct, allowing tools to specify whether additional properties are allowed in their input schema.Changes
AdditionalPropertiesfield toToolArgumentsSchemastructMarshalJSONto includeadditionalPropertiesin output when setWithSchemaAdditionalPropertiesToolOption functionExample Usage
Notes
anytype to supportfalse,true, or schema object values per JSON Schema specWithSchemaAdditionalPropertiesto distinguish from existingAdditionalPropertiesPropertyOptionFixes #672
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.