Skip to content

fix: strip and validate tool outputSchema and inputSchema#860

Open
DaleSeo wants to merge 2 commits into
mainfrom
fix/output-schema-strip-title-description
Open

fix: strip and validate tool outputSchema and inputSchema#860
DaleSeo wants to merge 2 commits into
mainfrom
fix/output-schema-strip-title-description

Conversation

@DaleSeo
Copy link
Copy Markdown
Member

@DaleSeo DaleSeo commented May 28, 2026

Motivation and Context

PR #856 cleaned up inputSchema by stripping the unnecessary top-level title and description fields, since the MCP spec does not require them and they only add noise to the LLM context. The same noise applies to outputSchema, and the spec's official examples for both schemas omit those top-level fields in identical fashion. This PR extends the same cleanup to outputSchema for symmetry.

image

EDIT: While auditing the symmetry, I found two related gaps and also addressed them. First, schema_for_input didn't enforce the MCP spec's type: "object" constraint, which schema_for_output already does. This meant that corner cases like Parameters<String> could silently create tool definitions that violated the spec. Now, the function is validated and returns Result<Arc<JsonObject>, String>, matching schema_for_output, with clear panics at call sites that previously couldn't fail. Second, schema_for_empty_input uses LazyLock to avoid allocating memory with each call.

How Has This Been Tested?

Added some unit tests and updated the integration test.

Breaking Changes

No breaking change. schema_for_input was added as a public function in PR #856 but has not been released yet.

Behavior change worth noting: tools whose input parameter type does not produce a type: "object" schema (e.g. Parameters<String>, Parameters<i32>) will now panic at construction time with a descriptive message rather than silently producing a spec-violating tool definition. Two tools inside this repo's own test suite fit that pattern and have been updated.

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

@DaleSeo DaleSeo self-assigned this May 28, 2026
@github-actions github-actions Bot added T-documentation Documentation improvements T-test Testing related changes T-config Configuration file changes T-core Core library changes T-handler Handler implementation changes T-macros Macro changes T-model Model/data structure changes labels May 28, 2026
@DaleSeo DaleSeo marked this pull request as ready for review May 28, 2026 21:41
@DaleSeo DaleSeo requested a review from a team as a code owner May 28, 2026 21:41
@jokemanfire jokemanfire requested a review from Copilot May 29, 2026 02:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens and harmonizes MCP tool schema generation by (1) stripping top-level title/description from both inputSchema and outputSchema, and (2) validating that generated tool schemas have a root JSON Schema type: "object" (as required by the MCP spec), failing fast with clearer panics at tool-construction call sites.

Changes:

  • Strip top-level title/description from tool outputSchema (matching the existing inputSchema cleanup) and add shared validation/stripping logic.
  • Make schema_for_input validate root type: "object" and return Result, updating call sites to unwrap with descriptive panic messages.
  • Update tests/fixtures to use structured request/response types (object schemas) and validate outputSchema serialization behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
README.md Updates documentation around schema generation (currently needs clarification; see PR comment).
crates/rmcp/src/handler/server/common.rs Adds shared validate+strip helper; enforces object-root schemas for tool input/output; caches results; optimizes empty-input schema via LazyLock.
crates/rmcp/src/model/tool.rs Updates with_input_schema to unwrap Result with a clearer panic (matching output behavior).
crates/rmcp/src/handler/server/router/tool/tool_traits.rs Updates default input_schema() to unwrap Result with a clearer panic.
crates/rmcp/src/handler/server/router/tool.rs Updates router builder paths to unwrap schema_for_input with clearer panics.
crates/rmcp-macros/src/tool.rs Updates macro-generated tool metadata to unwrap schema_for_input failures with descriptive panics.
crates/rmcp/tests/test_structured_output.rs Adjusts test tools to use object-typed parameter structs to comply with the new input-schema validation.
crates/rmcp/tests/test_list_tools_result.rs Changes tool to return Json<AddResult> so an outputSchema is generated and test can validate it.
crates/rmcp/tests/test_list_tools_result/list_tools_result.json Updates fixture to include outputSchema (without top-level title/description).

Comment thread README.md
```

The generated tool `inputSchema` is derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
The generated tool `inputSchema` and `outputSchema` are derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-config Configuration file changes T-core Core library changes T-documentation Documentation improvements T-handler Handler implementation changes T-macros Macro changes T-model Model/data structure changes T-test Testing related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants