diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index 9dd04564e..0599ea055 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -4165,7 +4165,8 @@ pub struct CallToolResult { // 1. Defaults `content` to `[]` when the field is missing (lenient per Postel's law) // 2. Requires at least one known field to be present, so that `CallToolResult` doesn't // greedily match arbitrary JSON objects when used inside `#[serde(untagged)]` enums -// (e.g. `ServerResult`), which would shadow `CustomResult`. +// (e.g. `ServerResult`), which would shadow `CustomResult`. `_meta` doesn't count, +// since any result type may carry it. // 3. Rejects non-`complete` result types so other `ServerResult` variants can match. // 4. Keeps a present `"structuredContent": null` as `Some(Value::Null)`, distinct // from an absent field, since structured content can be any JSON value. @@ -4202,11 +4203,10 @@ impl<'de> Deserialize<'de> for CallToolResult { if helper.content.is_none() && helper.structured_content.is_none() && helper.is_error.is_none() - && helper.meta.is_none() { return Err(serde::de::Error::custom( "expected at least one known CallToolResult field \ - (content, structuredContent, isError, or _meta)", + (content, structuredContent, or isError)", )); } diff --git a/crates/rmcp/tests/test_deserialization.rs b/crates/rmcp/tests/test_deserialization.rs index 1f2191864..92c806317 100644 --- a/crates/rmcp/tests/test_deserialization.rs +++ b/crates/rmcp/tests/test_deserialization.rs @@ -105,6 +105,37 @@ mod untagged_server_result { ); } + #[test] + fn meta_does_not_shadow_custom_result() { + // `_meta` can appear on any result, so it alone must not make + // CallToolResult swallow a custom result (e.g. SEP-2640 skills/list). + let payload = json!({ + "resultType": "complete", + "skills": [{ "uri": "skill://a/SKILL.md" }], + "ttlMs": 0, + "cacheScope": "private", + "_meta": { "x": true } + }); + let result = parse_result(wrap_response(payload.clone())); + let ServerResult::CustomResult(result) = result else { + panic!("expected CustomResult, got {result:?}"); + }; + assert_eq!(result.0, payload); + } + + #[test] + fn call_tool_result_with_meta_deserializes_to_correct_variant() { + let result = parse_result(wrap_response(json!({ + "content": [{ "type": "text", "text": "hi" }], + "_meta": {} + }))); + let ServerResult::CallToolResult(result) = result else { + panic!("expected CallToolResult, got {result:?}"); + }; + assert_eq!(result.content.len(), 1); + assert!(result.meta.is_some()); + } + #[test] fn input_required_result_with_meta_deserializes_to_correct_variant() { let result = parse_result(wrap_response(json!({ diff --git a/crates/rmcp/tests/test_result_type_wire.rs b/crates/rmcp/tests/test_result_type_wire.rs index a22933e35..b6ad82484 100644 --- a/crates/rmcp/tests/test_result_type_wire.rs +++ b/crates/rmcp/tests/test_result_type_wire.rs @@ -123,28 +123,21 @@ fn server_result_should_preserve_input_required_result_when_meta_present() { } #[test] -fn server_result_should_accept_legacy_call_tool_result_when_only_meta_present() { - let legacy = json!({ +fn server_result_should_not_treat_meta_only_result_as_call_tool_result() { + // `_meta` may appear on any result, so on its own it says nothing about the + // result being a tool call. + let payload = json!({ "_meta": { "example.com/replica": "r1", }, }); let result = - serde_json::from_value::(legacy).expect("deserialize legacy CallToolResult"); + serde_json::from_value::(payload.clone()).expect("deserialize ServerResult"); match result { - ServerResult::CallToolResult(result) => { - assert_eq!( - ( - result.result_type, - result.content.is_empty(), - result.meta.is_some() - ), - (None, true, true) - ); - } - _ => panic!("expected CallToolResult"), + ServerResult::CustomResult(result) => assert_eq!(result.0, payload), + other => panic!("expected CustomResult, got {other:?}"), } }