Skip to content

Commit db12e6c

Browse files
committed
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 b5d8d1e commit db12e6c

4 files changed

Lines changed: 250 additions & 44 deletions

File tree

docs/migration.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1515,7 +1515,7 @@ async def delete_folder(
15151515

15161516
The `confirm_delete` resolver reads the tool's own `path` argument by name, lists the folder, and only elicits when the folder is non-empty - an empty folder resolves to `Confirm(ok=True)` with no round-trip to the client. Because `delete_folder` annotates the result union, it handles every outcome: the user accepting and confirming, accepting but declining to delete (`ok=False`), declining the elicitation, or cancelling it.
15171517

1518-
The framework drives elicitation over whichever transport the negotiated protocol provides, so the resolver and tool code above is unchanged either way. At `2026-07-28` and later it returns an `InputRequiredResult` carrying the questions and resumes when the client retries `call_tool(..., input_responses=..., request_state=...)` (independent resolvers are batched into one round; a resolver that depends on another's answer is asked in a later round). At `2025-11-25` and earlier it issues a synchronous `elicitation/create` request mid-call. Resolved outcomes are carried in `request_state` across rounds so each resolver resolves once per call.
1518+
The framework drives elicitation over whichever transport the negotiated protocol provides, so the resolver and tool code above is unchanged either way. At `2026-07-28` and later it returns an `InputRequiredResult` carrying the questions and resumes when the client retries `call_tool(..., input_responses=..., request_state=...)` (independent resolvers are batched into one round; a resolver that depends on another's answer is asked in a later round). At `2025-11-25` and earlier it issues a synchronous `elicitation/create` request mid-call. Elicited answers are carried in `request_state` across rounds, so each question is asked once; a resolver that resolves without eliciting is pure and may re-run each round.
15191519

15201520
Resolved parameters are omitted from the tool's input schema, so the client never supplies them. Resolver parameters that cannot be classified, and cyclic resolver dependencies, raise at registration time.
15211521

src/mcp/server/mcpserver/context.py

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

241+
@property
242+
def protocol_version(self) -> str | None:
243+
"""The negotiated protocol version, or `None` outside of an active request."""
244+
return self._request_context.protocol_version if self._request_context is not None else None
245+
241246
@property
242247
def input_responses(self) -> InputResponses | None:
243248
"""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)