Skip to content

Commit 1555c0f

Browse files
authored
fix(tool): remove unnecessary schema validation (#375)
1 parent 05e9a04 commit 1555c0f

File tree

4 files changed

+45
-61
lines changed

4 files changed

+45
-61
lines changed

crates/rmcp/src/handler/server/router/tool.rs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use schemars::JsonSchema;
66
use crate::{
77
handler::server::tool::{
88
CallToolHandler, DynCallToolHandler, ToolCallContext, schema_for_type,
9-
validate_against_schema,
109
},
1110
model::{CallToolResult, Tool, ToolAnnotations},
1211
};
@@ -246,19 +245,6 @@ where
246245

247246
let result = (item.call)(context).await?;
248247

249-
// Validate structured content against output schema if present
250-
if let Some(ref output_schema) = item.attr.output_schema {
251-
// When output_schema is defined, structured_content is required
252-
if result.structured_content.is_none() {
253-
return Err(crate::ErrorData::invalid_params(
254-
"Tool with output_schema must return structured_content",
255-
None,
256-
));
257-
}
258-
// Validate the structured content against the schema
259-
validate_against_schema(result.structured_content.as_ref().unwrap(), output_schema)?;
260-
}
261-
262248
Ok(result)
263249
}
264250

crates/rmcp/src/handler/server/tool.rs

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -67,43 +67,6 @@ pub fn schema_for_type<T: JsonSchema>() -> JsonObject {
6767
}
6868
}
6969

70-
/// Validate that a JSON value conforms to basic type constraints from a schema.
71-
///
72-
/// Note: This is a basic validation that only checks type compatibility.
73-
/// For full JSON Schema validation, a dedicated validation library would be needed.
74-
pub fn validate_against_schema(
75-
value: &serde_json::Value,
76-
schema: &JsonObject,
77-
) -> Result<(), crate::ErrorData> {
78-
// Basic type validation
79-
if let Some(schema_type) = schema.get("type").and_then(|t| t.as_str()) {
80-
let value_type = get_json_value_type(value);
81-
82-
if schema_type != value_type {
83-
return Err(crate::ErrorData::invalid_params(
84-
format!(
85-
"Value type does not match schema. Expected '{}', got '{}'",
86-
schema_type, value_type
87-
),
88-
None,
89-
));
90-
}
91-
}
92-
93-
Ok(())
94-
}
95-
96-
fn get_json_value_type(value: &serde_json::Value) -> &'static str {
97-
match value {
98-
serde_json::Value::Null => "null",
99-
serde_json::Value::Bool(_) => "boolean",
100-
serde_json::Value::Number(_) => "number",
101-
serde_json::Value::String(_) => "string",
102-
serde_json::Value::Array(_) => "array",
103-
serde_json::Value::Object(_) => "object",
104-
}
105-
}
106-
10770
/// Call [`schema_for_type`] with a cache
10871
pub fn cached_schema_for_type<T: JsonSchema + std::any::Any>() -> Arc<JsonObject> {
10972
thread_local! {

crates/rmcp/src/model.rs

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ pub use extension::*;
1515
pub use meta::*;
1616
pub use prompt::*;
1717
pub use resource::*;
18-
use serde::{Deserialize, Serialize};
18+
use serde::{Deserialize, Serialize, de::DeserializeOwned};
1919
use serde_json::Value;
2020
pub use tool::*;
2121

@@ -1260,12 +1260,32 @@ impl CallToolResult {
12601260
}
12611261
}
12621262

1263-
/// Validate that content or structured content is provided
1264-
pub fn validate(&self) -> Result<(), &'static str> {
1265-
match (&self.content, &self.structured_content) {
1266-
(None, None) => Err("either content or structured_content must be provided"),
1267-
_ => Ok(()),
1263+
/// Convert the `structured_content` part of response into a certain type.
1264+
///
1265+
/// # About json schema validation
1266+
/// Since rust is a strong type language, we don't need to do json schema validation here.
1267+
///
1268+
/// But if you do have to validate the response data, you can use [`jsonschema`](https://crates.io/crates/jsonschema) crate.
1269+
pub fn into_typed<T>(self) -> Result<T, serde_json::Error>
1270+
where
1271+
T: DeserializeOwned,
1272+
{
1273+
let raw_text = match (self.structured_content, &self.content) {
1274+
(Some(value), _) => return serde_json::from_value(value),
1275+
(None, Some(contents)) => {
1276+
if let Some(text) = contents.first().and_then(|c| c.as_text()) {
1277+
let text = &text.text;
1278+
Some(text)
1279+
} else {
1280+
None
1281+
}
1282+
}
1283+
(None, None) => None,
1284+
};
1285+
if let Some(text) = raw_text {
1286+
return serde_json::from_str(text);
12681287
}
1288+
serde_json::from_value(serde_json::Value::Null)
12691289
}
12701290
}
12711291

@@ -1294,7 +1314,11 @@ impl<'de> Deserialize<'de> for CallToolResult {
12941314
};
12951315

12961316
// Validate mutual exclusivity
1297-
result.validate().map_err(serde::de::Error::custom)?;
1317+
if result.content.is_none() && result.structured_content.is_none() {
1318+
return Err(serde::de::Error::custom(
1319+
"CallToolResult must have either content or structured_content",
1320+
));
1321+
}
12981322

12991323
Ok(result)
13001324
}

crates/rmcp/tests/test_structured_output.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,13 +170,24 @@ async fn test_structured_error_in_call_result() {
170170

171171
#[tokio::test]
172172
async fn test_mutual_exclusivity_validation() {
173+
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
174+
pub struct Response {
175+
message: String,
176+
}
177+
let response = Response {
178+
message: "Hello".into(),
179+
};
173180
// Test that content and structured_content can both be passed separately
174-
let content_result = CallToolResult::success(vec![Content::text("Hello")]);
181+
let content_result = CallToolResult::success(vec![Content::json(response.clone()).unwrap()]);
175182
let structured_result = CallToolResult::structured(json!({"message": "Hello"}));
176183

177184
// Verify the validation
178-
assert!(content_result.validate().is_ok());
179-
assert!(structured_result.validate().is_ok());
185+
content_result
186+
.into_typed::<Response>()
187+
.expect("Failed to extract content");
188+
structured_result
189+
.into_typed::<Response>()
190+
.expect("Failed to extract content");
180191

181192
// Try to create a result with both fields
182193
let json_with_both = json!({

0 commit comments

Comments
 (0)