mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-09 07:09:11 +02:00
Store Tool objects in registry to fix namespace collision
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.
This commit is contained in:
parent
215e000fa6
commit
697215c685
4 changed files with 72 additions and 120 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue