Memoize built-in bound-method resolvers; stop mutating pre_validated

Review follow-ups on #2969:
- _resolver_key now keys any bound method (pure-python or built-in) by its
  underlying function/name plus __self__ identity, so a built-in bound method
  (no __func__, fresh object each access) referenced twice still memoizes to
  one call.
- call_fn_with_arg_validation copies the validated args before merging the
  injected kwargs, so a caller-provided pre_validated dict is never mutated.

Add regression tests.

Author: Kludex

src/mcp/server/mcpserver/resolve.py

@@ -134,13 +134,20 @@ def _wants_union(type_arg: Any) -> bool:
 def _resolver_key(fn: Callable[..., Any]) -> Hashable:
     """Identity key for memoizing a resolver.
 
-    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.
+    A bound method - pure-python (`inspect.ismethod`) or built-in (e.g. `obj.meth`
+    on a C-extension type) - is recreated on each attribute access, so `id(fn)`
+    differs every time. Key it by its underlying function (or name) plus its
+    `__self__` identity so `auth.login` referenced in two places memoizes to one
+    call. Everything else keys by `id`, so two distinct callables never collide
+    even if they compare equal.
     """
-    if inspect.ismethod(fn):
-        return (id(fn.__func__), id(fn.__self__))
+    bound_self = getattr(fn, "__self__", None)
+    if bound_self is not None:
+        # `__func__` (pure-python) has a stable identity; built-ins expose only a
+        # stable `__name__`. Use the function's id or the name's value accordingly.
+        func = getattr(fn, "__func__", None)
+        underlying: Hashable = id(func) if func is not None else getattr(fn, "__name__", id(fn))
+        return (underlying, id(bound_self))
     return id(fn)
 
 

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

@@ -91,7 +91,8 @@ async def call_fn_with_arg_validation(
         validating twice can re-run `default_factory`/stateful validators and hand the
         function different values than a caller already observed.
         """
-        arguments_parsed_dict = (
+        # Copy so a caller-provided `pre_validated` dict is never mutated in place.
+        arguments_parsed_dict = dict(
             pre_validated if pre_validated is not None else self.validate_arguments(arguments_to_validate)
         )
 

tests/server/mcpserver/test_func_metadata.py

@@ -155,6 +155,28 @@ async def test_complex_function_runtime_arg_validation_with_json():
     assert result == "ok!"
 
 
+@pytest.mark.anyio
+async def test_call_fn_does_not_mutate_pre_validated():
+    """A caller-provided `pre_validated` dict must not be mutated by the call."""
+
+    def fn(x: int, ctx: str) -> str:
+        return f"{x}:{ctx}"
+
+    meta = func_metadata(fn, skip_names=["ctx"])
+    pre_validated = meta.validate_arguments({"x": 1})
+    snapshot = dict(pre_validated)
+
+    result = await meta.call_fn_with_arg_validation(
+        fn,
+        fn_is_async=False,
+        arguments_to_validate={"x": 1},
+        arguments_to_pass_directly={"ctx": "injected"},
+        pre_validated=pre_validated,
+    )
+    assert result == "1:injected"
+    assert pre_validated == snapshot  # `ctx` was not leaked into the caller's dict
+
+
 def test_str_vs_list_str():
     """Test handling of string vs list[str] type annotations.
 

tests/server/mcpserver/test_resolve.py

@@ -17,7 +17,7 @@
     Resolve,
 )
 from mcp.server.mcpserver.exceptions import InvalidSignature
-from mcp.server.mcpserver.resolve import find_resolved_parameters
+from mcp.server.mcpserver.resolve import _resolver_key, find_resolved_parameters
 from mcp.server.mcpserver.tools.base import Tool
 from mcp.types import ElicitRequestParams, ElicitResult, TextContent
 
@@ -432,3 +432,27 @@ async def never(context: ClientRequestContext, params: ElicitRequestParams) -> E
     async with Client(mcp, mode="legacy", elicitation_callback=never) as client:
         assert await _text(client, "run", {}) == "1:1"
     assert counter["n"] == 1
+
+
+def test_resolver_key_is_stable_for_methods_and_distinct_callables():
+    class Service:
+        def handler(self) -> None: ...  # pragma: no cover
+
+    a, b = Service(), Service()
+
+    # Pure-python bound methods: stable across accesses, distinct per instance.
+    assert _resolver_key(a.handler) == _resolver_key(a.handler)
+    assert _resolver_key(a.handler) != _resolver_key(b.handler)
+
+    # Built-in bound methods (no `__func__`): fresh object each access, but the key
+    # is stable and keyed to `__self__`.
+    items: list[int] = []
+    others: list[int] = []
+    assert _resolver_key(items.append) == _resolver_key(items.append)
+    assert _resolver_key(items.append) != _resolver_key(others.append)
+    assert _resolver_key(items.append) != _resolver_key(items.pop)
+
+    # Plain functions key by identity.
+    def fn() -> None: ...  # pragma: no cover
+
+    assert _resolver_key(fn) == _resolver_key(fn)