Validate resolver tool args once; key resolvers by method identity

Review follow-ups on #2969:
- Tool.run validated arguments twice when resolvers were present (once to feed
  resolvers, once in call_fn_with_arg_validation). A field with default_factory
  or a stateful validator could hand a by-name resolver a different value than
  the tool body. Validate once and pass it through via a new pre_validated
  argument so both observe the same value.
- Key the resolver cache/plans by (id(__func__), id(__self__)) for bound methods
  and id(fn) otherwise, instead of the callable's equality, so two distinct
  callables that compare equal can no longer share a plan/cache entry while
  bound-method memoization still works.

Add regression tests.

Author: Kludex

src/mcp/server/mcpserver/resolve.py

@@ -132,17 +132,16 @@ def _wants_union(type_arg: Any) -> bool:
 
 
 def _resolver_key(fn: Callable[..., Any]) -> Hashable:
-    """Stable, equality-based key for memoizing a resolver.
+    """Identity key for memoizing a resolver.
 
-    Bound methods are recreated on each attribute access (`id(auth.login)` differs
-    every time) but hash/compare by `(__func__, __self__)`, so the callable itself
-    is the right key. Falls back to `id` only for the rare unhashable callable.
+    A bound method is recreated on each attribute access (`id(auth.login)` differs
+    every time), so key it by `(id(__func__), id(__self__))` to keep `auth.login`
+    referenced in two places memoized to one call. Everything else keys by `id`,
+    so two distinct callables never collide even if they compare equal.
     """
-    try:
-        hash(fn)
-    except TypeError:  # pragma: no cover - unhashable callables are pathological
-        return id(fn)
-    return fn
+    if inspect.ismethod(fn):
+        return (id(fn.__func__), id(fn.__self__))
+    return id(fn)
 
 
 def build_resolver_plans(

src/mcp/server/mcpserver/tools/base.py

@@ -128,15 +128,23 @@ async def run(
             pass_directly: dict[str, Any] = {}
             if self.context_kwarg is not None:
                 pass_directly[self.context_kwarg] = context
+
+            # Resolvers see the same validated arguments the tool body receives:
+            # validate once and reuse it, so a `default_factory`/stateful validator
+            # can't hand a by-name resolver a different value than the body.
+            pre_validated: dict[str, Any] | None = None
             if self.resolved_params:
-                tool_args = self.fn_metadata.validate_arguments(arguments)
-                pass_directly |= await resolve_arguments(self.resolved_params, self.resolver_plans, tool_args, context)
+                pre_validated = self.fn_metadata.validate_arguments(arguments)
+                pass_directly |= await resolve_arguments(
+                    self.resolved_params, self.resolver_plans, pre_validated, context
+                )
 
             result = await self.fn_metadata.call_fn_with_arg_validation(
                 self.fn,
                 self.is_async,
                 arguments,
                 pass_directly or None,
+                pre_validated=pre_validated,
             )
 
             if convert_result:

src/mcp/server/mcpserver/utilities/func_metadata.py

@@ -81,13 +81,19 @@ async def call_fn_with_arg_validation(
         fn_is_async: bool,
         arguments_to_validate: dict[str, Any],
         arguments_to_pass_directly: dict[str, Any] | None,
+        pre_validated: dict[str, Any] | None = None,
     ) -> Any:
         """Call the given function with arguments validated and injected.
 
         Arguments are first attempted to be parsed from JSON, then validated against
-        the argument model, before being passed to the function.
+        the argument model, before being passed to the function. Pass `pre_validated`
+        (the output of `validate_arguments`) to reuse an earlier validation pass -
+        validating twice can re-run `default_factory`/stateful validators and hand the
+        function different values than a caller already observed.
         """
-        arguments_parsed_dict = self.validate_arguments(arguments_to_validate)
+        arguments_parsed_dict = (
+            pre_validated if pre_validated is not None else self.validate_arguments(arguments_to_validate)
+        )
 
         arguments_parsed_dict |= arguments_to_pass_directly or {}
 

tests/server/mcpserver/test_resolve.py

@@ -403,3 +403,32 @@ async def tool(value: Annotated[Login, Resolve(service.a)]) -> str:
 
     with pytest.raises(InvalidSignature, match="cyclic"):
         Tool.from_function(tool)
+
+
+@pytest.mark.anyio
+async def test_resolver_and_body_see_the_same_validated_default():
+    mcp = MCPServer(name="DefaultFactory")
+    counter = {"n": 0}
+
+    def next_id() -> int:
+        counter["n"] += 1
+        return counter["n"]
+
+    # A by-name resolver and the tool body must observe one validation pass, so the
+    # `default_factory` runs once and both see the same generated value.
+    async def echo_id(request_id: int) -> int:
+        return request_id
+
+    @mcp.tool()
+    async def run(
+        request_id: Annotated[int, Field(default_factory=next_id)],
+        resolved_id: Annotated[int, Resolve(echo_id)],
+    ) -> str:
+        return f"{request_id}:{resolved_id}"
+
+    async def never(context: ClientRequestContext, params: ElicitRequestParams) -> ElicitResult:  # pragma: no cover
+        raise AssertionError("should not elicit")
+
+    async with Client(mcp, mode="legacy", elicitation_callback=never) as client:
+        assert await _text(client, "run", {}) == "1:1"
+    assert counter["n"] == 1