Skip to content

Commit 31ccb80

Browse files
Kludexmaxisbey
authored andcommitted
Fix resolver MRTR edge cases from review
- In-call cache keyed by _resolver_key (instance-distinct) again; _state_key adds id(__self__) for the wire key, so two instances of one bound method no longer collide and silently share an outcome. - resolve_arguments reads ctx.protocol_version (new Context property, None outside a request) instead of dereferencing request_context, so direct MCPServer.call_tool() works for tools whose resolvers never elicit. - request_state persists only elicited outcomes (always validated models); a resolver that resolves without eliciting is pure and re-runs each round. Fixes the json.dumps crash on non-serializable returns (datetime/set/...) and the dict-degradation of restored values. - _elicit_return_schema handles a bare Elicit[T] return (not only unions). - _INPUT_REQUIRED_VERSION pinned to '2026-07-28' instead of LATEST_MODERN_VERSION. - accept with no content raises ToolError instead of silently reporting cancel. - Independent nested resolver deps batch into one round (catch _Pending per dep). - Test cleanup: drop dead helpers, hoist CreateMessageResult import. Add regression tests for each; document the narrowed elicited-only persistence.
1 parent 4cebc58 commit 31ccb80

3 files changed

Lines changed: 249 additions & 43 deletions

File tree

src/mcp/server/mcpserver/context.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,11 @@ def request_id(self) -> str:
232232
"""Get the unique ID for this request."""
233233
return str(self.request_context.request_id)
234234

