Add cache_hints constructor map for SEP-2549 caching hints#3015
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/mcp/server/caching.py">
<violation number="1" location="src/mcp/server/caching.py:90">
P2: Unknown `cache_hints` keys are formatted as sortable strings, so non-string keys can raise `TypeError` before the intended `ValueError` validation error.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| """ | ||
| if cache_hints is None: | ||
| return {} | ||
| unknown = sorted(method for method in cache_hints if method not in CACHEABLE_METHODS) |
There was a problem hiding this comment.
P2: Unknown cache_hints keys are formatted as sortable strings, so non-string keys can raise TypeError before the intended ValueError validation error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/caching.py, line 90:
<comment>Unknown `cache_hints` keys are formatted as sortable strings, so non-string keys can raise `TypeError` before the intended `ValueError` validation error.</comment>
<file context>
@@ -0,0 +1,98 @@
+ """
+ if cache_hints is None:
+ return {}
+ unknown = sorted(method for method in cache_hints if method not in CACHEABLE_METHODS)
+ if unknown:
+ raise ValueError(f"cache_hints keys must be cacheable methods (see CacheableMethod); got: {', '.join(unknown)}")
</file context>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| # Fill cache hints on the typed result, before the serialize sieve | ||
| # decides whether the negotiated version carries the fields at all. | ||
| # `input_required` interim results are not `CacheableResult` models, | ||
| # so the MRTR carve-out (no hints on them) holds by shape. | ||
| if isinstance(result, CacheableResult) and (hint := self.server.cache_hints.get(method)) is not None: | ||
| result = apply_cache_hint(result, hint) |
There was a problem hiding this comment.
🟡 When a lowlevel handler (registered via add_request_handler, or a middleware short-circuit) returns a plain dict for a cacheable method like tools/list, the new hint-stamping step is skipped because it only runs for isinstance(result, CacheableResult), so the configured cache_hints entry is silently dropped and the 2026 wire emits the schema defaults ttlMs: 0 / cacheScope: "private" instead. Consider also filling absent keys on the dict path, or documenting the limitation in docs/advanced/caching.md.
Extended reasoning...
What the bug is. HandlerResult (src/mcp/server/context.py:114) is BaseModel | dict[str, Any] | None, so a handler registered with add_request_handler (and a server-tier middleware that short-circuits) may legitimately return a plain dict for one of the six cacheable methods. The PR's stamping step in src/mcp/server/runner.py:205 is gated on isinstance(result, CacheableResult), which a dict never satisfies, so apply_cache_hint is never invoked for dict results. The dict then flows through _serialize → serialize_server_result, where the 2026-07-28 surface model fills its non-None defaults (ttlMs=0, cacheScope="private"), and exclude_none=True keeps them — so the wire actively asserts "immediately stale, never shared" despite the operator's configured CacheHint.\n\nCode path. Concrete walk-through:\n\n1. Server("srv", cache_hints={"tools/list": CacheHint(ttl_ms=60_000, scope="public")}).\n2. server.add_request_handler("tools/list", PaginatedRequestParams, handler) where handler returns {"tools": [{"name": "t", "inputSchema": {"type": "object"}}]} — exactly the shape exercised by existing tests such as test_runner_handler_returning_malformed_dict_for_spec_method_is_internal_error (well-formed variant) and the _dump_result dict branch.\n3. A 2026-07-28 client calls tools/list. In _inner, result is a dict, so isinstance(result, CacheableResult) is False and the apply_cache_hint line at runner.py:205-206 is skipped.\n4. _serialize calls serialize_server_result("tools/list", "2026-07-28", dumped); the result model validates the dict, fills ttlMs=0 / cacheScope="private" from its defaults, and dumps them onto the wire.\n5. The client sees ttlMs: 0, cacheScope: "private" — the operator's 60s/public hint never applied, with no warning or error.\n\nWhy existing code doesn't prevent it. The precedence mechanism (model_fields_set in apply_cache_hint) only exists on the typed-result path; a dict has no model_fields_set, and the serialize sieve has no knowledge of cache_hints. The PR's tests all return typed result models, so this path is untested. This contradicts the documented model in docs/advanced/caching.md ("explicit beats configured, configured beats default", "emitting them is the server's job, and the SDK does it for you"): a dict that omits both keys set neither field explicitly, yet still gets the defaults rather than the configured hint.\n\nImpact. Narrow but real: it only bites the combination of cache_hints + a dict-returning handler for one of the six cacheable methods at a 2026 negotiation. All typed on_* constructor handlers and the entire MCPServer tier return result models and are unaffected, and the failure mode is conservative (defaults mean no caching, never an unsafe over-share). The authors clearly considered nearby edge cases (the model_construct caveat in apply_cache_hint's docstring), but this one is neither stamped nor documented.\n\nHow to fix. Either (a) extend the runner so dict results for cacheable methods also get the configured hint, filling only keys absent from the dict (e.g. check "ttlMs" not in result / "cacheScope" not in result before inserting the hint values), preserving the per-field precedence model; or (b) add a sentence to docs/advanced/caching.md noting that cache_hints only applies to typed result models, so handlers returning raw dicts (and short-circuiting middleware) own the ttlMs/cacheScope keys themselves.
Server and MCPServer take cache_hints={method: CacheHint(...)} to set
ttlMs/cacheScope on the six cacheable results server-wide. The runner
fills the typed result after the handler returns, so fields a handler
sets explicitly win, per field (via model_fields_set), and the existing
serialize sieve keeps pre-2026 wires clean. Keys are typed as the
CacheableMethod Literal so editors autocomplete them and flag typos;
runtime validation still rejects bad keys and values at construction
for untyped callers.
Part of #2899.
The caching page covered only server authoring. Add a 'What the client sees' section: the hints arrive as parsed ttl_ms/cache_scope fields on every cacheable result, the SDK does not act on them, and the supported path today is reading the fields and doing your own freshness and scope bookkeeping. Covers the legacy-server case (absent fields show the conservative model defaults) and the model_fields_set wire-presence check, with a tested example.
| CacheableResultT = TypeVar("CacheableResultT", bound=types.CacheableResult) | ||
|
|
||
|
|
||
| def apply_cache_hint(result: CacheableResultT, hint: CacheHint) -> CacheableResultT: | ||
| """Fill `ttl_ms`/`cache_scope` on `result` from `hint`. | ||
|
|
||
| Per-field: a field the handler set explicitly - even to its default value, | ||
| tracked via `model_fields_set` - is left alone; only unset fields take the |
There was a problem hiding this comment.
🟡 The docstring caveat "A handler constructing results with model_construct bypasses that tracking and is treated as having set nothing" is factually wrong: pydantic's model_construct defaults __pydantic_fields_set__ to the keys actually passed, so an explicit ttl_ms/cache_scope passed via model_construct still wins over the configured hint, exactly like normal construction. The runtime behavior is correct — only the sentence needs correcting or dropping (it only holds when a handler passes _fields_set=set() explicitly).
Extended reasoning...
What the docstring claims vs. what pydantic does. apply_cache_hint's docstring (src/mcp/server/caching.py:56-63) says a handler that builds its result with model_construct "bypasses that tracking and is treated as having set nothing." In pydantic v2, however, model_construct only treats the result as having set nothing when _fields_set is explicitly passed (e.g. _fields_set=set()). When _fields_set is omitted — the common case — pydantic computes __pydantic_fields_set__ from the keyword arguments actually passed, the same as normal validation-based construction.
Concrete proof (verified against the installed pydantic 2.12.5):
from mcp_types import ListToolsResult
from mcp.server.caching import CacheHint, apply_cache_hint
r = ListToolsResult.model_construct(tools=[], ttl_ms=10)
print(r.model_fields_set) # {'tools', 'ttl_ms'}
filled = apply_cache_hint(r, CacheHint(ttl_ms=60_000, scope="public"))
print(filled.ttl_ms) # 10 — the handler's explicit value survives
print(filled.cache_scope) # "public" — only the unset field takes the hintStep by step: (1) model_construct(tools=[], ttl_ms=10) is called with no _fields_set; (2) pydantic populates __pydantic_fields_set__ = {'tools', 'ttl_ms'} from the passed keys; (3) apply_cache_hint checks "ttl_ms" not in result.model_fields_set, which is False, so it leaves ttl_ms=10 alone and only fills cache_scope. That is identical to the behavior for a normally-constructed ListToolsResult(tools=[], ttl_ms=10) — there is no "bypass".
Why it matters. The runtime code is correct and consistent with the documented precedence model (explicit beats configured, per field), so this is not a behavioral bug. But the caveat could mislead lowlevel-handler authors into believing that using model_construct causes their explicitly-set ttl_ms/cache_scope to be clobbered by the server-wide cache_hints map, prompting unnecessary workarounds (or avoidance of model_construct altogether). The only scenario the sentence accurately describes is a handler that passes _fields_set=set() explicitly, or one that omits the fields entirely — and the latter is true of normal construction too.
How to fix. Either drop the sentence, or reword it to describe the actual edge case, e.g.: "A handler that calls model_construct with an explicit _fields_set that omits these fields is treated as having set nothing for them; by default model_construct tracks the fields actually passed, so explicit values still win."
All three verifiers independently confirmed the factual claim empirically against pydantic 2.12.5 in this repo; none refuted it. Docstring-only, non-blocking.
2 participants
Adds a
cache_hintsconstructor option toServerandMCPServerso server operators can set the SEP-2549 caching hints (ttlMs/cacheScope) emitted on the six cacheable results.Motivation and Context
The 2026-07-28 schema requires
ttlMs/cacheScopeon the results oftools/list,prompts/list,resources/list,resources/templates/list,resources/read, andserver/discover. The models default them to0/"private"(immediately stale, never shared) — wire-valid and safe, but until now there was no way for anMCPServeroperator to declare anything else: every high-level server told every client "don't cache this", regardless of intent. Lowlevel handlers could set the fields on the results they return, but only per handler.This adds the server-wide knob:
Design notes:
Server(next toinstructions=);MCPServerforwards it. One mechanism covers all six methods, includingserver/discover, whose default handler is lowlevel-owned.ServerRunnerfills the typed result after the handler returns and before serialization, so the existing version sieve still strips the fields for pre-2026 peers, andresultType: "input_required"results are never stamped (they aren'tCacheableResultmodels).model_fields_set: a field the handler set explicitly — even to the default value — always wins over the configured hint. A configured hint can never clobber a handler's explicitcache_scope.CacheableMethodLiteral(the analogue of the TypeScript SDK'sPartial<Record<CacheableResultMethod, CacheHint>>), so editors autocomplete them and type checkers flag typos. Runtime validation still rejects unknown keys (ValueError) and non-CacheHintvalues (TypeError) at construction for untyped callers.cacheScopereaches every page of a paginated list by construction: the map is keyed by method, not cursor.How Has This Been Tested?
tests/server/test_caching.py: hint validation, per-field precedence (including explicit-default-wins), wire round-trips on both server tiers, discover, and per-page scope consistency.tests/server/test_runner.pyproving configured hints never reach a 2025-11-25 peer.docs/advanced/caching.mdwith runnabledocs_src/caching/examples pinned bytests/docs_src/test_caching.py.cachingscenario (CI) covers the emitted-hints requirements.Breaking Changes
None — new optional keyword argument; defaults unchanged.
Types of changes
Checklist
Additional context
Part of #2899. Client-side cache honoring (a freshness cache that uses these hints) and a per-registration
@mcp.resource(..., cache_hint=...)override are intentionally out of scope for this PR.AI Disclaimer