Spaces:
Running
Running
Jeremiah Lowin Claude commited on
Fix OpenAPI tool name registration when modified by mcp_component_fn (#1096)
Browse filesResolves issue where tools modified by mcp_component_fn were registered
with original names but accessible with modified names, causing "Unknown tool"
errors. Now tools are registered using their final modified names.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Claude <noreply@anthropic.com>
src/fastmcp/server/openapi.py
CHANGED
|
@@ -847,10 +847,13 @@ class FastMCPOpenAPI(FastMCP):
|
|
| 847 |
f"Using component as-is."
|
| 848 |
)
|
| 849 |
|
|
|
|
|
|
|
|
|
|
| 850 |
# Register the tool by directly assigning to the tools dictionary
|
| 851 |
-
self._tool_manager._tools[
|
| 852 |
logger.debug(
|
| 853 |
-
f"Registered TOOL: {
|
| 854 |
)
|
| 855 |
|
| 856 |
def _create_openapi_resource(
|
|
@@ -897,10 +900,13 @@ class FastMCPOpenAPI(FastMCP):
|
|
| 897 |
f"Using component as-is."
|
| 898 |
)
|
| 899 |
|
|
|
|
|
|
|
|
|
|
| 900 |
# Register the resource by directly assigning to the resources dictionary
|
| 901 |
-
self._resource_manager._resources[
|
| 902 |
logger.debug(
|
| 903 |
-
f"Registered RESOURCE: {
|
| 904 |
)
|
| 905 |
|
| 906 |
def _create_openapi_template(
|
|
@@ -976,8 +982,11 @@ class FastMCPOpenAPI(FastMCP):
|
|
| 976 |
f"Using component as-is."
|
| 977 |
)
|
| 978 |
|
|
|
|
|
|
|
|
|
|
| 979 |
# Register the template by directly assigning to the templates dictionary
|
| 980 |
-
self._resource_manager._templates[
|
| 981 |
logger.debug(
|
| 982 |
-
f"Registered TEMPLATE: {
|
| 983 |
)
|
|
|
|
| 847 |
f"Using component as-is."
|
| 848 |
)
|
| 849 |
|
| 850 |
+
# Use the potentially modified tool name as the registration key
|
| 851 |
+
final_tool_name = tool.name
|
| 852 |
+
|
| 853 |
# Register the tool by directly assigning to the tools dictionary
|
| 854 |
+
self._tool_manager._tools[final_tool_name] = tool
|
| 855 |
logger.debug(
|
| 856 |
+
f"Registered TOOL: {final_tool_name} ({route.method} {route.path}) with tags: {route.tags}"
|
| 857 |
)
|
| 858 |
|
| 859 |
def _create_openapi_resource(
|
|
|
|
| 900 |
f"Using component as-is."
|
| 901 |
)
|
| 902 |
|
| 903 |
+
# Use the potentially modified resource URI as the registration key
|
| 904 |
+
final_resource_uri = str(resource.uri)
|
| 905 |
+
|
| 906 |
# Register the resource by directly assigning to the resources dictionary
|
| 907 |
+
self._resource_manager._resources[final_resource_uri] = resource
|
| 908 |
logger.debug(
|
| 909 |
+
f"Registered RESOURCE: {final_resource_uri} ({route.method} {route.path}) with tags: {route.tags}"
|
| 910 |
)
|
| 911 |
|
| 912 |
def _create_openapi_template(
|
|
|
|
| 982 |
f"Using component as-is."
|
| 983 |
)
|
| 984 |
|
| 985 |
+
# Use the potentially modified template URI as the registration key
|
| 986 |
+
final_template_uri = template.uri_template
|
| 987 |
+
|
| 988 |
# Register the template by directly assigning to the templates dictionary
|
| 989 |
+
self._resource_manager._templates[final_template_uri] = template
|
| 990 |
logger.debug(
|
| 991 |
+
f"Registered TEMPLATE: {final_template_uri} ({route.method} {route.path}) with tags: {route.tags}"
|
| 992 |
)
|
tests/server/openapi/test_route_map_fn.py
CHANGED
|
@@ -1,5 +1,7 @@
|
|
| 1 |
"""Tests for the route_map_fn and component_fn functionality in FastMCPOpenAPI."""
|
| 2 |
|
|
|
|
|
|
|
| 3 |
import httpx
|
| 4 |
import pytest
|
| 5 |
|
|
@@ -372,3 +374,79 @@ def test_route_map_fn_can_rescue_excluded_routes(sample_openapi_spec, http_clien
|
|
| 372 |
assert "getAdminSettings" not in tools
|
| 373 |
assert "updateAdminSettings" not in tools
|
| 374 |
assert "getData" not in tools
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
"""Tests for the route_map_fn and component_fn functionality in FastMCPOpenAPI."""
|
| 2 |
|
| 3 |
+
from unittest.mock import AsyncMock
|
| 4 |
+
|
| 5 |
import httpx
|
| 6 |
import pytest
|
| 7 |
|
|
|
|
| 374 |
assert "getAdminSettings" not in tools
|
| 375 |
assert "updateAdminSettings" not in tools
|
| 376 |
assert "getData" not in tools
|
| 377 |
+
|
| 378 |
+
|
| 379 |
+
class TestComponentFnToolNameModificationBug:
|
| 380 |
+
"""Test that mcp_component_fn can modify tool names without breaking access (Issue #1091)."""
|
| 381 |
+
|
| 382 |
+
@pytest.fixture
|
| 383 |
+
def mocked_http_client(self):
|
| 384 |
+
"""Mock HTTP client that returns successful responses."""
|
| 385 |
+
from unittest.mock import MagicMock
|
| 386 |
+
|
| 387 |
+
mock_client = AsyncMock(spec=httpx.AsyncClient)
|
| 388 |
+
|
| 389 |
+
# Mock a successful response
|
| 390 |
+
mock_response = MagicMock()
|
| 391 |
+
mock_response.status_code = 200
|
| 392 |
+
mock_response.json.return_value = {"result": "success"}
|
| 393 |
+
mock_response.raise_for_status.return_value = None
|
| 394 |
+
|
| 395 |
+
mock_client.request.return_value = mock_response
|
| 396 |
+
return mock_client
|
| 397 |
+
|
| 398 |
+
@pytest.fixture
|
| 399 |
+
def server_with_modified_tool_names(self, sample_openapi_spec, mocked_http_client):
|
| 400 |
+
"""Server with tool names modified by mcp_component_fn."""
|
| 401 |
+
|
| 402 |
+
def modify_tool_names(route, component):
|
| 403 |
+
"""Modify tool names by adding v1_removed_ prefix."""
|
| 404 |
+
from fastmcp.server.openapi import OpenAPITool
|
| 405 |
+
|
| 406 |
+
if isinstance(component, OpenAPITool):
|
| 407 |
+
if component.name.startswith("get"):
|
| 408 |
+
component.name = "v1_removed_" + component.name
|
| 409 |
+
|
| 410 |
+
return FastMCPOpenAPI(
|
| 411 |
+
openapi_spec=sample_openapi_spec,
|
| 412 |
+
client=mocked_http_client,
|
| 413 |
+
name="Test Server",
|
| 414 |
+
mcp_component_fn=modify_tool_names,
|
| 415 |
+
)
|
| 416 |
+
|
| 417 |
+
def test_registration(self, server_with_modified_tool_names):
|
| 418 |
+
"""Test that modified tool names are properly registered."""
|
| 419 |
+
tools = server_with_modified_tool_names._tool_manager._tools
|
| 420 |
+
|
| 421 |
+
# Tool should be registered with the modified name
|
| 422 |
+
assert "v1_removed_getUserById" in tools
|
| 423 |
+
assert "v1_removed_getAdminSettings" in tools
|
| 424 |
+
assert "v1_removed_getData" in tools
|
| 425 |
+
|
| 426 |
+
# The tool object should have the same name as the registration key
|
| 427 |
+
for key, tool in tools.items():
|
| 428 |
+
if key.startswith("v1_removed_"):
|
| 429 |
+
assert tool.name == key
|
| 430 |
+
|
| 431 |
+
async def test_client_access(self, server_with_modified_tool_names):
|
| 432 |
+
"""Test that modified tool names are accessible via client."""
|
| 433 |
+
from fastmcp.client import Client
|
| 434 |
+
|
| 435 |
+
async with Client(server_with_modified_tool_names) as client:
|
| 436 |
+
# List tools to verify they are exposed correctly
|
| 437 |
+
available_tools = await client.list_tools()
|
| 438 |
+
tool_names = [tool.name for tool in available_tools]
|
| 439 |
+
|
| 440 |
+
# Verify the modified tool names are available
|
| 441 |
+
assert "v1_removed_getUserById" in tool_names
|
| 442 |
+
assert "v1_removed_getAdminSettings" in tool_names
|
| 443 |
+
assert "v1_removed_getData" in tool_names
|
| 444 |
+
|
| 445 |
+
async def test_client_call(self, server_with_modified_tool_names):
|
| 446 |
+
"""Test that modified tool names can be called via client."""
|
| 447 |
+
from fastmcp.client import Client
|
| 448 |
+
|
| 449 |
+
async with Client(server_with_modified_tool_names) as client:
|
| 450 |
+
# This should work without "Unknown tool" error
|
| 451 |
+
result = await client.call_tool("v1_removed_getData", {})
|
| 452 |
+
assert result.data == {"result": "success"}
|