diff --git a/fastapi_mcp/server.py b/fastapi_mcp/server.py index fbff6e8..c723a67 100644 --- a/fastapi_mcp/server.py +++ b/fastapi_mcp/server.py @@ -31,6 +31,7 @@ def __init__( exclude_operations: Optional[List[str]] = None, include_tags: Optional[List[str]] = None, exclude_tags: Optional[List[str]] = None, + disable_warnings: bool = False, ): """ Create an MCP server from a FastAPI app. @@ -50,6 +51,7 @@ def __init__( exclude_operations: List of operation IDs to exclude from MCP tools. Cannot be used with include_operations. include_tags: List of tags to include as MCP tools. Cannot be used with exclude_tags. exclude_tags: List of tags to exclude from MCP tools. Cannot be used with include_tags. + disable_warnings: Set to True to disable tool conversion best practice warnings. """ # Validate operation and tag filtering options if include_operations is not None and exclude_operations is not None: @@ -73,6 +75,7 @@ def __init__( self._exclude_operations = exclude_operations self._include_tags = include_tags self._exclude_tags = exclude_tags + self._disable_warnings = disable_warnings self._http_client = http_client or httpx.AsyncClient() @@ -97,6 +100,11 @@ def setup_server(self) -> None: # Filter tools based on operation IDs and tags self.tools = self._filter_tools(all_tools, openapi_schema) + + # Check for warnings + self._warn_if_too_many_tools() + self._warn_if_non_get_endpoints() + self._warn_if_auto_generated_operation_ids() # Determine base URL if not provided if not self._base_url: @@ -139,6 +147,79 @@ async def handle_call_tool( self.server = mcp_server + def _warn_if_too_many_tools(self) -> None: + """ + Issue a warning if there are too many tools exposed, which may impact user experience. + """ + if self._disable_warnings: + return + + if len(self.tools) > 10: + logger.warning( + f"More than 10 tools exposed ({len(self.tools)}), which may impact user experience. " + f"Consider filtering tools to make the MCP more usable to the LLM. " + f"To disable this warning, use disable_warnings=True when creating FastApiMCP." + ) + + def _warn_if_non_get_endpoints(self) -> None: + """ + Issue a warning if non-GET endpoints are exposed as tools. + """ + if self._disable_warnings: + return + + non_get_tools = [] + for tool_name in self.operation_map: + if self.operation_map[tool_name]["method"].lower() != "get": + non_get_tools.append(f"{tool_name} ({self.operation_map[tool_name]['method'].upper()})") + + if non_get_tools: + logger.warning( + f"Non-GET endpoints exposed as tools: {', '.join(non_get_tools)}. " + f"Using POST, DELETE, PUT, or PATCH endpoints as tools may lead to unwanted side effects " + f"and unexpected behaviors when called by LLMs. Consider using include/exclude filters " + f"to limit exposed endpoints to GET methods only, or ensure proper validation is in place. " + f"To disable this warning, use disable_warnings=True when creating FastApiMCP." + ) + + def _warn_if_auto_generated_operation_ids(self) -> None: + """ + Issue a warning if auto-generated operation IDs are detected by checking if operation_id + was explicitly provided in FastAPI route definitions. + """ + if self._disable_warnings: + return + + # Track routes with explicitly set operation_ids + explicit_operation_ids = set() + + # Check each route to see if operation_id was explicitly provided + for route in self.fastapi.routes: + # Skip special routes or routes without endpoint functions + if not hasattr(route, "endpoint"): + continue + + # Check if the route has an explicitly set operation_id + # This is the main attribute to check in FastAPI routes + if hasattr(route, "operation_id") and route.operation_id: + explicit_operation_ids.add(route.operation_id) + + # Also check if operation_id is in the route's openapi_extra attribute + # This is another way for operation_ids to be set in FastAPI + if hasattr(route, "openapi_extra") and route.openapi_extra: + if "operationId" in route.openapi_extra: + explicit_operation_ids.add(route.openapi_extra["operationId"]) + + # For each tool, check if it corresponds to a route with an auto-generated operation_id + for tool in self.tools: + if tool.name not in explicit_operation_ids: + logger.warning( + f"Tool '{tool.name}' appears to have an auto-generated operation_id. " + f"LLMs may struggle to use this tool effectively. Consider adding an explicit operation_id " + f"to the route or excluding it from MCP tools. " + f"To disable this warning, use disable_warnings=True when creating FastApiMCP." + ) + def mount(self, router: Optional[FastAPI | APIRouter] = None, mount_path: str = "/mcp") -> None: """ Mount the MCP server to **any** FastAPI app or APIRouter. diff --git a/tests/test_warnings.py b/tests/test_warnings.py new file mode 100644 index 0000000..35b54ac --- /dev/null +++ b/tests/test_warnings.py @@ -0,0 +1,230 @@ +import pytest +from fastapi import FastAPI, APIRouter +from fastapi_mcp import FastApiMCP +import logging + + +@pytest.fixture +def app_with_too_many_tools(): + """Create a FastAPI app with more than 10 endpoints to trigger the 'too many tools' warning.""" + app = FastAPI( + title="App with Too Many Tools", + description="An app with more than 10 endpoints to test warnings", + ) + + # Create more than 10 GET endpoints + for i in range(11): + @app.get(f"/items/{i}", operation_id=f"get_item_{i}") + async def get_item(item_id: int = i): + return {"item_id": item_id, "name": f"Item {item_id}"} + + return app + + +@pytest.fixture +def app_with_non_get_endpoints(): + """Create a FastAPI app with non-GET endpoints to trigger the warning.""" + app = FastAPI( + title="App with Non-GET Endpoints", + description="An app with various HTTP methods to test warnings", + ) + + @app.get("/items", operation_id="list_items") + async def list_items(): + return [{"id": 1, "name": "Item 1"}, {"id": 2, "name": "Item 2"}] + + @app.post("/items", operation_id="create_item") + async def create_item(item: dict): + return {"id": 3, **item} + + @app.put("/items/{item_id}", operation_id="update_item") + async def update_item(item_id: int, item: dict): + return {"id": item_id, **item} + + @app.delete("/items/{item_id}", operation_id="delete_item") + async def delete_item(item_id: int): + return {"message": f"Item {item_id} deleted"} + + return app + + +@pytest.fixture +def app_with_auto_generated_ids(): + """Create a FastAPI app with auto-generated operation IDs to trigger the warning.""" + app = FastAPI( + title="App with Auto-generated IDs", + description="An app with auto-generated operation IDs to test warnings", + ) + + # Routes with auto-generated operation IDs (no explicit operation_id provided) + @app.get("/auto-generated") + async def auto_generated_route(): + return {"message": "Auto-generated operation_id"} + + @app.get("/auto-generated-2") + async def auto_generated_route_get(): + return {"message": "Another auto-generated operation_id"} + + # Route with explicit operation_id + @app.get("/explicit", operation_id="explicit_operation_id") + async def explicit_route(): + return {"message": "Explicit operation_id"} + + return app + + +def test_warn_if_too_many_tools(app_with_too_many_tools, caplog): + """Test that a warning is issued when there are too many tools.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance + _ = FastApiMCP(app_with_too_many_tools) + + # Check that warning was logged + assert any("More than 10 tools exposed" in record.message for record in caplog.records) + assert any("To disable this warning" in record.message for record in caplog.records) + + +def test_warn_if_too_many_tools_no_warning(app_with_too_many_tools, caplog): + """Test that no warning is issued when disable_warnings=True.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance with warnings disabled + _ = FastApiMCP(app_with_too_many_tools, disable_warnings=True) + + # Check that no warning was logged + assert not any("More than 10 tools exposed" in record.message for record in caplog.records) + + +def test_warn_if_non_get_endpoints(app_with_non_get_endpoints, caplog): + """Test that a warning is issued when there are non-GET endpoints.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance + _ = FastApiMCP(app_with_non_get_endpoints) + + # Check that warning was logged + assert any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) + assert any("create_item (POST)" in record.message for record in caplog.records) + assert any("update_item (PUT)" in record.message for record in caplog.records) + assert any("delete_item (DELETE)" in record.message for record in caplog.records) + assert any("To disable this warning" in record.message for record in caplog.records) + + +def test_warn_if_non_get_endpoints_no_warning(app_with_non_get_endpoints, caplog): + """Test that no warning is issued when disable_warnings=True.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance with warnings disabled + _ = FastApiMCP(app_with_non_get_endpoints, disable_warnings=True) + + # Check that no warning was logged + assert not any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) + + +def test_warn_if_auto_generated_operation_ids(app_with_auto_generated_ids, caplog): + """Test that warnings are issued for auto-generated operation IDs.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance + _ = FastApiMCP(app_with_auto_generated_ids) + + # Check that warning was logged for auto-generated IDs but not for explicit ones + assert any("auto_generated_route" in record.message for record in caplog.records) + assert any("auto_generated_route_get" in record.message for record in caplog.records) + assert not any("explicit_operation_id" in record.message for record in caplog.records) + assert any("To disable this warning" in record.message for record in caplog.records) + + +def test_warn_if_auto_generated_operation_ids_no_warning(app_with_auto_generated_ids, caplog): + """Test that no warning is issued when disable_warnings=True.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance with warnings disabled + _ = FastApiMCP(app_with_auto_generated_ids, disable_warnings=True) + + # Check that no warning was logged + assert not any("auto_generated_route" in record.message for record in caplog.records) + assert not any("auto_generated_route_get" in record.message for record in caplog.records) + + +def test_disable_all_warnings(app_with_too_many_tools, caplog): + """Test that all warnings can be disabled via the disable_warnings parameter.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create FastApiMCP instance with warnings disabled + _ = FastApiMCP(app_with_too_many_tools, disable_warnings=True) + + # Check that no warnings were logged + assert not any("More than 10 tools exposed" in record.message for record in caplog.records) + assert not any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) + assert not any("auto_generated_route" in record.message for record in caplog.records) + + +def test_integration_all_warnings(caplog): + """Test that all warnings are issued during server setup when needed.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create a FastAPI app with all warning scenarios + app = FastAPI() + router = APIRouter() + + # Auto-generated operation IDs + @router.get("/items/") + async def get_items(): + return {"items": []} + + # Non-GET endpoint + @router.post("/items/") + async def create_item(): + return {"message": "Item created"} + + # Add enough routes to trigger the "too many tools" warning + for i in range(10): + @router.get(f"/other-route-{i}/") + async def other_route_get(): + return {"message": "OK"} + + app.include_router(router) + + # Create FastApiMCP instance + _ = FastApiMCP(app) + + # Check that all warnings were logged + assert any("More than 10 tools exposed" in record.message for record in caplog.records) + assert any("Non-GET endpoints exposed as tools" in record.message for record in caplog.records) + assert any("create_item_items__post (POST)" in record.message for record in caplog.records) + assert any("appears to have an auto-generated operation_id" in record.message for record in caplog.records) + + +def test_integration_warnings_disabled(caplog): + """Test that warnings are not issued during server setup when disable_warnings=True.""" + # Set up logging capture + caplog.set_level(logging.WARNING) + + # Create a FastAPI app with all warning scenarios + app = FastAPI() + + # Auto-generated operation ID + @app.get("/items/") + async def get_items(): + return {"items": []} + + # Non-GET endpoint + @app.post("/items/") + async def create_item(): + return {"message": "Item created"} + + # Create FastApiMCP instance with warnings disabled + _ = FastApiMCP(app, disable_warnings=True) + + # Check that no warnings were logged + assert len(caplog.records) == 0 \ No newline at end of file