Skip to content

Add schema validation to lowlevel server #1005

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.

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

Merged
merged 5 commits into from
Jun 25, 2025

Conversation

bhosmer-ant
Copy link
Contributor

@bhosmer-ant bhosmer-ant commented Jun 23, 2025

Lowlevel server

  • validates incoming tool call arguments against tool's inputSchema
    • note: @server.call_tool(validate_input=False) can be used to disable input validation
  • validates tool call results against tool's outputSchema, if defined

Motivation and Context

Previously lowlevel server did no schema validation for tool calls.

How Has This Been Tested?

New tests added, old tests (which now run lowlevel tool calls through schema validation) succeed without change.

Breaking Changes

Possible that some tools defined in the lowlevel server "just worked" against invalid incoming arguments, these will now fail validation. @server.call_tool(validate_input=False) can be used in this situation.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

In addition to adding validation to the lowlevel server, FastMCP delegates to the lowlevel server and will depend on validation being performed here.

@bhosmer-ant bhosmer-ant requested a review from ihrpr June 23, 2025 18:11
@bhosmer-ant bhosmer-ant force-pushed the basil/lowlevel_schema_validation branch 2 times, most recently from 50311b7 to fff4cf9 Compare June 23, 2025 19:33
@bhosmer-ant bhosmer-ant marked this pull request as ready for review June 23, 2025 19:55
@bhosmer-ant bhosmer-ant force-pushed the basil/lowlevel_schema_validation branch 3 times, most recently from b9cd587 to 727660f Compare June 23, 2025 20:28
ihrpr
ihrpr previously approved these changes Jun 24, 2025
Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

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

Looks good. Left some comments about types and clarifying examples

- Add jsonschema dependency for schema validation
- Implement tool definition cache in Server class that gets refreshed when list_tools is called
- Add _validate_tool_arguments helper method to validate tool arguments against inputSchema
- Update call_tool handler to validate arguments before execution
- Log warning and skip validation for tools not found in cache
- Add comprehensive tests for validation scenarios

This ensures tool arguments are validated against their JSON schemas before execution,
providing better error messages and preventing invalid tool calls from reaching handlers.
- Refactor code to extract _get_tool_definition helper and simplify validation
- Update call_tool to support three return types: content only, dict only, or both
- Add outputSchema validation that checks structured content matches the schema
- Serialize dict-only results to JSON text content
- Factor error result construction into _make_error_result helper
- Add comprehensive tests for all output validation scenarios

The server now validates tool outputs against their defined schemas, providing
better error messages and ensuring tool responses match their contracts.
@bhosmer-ant bhosmer-ant force-pushed the basil/lowlevel_schema_validation branch from 5b75167 to 193b28f Compare June 25, 2025 05:52
Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

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

LGTM

@bhosmer-ant bhosmer-ant merged commit c8bbfc0 into main Jun 25, 2025
20 of 21 checks passed
@bhosmer-ant bhosmer-ant deleted the basil/lowlevel_schema_validation branch June 25, 2025 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants