From 947ab05ab6f18188d116c5917b45acb650e8695d Mon Sep 17 00:00:00 2001 From: Wahib El Khadiri Date: Wed, 16 Sep 2026 20:20:04 +0200 Subject: [PATCH 1/2] fix(rmcp): tolerate empty cacheScope instead of silently dropping the whole result ListToolsResult/ReadResourceResult.cache_scope: Option had no custom deserializer, unlike the sibling ttl_ms field, which already tolerates out-of-range input via deserialize_ttl_ms. A server sending cacheScope: "" (SEP-2549 only permits "public"/"private"/absent) failed deserialization of the whole result - and because ServerResult is #[serde(untagged)], that failure doesn't surface as an error. It falls through variant-by-variant to CustomResult (the catch-all), so callers silently lose typed access to .tools/.contents instead of getting a clear error or a usable result. Add deserialize_cache_scope, mirroring the existing deserialize_ttl_ms normalize-don't-error pattern: "" and null are treated as absent, everything else delegates to CacheScope's normal deserialization, so a genuinely invalid value (e.g. "PUBLIC") still errors - it just no longer takes the whole result down with it. Add a regression test covering both direct ListToolsResult deserialization and the full ServerResult untagged path, asserting the tool list survives instead of degrading to CustomResult. Signed-off-by: Wahib El Khadiri --- crates/rmcp/src/model.rs | 33 +++++++++++++++++++++++++-- crates/rmcp/tests/test_cache_hints.rs | 27 +++++++++++++++++++++- 2 files changed, 57 insertions(+), 3 deletions(-) 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..5d89f35fd 100644 --- a/crates/rmcp/tests/test_cache_hints.rs +++ b/crates/rmcp/tests/test_cache_hints.rs @@ -1,6 +1,31 @@ -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()) From a3f45f3a845926d9d005f4b2217fcf692ef77684 Mon Sep 17 00:00:00 2001 From: WAHIB-EL-KHADIRI Date: Mon, 21 Sep 2026 21:17:23 +0200 Subject: [PATCH 2/2] style: wrap the cache-hints test imports to satisfy rustfmt The Code Formatting job runs `cargo +nightly fmt --all -- --check`, and the single-line `use rmcp::model::{...}` in this test exceeded the configured width, failing the only red check on the PR. No behaviour change. Co-Authored-By: Claude Opus 5 Signed-off-by: WAHIB-EL-KHADIRI --- crates/rmcp/tests/test_cache_hints.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/rmcp/tests/test_cache_hints.rs b/crates/rmcp/tests/test_cache_hints.rs index 5d89f35fd..ebf78ff6b 100644 --- a/crates/rmcp/tests/test_cache_hints.rs +++ b/crates/rmcp/tests/test_cache_hints.rs @@ -1,4 +1,6 @@ -use rmcp::model::{CacheScope, ListToolsResult, ReadResourceResult, ResourceContents, ServerResult}; +use rmcp::model::{ + CacheScope, ListToolsResult, ReadResourceResult, ResourceContents, ServerResult, +}; use serde_json::json; #[test]