fix(rmcp): tolerate empty cacheScope instead of silently dropping the whole result - #1281
Open
WAHIB-EL-KHADIRI wants to merge 2 commits into
Conversation
… 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>
github-actions
Bot
added
T-test
Rchie121
approved these changes
Sep 17, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ListToolsResult/ReadResourceResult.cache_scope: Option<CacheScope>has nocustom deserializer, unlike the sibling
ttl_msfield, which alreadytolerates out-of-range input via
deserialize_ttl_ms. A server sendingcacheScope: ""(SEP-2549 only permits"public"/"private"/absent) failsdeserialization of the whole result — and because
ServerResultis#[serde(untagged)], that failure doesn't surface as an error. It fallsthrough variant-by-variant to
CustomResult(the catch-all), so callerssilently lose typed access to
.tools/.contentsinstead of getting aclear error or a usable result.
Fix mirrors the existing
deserialize_ttl_msnormalize-don't-error pattern:a new
deserialize_cache_scopetreats""/nullas absent, delegateseverything else to
CacheScope's normal deserialization (so a genuinelyinvalid value like
"PUBLIC"still errors, it just no longer takes thewhole result down with it).
Test: added
repro_empty_cache_scope_drops_every_tool_via_untagged_fallthroughto
test_cache_hints.rs, asserting both directListToolsResultdeserialization and the full
ServerResultuntagged path preserve the toollist instead of degrading to
CustomResult.Test results:
cargo test -p rmcp --test test_cache_hints→ 6/6 pass (5pre-existing + 1 new), no regressions.
Closes #1242.