Skip to content

Commit d1cebd1

Browse files
committed
Apply configured cache hints to mapping handler results and fix unknown-key error formatting
1 parent efac31f commit d1cebd1

4 files changed

Lines changed: 98 additions & 9 deletions

File tree

src/mcp/server/caching.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ def validate_cache_hints(cache_hints: Mapping[Any, Any] | None) -> dict[str, Cac
7373
"""
7474
if cache_hints is None:
7575
return {}
76-
unknown = sorted(method for method in cache_hints if method not in CACHEABLE_METHODS)
76+
# Keys come from an untyped mapping, so format via repr: a non-string key
77+
# must produce this ValueError too, not a TypeError from sorted/join.
78+
unknown = sorted(repr(method) for method in cache_hints if method not in CACHEABLE_METHODS)
7779
if unknown:
7880
raise ValueError(f"cache_hints keys must be cacheable methods (see CacheableMethod); got: {', '.join(unknown)}")
7981
validated: dict[str, CacheHint] = {}

src/mcp/server/runner.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,19 +198,27 @@ async def _inner(ctx: ServerRequestContext[LifespanT, Any]) -> HandlerResult:
198198
if isinstance(result, ErrorData):
199199
# Raise inside the chain so middleware observes the failure.
200200
raise MCPError.from_error_data(result)
201-
# Fill cache hints on the typed result, before the serialize sieve
201+
# Fill cache hints on the handler result, before the serialize sieve
202202
# decides whether the negotiated version carries the fields at all.
203-
# `input_required` interim results are not `CacheableResult` models,
204-
# so the MRTR carve-out (no hints on them) holds by shape.
205-
if isinstance(result, CacheableResult) and (hint := self.server.cache_hints.get(method)) is not None:
206-
result = apply_cache_hint(result, hint)
203+
# `input_required` interim results are not `CacheableResult` models
204+
# and mapping results declaring that shape are skipped explicitly,
205+
# so the MRTR carve-out (no hints on them) holds on both paths.
206+
if (hint := self.server.cache_hints.get(method)) is not None:
207+
if isinstance(result, CacheableResult):
208+
result = apply_cache_hint(result, hint)
209+
elif isinstance(result, Mapping) and result.get("resultType") != "input_required":
210+
# Same per-field precedence as `apply_cache_hint`: wire keys the
211+
# handler put in the mapping win. Fresh dict, so a mapping the
212+
# handler may still hold an alias to is never mutated.
213+
result = {"ttlMs": hint.ttl_ms, "cacheScope": hint.scope, **result}
207214
# Dump and serialize inside the chain so the OpenTelemetry span (the
208215
# outermost middleware) records a failing handler return shape too.
209216
return self._serialize(method, version, result)
210217

211218
call = self._compose_server_middleware(_inner)
212219
# `_inner` already produced the wire dict; a middleware that short-circuited
213-
# without `call_next` is trusted to return its own well-formed result.
220+
# without `call_next` is trusted to return its own well-formed result,
221+
# configured cache hints included.
214222
result = _dump_result(await call(ctx))
215223
if method == "initialize":
216224
# Commit only on chain success, so a middleware veto leaves no state.

tests/docs_src/test_caching.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ async def test_a_non_cacheable_method_is_rejected_at_construction() -> None:
4848
with pytest.raises(ValueError) as exc:
4949
MCPServer("Weather", cache_hints=cast(Any, {"tools/call": CacheHint(ttl_ms=1_000)}))
5050
assert str(exc.value) == snapshot(
51-
"cache_hints keys must be cacheable methods (see CacheableMethod); got: tools/call"
51+
"cache_hints keys must be cacheable methods (see CacheableMethod); got: 'tools/call'"
5252
)
5353

5454

tests/server/test_caching.py

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
import pytest
77
from inline_snapshot import snapshot
88
from mcp_types import (
9+
InputRequiredResult,
910
ListResourcesResult,
1011
ListToolsResult,
1112
PaginatedRequestParams,
13+
ReadResourceRequestParams,
1214
Resource,
1315
Tool,
1416
)
@@ -68,7 +70,7 @@ def test_a_non_cacheable_method_in_cache_hints_is_rejected_at_server_constructio
6870
with pytest.raises(ValueError) as exc:
6971
Server("srv", cache_hints=cast(Any, {"tools/call": CacheHint()}))
7072
assert str(exc.value) == snapshot(
71-
"cache_hints keys must be cacheable methods (see CacheableMethod); got: tools/call"
73+
"cache_hints keys must be cacheable methods (see CacheableMethod); got: 'tools/call'"
7274
)
7375

7476

@@ -81,6 +83,83 @@ def test_a_non_cache_hint_value_is_rejected_at_server_construction() -> None:
8183
assert str(exc.value) == snapshot("cache_hints['tools/list'] must be a CacheHint, got dict")
8284

8385

86+
def test_a_non_string_cache_hints_key_is_rejected_with_the_unknown_key_error() -> None:
87+
"""SDK-defined: `cache_hints` is deliberately loose for config-shaped callers,
88+
so a non-string key takes the same unknown-key ValueError as a typo - not a
89+
TypeError from formatting the message."""
90+
with pytest.raises(ValueError) as exc:
91+
Server("srv", cache_hints=cast(Any, {42: CacheHint()}))
92+
assert str(exc.value) == snapshot("cache_hints keys must be cacheable methods (see CacheableMethod); got: 42")
93+
94+
95+
async def test_a_dict_returning_handler_takes_the_configured_hint() -> None:
96+
"""SDK-defined: the construction-time hint also stamps a handler that returns
97+
a raw dict for a cacheable method, so the 2026-07-28 surface (where both
98+
fields are required) accepts it and the wire carries the hint's values."""
99+
hint = CacheHint(ttl_ms=60_000, scope="public")
100+
101+
async def list_tools(ctx: ServerRequestContext[Any], params: PaginatedRequestParams) -> dict[str, Any]:
102+
return {"tools": [], "resultType": "complete"}
103+
104+
server = Server("srv", cache_hints={"tools/list": hint})
105+
server.add_request_handler("tools/list", PaginatedRequestParams, list_tools)
106+
async with Client(server) as client:
107+
result = await client.list_tools()
108+
assert result.ttl_ms == hint.ttl_ms
109+
assert result.cache_scope == hint.scope
110+
111+
112+
async def test_a_dict_provided_ttl_wins_and_the_hint_fills_only_the_missing_scope() -> None:
113+
"""SDK-defined precedence, dict path: wire keys the handler put in the dict
114+
win, mirroring `model_fields_set` semantics on the model path - the hint
115+
fills only the absent `cacheScope`."""
116+
117+
async def list_tools(ctx: ServerRequestContext[Any], params: PaginatedRequestParams) -> dict[str, Any]:
118+
return {"tools": [], "resultType": "complete", "ttlMs": 25}
119+
120+
server = Server("srv", cache_hints={"tools/list": CacheHint(ttl_ms=60_000, scope="public")})
121+
server.add_request_handler("tools/list", PaginatedRequestParams, list_tools)
122+
async with Client(server) as client:
123+
result = await client.list_tools()
124+
assert result.ttl_ms == 25
125+
assert result.cache_scope == "public"
126+
127+
128+
async def test_a_dict_returning_handler_leaks_no_hint_fields_to_a_2025_session() -> None:
129+
"""SDK-defined era gate: the stamp runs version-independently, but the 2025
130+
serialize sieve still strips `ttlMs`/`cacheScope` from a dict result - the
131+
client model parses them as unset, not as wire values."""
132+
133+
async def list_tools(ctx: ServerRequestContext[Any], params: PaginatedRequestParams) -> dict[str, Any]:
134+
return {"tools": []}
135+
136+
server = Server("srv", cache_hints={"tools/list": CacheHint(ttl_ms=60_000, scope="public")})
137+
server.add_request_handler("tools/list", PaginatedRequestParams, list_tools)
138+
async with Client(server, mode="legacy") as client:
139+
result = await client.list_tools()
140+
assert "ttl_ms" not in result.model_fields_set
141+
assert "cache_scope" not in result.model_fields_set
142+
143+
144+
async def test_an_input_required_shaped_dict_is_never_stamped() -> None:
145+
"""Spec-mandated MRTR carve-out: an interim `input_required` result carries no
146+
cache hints even on a hinted cacheable method. The runner's stamp skips a
147+
dict declaring that shape (and the serialize surface would drop stray hint
148+
keys regardless), so the full dump is exactly what the handler returned."""
149+
150+
async def read_resource(ctx: ServerRequestContext[Any], params: ReadResourceRequestParams) -> dict[str, Any]:
151+
return {"resultType": "input_required", "requestState": "s1"}
152+
153+
server = Server("srv", cache_hints={"resources/read": CacheHint(ttl_ms=60_000, scope="public")})
154+
server.add_request_handler("resources/read", ReadResourceRequestParams, read_resource)
155+
async with Client(server) as client:
156+
result = await client.session.read_resource("res://x", allow_input_required=True)
157+
assert isinstance(result, InputRequiredResult)
158+
assert result.model_dump(by_alias=True, exclude_none=True) == snapshot(
159+
{"resultType": "input_required", "requestState": "s1"}
160+
)
161+
162+
84163
async def test_server_cache_hints_reach_the_wire_for_a_bare_handler_result() -> None:
85164
"""SDK-defined: a lowlevel handler that never thinks about caching emits the
86165
server-wide hint configured at construction."""

0 commit comments

Comments
 (0)