235+
@property
236+
def protocol_version(self) -> str | None:
237+
"""The negotiated protocol version, or `None` outside of an active request."""
238+
return self._request_context.protocol_version if self._request_context is not None else None
239+
235240
@property
236241
def input_responses(self) -> InputResponses | None:
237242
"""Client responses to a prior `InputRequiredResult.input_requests`.

src/mcp/server/mcpserver/resolve.py

Lines changed: 68 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@
1212
and resumes when the client retries with `input_responses`/`request_state`
1313
(independent resolvers are asked in one round; a resolver depending on another's
1414
answer is asked in a later round). At <= 2025-11-25 it issues a synchronous
15-
`elicitation/create` request mid-call. Resolved outcomes are carried in
16-
`request_state` across rounds, so each resolver resolves once per logical call.
15+
`elicitation/create` request mid-call. Only *elicited* outcomes are carried in
16+
`request_state` across rounds (so the user is asked each question once); a
17+
resolver that returns a value without eliciting is pure and may re-run each round.
1718
1819
Whether the consumer receives the unwrapped model or the full
1920
`ElicitationResult` union is decided by the consumer's annotation:
@@ -26,6 +27,7 @@
2627
from __future__ import annotations
2728

2829
import inspect
30+
import types
2931
import typing
3032
from collections.abc import Callable, Hashable, Mapping
3133
from typing import Annotated, Any, Generic, Literal, TypeGuard, get_args, get_origin
@@ -39,7 +41,7 @@
3941
InputRequiredResult,
4042
InputResponses,
4143
)
42-
from mcp_types.version import LATEST_MODERN_VERSION, is_version_at_least
44+
from mcp_types.version import is_version_at_least
4345
from pydantic import BaseModel, ValidationError
4446
from typing_extensions import TypeVar
4547

@@ -61,7 +63,8 @@
6163

6264
# First protocol revision whose `tools/call` carries elicitation inside
6365
# `InputRequiredResult` rather than as a standalone server-to-client request.
64-
_INPUT_REQUIRED_VERSION = LATEST_MODERN_VERSION # "2026-07-28"
66+
# Pinned (not `LATEST_MODERN_VERSION`, which moves when newer revisions are added).
67+
_INPUT_REQUIRED_VERSION = "2026-07-28"
6568
_STATE_VERSION = 1
6669

6770

@@ -158,10 +161,12 @@ def find_resolved_parameters(fn: Callable[..., Any]) -> dict[str, tuple[Resolve,
158161
def _elicit_return_schema(return_annotation: Any) -> type[BaseModel] | None:
159162
"""Extract `T` from a resolver return type's `Elicit[T]` arm, if present.
160163
161-
Lets an outcome restored from `request_state` (a plain dict) be re-validated
162-
into its model so dependent resolvers and tools receive a typed value.
164+
Handles a bare `-> Elicit[T]` and a `-> T | Elicit[T]` union. Lets an elicited
165+
outcome restored from `request_state` (a plain dict) be re-validated into its
166+
model so dependent resolvers and tools receive a typed value.
163167
"""
164-
candidates = get_args(return_annotation) if get_origin(return_annotation) is not None else (return_annotation,)
168+
# A bare `Elicit[T]` is itself a candidate; a union contributes its members.
169+
candidates = get_args(return_annotation) if _is_union(return_annotation) else (return_annotation,)
165170
for candidate in candidates:
166171
if get_origin(candidate) is Elicit:
167172
schema = get_args(candidate)[0]
@@ -170,6 +175,10 @@ def _elicit_return_schema(return_annotation: Any) -> type[BaseModel] | None:
170175
return None
171176

172177

178+
def _is_union(annotation: Any) -> bool:
179+
return get_origin(annotation) in (typing.Union, types.UnionType)
180+
181+
173182
def _wants_union(type_arg: Any) -> bool:
174183
"""True when `type_arg` is an `ElicitationResult` member (or a union of them).
175184
@@ -296,19 +305,26 @@ def __init__(
296305
self.input_required = input_required
297306
self.answers: InputResponses = context.input_responses or {} if input_required else {}
298307
self.state = _decode_state(context.request_state) if input_required else {}
299-
self.cache: dict[str, ElicitationResult[Any]] = {}
308+
# In-call dedup keyed by resolver identity (distinguishes two instances of
309+
# the same bound method); `elicited` holds only outcomes that came from an
310+
# elicitation, keyed by their wire key - these are what `request_state`
311+
# persists, since pure resolvers are cheap to re-run each round.
312+
self.cache: dict[Hashable, ElicitationResult[Any]] = {}
313+
self.elicited: dict[str, ElicitationResult[Any]] = {}
300314
self.pending: InputRequests = {}
301315

302316

303317
def _state_key(fn: Callable[..., Any]) -> str:
304-
"""Process-stable wire key for a resolver.
318+
"""Process-stable wire key for a resolver's elicitation.
305319
306-
`id`-based keys aren't stable across `input_required` rounds (a retry may land
307-
on a different worker), so memoize and key `input_requests`/`request_state` by
308-
the resolver's `module:qualname`. Two consumers of the same resolver therefore
309-
share one cache entry, one question, and one stored outcome.
320+
`id(fn)` isn't stable across `input_required` rounds, so key `input_requests` /
321+
`request_state` by `module:qualname`. Bound methods add their `__self__` id so
322+
two instances of the same method get distinct questions and stored outcomes
323+
(the registered `Resolve(...)` holds the instance for the call's lifetime).
310324
"""
311-
return f"{getattr(fn, '__module__', '')}:{getattr(fn, '__qualname__', fn)}"
325+
base = f"{getattr(fn, '__module__', '')}:{getattr(fn, '__qualname__', fn)!s}"
326+
bound_self = getattr(fn, "__self__", None)
327+
return f"{base}#{id(bound_self)}" if bound_self is not None else base
312328

313329

314330
async def resolve_arguments(
@@ -324,14 +340,19 @@ async def resolve_arguments(
324340
negotiated protocol is >= 2026-07-28), returns an `InputRequiredResult`
325341
carrying the batched questions instead; the tool body is not run.
326342
327-
Each resolver runs at most once per logical call - across multiple
328-
`input_required` rounds, resolved outcomes are carried in `request_state`.
343+
An eliciting resolver asks its question once - its answer is carried in
344+
`request_state` across rounds - while a resolver that resolves without
345+
eliciting is pure and may re-run on each round.
329346
330347
Raises:
331348
ToolError: If an elicited value is declined or cancelled and the consumer
332349
asked for the unwrapped model (rather than the result union).
333350
"""
334-
res = _Resolution(plans, tool_args, context, uses_input_required(context.request_context.protocol_version))
351+
# `ctx.protocol_version` is `None` outside an active request: `MCPServer.call_tool()`
352+
# called directly builds such a `Context`, and a tool whose resolvers never elicit
353+
# must still work there. A missing version means the synchronous (non-input_required)
354+
# transport, which never reaches a server-to-client request anyway.
355+
res = _Resolution(plans, tool_args, context, uses_input_required(context.protocol_version))
335356
injected: dict[str, Any] = {}
336357
for name, (marker, wants_union) in resolved_params.items():
337358
try:
@@ -341,39 +362,49 @@ async def resolve_arguments(
341362
injected[name] = outcome if wants_union else _unwrap(outcome, name)
342363

343364
if res.pending:
344-
return InputRequiredResult(input_requests=res.pending, request_state=_encode_state(res.cache))
365+
return InputRequiredResult(input_requests=res.pending, request_state=_encode_state(res.elicited))
345366
return injected
346367

347368

348369
async def _resolve(fn: Callable[..., Any], res: _Resolution) -> ElicitationResult[Any]:
349-
"""Resolve one resolver, memoized by its process-stable state key.
370+
"""Resolve one resolver, deduped within the call by its resolver identity.
350371
351372
Raises `_Pending` when the resolver (or one of its dependencies) needs client
352373
input that has not arrived yet.
353374
"""
354-
key = _state_key(fn)
355-
if key in res.cache:
356-
return res.cache[key]
357-
if key in res.pending:
375+
cache_key = _resolver_key(fn)
376+
if cache_key in res.cache:
377+
return res.cache[cache_key]
378+
379+
plan = res.plans[cache_key]
380+
wire_key = _state_key(fn)
381+
if wire_key in res.pending:
358382
# Already asked this round by another consumer; don't run the resolver again.
359383
raise _Pending
360-
361-
plan = res.plans[_resolver_key(fn)]
362-
if key in res.state:
363-
outcome = _outcome_from_state(res.state[key], plan.elicit_schema)
364-
res.cache[key] = outcome
384+
if wire_key in res.state:
385+
outcome = _outcome_from_state(res.state[wire_key], plan.elicit_schema)
386+
res.cache[cache_key] = outcome
365387
return outcome
366388

367389
kwargs: dict[str, Any] = {}
390+
dep_pending = False
368391
for param_name, param_plan in plan.params.items():
369392
if param_plan.kind == "context":
370393
kwargs[param_name] = res.context
371394
elif param_plan.kind == "by_name":
372395
kwargs[param_name] = res.tool_args[param_name]
373396
else:
374397
assert param_plan.resolve is not None
375-
dep_outcome = await _resolve(param_plan.resolve.fn, res)
398+
try:
399+
# Visit every dependency so independent ones that need input are all
400+
# collected into `res.pending` and batched into a single round.
401+
dep_outcome = await _resolve(param_plan.resolve.fn, res)
402+
except _Pending:
403+
dep_pending = True
404+
continue
376405
kwargs[param_name] = dep_outcome if param_plan.wants_union else _unwrap(dep_outcome, param_name)
406+
if dep_pending:
407+
raise _Pending
377408

378409
result: Any
379410
if plan.is_async:
@@ -382,13 +413,15 @@ async def _resolve(fn: Callable[..., Any], res: _Resolution) -> ElicitationResul
382413
result = await anyio.to_thread.run_sync(lambda: fn(**kwargs))
383414

384415
if _is_elicit(result):
385-
outcome = await _elicit(result, key, res)
416+
outcome = await _elicit(result, wire_key, res)
417+
res.elicited[wire_key] = outcome
386418
else:
387419
# A resolver may return any type (not just `BaseModel`), so accept it as the
388-
# outcome without validating against the schema bound.
420+
# outcome without validating against the schema bound. Plain outcomes are not
421+
# persisted in `request_state`; the resolver re-runs next round instead.
389422
outcome = _accepted(result)
390423

391-
res.cache[key] = outcome
424+
res.cache[cache_key] = outcome
392425
return outcome
393426

394427

@@ -403,7 +436,9 @@ async def _elicit(elicit: Elicit[Any], key: str, res: _Resolution) -> Elicitatio
403436
raise _Pending
404437
if not isinstance(answer, ElicitResult):
405438
raise ToolError(f"Resolver {key!r} received a non-elicitation response")
406-
if answer.action == "accept" and answer.content is not None:
439+
if answer.action == "accept":
440+
if answer.content is None:
441+
raise ToolError(f"Resolver {key!r} received an accepted elicitation with no content")
407442
return AcceptedElicitation(data=elicit.schema.model_validate(answer.content))
408443
if answer.action == "decline":
409444
return DeclinedElicitation()

0 commit comments

Comments
 (0)