feat: add client-side TTL-honoring response cache (SEP-2549)#1025
feat: add client-side TTL-honoring response cache (SEP-2549)#1025alexhancock wants to merge 1 commit into
Conversation
Add a configurable client response cache in the service layer that honors SEP-2549 caching hints (ttlMs / cacheScope) for tools/list, prompts/list, resources/list, resources/templates/list, and resources/read. Closes #974 Co-Authored-by: John-Francis Nnadi <nnadifrancis23@gmail.com>
| // Cache keys are built only from the request method plus the parameters that | ||
| // affect the result (SEP-2549). Request `_meta` (progress tokens, trace | ||
| // context, etc.) does not affect the result, so it is deliberately excluded to | ||
| // avoid fragmenting the cache across otherwise-identical requests. | ||
| fn discover_cache_key() -> String { | ||
| // `server/discover` carries no result-affecting parameters. | ||
| DISCOVER_CACHE_PREFIX.to_string() | ||
| } | ||
|
|
||
| fn list_response_cache_key(prefix: &str, params: &Option<PaginatedRequestParams>) -> String { | ||
| // Only the pagination cursor affects which page is returned. | ||
| let cursor = params.as_ref().and_then(|params| params.cursor.as_deref()); | ||
| let cursor = | ||
| serde_json::to_string(&cursor).expect("serializing a pagination cursor cannot fail"); | ||
| format!("{prefix}{cursor}") | ||
| } |
There was a problem hiding this comment.
RequestMetaObject permits arbitrary extension keys that handlers can use to vary a result, so two calls with the same cursor but different _meta can currently share one cached response. How should result-affecting metadata participate in cache identity without fragmenting entries on transport-only fields such as progress and trace context?
| Err(error) => { | ||
| if let Some(ServerResult::ListToolsResult(result)) = | ||
| self.stale_cached_response(&cache_key).await | ||
| { | ||
| return Ok(result); | ||
| } | ||
| if uses_cursor { | ||
| self.invalidate_tool_cache().await; | ||
| } |
There was a problem hiding this comment.
With the default stale-on-error setting, an expired cursor page remains available when the server rejects that cursor, so this early return bypasses the page-set invalidation below.
| async fn private_entries_are_isolated_between_client_peers() { | ||
| let first = disconnected_peer(); | ||
| let second = disconnected_peer(); | ||
| let key = list_response_cache_key(TOOL_LIST_CACHE_PREFIX, &None); | ||
| first | ||
| .cache_response( | ||
| key.clone(), | ||
| ServerResult::ListToolsResult(tools_result(Some(5_000), Some(CacheScope::Private))), | ||
| Some(5_000), | ||
| Some(CacheScope::Private), | ||
| ) | ||
| .await; | ||
|
|
||
| assert!(first.cached_response(&key).await.is_some()); | ||
| assert!(second.cached_response(&key).await.is_none()); |
There was a problem hiding this comment.
Separate peers always own separate cache maps, so this test passes even if private_partition is ignored. Could it exercise two authorization partitions on the same peer to cover the isolation guarantee introduced by this cache?
Add a configurable client response cache in the service layer that honors SEP-2549 caching hints (ttlMs / cacheScope) for tools/list, prompts/list, resources/list, resources/templates/list, and resources/read.
Closes #974
Motivation and Context
Needed to support https://modelcontextprotocol.io/specification/draft/server/utilities/caching
How Has This Been Tested?
CI/Tests
Breaking Changes
N/A
Types of changes
Checklist
Additional context
Original PR (heavily adapted for this): #975
https://github.com/starsaintf Marked as Co-Author