Skip to content

Commit 1bbc469

Browse files
committed
Own the response envelope in one outbound pass
Cache-hint filling and the serverInfo stamp were two separate if-blocks in ServerRunner._on_request, one of them applied after the middleware chain by patching the returned dict. _serialize now owns the whole outbound envelope (hints, dump, per-version sieve, serverInfo stamp) as the counterpart of the inbound classification ladder, and _dump_result copies dict results so a handler-retained object can never be mutated by wire shaping. Behavior change: a middleware that short-circuits without call_next now owns its result envelope entirely, per the existing trust contract; it is no longer stamped after the fact.
1 parent 8eb8d70 commit 1bbc469

2 files changed

Lines changed: 61 additions & 51 deletions

File tree

src/mcp/server/runner.py

Lines changed: 56 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,10 @@ def _dump_result(result: Any) -> dict[str, Any]:
112112
if isinstance(result, BaseModel):
113113
return result.model_dump(by_alias=True, mode="json", exclude_none=True)
114114
if isinstance(result, dict):
115-
return cast(dict[str, Any], result)
115+
# Copied so callers own the returned dict: handlers and middleware may
116+
# retain the object they returned, and the outbound pipeline shapes the
117+
# wire form without reaching into anything the handler still holds.
118+
return dict(cast(dict[str, Any], result))
116119
raise TypeError(f"handler returned {type(result).__name__}; expected BaseModel, dict, or None")
117120

118121

@@ -210,38 +213,16 @@ async def _inner(ctx: ServerRequestContext[LifespanT, Any]) -> HandlerResult:
210213
if isinstance(result, ErrorData):
211214
# Raise inside the chain so middleware observes the failure.
212215
raise MCPError.from_error_data(result)
213-
# Fill cache hints on the handler result, before the serialize sieve
214-
# decides whether the negotiated version carries the fields at all.
215-
# MRTR carve-out: `input_required` interim results, typed or mapping, never get hints.
216-
if (hint := self.server.cache_hints.get(method)) is not None:
217-
if isinstance(result, CacheableResult):
218-
result = apply_cache_hint(result, hint)
219-
elif isinstance(result, Mapping) and not _methods.is_input_required(result):
220-
# Hint keys first so wire keys the handler set win, matching `apply_cache_hint` precedence.
221-
result = {"ttlMs": hint.ttl_ms, "cacheScope": hint.scope, **result}
222-
# Dump and serialize inside the chain so the OpenTelemetry span (the
216+
# Shape for the wire inside the chain so the OpenTelemetry span (the
223217
# outermost middleware) records a failing handler return shape too.
224218
return self._serialize(method, version, result)
225219

226220
call = self._compose_server_middleware(_inner)
227221
# `_inner` already produced the wire dict; a middleware that short-circuited
228-
# without `call_next` is trusted to return its own well-formed result.
222+
# without `call_next` is trusted to return its own well-formed result -
223+
# including its response envelope. The pipeline never patches it up after
224+
# the fact.
229225
result = _dump_result(await call(ctx))
230-
# Spec 2026-07-28 (#3002): stamp `serverInfo` into every modern result's
231-
# `_meta`. This runs after the middleware chain - the one exit every
232-
# result takes, including middleware short-circuits - so coverage holds
233-
# by construction. Custom-method and short-circuit results pass through
234-
# by reference, so stamping builds a shallow copy rather than mutating a
235-
# dict the handler may retain. A handler-authored value wins; a
236-
# non-mapping `_meta` is the handler's to own, not ours to clobber.
237-
if self.server.include_server_info and version in MODERN_PROTOCOL_VERSIONS:
238-
raw_meta = result.get("_meta")
239-
if raw_meta is None:
240-
result = {**result, "_meta": {SERVER_INFO_META_KEY: dict(self.server.server_info_stamp)}}
241-
elif isinstance(raw_meta, dict):
242-
meta = cast("dict[str, Any]", raw_meta)
243-
if meta.get(SERVER_INFO_META_KEY) is None:
244-
result = {**result, "_meta": {**meta, SERVER_INFO_META_KEY: dict(self.server.server_info_stamp)}}
245226
if method == "initialize":
246227
# Commit only on chain success, so a middleware veto leaves no state.
247228
# Race-free: the read loop is parked until this call returns.
@@ -349,27 +330,59 @@ def _make_context(
349330
close_standalone_sse_stream=close_standalone_sse_stream,
350331
)
351332

