From d3c44756da0e3e495af23eae8e7442a4cfc99823 Mon Sep 17 00:00:00 2001 From: 4t145 Date: Thu, 14 Aug 2025 18:54:52 +0800 Subject: [PATCH] fix(tool): remove unnecessary schema validation --- crates/rmcp/src/handler/server/router/tool.rs | 14 ------- crates/rmcp/src/handler/server/tool.rs | 37 ------------------ crates/rmcp/src/model.rs | 38 +++++++++++++++---- crates/rmcp/tests/test_structured_output.rs | 17 +++++++-- 4 files changed, 45 insertions(+), 61 deletions(-) diff --git a/crates/rmcp/src/handler/server/router/tool.rs b/crates/rmcp/src/handler/server/router/tool.rs index 2a100de9..4a5ba34f 100644 --- a/crates/rmcp/src/handler/server/router/tool.rs +++ b/crates/rmcp/src/handler/server/router/tool.rs @@ -6,7 +6,6 @@ use schemars::JsonSchema; use crate::{ handler::server::tool::{ CallToolHandler, DynCallToolHandler, ToolCallContext, schema_for_type, - validate_against_schema, }, model::{CallToolResult, Tool, ToolAnnotations}, }; @@ -246,19 +245,6 @@ where let result = (item.call)(context).await?; - // Validate structured content against output schema if present - if let Some(ref output_schema) = item.attr.output_schema { - // When output_schema is defined, structured_content is required - if result.structured_content.is_none() { - return Err(crate::ErrorData::invalid_params( - "Tool with output_schema must return structured_content", - None, - )); - } - // Validate the structured content against the schema - validate_against_schema(result.structured_content.as_ref().unwrap(), output_schema)?; - } - Ok(result) } diff --git a/crates/rmcp/src/handler/server/tool.rs b/crates/rmcp/src/handler/server/tool.rs index 8d5c8213..bdb33698 100644 --- a/crates/rmcp/src/handler/server/tool.rs +++ b/crates/rmcp/src/handler/server/tool.rs @@ -67,43 +67,6 @@ pub fn schema_for_type() -> JsonObject { } } -/// Validate that a JSON value conforms to basic type constraints from a schema. -/// -/// Note: This is a basic validation that only checks type compatibility. -/// For full JSON Schema validation, a dedicated validation library would be needed. -pub fn validate_against_schema( - value: &serde_json::Value, - schema: &JsonObject, -) -> Result<(), crate::ErrorData> { - // Basic type validation - if let Some(schema_type) = schema.get("type").and_then(|t| t.as_str()) { - let value_type = get_json_value_type(value); - - if schema_type != value_type { - return Err(crate::ErrorData::invalid_params( - format!( - "Value type does not match schema. Expected '{}', got '{}'", - schema_type, value_type - ), - None, - )); - } - } - - Ok(()) -} - -fn get_json_value_type(value: &serde_json::Value) -> &'static str { - match value { - serde_json::Value::Null => "null", - serde_json::Value::Bool(_) => "boolean", - serde_json::Value::Number(_) => "number", - serde_json::Value::String(_) => "string", - serde_json::Value::Array(_) => "array", - serde_json::Value::Object(_) => "object", - } -} - /// Call [`schema_for_type`] with a cache pub fn cached_schema_for_type() -> Arc { thread_local! { diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index f8a44933..c51eaa1e 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -15,7 +15,7 @@ pub use extension::*; pub use meta::*; pub use prompt::*; pub use resource::*; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Serialize, de::DeserializeOwned}; use serde_json::Value; pub use tool::*; @@ -1260,12 +1260,32 @@ impl CallToolResult { } } - /// Validate that content or structured content is provided - pub fn validate(&self) -> Result<(), &'static str> { - match (&self.content, &self.structured_content) { - (None, None) => Err("either content or structured_content must be provided"), - _ => Ok(()), + /// Convert the `structured_content` part of response into a certain type. + /// + /// # About json schema validation + /// Since rust is a strong type language, we don't need to do json schema validation here. + /// + /// But if you do have to validate the response data, you can use [`jsonschema`](https://crates.io/crates/jsonschema) crate. + pub fn into_typed(self) -> Result + where + T: DeserializeOwned, + { + let raw_text = match (self.structured_content, &self.content) { + (Some(value), _) => return serde_json::from_value(value), + (None, Some(contents)) => { + if let Some(text) = contents.first().and_then(|c| c.as_text()) { + let text = &text.text; + Some(text) + } else { + None + } + } + (None, None) => None, + }; + if let Some(text) = raw_text { + return serde_json::from_str(text); } + serde_json::from_value(serde_json::Value::Null) } } @@ -1294,7 +1314,11 @@ impl<'de> Deserialize<'de> for CallToolResult { }; // Validate mutual exclusivity - result.validate().map_err(serde::de::Error::custom)?; + if result.content.is_none() && result.structured_content.is_none() { + return Err(serde::de::Error::custom( + "CallToolResult must have either content or structured_content", + )); + } Ok(result) } diff --git a/crates/rmcp/tests/test_structured_output.rs b/crates/rmcp/tests/test_structured_output.rs index f8e8beb0..171213e0 100644 --- a/crates/rmcp/tests/test_structured_output.rs +++ b/crates/rmcp/tests/test_structured_output.rs @@ -170,13 +170,24 @@ async fn test_structured_error_in_call_result() { #[tokio::test] async fn test_mutual_exclusivity_validation() { + #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] + pub struct Response { + message: String, + } + let response = Response { + message: "Hello".into(), + }; // Test that content and structured_content can both be passed separately - let content_result = CallToolResult::success(vec![Content::text("Hello")]); + let content_result = CallToolResult::success(vec![Content::json(response.clone()).unwrap()]); let structured_result = CallToolResult::structured(json!({"message": "Hello"})); // Verify the validation - assert!(content_result.validate().is_ok()); - assert!(structured_result.validate().is_ok()); + content_result + .into_typed::() + .expect("Failed to extract content"); + structured_result + .into_typed::() + .expect("Failed to extract content"); // Try to create a result with both fields let json_with_both = json!({