diff --git a/libs/arcade-mcp-server/arcade_mcp_server/__main__.py b/libs/arcade-mcp-server/arcade_mcp_server/__main__.py index 12cebf6b..15b1ebae 100644 --- a/libs/arcade-mcp-server/arcade_mcp_server/__main__.py +++ b/libs/arcade-mcp-server/arcade_mcp_server/__main__.py @@ -20,7 +20,6 @@ Usage: """ import asyncio -import logging import os import sys from pathlib import Path @@ -32,23 +31,11 @@ from arcade_core.toolkit import ToolkitLoadError from dotenv import load_dotenv from loguru import logger +from arcade_mcp_server.logging_utils import intercept_standard_logging from arcade_mcp_server.server import MCPServer from arcade_mcp_server.settings import MCPSettings -# Logging setup with Loguru -class LoguruInterceptHandler(logging.Handler): - """Intercept standard logging and route to Loguru.""" - - def emit(self, record: logging.LogRecord) -> None: - try: - level = logger.level(record.levelname).name - except ValueError: - level = str(record.levelno) - - logger.opt(exception=record.exc_info).log(level, record.getMessage()) - - def setup_logging(level: str = "INFO", stdio_mode: bool = False) -> None: """Configure logging with Loguru.""" # Remove existing handlers @@ -74,7 +61,7 @@ def setup_logging(level: str = "INFO", stdio_mode: bool = False) -> None: ) # Intercept standard logging - logging.basicConfig(handlers=[LoguruInterceptHandler()], level=0, force=True) + intercept_standard_logging() def initialize_tool_catalog( diff --git a/libs/arcade-mcp-server/arcade_mcp_server/logging_utils.py b/libs/arcade-mcp-server/arcade_mcp_server/logging_utils.py new file mode 100644 index 00000000..017fa28f --- /dev/null +++ b/libs/arcade-mcp-server/arcade_mcp_server/logging_utils.py @@ -0,0 +1,30 @@ +"""Shared logging utilities for MCP server.""" + +import logging + +from loguru import logger + + +class LoguruInterceptHandler(logging.Handler): + """Intercept standard logging and route to Loguru. + + This handler bridges the standard Python logging module with Loguru, + ensuring all logs (from both systems) use the same formatting. + """ + + def emit(self, record: logging.LogRecord) -> None: + try: + level = logger.level(record.levelname).name + except ValueError: + level = str(record.levelno) + + logger.opt(exception=record.exc_info).log(level, record.getMessage()) + + +def intercept_standard_logging() -> None: + """Configure standard logging to route through Loguru. + + This should be called after Loguru is configured to ensure all + standard logging calls are intercepted and formatted consistently. + """ + logging.basicConfig(handlers=[LoguruInterceptHandler()], level=0, force=True) diff --git a/libs/arcade-mcp-server/arcade_mcp_server/mcp_app.py b/libs/arcade-mcp-server/arcade_mcp_server/mcp_app.py index 72d035e4..d1e405b7 100644 --- a/libs/arcade-mcp-server/arcade_mcp_server/mcp_app.py +++ b/libs/arcade-mcp-server/arcade_mcp_server/mcp_app.py @@ -24,6 +24,7 @@ from loguru import logger from watchfiles import watch from arcade_mcp_server.exceptions import ServerError +from arcade_mcp_server.logging_utils import intercept_standard_logging from arcade_mcp_server.server import MCPServer from arcade_mcp_server.settings import MCPSettings, ServerSettings from arcade_mcp_server.types import Prompt, PromptMessage, Resource @@ -217,6 +218,9 @@ class MCPApp: diagnose=(self.log_level == "DEBUG"), ) + # Intercept standard logging and route through Loguru + intercept_standard_logging() + def add_tool( self, func: Callable[P, T], diff --git a/libs/arcade-mcp-server/arcade_mcp_server/server.py b/libs/arcade-mcp-server/arcade_mcp_server/server.py index ff3e55b4..62b1f464 100644 --- a/libs/arcade-mcp-server/arcade_mcp_server/server.py +++ b/libs/arcade-mcp-server/arcade_mcp_server/server.py @@ -307,6 +307,10 @@ class MCPServer: await self._tool_manager.load_from_catalog(self._initial_catalog) except Exception: logger.exception("Failed to load tools from initial catalog") + + # Check for missing secrets and log warnings (only when worker routes are disabled) + await self._check_and_warn_missing_secrets() + await self._resource_manager.start() await self._prompt_manager.start() await self.lifespan_manager.startup() @@ -654,6 +658,40 @@ class MCPServer: return tool_context + async def _check_and_warn_missing_secrets(self) -> None: + """ + Check for missing tool secrets and log warnings. + + This method is called during server startup to provide early feedback + about missing configuration. It only runs when worker routes are disabled + (when ARCADE_WORKER_SECRET is not set), as worker routes receive secrets + with tool execution information. + """ + # Skip validation if worker routes are enabled + if self.settings.arcade.server_secret: + logger.debug("Skipping secret validation check - worker routes are enabled") + return + + # Get all available secrets from settings and environment + available_secrets = set(self.settings.tool_secrets().keys()) | set(os.environ.keys()) + + # Check each tool for missing secrets + managed_tools = await self._tool_manager.registry.list() + for managed_tool in managed_tools: + tool = managed_tool["materialized"] + if tool.definition.requirements and tool.definition.requirements.secrets: + missing_secrets = [] + for secret_requirement in tool.definition.requirements.secrets: + if secret_requirement.key not in available_secrets: + missing_secrets.append(secret_requirement.key) + + if missing_secrets: + secret_list = "', '".join(missing_secrets) + tool_name = tool.definition.name + logger.warning( + f"Tool '{tool_name}' declares secret(s) '{secret_list}' which is/are not set. It will return an error if called." + ) + async def _handle_call_tool( self, message: CallToolRequest, diff --git a/libs/arcade-mcp-server/pyproject.toml b/libs/arcade-mcp-server/pyproject.toml index 9aa9fb27..9917b4a4 100644 --- a/libs/arcade-mcp-server/pyproject.toml +++ b/libs/arcade-mcp-server/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "arcade-mcp-server" -version = "1.10.3" +version = "1.11.0" description = "Model Context Protocol (MCP) server framework for Arcade.dev" readme = "README.md" authors = [{ name = "Arcade.dev" }] diff --git a/libs/tests/arcade_mcp_server/test_server.py b/libs/tests/arcade_mcp_server/test_server.py index c8ba3a74..0ab9e716 100644 --- a/libs/tests/arcade_mcp_server/test_server.py +++ b/libs/tests/arcade_mcp_server/test_server.py @@ -2,9 +2,11 @@ import asyncio import contextlib -from unittest.mock import AsyncMock, Mock from typing import Annotated +from unittest.mock import AsyncMock, Mock + import pytest +from arcade_core.auth import OAuth2 from arcade_core.catalog import MaterializedTool, ToolMeta, create_func_models from arcade_core.errors import ToolRuntimeError from arcade_core.schema import ( @@ -20,7 +22,7 @@ from arcade_core.schema import ( ToolSecretRequirement, ValueSchema, ) -from arcade_core.auth import OAuth2 +from arcade_mcp_server import tool from arcade_mcp_server.middleware import Middleware from arcade_mcp_server.server import MCPServer from arcade_mcp_server.session import InitializationState @@ -35,7 +37,6 @@ from arcade_mcp_server.types import ( ListToolsResult, PingRequest, ) -from arcade_mcp_server import tool class TestMCPServer: @@ -998,7 +999,7 @@ class TestMCPServer: @tool(requires_secrets=["SECRET_KEY"]) def secret_tool_func(text: Annotated[str, "Input text"]) -> Annotated[str, "Secret text"]: """Secret tool function""" - return f"Secret" + return "Secret" input_model, output_model = create_func_models(secret_tool_func) meta = ToolMeta(module=secret_tool_func.__module__, toolkit="TestToolkit") @@ -1068,8 +1069,13 @@ class TestMCPServer: ), ) - @tool(requires_auth=OAuth2(id="test-provider", scopes=["test.scope"]), requires_secrets=["API_KEY"]) - def combined_tool_func(text: Annotated[str, "Input text"]) -> Annotated[str, "Combined text"]: + @tool( + requires_auth=OAuth2(id="test-provider", scopes=["test.scope"]), + requires_secrets=["API_KEY"], + ) + def combined_tool_func( + text: Annotated[str, "Input text"], + ) -> Annotated[str, "Combined text"]: """Combined tool function""" return f"Combined: {text}" @@ -1103,7 +1109,8 @@ class TestMCPServer: assert response.result.isError is True assert "HTTP transport" in response.result.structuredContent["message"] assert ( - "authorization or access to sensitive secrets" in response.result.structuredContent["message"] + "authorization or access to sensitive secrets" + in response.result.structuredContent["message"] ) @pytest.mark.asyncio @@ -1199,3 +1206,352 @@ class TestMCPServer: assert isinstance(response, JSONRPCResponse) assert isinstance(response.result, CallToolResult) assert response.result.isError is False + + +class TestMissingSecretsWarnings: + """Test startup warnings for missing tool secrets.""" + + @pytest.mark.asyncio + async def test_warns_missing_secrets_on_startup(self, tool_catalog, mcp_settings, caplog): + """Test that missing secrets trigger warnings during server startup.""" + import logging + + # Create tool definition with secret requirements + tool_def = ToolDefinition( + name="fetch_data", + fully_qualified_name="TestToolkit.fetch_data", + description="Fetch data from API.", + toolkit=ToolkitDefinition( + name="TestToolkit", description="Test toolkit", version="1.0.0" + ), + input=ToolInput( + parameters=[ + InputParameter( + name="query", + required=True, + description="Search query", + value_schema=ValueSchema(val_type="string"), + ) + ] + ), + output=ToolOutput(description="Result", value_schema=ValueSchema(val_type="string")), + requirements=ToolRequirements( + secrets=[ + ToolSecretRequirement(key="API_KEY", description="API Key"), + ToolSecretRequirement(key="SECRET_TOKEN", description="Secret Token"), + ] + ), + ) + + @tool + def fetch_data(query: Annotated[str, "Search query"]) -> Annotated[str, "Result"]: + """Fetch data from API.""" + return f"Data for {query}" + + # Add tool to catalog + + input_model, output_model = create_func_models(fetch_data) + meta = ToolMeta(module=fetch_data.__module__, toolkit="TestToolkit") + materialized = MaterializedTool( + tool=fetch_data, + definition=tool_def, + meta=meta, + input_model=input_model, + output_model=output_model, + ) + tool_catalog._tools[tool_def.get_fully_qualified_name()] = materialized + + # Clear any existing secrets from environment + import os + + old_api_key = os.environ.pop("API_KEY", None) + old_secret_token = os.environ.pop("SECRET_TOKEN", None) + + try: + # Ensure worker routes are disabled (no ARCADE_WORKER_SECRET) + mcp_settings.arcade.server_secret = None + + # Create and start server + with caplog.at_level(logging.WARNING): + server = MCPServer( + catalog=tool_catalog, + name="Test Server", + version="1.0.0", + settings=mcp_settings, + ) + await server.start() + + # Check for warning message + warning_messages = [ + rec.message for rec in caplog.records if rec.levelno == logging.WARNING + ] + + # Should have a warning about missing secrets + assert any("fetch_data" in msg and "API_KEY" in msg for msg in warning_messages), ( + f"Expected warning about missing API_KEY for fetch_data. Got: {warning_messages}" + ) + assert any( + "fetch_data" in msg and "SECRET_TOKEN" in msg for msg in warning_messages + ), ( + f"Expected warning about missing SECRET_TOKEN for fetch_data. Got: {warning_messages}" + ) + + await server.stop() + finally: + # Restore environment + if old_api_key is not None: + os.environ["API_KEY"] = old_api_key + if old_secret_token is not None: + os.environ["SECRET_TOKEN"] = old_secret_token + + @pytest.mark.asyncio + async def test_no_warning_when_secrets_present(self, tool_catalog, mcp_settings, caplog): + """Test that no warnings are shown when secrets are available.""" + import logging + + # Create tool definition with secret requirements + tool_def = ToolDefinition( + name="secure_tool", + fully_qualified_name="TestToolkit.secure_tool", + description="Secure tool.", + toolkit=ToolkitDefinition( + name="TestToolkit", description="Test toolkit", version="1.0.0" + ), + input=ToolInput( + parameters=[ + InputParameter( + name="data", + required=True, + description="Data", + value_schema=ValueSchema(val_type="string"), + ) + ] + ), + output=ToolOutput(description="Result", value_schema=ValueSchema(val_type="string")), + requirements=ToolRequirements( + secrets=[ToolSecretRequirement(key="PRESENT_KEY", description="Present Key")] + ), + ) + + @tool + def secure_tool(data: Annotated[str, "Data"]) -> Annotated[str, "Result"]: + """Secure tool.""" + return f"Processed {data}" + + # Add tool to catalog + + input_model, output_model = create_func_models(secure_tool) + meta = ToolMeta(module=secure_tool.__module__, toolkit="TestToolkit") + materialized = MaterializedTool( + tool=secure_tool, + definition=tool_def, + meta=meta, + input_model=input_model, + output_model=output_model, + ) + tool_catalog._tools[tool_def.get_fully_qualified_name()] = materialized + + # Set the secret in environment + import os + + old_value = os.environ.get("PRESENT_KEY") + os.environ["PRESENT_KEY"] = "test-value" + + try: + # Ensure worker routes are disabled + mcp_settings.arcade.server_secret = None + + # Create and start server + with caplog.at_level(logging.WARNING): + server = MCPServer( + catalog=tool_catalog, + name="Test Server", + version="1.0.0", + settings=mcp_settings, + ) + await server.start() + + # Check that no warning is logged for this tool + warning_messages = [ + rec.message for rec in caplog.records if rec.levelno == logging.WARNING + ] + assert not any( + "secure_tool" in msg and "PRESENT_KEY" in msg for msg in warning_messages + ), f"Should not warn about PRESENT_KEY when it's set. Got: {warning_messages}" + + await server.stop() + finally: + # Restore environment + if old_value is not None: + os.environ["PRESENT_KEY"] = old_value + else: + os.environ.pop("PRESENT_KEY", None) + + @pytest.mark.asyncio + async def test_no_warning_when_worker_routes_enabled(self, tool_catalog, mcp_settings, caplog): + """Test that warnings are skipped when worker routes are enabled.""" + import logging + + # Create tool definition with secret requirements + tool_def = ToolDefinition( + name="worker_tool", + fully_qualified_name="TestToolkit.worker_tool", + description="Worker tool.", + toolkit=ToolkitDefinition( + name="TestToolkit", description="Test toolkit", version="1.0.0" + ), + input=ToolInput( + parameters=[ + InputParameter( + name="param", + required=True, + description="Param", + value_schema=ValueSchema(val_type="string"), + ) + ] + ), + output=ToolOutput(description="Result", value_schema=ValueSchema(val_type="string")), + requirements=ToolRequirements( + secrets=[ToolSecretRequirement(key="WORKER_API_KEY", description="Worker API Key")] + ), + ) + + @tool + def worker_tool(param: Annotated[str, "Param"]) -> Annotated[str, "Result"]: + """Worker tool.""" + return f"Result: {param}" + + # Add tool to catalog + + input_model, output_model = create_func_models(worker_tool) + meta = ToolMeta(module=worker_tool.__module__, toolkit="TestToolkit") + materialized = MaterializedTool( + tool=worker_tool, + definition=tool_def, + meta=meta, + input_model=input_model, + output_model=output_model, + ) + tool_catalog._tools[tool_def.get_fully_qualified_name()] = materialized + + # Clear the secret from environment + import os + + old_value = os.environ.pop("WORKER_API_KEY", None) + + try: + # Enable worker routes by setting ARCADE_WORKER_SECRET + mcp_settings.arcade.server_secret = "test-worker-secret" + + # Create and start server + with caplog.at_level(logging.WARNING): + server = MCPServer( + catalog=tool_catalog, + name="Test Server", + version="1.0.0", + settings=mcp_settings, + ) + await server.start() + + # Check that no warning is logged (worker routes are enabled) + warning_messages = [ + rec.message for rec in caplog.records if rec.levelno == logging.WARNING + ] + assert not any( + "worker_tool" in msg and "WORKER_API_KEY" in msg for msg in warning_messages + ), f"Should not warn when worker routes are enabled. Got: {warning_messages}" + + await server.stop() + finally: + # Restore environment + if old_value is not None: + os.environ["WORKER_API_KEY"] = old_value + + @pytest.mark.asyncio + async def test_warning_format(self, tool_catalog, mcp_settings, caplog): + """Test that warnings use the expected format.""" + import logging + + # Create tool definition with secret requirement + tool_def = ToolDefinition( + name="format_test_tool", + fully_qualified_name="TestToolkit.format_test_tool", + description="Format test tool.", + toolkit=ToolkitDefinition( + name="TestToolkit", description="Test toolkit", version="1.0.0" + ), + input=ToolInput( + parameters=[ + InputParameter( + name="x", + required=True, + description="Input", + value_schema=ValueSchema(val_type="integer"), + ) + ] + ), + output=ToolOutput(description="Output", value_schema=ValueSchema(val_type="integer")), + requirements=ToolRequirements( + secrets=[ + ToolSecretRequirement(key="FORMAT_TEST_KEY", description="Format Test Key") + ] + ), + ) + + @tool + def format_test_tool(x: Annotated[int, "Input"]) -> Annotated[int, "Output"]: + """Format test tool.""" + return x * 2 + + # Add tool to catalog + input_model, output_model = create_func_models(format_test_tool) + meta = ToolMeta(module=format_test_tool.__module__, toolkit="TestToolkit") + materialized = MaterializedTool( + tool=format_test_tool, + definition=tool_def, + meta=meta, + input_model=input_model, + output_model=output_model, + ) + tool_catalog._tools[tool_def.get_fully_qualified_name()] = materialized + + # Clear the secret from environment + import os + + old_value = os.environ.pop("FORMAT_TEST_KEY", None) + + try: + # Ensure worker routes are disabled + mcp_settings.arcade.server_secret = None + + # Create and start server + with caplog.at_level(logging.WARNING): + server = MCPServer( + catalog=tool_catalog, + name="Test Server", + version="1.0.0", + settings=mcp_settings, + ) + await server.start() + + # Check warning format matches specification + warning_messages = [ + rec.message for rec in caplog.records if rec.levelno == logging.WARNING + ] + + # Find the warning for our tool + matching_warnings = [msg for msg in warning_messages if "format_test_tool" in msg] + assert len(matching_warnings) > 0, ( + f"Expected warning for format_test_tool. Got: {warning_messages}" + ) + + warning = matching_warnings[0] + # Check format: "⚠ Tool 'name' declares secret(s) 'KEY' which are not set" + assert "Tool 'format_test_tool'" in warning + assert "not set" in warning + + await server.stop() + finally: + # Restore environment + if old_value is not None: + os.environ["FORMAT_TEST_KEY"] = old_value