352-
@staticmethod
353-
def _serialize(method: str, version: str, result: HandlerResult) -> dict[str, Any]:
354-
"""Dump a handler result to the wire dict, serializing spec methods.
355-
356-
Runs inside the middleware chain so the OpenTelemetry span observes a
357-
failing return shape (unsupported type, malformed spec result) as an
358-
error rather than closing on a request that the client sees fail.
333+
def _serialize(self, method: str, version: str, result: HandlerResult) -> dict[str, Any]:
334+
"""Shape a handler result into its wire form: the outbound counterpart
335+
of the inbound classification ladder.
336+
337+
One pass owns the whole response envelope, in order: cache hints fill
338+
`ttlMs`/`cacheScope` the handler left unset, spec-method results are
339+
validated and sieved by the per-version surface, and 2026-era results
340+
get the `serverInfo` `_meta` stamp (spec #3002). Runs inside the
341+
middleware chain so the OpenTelemetry span observes a failing return
342+
shape (unsupported type, malformed spec result) as an error rather
343+
than closing on a request that the client sees fail - and so a
344+
middleware that short-circuits without `call_next` owns its result,
345+
envelope included.
359346
"""
347+
# MRTR carve-out: `input_required` interim results, typed or mapping, never get hints.
348+
if (hint := self.server.cache_hints.get(method)) is not None:
349+
if isinstance(result, CacheableResult):
350+
result = apply_cache_hint(result, hint)
351+
elif isinstance(result, Mapping) and not _methods.is_input_required(result):
352+
# Hint keys first so wire keys the handler set win, matching `apply_cache_hint` precedence.
353+
result = {"ttlMs": hint.ttl_ms, "cacheScope": hint.scope, **result}
360354
dumped = _dump_result(result)
361355
# TODO(L56): reject resultType values outside {"complete", "input_required"} unless the
362356
# corresponding extension is in this request's _meta clientCapabilities.extensions; the
363357
# explicit MUST-reject is client-side (basic/index.mdx ResultType), this enforces it proactively.
364-
if method not in _methods.SPEC_CLIENT_METHODS:
365-
return dumped
366-
try:
367-
return _methods.serialize_server_result(method, version, dumped)
368-
except ValidationError:
369-
# Server bug, not client fault. Detail stays in the server log:
370-
# pydantic messages echo the result body.
371-
logger.exception("handler for %r returned an invalid result", method)
372-
raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None
358+
if method in _methods.SPEC_CLIENT_METHODS:
359+
try:
360+
dumped = _methods.serialize_server_result(method, version, dumped)
361+
except ValidationError:
362+
# Server bug, not client fault. Detail stays in the server log:
363+
# pydantic messages echo the result body.
364+
logger.exception("handler for %r returned an invalid result", method)
365+
raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None
366+
return self._stamp_server_info(version, dumped)
367+
368+
def _stamp_server_info(self, version: str, result: dict[str, Any]) -> dict[str, Any]:
369+
"""Fill the `serverInfo` `_meta` stamp on a 2026-era result (spec #3002).
370+
371+
A handler-authored value wins, a non-mapping `_meta` is the handler's
372+
to own, and handshake-era results are never stamped. `result` is
373+
pipeline-owned (`_dump_result` copies), but `_meta` may still be the
374+
handler's object, so the stamp replaces it rather than writing into it.
375+
"""
376+
if not self.server.include_server_info or version not in MODERN_PROTOCOL_VERSIONS:
377+
return result
378+
raw_meta = result.get("_meta")
379+
if raw_meta is None:
380+
result["_meta"] = {SERVER_INFO_META_KEY: dict(self.server.server_info_stamp)}
381+
elif isinstance(raw_meta, dict):
382+
meta = cast("dict[str, Any]", raw_meta)
383+
if meta.get(SERVER_INFO_META_KEY) is None:
384+
result["_meta"] = {**meta, SERVER_INFO_META_KEY: dict(self.server.server_info_stamp)}
385+
return result
373386

374387
@staticmethod
375388
def _negotiate_initialize(params: Mapping[str, Any] | None) -> tuple[InitializeRequestParams, str]:

tests/server/test_runner.py

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -995,10 +995,10 @@ async def custom(ctx: Ctx, params: RequestParams) -> dict[str, Any]:
995995

996996

997997
@pytest.mark.anyio
998-
async def test_modern_short_circuit_middleware_result_still_carries_the_server_info_stamp(server: SrvT):
999-
"""SDK-defined: the serverInfo stamp is applied at the runner's single exit,
1000-
after the middleware chain, so even a middleware that answers without
1001-
calling `call_next` produces an identified result (spec 2026-07-28, #3002)."""
998+
async def test_modern_short_circuit_middleware_owns_its_result_envelope(server: SrvT):
999+
"""SDK-defined: a middleware that answers without calling `call_next` is
1000+
trusted to return its own well-formed result, response envelope included -
1001+
the outbound pipeline (and its serverInfo stamp) never patches it up."""
10021002

10031003
async def short_circuit(ctx: Ctx, call_next: Any) -> Any:
10041004
return {"ok": True}
@@ -1007,10 +1007,7 @@ async def short_circuit(ctx: Ctx, call_next: Any) -> Any:
10071007
born_ready = Connection.from_envelope(LATEST_MODERN_VERSION, None, None)
10081008
async with connected_runner(server, initialized=False, connection=born_ready) as (client, _):
10091009
result = await client.send_raw_request("myorg/anything", None)
1010-
assert result == {
1011-
"ok": True,
1012-
"_meta": {SERVER_INFO_META_KEY: {"name": "test-server", "version": "0.0.1"}},
1013-
}
1010+
assert result == {"ok": True}
10141011

10151012

10161013
@pytest.mark.anyio

0 commit comments

Comments
 (0)