From 2d5225b09a4b00ac915273345a50d0e6f6b63e87 Mon Sep 17 00:00:00 2001 From: Cabeza Date: Mon, 12 Oct 2026 13:18:39 +0800 Subject: [PATCH] fix(mcpserver): stop a bare NameError escaping tool registration on 3.14 Since Python 3.14 (PEP 649/749) annotations are evaluated lazily, so a tool may legally name a type the module defines further down: @app.tool() def search(q: Query) -> list[Hit]: ... class Query(BaseModel): ... `MCPServer.tool()` builds the `Tool` eagerly, and three of its steps call `inspect.signature(fn)` while only needing the parameter *names*. The default `annotation_format` evaluates `__annotate__`, so registration died with a bare `NameError: name 'Query' is not defined` raised from inside `inspect`. Those steps now read names through `signature_parameters()`, which asks for `Format.FORWARDREF` on 3.14+ and keeps the plain call on older Pythons, where string annotations are already returned unevaluated. Registration then reaches the step that genuinely needs the types and fails there with the `InvalidSignature` it already produced for a quoted annotation -- now naming the unresolved type, so the error says which name to move above the decorated function instead of only that something could not be evaluated. Deferring the whole schema build so the tool would register and resolve on first use is the other half of the issue and is left to the maintainers' preference; this change makes the failure consistent and actionable either way. Refs #3668 --- src/mcp/server/mcpserver/resolve.py | 7 ++- src/mcp/server/mcpserver/server.py | 12 ++--- .../mcpserver/utilities/func_metadata.py | 8 +++- src/mcp/shared/_callable_inspection.py | 22 ++++++++- tests/server/mcpserver/test_func_metadata.py | 20 +++++++++ tests/server/mcpserver/test_resolve.py | 21 +++++++++ tests/server/mcpserver/test_server.py | 45 +++++++++++++++++++ 7 files changed, 123 insertions(+), 12 deletions(-) diff --git a/src/mcp/server/mcpserver/resolve.py b/src/mcp/server/mcpserver/resolve.py index 2afbc516e3..67df5c05ab 100644 --- a/src/mcp/server/mcpserver/resolve.py +++ b/src/mcp/server/mcpserver/resolve.py @@ -81,7 +81,7 @@ from mcp.server.mcpserver.exceptions import InvalidSignature, ToolError from mcp.server.request_state import compact_json from mcp.server.validation import validate_tool_use_result_messages, wants_sampling_tools -from mcp.shared._callable_inspection import is_async_callable +from mcp.shared._callable_inspection import is_async_callable, signature_parameters from mcp.shared.exceptions import MCPError from mcp.shared.message import ServerMessageMetadata @@ -226,7 +226,7 @@ def find_resolved_parameters(fn: Callable[..., Any]) -> dict[str, tuple[Resolve, """ hints = _type_hints(fn) resolved: dict[str, tuple[Resolve, bool]] = {} - for name in inspect.signature(fn).parameters: + for name in signature_parameters(fn): annotation = hints.get(name) if get_origin(annotation) is not Annotated: # A `Resolve` marker is only honored at the top level; flag (rather than @@ -373,10 +373,9 @@ def analyze(fn: Callable[..., Any], stack: tuple[Hashable, ...]) -> None: wire_key = base if seen == 0 else f"{base}#{seen}" hints = _type_hints(fn) - sig = inspect.signature(fn) params: dict[str, _ParamPlan] = {} nested: list[Callable[..., Any]] = [] - for param_name in sig.parameters: + for param_name in signature_parameters(fn): annotation = hints.get(param_name) if annotation is not None and _is_context_annotation(annotation): params[param_name] = _ParamPlan("context") diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index 99b5a56c0a..1e8c9a51c3 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -100,6 +100,7 @@ ) from mcp.server.subscriptions import InMemorySubscriptionBus, ListenHandler, SubscriptionBus from mcp.server.transport_security import DEFAULT_MAX_REQUEST_BODY_SIZE, TransportSecuritySettings +from mcp.shared._callable_inspection import signature_parameters from mcp.shared.exceptions import MCPError from mcp.shared.uri_template import UriTemplate @@ -870,9 +871,12 @@ async def get_weather(city: str) -> str: uri_params = set(parsed.variable_names) def decorator(fn: _CallableT) -> _CallableT: - sig = inspect.signature(fn) context_param = find_context_parameter(fn) - func_params = {p for p in sig.parameters.keys() if p != context_param} + # Names only: on Python 3.14+ evaluating the annotations here would + # raise for a type the module has not finished defining. The one + # place below that needs a default reads it from the same object. + parameters = signature_parameters(fn) + func_params = {p for p in parameters if p != context_param} # Template/static is decided purely by the URI: variables # present means template, none means static. @@ -889,9 +893,7 @@ def decorator(fn: _CallableT) -> _CallableT: # one, the author only finds out on the first request that # omits it, as an opaque internal error. missing_defaults = sorted( - name - for name in parsed.query_variable_names - if sig.parameters[name].default is inspect.Parameter.empty + name for name in parsed.query_variable_names if parameters[name].default is inspect.Parameter.empty ) if missing_defaults: raise ValueError( diff --git a/src/mcp/server/mcpserver/utilities/func_metadata.py b/src/mcp/server/mcpserver/utilities/func_metadata.py index 0ffac07c4e..af161f1081 100644 --- a/src/mcp/server/mcpserver/utilities/func_metadata.py +++ b/src/mcp/server/mcpserver/utilities/func_metadata.py @@ -320,10 +320,14 @@ def func_metadata( """ try: sig = inspect.signature(func, eval_str=True) - except NameError as e: # pragma: no cover + except NameError as e: # This raise could perhaps be skipped, and we (MCPServer) just call # model_rebuild right before using it 🤷 - raise InvalidSignature(f"Unable to evaluate type annotations for callable {func.__name__!r}") from e + # `e` names the unresolved annotation, which is the part the caller can + # act on: on Python 3.14+ (PEP 649) a type defined below the decorated + # function is simply not there yet, and "name 'Query' is not defined" + # says so where a bare "unable to evaluate" does not. + raise InvalidSignature(f"Unable to evaluate type annotations for callable {func.__name__!r}: {e}") from e params = sig.parameters dynamic_pydantic_model_params: dict[str, Any] = {} for param in params.values(): diff --git a/src/mcp/shared/_callable_inspection.py b/src/mcp/shared/_callable_inspection.py index 0e89e446f8..4fad80ce55 100644 --- a/src/mcp/shared/_callable_inspection.py +++ b/src/mcp/shared/_callable_inspection.py @@ -8,9 +8,13 @@ import functools import inspect -from collections.abc import Awaitable, Callable +import sys +from collections.abc import Awaitable, Callable, Mapping from typing import Any, TypeGuard, TypeVar, overload +if sys.version_info >= (3, 14): # pragma: no cover - version-dependent import + import annotationlib + T = TypeVar("T") AwaitableCallable = Callable[..., Awaitable[T]] @@ -31,3 +35,19 @@ def is_async_callable(obj: Any) -> Any: return inspect.iscoroutinefunction(obj) or ( callable(obj) and inspect.iscoroutinefunction(getattr(obj, "__call__", None)) ) + + +def signature_parameters(fn: Callable[..., Any]) -> Mapping[str, inspect.Parameter]: + """Parameters of `fn` by name, without evaluating its annotations. + + Callers that only need the names must not force evaluation: on Python 3.14+ + (PEP 649) annotations are evaluated lazily through `__annotate__`, and + `inspect.signature` evaluates them by default, so a parameter whose type the + module has not finished defining raises a bare `NameError` from a call that + never wanted the type. `Format.FORWARDREF` leaves them unevaluated; on older + Pythons string annotations are already returned unevaluated, so the plain + call is safe there. + """ + if sys.version_info >= (3, 14): + return inspect.signature(fn, annotation_format=annotationlib.Format.FORWARDREF).parameters + return inspect.signature(fn).parameters diff --git a/tests/server/mcpserver/test_func_metadata.py b/tests/server/mcpserver/test_func_metadata.py index dba0637ded..62e8a71f14 100644 --- a/tests/server/mcpserver/test_func_metadata.py +++ b/tests/server/mcpserver/test_func_metadata.py @@ -1520,3 +1520,23 @@ def fn() -> StepA | StepB: ... # pragma: no branch meta = func_metadata(fn) assert meta.output_schema is None + + +def test_unresolvable_annotation_names_the_callable_and_the_missing_type(): + """A forward-referenced type that does not exist yet has to say which one. + + On Python 3.14+ (PEP 649) annotations are evaluated lazily, so a type defined + below the decorated function is simply not there yet; the caller can only act + on the error if it names the missing type. On older Pythons the same message + comes from a quoted annotation. See #3668. + """ + + # The undefined name is the point of the test. + def fn(q: "NotDefinedYet") -> str: ... # noqa: F821 # pragma: no branch + + with pytest.raises(InvalidSignature) as exc_info: + func_metadata(fn) + + message = str(exc_info.value) + assert "'fn'" in message + assert "NotDefinedYet" in message diff --git a/tests/server/mcpserver/test_resolve.py b/tests/server/mcpserver/test_resolve.py index 49f0f1314f..fd99dd928c 100644 --- a/tests/server/mcpserver/test_resolve.py +++ b/tests/server/mcpserver/test_resolve.py @@ -2,6 +2,7 @@ import json import logging +import sys from collections.abc import Callable from datetime import datetime from typing import Annotated, Any, Literal, TypeVar, cast @@ -2875,3 +2876,23 @@ async def whoami(login: Annotated[Login, Resolve(ask)]) -> str: assert exc_info.value.code == MISSING_REQUIRED_CLIENT_CAPABILITY assert exc_info.value.error.data == {"requiredCapabilities": {"elicitation": {"form": {}}}} + + +@pytest.mark.skipif(sys.version_info < (3, 14), reason="PEP 649 lazy annotations are 3.14+") +def test_find_resolved_parameters_reads_names_without_evaluating_annotations(): + """Only the names are needed here, so an unresolvable annotation must not raise. + + On Python 3.14+ (PEP 649) annotations are evaluated lazily and + `inspect.signature` evaluates them by default, so a parameter whose type the + module has not finished defining raises a bare `NameError` from a call that + only wanted the names. #3668. + """ + from mcp.server.mcpserver.resolve import find_resolved_parameters + + # Built with exec so the undefined name is never statically visible: on older + # Pythons an unquoted undefined annotation raises where the function is + # defined, which would make this module fail to import. + namespace: dict[str, Any] = {} + exec("def fn(q: NotDefinedYet) -> str: ...", namespace) # noqa: S102 + + assert find_resolved_parameters(namespace["fn"]) == {} diff --git a/tests/server/mcpserver/test_server.py b/tests/server/mcpserver/test_server.py index 2b95a12290..3e5adb08b9 100644 --- a/tests/server/mcpserver/test_server.py +++ b/tests/server/mcpserver/test_server.py @@ -1,5 +1,6 @@ import base64 import logging +import sys from collections.abc import AsyncIterator from contextlib import asynccontextmanager from dataclasses import dataclass @@ -57,6 +58,7 @@ from mcp.server.context import ServerRequestContext from mcp.server.mcpserver import Context, MCPServer, RequestStateSecurity, Resolve, ResourceSecurity from mcp.server.mcpserver.exceptions import ( + InvalidSignature, ResourceError, ResourceNotFoundError, ToolError, @@ -3318,3 +3320,46 @@ async def refuse_listen(ctx: ServerRequestContext[Any, Any], call_next: Any) -> pass # pragma: no cover - the refusal precedes the stream assert exc_info.value.error.code == INVALID_REQUEST assert exc_info.value.error.message == "not permitted to watch the requested resources" + + +# The issue's repro: a tool whose parameter and return types are defined *below* +# it, which PEP 649 (3.14+) makes legal without quoting. Built with exec so the +# undefined names are never statically visible -- on older Pythons an unquoted +# undefined annotation raises where the function is defined, which would make +# this module fail to import. +_FORWARD_REF_TOOL_SOURCE = """ +from pydantic import BaseModel + +app = MCPServer("repro") + + +@app.tool() +def search(q: Query) -> list[Hit]: + \"\"\"Search.\"\"\" + return [Hit(id=1)] + + +class Query(BaseModel): + text: str + + +class Hit(BaseModel): + id: int +""" + + +@pytest.mark.skipif(sys.version_info < (3, 14), reason="PEP 649 lazy annotations are 3.14+") +async def test_tool_with_types_defined_below_it_fails_with_a_named_error(): + """Registration must not leak a bare NameError from inside inspect.signature. + + It raises `InvalidSignature` naming the tool and the type that could not be + resolved yet, the same error a quoted annotation already produced. #3668. + """ + namespace: dict[str, Any] = {"MCPServer": MCPServer} + + with pytest.raises(InvalidSignature) as exc_info: + exec(_FORWARD_REF_TOOL_SOURCE, namespace) # noqa: S102 + + message = str(exc_info.value) + assert "'search'" in message + assert "Query" in message