From 697215c685ef60179c478b961f32dfead02f9d72 Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Mon, 2 Mar 2026 19:49:24 -0500 Subject: [PATCH] Store Tool objects in registry to fix namespace collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The registry previously mapped global_key → local_name (string), which meant two mounted children with the same tool name would resolve ambiguously. Now it maps global_key → Tool (the object), so call_tool uses the tool directly — no ContextVar, no transform bypass needed. --- src/fastmcp/server/providers/base.py | 20 +--- .../local_provider/decorators/tools.py | 2 +- src/fastmcp/server/server.py | 93 ++++++++++--------- tests/test_app_tool_global_keys.py | 77 ++++----------- 4 files changed, 72 insertions(+), 120 deletions(-) diff --git a/src/fastmcp/server/providers/base.py b/src/fastmcp/server/providers/base.py index 2c485e1c0..2c27b6521 100644 --- a/src/fastmcp/server/providers/base.py +++ b/src/fastmcp/server/providers/base.py @@ -30,7 +30,6 @@ from __future__ import annotations from collections.abc import AsyncIterator, Sequence from contextlib import asynccontextmanager -from contextvars import ContextVar from functools import partial from typing import TYPE_CHECKING, Literal, cast @@ -48,17 +47,11 @@ from fastmcp.utilities.versions import VersionSpec, version_sort_key if TYPE_CHECKING: from fastmcp.server.transforms import Transform -# When set to True, Provider.get_tool() bypasses the transform chain -# (Namespace, ToolTransform, etc.) and calls _get_tool() directly. -# Used by FastMCP.call_tool() when resolving app tool global keys — -# these keys are stable identifiers that must not be transformed. -_APP_TOOL_CALL: ContextVar[bool] = ContextVar("_APP_TOOL_CALL", default=False) - -# Module-level registry mapping app tool global keys to their local names. +# Module-level registry mapping app tool global keys to Tool objects. # Populated by _maybe_generate_app_global_key when tools with "app" in # their visibility are registered. Checked by FastMCP.call_tool() to -# resolve global keys without going through the transform chain. -_APP_TOOL_REGISTRY: dict[str, str] = {} +# resolve and execute tools directly, bypassing the transform chain. +_APP_TOOL_REGISTRY: dict[str, Tool] = {} class Provider: @@ -171,11 +164,6 @@ class Provider: (FastMCP) performs enabled filtering after all transforms complete, allowing session-level transforms to override provider-level disables. - When ``_APP_TOOL_CALL`` is set, the transform chain is bypassed - entirely and ``_get_tool`` is called directly. This is used for - app tool global key resolution where transforms must not alter - the lookup name. - Args: name: The transformed tool name to look up. version: Optional version filter. If None, returns highest version. @@ -183,8 +171,6 @@ class Provider: Returns: The tool if found (may be marked disabled), None if not found. """ - if _APP_TOOL_CALL.get(): - return await self._get_tool(name, version) async def base(n: str, version: VersionSpec | None = None) -> Tool | None: return await self._get_tool(n, version) diff --git a/src/fastmcp/server/providers/local_provider/decorators/tools.py b/src/fastmcp/server/providers/local_provider/decorators/tools.py index 66df767f9..5dab6161e 100644 --- a/src/fastmcp/server/providers/local_provider/decorators/tools.py +++ b/src/fastmcp/server/providers/local_provider/decorators/tools.py @@ -147,7 +147,7 @@ def _maybe_generate_app_global_key(tool: Tool) -> None: global_key = f"{tool.name}-{uuid.uuid4().hex[:8]}" ui["globalKey"] = global_key - _APP_TOOL_REGISTRY[global_key] = tool.name + _APP_TOOL_REGISTRY[global_key] = tool def _maybe_apply_prefab_ui(provider: LocalProvider, tool: Tool) -> None: diff --git a/src/fastmcp/server/server.py b/src/fastmcp/server/server.py index d8d5d7277..28cf59442 100644 --- a/src/fastmcp/server/server.py +++ b/src/fastmcp/server/server.py @@ -68,7 +68,7 @@ from fastmcp.server.middleware import Middleware, MiddlewareContext from fastmcp.server.mixins import LifespanMixin, MCPOperationsMixin, TransportMixin from fastmcp.server.providers import LocalProvider, Provider from fastmcp.server.providers.aggregate import AggregateProvider -from fastmcp.server.providers.base import _APP_TOOL_CALL, _APP_TOOL_REGISTRY +from fastmcp.server.providers.base import _APP_TOOL_REGISTRY from fastmcp.server.tasks.config import TaskConfig, TaskMeta from fastmcp.server.telemetry import server_span from fastmcp.server.transforms import ( @@ -974,52 +974,57 @@ class FastMCP( ) # Core logic: find and execute tool (providers queried in parallel) - # If the name matches an app tool global key, resolve it to the - # local name and set _APP_TOOL_CALL so provider transforms - # (Namespace, ToolTransform) are bypassed during resolution. - app_tool_token = None - if name in _APP_TOOL_REGISTRY: - name = _APP_TOOL_REGISTRY[name] - app_tool_token = _APP_TOOL_CALL.set(True) - - try: - with server_span( - f"tools/call {name}", "tools/call", self.name, "tool", name - ) as span: + with server_span( + f"tools/call {name}", "tools/call", self.name, "tool", name + ) as span: + # If the name matches an app tool global key, use the + # Tool object directly — bypassing the transform chain. + # This avoids namespace ambiguity when multiple mounted + # children have tools with the same local name. + if name in _APP_TOOL_REGISTRY: + tool = _APP_TOOL_REGISTRY[name] + # Auth still fires for global key calls + skip_auth, token = _get_auth_context() + if not skip_auth and tool.auth is not None: + ctx = AuthContext(token=token, component=tool) + try: + if not await run_auth_checks(tool.auth, ctx): + raise NotFoundError(f"Unknown tool: {name!r}") + except AuthorizationError: + raise NotFoundError(f"Unknown tool: {name!r}") from None + else: tool = await self.get_tool(name, version=version) - if tool is None: - raise NotFoundError(f"Unknown tool: {name!r}") - span.set_attributes(tool.get_span_attributes()) - if task_meta is not None and task_meta.fn_key is None: - task_meta = replace(task_meta, fn_key=tool.key) - try: - return await tool._run(arguments or {}, task_meta=task_meta) - except FastMCPError: - logger.exception(f"Error calling tool {name!r}") - raise - except (ValidationError, PydanticValidationError): - logger.exception(f"Error validating tool {name!r}") - raise - except Exception as e: - logger.exception(f"Error calling tool {name!r}") - # Handle actionable errors that should reach the LLM - # even when masking is enabled - if isinstance(e, httpx.HTTPStatusError): - if e.response.status_code == 429: - raise ToolError( - "Rate limited by upstream API, please retry later" - ) from e - if isinstance(e, httpx.TimeoutException): + + if tool is None: + raise NotFoundError(f"Unknown tool: {name!r}") + span.set_attributes(tool.get_span_attributes()) + if task_meta is not None and task_meta.fn_key is None: + task_meta = replace(task_meta, fn_key=tool.key) + try: + return await tool._run(arguments or {}, task_meta=task_meta) + except FastMCPError: + logger.exception(f"Error calling tool {name!r}") + raise + except (ValidationError, PydanticValidationError): + logger.exception(f"Error validating tool {name!r}") + raise + except Exception as e: + logger.exception(f"Error calling tool {name!r}") + # Handle actionable errors that should reach the LLM + # even when masking is enabled + if isinstance(e, httpx.HTTPStatusError): + if e.response.status_code == 429: raise ToolError( - "Upstream request timed out, please retry" + "Rate limited by upstream API, please retry later" ) from e - # Standard masking logic - if self._mask_error_details: - raise ToolError(f"Error calling tool {name!r}") from e - raise ToolError(f"Error calling tool {name!r}: {e}") from e - finally: - if app_tool_token is not None: - _APP_TOOL_CALL.reset(app_tool_token) + if isinstance(e, httpx.TimeoutException): + raise ToolError( + "Upstream request timed out, please retry" + ) from e + # Standard masking logic + if self._mask_error_details: + raise ToolError(f"Error calling tool {name!r}") from e + raise ToolError(f"Error calling tool {name!r}: {e}") from e @overload async def read_resource( diff --git a/tests/test_app_tool_global_keys.py b/tests/test_app_tool_global_keys.py index cb196a146..6d1612f57 100644 --- a/tests/test_app_tool_global_keys.py +++ b/tests/test_app_tool_global_keys.py @@ -9,8 +9,8 @@ from fastmcp import FastMCP from fastmcp.exceptions import NotFoundError from fastmcp.server.apps import AppConfig from fastmcp.server.auth import AuthContext -from fastmcp.server.providers.base import _APP_TOOL_CALL, _APP_TOOL_REGISTRY -from fastmcp.tools.tool import ToolResult +from fastmcp.server.providers.base import _APP_TOOL_REGISTRY +from fastmcp.tools.tool import Tool, ToolResult def _get_text(result: ToolResult) -> str: @@ -80,7 +80,7 @@ class TestGlobalKeyGeneration: assert len(_APP_TOOL_REGISTRY) == 0 - def test_global_key_maps_to_local_name(self): + def test_global_key_maps_to_tool_object(self): mcp = FastMCP("test") @mcp.tool(app=AppConfig(resource_uri="ui://app/view.html", visibility=["app"])) @@ -88,7 +88,9 @@ class TestGlobalKeyGeneration: return "done" key = next(iter(_APP_TOOL_REGISTRY)) - assert _APP_TOOL_REGISTRY[key] == "action" + tool = _APP_TOOL_REGISTRY[key] + assert isinstance(tool, Tool) + assert tool.name == "action" def test_two_tools_get_different_keys(self): mcp = FastMCP("test") @@ -209,22 +211,22 @@ class TestCallToolMounted: result = await server_a.call_tool(global_key, {}) assert _get_text(result) == "from C" - async def test_duplicate_servers_no_collision(self): - """Two identical servers mounted get different global keys.""" + async def test_same_name_tools_on_different_children(self): + """Two children with the same tool name resolve to correct child.""" - def make_child() -> FastMCP: + def make_child(val: str) -> FastMCP: child = FastMCP("child") @child.tool( app=AppConfig(resource_uri="ui://app/view.html", visibility=["app"]) ) def action() -> str: - return "done" + return val return child - child1 = make_child() - child2 = make_child() + child1 = make_child("from-child1") + child2 = make_child("from-child2") parent = FastMCP("parent") parent.mount(child1, namespace="a") @@ -232,7 +234,13 @@ class TestCallToolMounted: keys = list(_APP_TOOL_REGISTRY.keys()) assert len(keys) == 2 - assert keys[0] != keys[1] + + results = set() + for k in keys: + result = await parent.call_tool(k, {}) + results.add(_get_text(result)) + + assert results == {"from-child1", "from-child2"} async def test_mount_without_namespace(self): """Global keys work even without a namespace.""" @@ -323,50 +331,3 @@ class TestAuthWithGlobalKeys: with pytest.raises(NotFoundError): await mcp.call_tool(global_key, {}) - - -# --------------------------------------------------------------------------- -# ContextVar safety -# --------------------------------------------------------------------------- - - -class TestContextVarSafety: - async def test_contextvar_reset_after_call(self): - """_APP_TOOL_CALL must be False after a global key call completes.""" - mcp = FastMCP("test") - - @mcp.tool(app=AppConfig(resource_uri="ui://app/view.html", visibility=["app"])) - def check() -> str: - return "ok" - - global_key = next(iter(_APP_TOOL_REGISTRY)) - await mcp.call_tool(global_key, {}) - assert _APP_TOOL_CALL.get() is False - - async def test_contextvar_reset_on_error(self): - """_APP_TOOL_CALL must be reset even if the tool raises.""" - mcp = FastMCP("test") - - @mcp.tool(app=AppConfig(resource_uri="ui://app/view.html", visibility=["app"])) - def failing() -> str: - raise ValueError("boom") - - global_key = next(iter(_APP_TOOL_REGISTRY)) - with pytest.raises(Exception): - await mcp.call_tool(global_key, {}) - assert _APP_TOOL_CALL.get() is False - - async def test_normal_call_does_not_set_contextvar(self): - """Regular tool calls must not trigger _APP_TOOL_CALL.""" - mcp = FastMCP("test") - - called_with_flag = None - - @mcp.tool - def probe() -> str: - nonlocal called_with_flag - called_with_flag = _APP_TOOL_CALL.get() - return "ok" - - await mcp.call_tool("probe", {}) - assert called_with_flag is False