Skip to content

Conversation

@shorwood
Copy link
Contributor

@shorwood shorwood commented May 14, 2025

This commit adds JSON Schema support for all model types by implementing schemars::JsonSchema trait. The feature is gated behind the new schemars feature flag. This enables automatic schema generation for API documentation and validation purposes. Added tests to verify schema generation for client and server JSON-RPC messages.

Motivation and Context

This change facilitates schema-driven tooling, enabling better API validation, documentation generation, and integration with OpenAPI-based systems. It addresses the need for structured metadata across all model types.

How Has This Been Tested?

  • Unit tests were added to ensure correct schema generation.
  • Verified against example client and server JSON-RPC message structures.
  • Tests were added for two representative schema types: client and server JSON-RPC messages.
  • These types were selected because they internally reference all supported model subtypes, ensuring comprehensive schema coverage.

Breaking Changes

No breaking changes. The new feature is opt-in and does not affect default builds.

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

The schemars feature is isolated for minimal impact. Feel free to suggest improvements or additional test scenarios.

}

fn json_schema(generator: &mut schemars::SchemaGenerator) -> schemars::schema::Schema {
generator.subschema_for::<serde_json::Value>()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unsure about this, feel free to suggest improvements.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's no need to implement JsonSchema for extensions.

Copy link
Contributor Author

@shorwood shorwood May 15, 2025

Choose a reason for hiding this comment

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

You're right, I adjusted some models like so:

#[schemars(skip)]
pub extensions: Extensions,

Ok with this ?

@shorwood shorwood force-pushed the shorwood/schemars-json-schema branch from 3c6ebb4 to 330dfc4 Compare May 14, 2025 17:05
@4t145 4t145 requested a review from Copilot May 15, 2025 02:06
Copy link
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 implements JSON Schema support for all model types by adding the schemars::JsonSchema derive where applicable and updating tests to verify schema generation.

  • Adds JSON Schema derivations on all model structs and enums via cfg_attr.
  • Introduces new tests that compare generated schema against expected JSON files.
  • Updates Cargo.toml to include schemars with additional features and test configuration.

Reviewed Changes

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

File Description
crates/rmcp/tests/test_message_schema.rs Adds tests for JSON schema generation of client/server JSON-RPC messages.
crates/rmcp/src/model/* Updates model definitions to derive JsonSchema when the schemars feature is enabled.
crates/rmcp/Cargo.toml Adds and configures the schemars dependency with extra features.
README.md Documents the newly added schemars feature.

let schema = schema_for!(ClientJsonRpcMessage);
let schema_str = serde_json::to_string_pretty(&schema).unwrap();
let expected = std::fs::read_to_string("tests/test_message_schema/client_json_rpc_message_schema.json").unwrap();
assert_eq!(schema_str, expected, "Schema generation for ClientJsonRpcMessage should match expected output");
Copy link

Copilot AI May 15, 2025

Choose a reason for hiding this comment

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

Comparing the formatted JSON schema as a string can be brittle due to potential whitespace or key ordering differences. Instead, parse both 'schema_str' and 'expected' into JSON values and compare those for a more robust test.

Suggested change
assert_eq!(schema_str, expected, "Schema generation for ClientJsonRpcMessage should match expected output");
let schema_json: serde_json::Value = serde_json::from_str(&schema_str).unwrap();
let expected_json: serde_json::Value = serde_json::from_str(&expected).unwrap();
assert_eq!(schema_json, expected_json, "Schema generation for ClientJsonRpcMessage should match expected output");

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's reasonable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

@4t145
Copy link
Collaborator

4t145 commented May 15, 2025

Looks good to me. Just need to fix CI and suggestions from coplilot, please.

shorwood added 2 commits May 15, 2025 17:46
This commit adds JSON Schema support for all model types by implementing
`schemars::JsonSchema` trait. The feature is gated behind the new
`schemars` feature flag. This enables automatic schema generation for
API documentation and validation purposes. Added tests to verify schema
generation for client and server JSON-RPC messages.
This commit adds a manual implementation of `JsonSchema` trait for the
`NumberOrString` enum to properly represent its union type nature in
JSON Schema. The schema now correctly specifies that the type can be
either a number or a string using the `oneOf` validation keyword.
@shorwood shorwood force-pushed the shorwood/schemars-json-schema branch from f7c895a to 35976a6 Compare May 15, 2025 15:47
The `Extensions` type was incorrectly included in JSON schema
generation, which could lead to confusing API documentation. This commit
adds `#[schemars(skip)]` attribute to all `extensions` fields in request
and notification structs, and removes the manual `JsonSchema`
implementation for the `Extensions` type since it's an internal
implementation detail that shouldn't be exposed in the schema.
@shorwood shorwood force-pushed the shorwood/schemars-json-schema branch from 35976a6 to e211254 Compare May 15, 2025 15:49
@4t145 4t145 merged commit e9a5ae9 into modelcontextprotocol:main May 15, 2025
9 checks passed
@4t145
Copy link
Collaborator

4t145 commented May 15, 2025

Merged, thank you for PR!

@github-actions github-actions bot mentioned this pull request Jul 2, 2025
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