Skip to content

Commit a47cbaa

Browse files
authored
fix: match shape of the calltoolresult schema (#377)
BREAKING CHANGE: makes the `content` field non-optional
1 parent 81ff315 commit a47cbaa

File tree

5 files changed

+26
-29
lines changed

5 files changed

+26
-29
lines changed

crates/rmcp/src/model.rs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ impl<'de> Deserialize<'de> for ProtocolVersion {
183183
pub enum NumberOrString {
184184
/// A numeric identifier
185185
Number(u32),
186-
/// A string identifier
186+
/// A string identifier
187187
String(Arc<str>),
188188
}
189189

@@ -1257,8 +1257,7 @@ pub type CreateElicitationRequest =
12571257
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
12581258
pub struct CallToolResult {
12591259
/// The content returned by the tool (text, images, etc.)
1260-
#[serde(skip_serializing_if = "Option::is_none")]
1261-
pub content: Option<Vec<Content>>,
1260+
pub content: Vec<Content>,
12621261
/// An optional JSON object that represents the structured result of the tool call
12631262
#[serde(skip_serializing_if = "Option::is_none")]
12641263
pub structured_content: Option<Value>,
@@ -1271,15 +1270,15 @@ impl CallToolResult {
12711270
/// Create a successful tool result with unstructured content
12721271
pub fn success(content: Vec<Content>) -> Self {
12731272
CallToolResult {
1274-
content: Some(content),
1273+
content,
12751274
structured_content: None,
12761275
is_error: Some(false),
12771276
}
12781277
}
12791278
/// Create an error tool result with unstructured content
12801279
pub fn error(content: Vec<Content>) -> Self {
12811280
CallToolResult {
1282-
content: Some(content),
1281+
content,
12831282
structured_content: None,
12841283
is_error: Some(true),
12851284
}
@@ -1300,7 +1299,7 @@ impl CallToolResult {
13001299
/// ```
13011300
pub fn structured(value: Value) -> Self {
13021301
CallToolResult {
1303-
content: Some(vec![Content::text(value.to_string())]),
1302+
content: vec![Content::text(value.to_string())],
13041303
structured_content: Some(value),
13051304
is_error: Some(false),
13061305
}
@@ -1325,7 +1324,7 @@ impl CallToolResult {
13251324
/// ```
13261325
pub fn structured_error(value: Value) -> Self {
13271326
CallToolResult {
1328-
content: Some(vec![Content::text(value.to_string())]),
1327+
content: vec![Content::text(value.to_string())],
13291328
structured_content: Some(value),
13301329
is_error: Some(true),
13311330
}
@@ -1341,10 +1340,10 @@ impl CallToolResult {
13411340
where
13421341
T: DeserializeOwned,
13431342
{
1344-
let raw_text = match (self.structured_content, &self.content) {
1343+
let raw_text = match (self.structured_content, &self.content.first()) {
13451344
(Some(value), _) => return serde_json::from_value(value),
13461345
(None, Some(contents)) => {
1347-
if let Some(text) = contents.first().and_then(|c| c.as_text()) {
1346+
if let Some(text) = contents.as_text() {
13481347
let text = &text.text;
13491348
Some(text)
13501349
} else {
@@ -1379,13 +1378,13 @@ impl<'de> Deserialize<'de> for CallToolResult {
13791378

13801379
let helper = CallToolResultHelper::deserialize(deserializer)?;
13811380
let result = CallToolResult {
1382-
content: helper.content,
1381+
content: helper.content.unwrap_or_default(),
13831382
structured_content: helper.structured_content,
13841383
is_error: helper.is_error,
13851384
};
13861385

13871386
// Validate mutual exclusivity
1388-
if result.content.is_none() && result.structured_content.is_none() {
1387+
if result.content.is_empty() && result.structured_content.is_none() {
13891388
return Err(serde::de::Error::custom(
13901389
"CallToolResult must have either content or structured_content",
13911390
));

crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema.json

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -304,10 +304,7 @@
304304
"properties": {
305305
"content": {
306306
"description": "The content returned by the tool (text, images, etc.)",
307-
"type": [
308-
"array",
309-
"null"
310-
],
307+
"type": "array",
311308
"items": {
312309
"$ref": "#/definitions/Annotated"
313310
}
@@ -322,7 +319,10 @@
322319
"structuredContent": {
323320
"description": "An optional JSON object that represents the structured result of the tool call"
324321
}
325-
}
322+
},
323+
"required": [
324+
"content"
325+
]
326326
},
327327
"CancelledNotificationMethod": {
328328
"type": "string",

crates/rmcp/tests/test_structured_output.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,10 @@ async fn test_structured_content_in_call_result() {
122122

123123
let result = CallToolResult::structured(structured_data.clone());
124124

125-
assert!(result.content.is_some());
125+
assert!(!result.content.is_empty());
126126
assert!(result.structured_content.is_some());
127127

128-
let contents = result.content.unwrap();
128+
let contents = result.content;
129129

130130
assert_eq!(contents.len(), 1);
131131

@@ -150,10 +150,10 @@ async fn test_structured_error_in_call_result() {
150150

151151
let result = CallToolResult::structured_error(error_data.clone());
152152

153-
assert!(result.content.is_some());
153+
assert!(!result.content.is_empty());
154154
assert!(result.structured_content.is_some());
155155

156-
let contents = result.content.unwrap();
156+
let contents = result.content;
157157

158158
assert_eq!(contents.len(), 1);
159159

@@ -217,10 +217,10 @@ async fn test_structured_return_conversion() {
217217

218218
// Tools which return structured content should also return a serialized version as
219219
// Content::text for backwards compatibility.
220-
assert!(call_result.content.is_some());
220+
assert!(!call_result.content.is_empty());
221221
assert!(call_result.structured_content.is_some());
222222

223-
let contents = call_result.content.unwrap();
223+
let contents = call_result.content;
224224

225225
assert_eq!(contents.len(), 1);
226226

@@ -278,5 +278,5 @@ async fn test_output_schema_requires_structured_content() {
278278

279279
// Verify it has structured_content and content
280280
assert!(call_result.structured_content.is_some());
281-
assert!(call_result.content.is_some());
281+
assert!(!call_result.content.is_empty());
282282
}

crates/rmcp/tests/test_tool_macros.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,7 @@ async fn test_optional_i64_field_with_null_input() -> anyhow::Result<()> {
301301

302302
let result_text = result
303303
.content
304-
.as_ref()
305-
.and_then(|contents| contents.first())
304+
.first()
306305
.and_then(|content| content.raw.as_text())
307306
.map(|text| text.text.as_str())
308307
.expect("Expected text content");
@@ -330,8 +329,7 @@ async fn test_optional_i64_field_with_null_input() -> anyhow::Result<()> {
330329

331330
let some_result_text = some_result
332331
.content
333-
.as_ref()
334-
.and_then(|contents| contents.first())
332+
.first()
335333
.and_then(|content| content.raw.as_text())
336334
.map(|text| text.text.as_str())
337335
.expect("Expected text content");

examples/simple-chat-client/src/chat.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ impl ChatSession {
8585
if result.is_error.is_some_and(|b| b) {
8686
self.messages
8787
.push(Message::user("tool call failed, mcp call error"));
88-
} else if let Some(contents) = &result.content {
89-
contents.iter().for_each(|content| {
88+
} else {
89+
result.content.iter().for_each(|content| {
9090
if let Some(content_text) = content.as_text() {
9191
let json_result = serde_json::from_str::<serde_json::Value>(
9292
&content_text.text,

0 commit comments

Comments
 (0)