Skip to content

Commit 947ab05

Browse files
fix(rmcp): tolerate empty cacheScope instead of silently dropping the whole result
ListToolsResult/ReadResourceResult.cache_scope: Option<CacheScope> 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 <wahibelkhadiri06@gmail.com>
1 parent fd7811f commit 947ab05

2 files changed

Lines changed: 57 additions & 3 deletions

File tree

‎crates/rmcp/src/model.rs‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1660,6 +1660,27 @@ where
16601660
Ok(value.map(|ttl_ms| ttl_ms.max(0) as u64))
16611661
}
16621662

1663+
/// Normalize a `cacheScope` value during deserialization.
1664+
///
1665+
/// Per SEP-2549, `cacheScope` MUST be `"public"`, `"private"`, or absent; some
1666+
/// servers instead send `""`. Because `ServerResult` is `#[serde(untagged)]`,
1667+
/// letting that value hard-error here would silently fall through to
1668+
/// `CustomResult` and drop the entire (otherwise valid) result. Treat an empty
1669+
/// string the same as an absent field rather than erroring.
1670+
fn deserialize_cache_scope<'de, D>(deserializer: D) -> Result<Option<CacheScope>, D::Error>
1671+
where
1672+
D: serde::Deserializer<'de>,
1673+
{
1674+
let value = Option::<Value>::deserialize(deserializer)?;
1675+
match value {
1676+
None | Some(Value::Null) => Ok(None),
1677+
Some(Value::String(s)) if s.is_empty() => Ok(None),
1678+
Some(value) => CacheScope::deserialize(value)
1679+
.map(Some)
1680+
.map_err(serde::de::Error::custom),
1681+
}
1682+
}
1683+
16631684
macro_rules! paginated_result {
16641685
($t:ident {
16651686
$i_item: ident: $t_item: ty
@@ -1697,7 +1718,11 @@ macro_rules! paginated_result {
16971718
/// Scope describing who may cache this result (SEP-2549).
16981719
/// Required by spec version 2026-07-28, but optional here to maintain compatibility
16991720
/// with older spec versions.
1700-
#[serde(default, skip_serializing_if = "Option::is_none")]
1721+
#[serde(
1722+
default,
1723+
deserialize_with = "deserialize_cache_scope",
1724+
skip_serializing_if = "Option::is_none"
1725+
)]
17011726
pub cache_scope: Option<CacheScope>,
17021727
pub $i_item: $t_item,
17031728
}
@@ -1847,7 +1872,11 @@ pub struct ReadResourceResult {
18471872
/// Scope describing who may cache this result (SEP-2549).
18481873
/// Required by spec version 2026-07-28, but optional here to maintain compatibility
18491874
/// with older spec versions.
1850-
#[serde(default, skip_serializing_if = "Option::is_none")]
1875+
#[serde(
1876+
default,
1877+
deserialize_with = "deserialize_cache_scope",
1878+
skip_serializing_if = "Option::is_none"
1879+
)]
18511880
pub cache_scope: Option<CacheScope>,
18521881
/// The actual content of the resource
18531882
pub contents: Vec<ResourceContents>,

‎crates/rmcp/tests/test_cache_hints.rs‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,31 @@
1-
use rmcp::model::{CacheScope, ListToolsResult, ReadResourceResult, ResourceContents};
1+
use rmcp::model::{CacheScope, ListToolsResult, ReadResourceResult, ResourceContents, ServerResult};
22
use serde_json::json;
33

4+
#[test]
5+
fn repro_empty_cache_scope_drops_every_tool_via_untagged_fallthrough() {
6+
let payload = json!({
7+
"tools": [{ "name": "search", "inputSchema": { "type": "object" } }],
8+
"cacheScope": ""
9+
});
10+
11+
let direct = serde_json::from_value::<ListToolsResult>(payload.clone());
12+
assert!(
13+
direct.is_ok(),
14+
"ListToolsResult itself must accept an empty cacheScope, got {direct:?}"
15+
);
16+
assert_eq!(direct.unwrap().tools.len(), 1);
17+
18+
let via_server_result: ServerResult =
19+
serde_json::from_value(payload).expect("ServerResult must deserialize this payload");
20+
match via_server_result {
21+
ServerResult::ListToolsResult(r) => assert_eq!(r.tools.len(), 1),
22+
other => panic!(
23+
"expected ListToolsResult, got {other:?} \
24+
(untagged fallthrough silently reinterpreted a valid tools/list result)"
25+
),
26+
}
27+
}
28+
429
#[test]
530
fn paginated_results_serialize_cache_hints_as_top_level_fields() {
631
let result = ListToolsResult::with_all_items(Vec::new())

0 commit comments

Comments
 (0)