Skip to content

Commit fbed447

Browse files
fix(rmcp): tolerate empty cacheScope instead of silently dropping the whole result (#1281)
* 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> * 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 <noreply@anthropic.com> Signed-off-by: WAHIB-EL-KHADIRI <wahibelkhadiri06@gmail.com> --------- Signed-off-by: Wahib El Khadiri <wahibelkhadiri06@gmail.com> Signed-off-by: WAHIB-EL-KHADIRI <wahibelkhadiri06@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 90516bf commit fbed447

2 files changed

Lines changed: 59 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: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,33 @@
1-
use rmcp::model::{CacheScope, ListToolsResult, ReadResourceResult, ResourceContents};
1+
use rmcp::model::{
2+
CacheScope, ListToolsResult, ReadResourceResult, ResourceContents, ServerResult,
3+
};
24
use serde_json::json;
35

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

0 commit comments

Comments
 (0)