From 38e42a7f5b604ededb875e1085773ac7eca34313 Mon Sep 17 00:00:00 2001 From: cetra3 Date: Mon, 18 Aug 2025 13:40:02 +0930 Subject: [PATCH] fix: match shape of the calltoolresult schema BREAKING CHANGE: makes the `content` field non-optional --- crates/rmcp/src/model.rs | 21 +++++++++---------- .../server_json_rpc_message_schema.json | 10 ++++----- crates/rmcp/tests/test_structured_output.rs | 14 ++++++------- crates/rmcp/tests/test_tool_macros.rs | 6 ++---- examples/simple-chat-client/src/chat.rs | 4 ++-- 5 files changed, 26 insertions(+), 29 deletions(-) diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index c51eaa1e..b9a591b8 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -181,7 +181,7 @@ impl<'de> Deserialize<'de> for ProtocolVersion { pub enum NumberOrString { /// A numeric identifier Number(u32), - /// A string identifier + /// A string identifier String(Arc), } @@ -1186,8 +1186,7 @@ pub type RootsListChangedNotification = NotificationNoParam>, + pub content: Vec, /// An optional JSON object that represents the structured result of the tool call #[serde(skip_serializing_if = "Option::is_none")] pub structured_content: Option, @@ -1200,7 +1199,7 @@ impl CallToolResult { /// Create a successful tool result with unstructured content pub fn success(content: Vec) -> Self { CallToolResult { - content: Some(content), + content, structured_content: None, is_error: Some(false), } @@ -1208,7 +1207,7 @@ impl CallToolResult { /// Create an error tool result with unstructured content pub fn error(content: Vec) -> Self { CallToolResult { - content: Some(content), + content, structured_content: None, is_error: Some(true), } @@ -1229,7 +1228,7 @@ impl CallToolResult { /// ``` pub fn structured(value: Value) -> Self { CallToolResult { - content: Some(vec![Content::text(value.to_string())]), + content: vec![Content::text(value.to_string())], structured_content: Some(value), is_error: Some(false), } @@ -1254,7 +1253,7 @@ impl CallToolResult { /// ``` pub fn structured_error(value: Value) -> Self { CallToolResult { - content: Some(vec![Content::text(value.to_string())]), + content: vec![Content::text(value.to_string())], structured_content: Some(value), is_error: Some(true), } @@ -1270,10 +1269,10 @@ impl CallToolResult { where T: DeserializeOwned, { - let raw_text = match (self.structured_content, &self.content) { + let raw_text = match (self.structured_content, &self.content.first()) { (Some(value), _) => return serde_json::from_value(value), (None, Some(contents)) => { - if let Some(text) = contents.first().and_then(|c| c.as_text()) { + if let Some(text) = contents.as_text() { let text = &text.text; Some(text) } else { @@ -1308,13 +1307,13 @@ impl<'de> Deserialize<'de> for CallToolResult { let helper = CallToolResultHelper::deserialize(deserializer)?; let result = CallToolResult { - content: helper.content, + content: helper.content.unwrap_or_default(), structured_content: helper.structured_content, is_error: helper.is_error, }; // Validate mutual exclusivity - if result.content.is_none() && result.structured_content.is_none() { + if result.content.is_empty() && result.structured_content.is_none() { return Err(serde::de::Error::custom( "CallToolResult must have either content or structured_content", )); diff --git a/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema.json b/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema.json index a0fa15e2..fabcf72d 100644 --- a/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema.json +++ b/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema.json @@ -304,10 +304,7 @@ "properties": { "content": { "description": "The content returned by the tool (text, images, etc.)", - "type": [ - "array", - "null" - ], + "type": "array", "items": { "$ref": "#/definitions/Annotated" } @@ -322,7 +319,10 @@ "structuredContent": { "description": "An optional JSON object that represents the structured result of the tool call" } - } + }, + "required": [ + "content" + ] }, "CancelledNotificationMethod": { "type": "string", diff --git a/crates/rmcp/tests/test_structured_output.rs b/crates/rmcp/tests/test_structured_output.rs index 171213e0..98821e5d 100644 --- a/crates/rmcp/tests/test_structured_output.rs +++ b/crates/rmcp/tests/test_structured_output.rs @@ -122,10 +122,10 @@ async fn test_structured_content_in_call_result() { let result = CallToolResult::structured(structured_data.clone()); - assert!(result.content.is_some()); + assert!(!result.content.is_empty()); assert!(result.structured_content.is_some()); - let contents = result.content.unwrap(); + let contents = result.content; assert_eq!(contents.len(), 1); @@ -150,10 +150,10 @@ async fn test_structured_error_in_call_result() { let result = CallToolResult::structured_error(error_data.clone()); - assert!(result.content.is_some()); + assert!(!result.content.is_empty()); assert!(result.structured_content.is_some()); - let contents = result.content.unwrap(); + let contents = result.content; assert_eq!(contents.len(), 1); @@ -217,10 +217,10 @@ async fn test_structured_return_conversion() { // Tools which return structured content should also return a serialized version as // Content::text for backwards compatibility. - assert!(call_result.content.is_some()); + assert!(!call_result.content.is_empty()); assert!(call_result.structured_content.is_some()); - let contents = call_result.content.unwrap(); + let contents = call_result.content; assert_eq!(contents.len(), 1); @@ -278,5 +278,5 @@ async fn test_output_schema_requires_structured_content() { // Verify it has structured_content and content assert!(call_result.structured_content.is_some()); - assert!(call_result.content.is_some()); + assert!(!call_result.content.is_empty()); } diff --git a/crates/rmcp/tests/test_tool_macros.rs b/crates/rmcp/tests/test_tool_macros.rs index 90791062..b7631ed5 100644 --- a/crates/rmcp/tests/test_tool_macros.rs +++ b/crates/rmcp/tests/test_tool_macros.rs @@ -301,8 +301,7 @@ async fn test_optional_i64_field_with_null_input() -> anyhow::Result<()> { let result_text = result .content - .as_ref() - .and_then(|contents| contents.first()) + .first() .and_then(|content| content.raw.as_text()) .map(|text| text.text.as_str()) .expect("Expected text content"); @@ -330,8 +329,7 @@ async fn test_optional_i64_field_with_null_input() -> anyhow::Result<()> { let some_result_text = some_result .content - .as_ref() - .and_then(|contents| contents.first()) + .first() .and_then(|content| content.raw.as_text()) .map(|text| text.text.as_str()) .expect("Expected text content"); diff --git a/examples/simple-chat-client/src/chat.rs b/examples/simple-chat-client/src/chat.rs index abbe0d2e..221389ed 100644 --- a/examples/simple-chat-client/src/chat.rs +++ b/examples/simple-chat-client/src/chat.rs @@ -85,8 +85,8 @@ impl ChatSession { if result.is_error.is_some_and(|b| b) { self.messages .push(Message::user("tool call failed, mcp call error")); - } else if let Some(contents) = &result.content { - contents.iter().for_each(|content| { + } else { + result.content.iter().for_each(|content| { if let Some(content_text) = content.as_text() { let json_result = serde_json::from_str::( &content_text.text,