Validate inputSchema for MCP tool definitions #4855
Conversation
- For both Sync/Async toolcallbacks, verify the inputSchema when setting the Spring AI ToolDefinition
- This is needed for the cases where the inputSchema doesn't include required JSON schema parameters such as "properties" when parameterless tool definition is used. This specific case is implicitly handled for both Spring AI FunctionToolCallback and McpTool annotated Tools and needs to be fixed for the case where MCP tools don't have valid inputSchema set.
- Applied the fix specifically for Sync and Async ToolCallbacks
- Added JsonSchemaUtils to ensure the schema is validated and fixed with the required parameters. Thanks to @liugddx for the fix from spring-projects#4832
- Add integration tests in OpenAiChatModelIT to verify MCP toolbacks with incorrect inputSchema is handled correctly
- Add MCP integration test to validate the same
Fixes spring-projects#4776
Co-authored-by: liugddx <liugddx@gmail.com>
Signed-off-by: Ilayaperumal Gopinathan <ilayaperumal.gopinathan@broadcom.com>
|
LGTM, thanks @ilayaperumalg |
liugddx
left a comment
liugddx
left a comment
There was a problem hiding this comment.
@ilayaperumalg I have some comments, just some suggestions. Generally LGTM
| Map<String, Object> schemaMap = ModelOptionsUtils.jsonToMap(inputSchema); | ||
| assertThat(schemaMap).isNotNull(); | ||
| assertThat(schemaMap).containsKey("type"); | ||
| assertThat(schemaMap.get("type")).isEqualTo("object"); |
There was a problem hiding this comment.
Add an assertion to preserve additionalProperties, ensuring that the field is retained after normalization.
| McpServerFeatures.SyncToolSpecification toolSpec = new McpServerFeatures.SyncToolSpecification( | ||
| parameterlessTool, (exchange, arguments) -> { | ||
| McpSchema.TextContent content = new McpSchema.TextContent( | ||
| "Current time: 2025-11-11T12:00:00Z"); |
There was a problem hiding this comment.
The McpToolCallbackParameterlessToolIT returns a fixed date string "2025-11-11T12:00:00Z". It can be changed to the current time to reduce the semantic misunderstanding caused by the "specific date string" (this does not affect correctness, only a readability suggestion).
| return DefaultToolDefinition.builder() | ||
| .name(this.prefixedToolName) | ||
| .description(this.tool.description()) | ||
| .inputSchema(ModelOptionsUtils.toJsonString(this.tool.inputSchema())) | ||
| .inputSchema( | ||
| JsonSchemaUtils.ensureValidInputSchema(ModelOptionsUtils.toJsonString(this.tool.inputSchema()))) | ||
| .build(); |
There was a problem hiding this comment.
The logic for constructing ToolDefinition in both the synchronous and asynchronous sections is almost identical. It is recommended to extract a common helper method to facilitate future maintenance.
- Move Sync/Async ToolDefinition creation into utils - Add assertion and updates to McpToolCallbackParameterlessToolIT Signed-off-by: Ilayaperumal Gopinathan <ilayaperumal.gopinathan@broadcom.com>
|
@liugddx thank you for the quick response reviewing the PR! Much appreciated. I have updated the PR addressing the review comments. |
|
LGTM |
3 participants
For both Sync/Async toolcallbacks, verify the inputSchema when setting the Spring AI ToolDefinition
Applied the fix specifically for Sync and Async ToolCallbacks
Added JsonSchemaUtils to ensure the schema is validated and fixed with the required parameters. Thanks to @liugddx for the fix from fix: ensure valid parameters schema for parameter-less functions in OpenAiApi #4832
Add integration tests in OpenAiChatModelIT to verify MCP tool callbacks with incorrect inputSchema is handled correctly
Add MCP integration test to validate the same
Fixes #4776