fix: treat empty cacheScope as omitted - #1241
Conversation
github-actions
Bot
added
T-test
ismail-rt
changed the title
a4bed59 to
2ccc934
Compare
An empty cacheScope string is invalid under SEP-2549, but some hosted servers emit it. Rejecting the whole tools/list payload currently drops every tool from an otherwise valid response.
2ccc934 to
611dc34
Compare
| /// How long clients may consider this response fresh, in milliseconds. | ||
| pub ttl_ms: u64, | ||
| /// Whether the cached result may be shared across authorization contexts. | ||
| pub cache_scope: CacheScope, |
There was a problem hiding this comment.
If a server is loose enough to emit "" on tools/list, is there a reason to expect it to be stricter on discover?
| let value = Option::<String>::deserialize(deserializer)?; | ||
| match value.as_deref() { | ||
| None | Some("") => Ok(None), | ||
| Some("public") => Ok(Some(CacheScope::Public)), | ||
| Some("private") => Ok(Some(CacheScope::Private)), | ||
| Some(other) => Err(serde::de::Error::unknown_variant( | ||
| other, | ||
| &["public", "private"], | ||
| )), | ||
| } |
There was a problem hiding this comment.
We can handle only that sentinel and delegate the rest to the derived impl.
| let value = Option::<String>::deserialize(deserializer)?; | |
| match value.as_deref() { | |
| None | Some("") => Ok(None), | |
| Some("public") => Ok(Some(CacheScope::Public)), | |
| Some("private") => Ok(Some(CacheScope::Private)), | |
| Some(other) => Err(serde::de::Error::unknown_variant( | |
| other, | |
| &["public", "private"], | |
| )), | |
| } | |
| use serde::de::IntoDeserializer; | |
| match Option::<std::borrow::Cow<'de, str>>::deserialize(deserializer)?.as_deref() { | |
| None | Some("") => Ok(None), | |
| Some(other) => CacheScope::deserialize(other.into_deserializer()).map(Some), | |
| } |
|
Additional production evidence for this compatibility fix: ChatGPT's curated Upwork app now reproduces the same failure boundary as the Codex reports in openai/codex#41437. Fresh OAuth succeeds and the curated tool schemas are visible, but the first authenticated Upwork operation returns no data and the host disables the Upwork tool surface. This was reproduced with account discovery, direct organization read, and a direct marketplace job read. A clean uninstall/reinstall and app-permission changes did not alter it. OpenAI support cases 14667036/14667065 and Upwork MCP support ticket #55494650 now have the protocol evidence. Given that this PR is mergeable and CI/CodeQL/conformance are green, the narrow exact-empty-string normalization here appears to be a useful client-side interoperability safeguard even if Upwork also fixes its server response. No credentials or private account identifiers included. |
3 participants
Fixes #1242.
Motivation and Context
SEP-2549 allows
cacheScopeto be"public","private", or omitted. Some hosted servers send"". The deserializer treats that as an unknown variant, sotools/list(andresources/read) fail to parse as the typed result.ServerResultis untagged. WhenListToolsResultrejects the payload, it falls through toCustomResult. The tools are on the wire and then disappear.Negative
ttlMsis already clamped to0. EmptycacheScopenow maps toNone. Unknown or whitespace values still error.How Has This Been Tested?
empty_cache_scope_is_treated_as_omittedincrates/rmcp/tests/test_cache_hints.rsunknown_cache_scope_still_errors("shared"and" "still fail)empty_cache_scope_list_tools_result_does_not_fall_throughincrates/rmcp/tests/test_deserialization.rsBreaking Changes
None. Empty
cacheScopepreviously failed to decode. It is now treated as omitted.Types of changes
Checklist