-
Notifications
You must be signed in to change notification settings - Fork 430
Implements outputSchema validation #566
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
Implements outputSchema validation #566
Conversation
95984d1 to
f30567b
Compare
|
Hi @jokemanfire, @4t145, @alexhancock, could one of you please review this PR? Thanks! 🙏 |
f30567b to
3c62ee8
Compare
3399b25 to
3283a0b
Compare
alexhancock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. LGTM other than the one recommendation!
| } | ||
|
|
||
| /// Call [`schema_for_output`] with a cache. | ||
| pub fn cached_schema_for_output<T: JsonSchema + std::any::Any>() -> Result<Arc<JsonObject>, String> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would just fold this caching logic into schema_for_output and only have the one public method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion, @alexhancock! I've consolidated the caching logic into schema_for_output.
77177f2 to
17e48aa
Compare
|
Thank you for fixing this, much appreciated! |
* feat: implement output schema validation * fix: calculator example comply MCP spec * refactor: merge cached_schema_for_output into schema_for_output
Fixes #532
Implements validation to enforce MCP specification requirement that tool
outputSchemamust have a root type of"object". Tools using structured output (Json<T>) where T is a primitive type will fail at compile time with clear error messages.Motivation and Context
As reported in issue #532, the MCP specification requires tool
outputSchemato have a root type of"object"(see MCP Tool Schema). The Rust SDK did not validate this, allowing spec-violating schemas to be generated.The latest spec published on Nov 25 states this more clearly:
How Has This Been Tested?
Noticed the
subtool in the calculator example in the repo doesn't comply with the MCP spec, which causes an issue with MCP Insepctor.rust-sdk/examples/servers/src/common/calculator.rs
Lines 46 to 49 in 94428d5
Now that the output schema validation is in place, the server panics during startup:
➜ servers git:(outputschema-validation) ✗ cargo run -p mcp-server-examples --example servers_calculator_stdio Compiling mcp-server-examples v0.1.5 (/Users/dale/work/rust-sdk/examples/servers) Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.31s Running `/Users/dale/work/rust-sdk/target/debug/examples/servers_calculator_stdio` 2025-11-29T18:50:44.901451Z INFO servers_calculator_stdio: Starting Calculator MCP server thread 'main' panicked at examples/servers/src/common/calculator.rs:46:5: Invalid output schema for Json<i32>: MCP specification requires tool outputSchema to have root type 'object', but found 'integer'. note: run with `RUST_BACKTRACE=1` environment variable to display a backtraceAfter fixing the calculator example code so that the
subtool returns unstructured output just like thesumtool, the server starts up without any issues and works well with the MCP Inspector.➜ servers git:(outputschema-validation) ✗ cargo run -p mcp-server-examples --example servers_calculator_stdio Compiling mcp-server-examples v0.1.5 (/Users/dale/work/rust-sdk/examples/servers) Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.41s Running `/Users/dale/work/rust-sdk/target/debug/examples/servers_calculator_stdio` 2025-11-29T19:05:00.312995Z INFO servers_calculator_stdio: Starting Calculator MCP serverAs suggested in issue #532, wrapping the primitive type also works as expected.
Breaking Changes
Existing valid code works unchanged:
i32,String) with no output stream continues to work<Json<i32>,<Json<String>) fails to compileTypes of changes
Checklist
Additional context
The TypeScript SDK also enforces this requirement in the Tool schema definition here: