diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index ec7b93b3e..6d3cdf01d 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -1660,6 +1660,27 @@ where Ok(value.map(|ttl_ms| ttl_ms.max(0) as u64)) } +/// Normalize a `cacheScope` value during deserialization. +/// +/// Per SEP-2549, `cacheScope` MUST be `"public"`, `"private"`, or absent; some +/// servers instead send `""`. Because `ServerResult` is `#[serde(untagged)]`, +/// letting that value hard-error here would silently fall through to +/// `CustomResult` and drop the entire (otherwise valid) result. Treat an empty +/// string the same as an absent field rather than erroring. +fn deserialize_cache_scope<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let value = Option::::deserialize(deserializer)?; + match value { + None | Some(Value::Null) => Ok(None), + Some(Value::String(s)) if s.is_empty() => Ok(None), + Some(value) => CacheScope::deserialize(value) + .map(Some) + .map_err(serde::de::Error::custom), + } +} + macro_rules! paginated_result { ($t:ident { $i_item: ident: $t_item: ty @@ -1697,7 +1718,11 @@ macro_rules! paginated_result { /// Scope describing who may cache this result (SEP-2549). /// Required by spec version 2026-07-28, but optional here to maintain compatibility /// with older spec versions. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "deserialize_cache_scope", + skip_serializing_if = "Option::is_none" + )] pub cache_scope: Option, pub $i_item: $t_item, } @@ -1847,7 +1872,11 @@ pub struct ReadResourceResult { /// Scope describing who may cache this result (SEP-2549). /// Required by spec version 2026-07-28, but optional here to maintain compatibility /// with older spec versions. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "deserialize_cache_scope", + skip_serializing_if = "Option::is_none" + )] pub cache_scope: Option, /// The actual content of the resource pub contents: Vec, diff --git a/crates/rmcp/tests/test_cache_hints.rs b/crates/rmcp/tests/test_cache_hints.rs index 2b6aecd8c..ebf78ff6b 100644 --- a/crates/rmcp/tests/test_cache_hints.rs +++ b/crates/rmcp/tests/test_cache_hints.rs @@ -1,6 +1,33 @@ -use rmcp::model::{CacheScope, ListToolsResult, ReadResourceResult, ResourceContents}; +use rmcp::model::{ + CacheScope, ListToolsResult, ReadResourceResult, ResourceContents, ServerResult, +}; use serde_json::json; +#[test] +fn repro_empty_cache_scope_drops_every_tool_via_untagged_fallthrough() { + let payload = json!({ + "tools": [{ "name": "search", "inputSchema": { "type": "object" } }], + "cacheScope": "" + }); + + let direct = serde_json::from_value::(payload.clone()); + assert!( + direct.is_ok(), + "ListToolsResult itself must accept an empty cacheScope, got {direct:?}" + ); + assert_eq!(direct.unwrap().tools.len(), 1); + + let via_server_result: ServerResult = + serde_json::from_value(payload).expect("ServerResult must deserialize this payload"); + match via_server_result { + ServerResult::ListToolsResult(r) => assert_eq!(r.tools.len(), 1), + other => panic!( + "expected ListToolsResult, got {other:?} \ + (untagged fallthrough silently reinterpreted a valid tools/list result)" + ), + } +} + #[test] fn paginated_results_serialize_cache_hints_as_top_level_fields() { let result = ListToolsResult::with_all_items(Vec::new())