diff --git a/README.md b/README.md index 7d828e1..52de722 100644 --- a/README.md +++ b/README.md @@ -99,8 +99,17 @@ time_tools = client.agent_tools(servers=['time']) # Get tools from multiple servers, only if healthy subset_tools = client.agent_tools(servers=['time', 'fetch']) -# Get tools from all servers regardless of health (not recommended) -all_tools_unfiltered = client.agent_tools(check_health=False) +# Filter by tool names (cross-cutting) +math_tools = client.agent_tools(tools=['add', 'multiply']) + +# Filter by qualified tool names +specific = client.agent_tools(tools=['time__get_current_time']) + +# Combine server and tool filtering +filtered = client.agent_tools( + servers=['time', 'math'], + tools=['add', 'get_current_time'] +) agent_config = AgentConfig( tools=client.agent_tools(), @@ -113,6 +122,20 @@ response = agent.run("What is the current time in Tokyo?") print(response) ``` +> [!IMPORTANT] +> Generated functions are cached for performance. Once cached, subsequent calls to `agent_tools()` return +> the cached functions immediately without refetching schemas, regardless of filter parameters. +> Use `refresh_cache=True` or call `client.clear_agent_tools_cache()` to force regeneration when tool schemas have changed. + +```python +# Force refresh cache to get latest schemas +fresh_tools = client.agent_tools(refresh_cache=True) + +# Or clear cache manually and call again +client.clear_agent_tools_cache() +fresh_tools = client.agent_tools() +``` + ## Examples A working SDK examples are available in the `examples/` folder, @@ -145,9 +168,9 @@ client = McpdClient(api_endpoint="http://localhost:8090", api_key="optional-key" * `client.tools(server_name: str) -> list[dict]` - Returns the tool schema definitions for only the specified server. -* `client.agent_tools(servers: list[str] | None = None, *, check_health: bool = True) -> list[Callable]` - Returns a list of self-contained, callable functions suitable for agentic frameworks. By default, filters to healthy servers only. Use `servers` to filter by server names, or `check_health=False` to include all servers regardless of health. +* `client.agent_tools(servers: list[str] | None = None, tools: list[str] | None = None, *, refresh_cache: bool = False) -> list[Callable]` - Returns a list of self-contained, callable functions suitable for agentic frameworks. By default, filters to healthy servers only. Use `servers` to filter by server names, `tools` to filter by tool names (supports both raw names like `'add'` and prefixed names like `'time__get_current_time'`), or `refresh_cache=True` to force regeneration of cached functions. Functions are cached - subsequent calls return cached functions immediately without refetching schemas. -* `client.clear_agent_tools_cache()` - Clears cached generated callable functions that are created when calling agent_tools(). +* `client.clear_agent_tools_cache()` - Clears cached generated callable functions. Call this to force regeneration when tool schemas have changed. * `client.has_tool(server_name: str, tool_name: str) -> bool` - Checks if a specific tool exists on a given server. diff --git a/src/mcpd/dynamic_caller.py b/src/mcpd/dynamic_caller.py index 33f49bd..58f01dd 100644 --- a/src/mcpd/dynamic_caller.py +++ b/src/mcpd/dynamic_caller.py @@ -10,9 +10,11 @@ from __future__ import annotations +from collections.abc import Callable from typing import TYPE_CHECKING from .exceptions import ToolNotFoundError +from .function_builder import TOOL_SEPARATOR if TYPE_CHECKING: from .mcpd_client import McpdClient @@ -109,7 +111,7 @@ def __init__(self, client: McpdClient, server_name: str): self._client = client self._server_name = server_name - def __getattr__(self, tool_name: str) -> callable: + def __getattr__(self, tool_name: str) -> Callable: """Create a callable function for the specified tool. When you access an attribute on a ServerProxy (e.g., time_server.get_current_time), @@ -161,7 +163,7 @@ def tool_function(**kwargs): return self._client._perform_call(self._server_name, tool_name, kwargs) # Add metadata to help with debugging and introspection - tool_function.__name__ = f"{self._server_name}__{tool_name}" + tool_function.__name__ = f"{self._server_name}{TOOL_SEPARATOR}{tool_name}" tool_function.__qualname__ = f"ServerProxy.{tool_name}" return tool_function diff --git a/src/mcpd/function_builder.py b/src/mcpd/function_builder.py index e87b544..e44ab05 100644 --- a/src/mcpd/function_builder.py +++ b/src/mcpd/function_builder.py @@ -11,7 +11,7 @@ from __future__ import annotations import re -from types import FunctionType +from collections.abc import Callable from typing import TYPE_CHECKING, Any from .exceptions import McpdError, ValidationError @@ -20,6 +20,13 @@ if TYPE_CHECKING: from .mcpd_client import McpdClient +TOOL_SEPARATOR = "__" +"""Separator used between server name and tool name in qualified tool names. + +Format: `{serverName}{TOOL_SEPARATOR}{toolName}` +Example: "time__get_current_time" where "time" is server and "get_current_time" is tool. +""" + class FunctionBuilder: """Builds callable Python functions from MCP tool JSON schemas. @@ -88,18 +95,18 @@ def _function_name(self, server_name: str, schema_name: str) -> str: """Generate a unique function name from server and tool names. This method creates a qualified function name by combining the server name - and tool name with a double underscore separator. Both names are sanitized - using _safe_name() to ensure the result is a valid Python identifier. + and tool name with TOOL_SEPARATOR. Both names are sanitized using _safe_name() + to ensure the result is a valid Python identifier. - The double underscore convention helps distinguish the server and tool - components while maintaining uniqueness across the entire function namespace. + The separator helps distinguish the server and tool components while maintaining + uniqueness across the entire function namespace. Args: server_name: The name of the MCP server hosting the tool. schema_name: The name of the tool from the schema definition. Returns: - A qualified function name in the format "{safe_server}__{safe_tool}". + A qualified function name in the format "{safe_server}{TOOL_SEPARATOR}{safe_tool}". The result is guaranteed to be a valid Python identifier. Example: @@ -112,11 +119,11 @@ def _function_name(self, server_name: str, schema_name: str) -> str: Note: This naming convention allows the generated function to be introspected - to determine its originating server and tool names by splitting on '__'. + to determine its originating server and tool names by splitting on TOOL_SEPARATOR. """ - return f"{self._safe_name(server_name)}__{self._safe_name(schema_name)}" + return f"{self._safe_name(server_name)}{TOOL_SEPARATOR}{self._safe_name(schema_name)}" - def create_function_from_schema(self, schema: dict[str, Any], server_name: str) -> FunctionType: + def create_function_from_schema(self, schema: dict[str, Any], server_name: str) -> Callable[..., Any]: """Create a callable Python function from an MCP tool's JSON Schema definition. This method generates a self-contained, callable function that validates parameters @@ -143,6 +150,8 @@ def create_function_from_schema(self, schema: dict[str, Any], server_name: str) - Comprehensive docstring with parameter descriptions - Raises ValidationError for missing required parameters - Returns the tool's execution result via client._perform_call() + - Has _server_name attribute containing the originating server name + - Has _tool_name attribute containing the original tool name Raises: McpdError: If function compilation fails due to invalid schema, @@ -167,9 +176,9 @@ def create_function_from_schema(self, schema: dict[str, Any], server_name: str) Note: The generated function includes validation logic that checks for required parameters at runtime and builds a parameters dictionary for the API call. - The function is cached using a key of "{server_name}__{tool_name}". + The function is cached using a key of "{server_name}{TOOL_SEPARATOR}{tool_name}". """ - cache_key = f"{server_name}__{schema.get('name', '')}" + cache_key = f"{server_name}{TOOL_SEPARATOR}{schema.get('name', '')}" if cache_key in self._function_cache: cached_func = self._function_cache[cache_key] @@ -187,12 +196,19 @@ def create_function_from_schema(self, schema: dict[str, Any], server_name: str) created_function = namespace[function_name] created_function.__annotations__ = annotations + # Add metadata attributes. + created_function._server_name = server_name + created_function._tool_name = schema["name"] + # Cache the function creation details - def create_function_instance(annotations: dict[str, Any]) -> FunctionType: + def create_function_instance(annotations: dict[str, Any]) -> Callable[..., Any]: temp_namespace = namespace.copy() exec(compiled_code, temp_namespace) new_func = temp_namespace[function_name] new_func.__annotations__ = annotations.copy() + # Add metadata attributes to cached instances as well. + new_func._server_name = server_name + new_func._tool_name = schema["name"] return new_func self._function_cache[cache_key] = { @@ -601,3 +617,11 @@ def clear_cache(self) -> None: >>> func3 = builder.create_function_from_schema(schema, "server1") # Recompiles """ self._function_cache.clear() + + def get_cached_functions(self) -> list[Callable[..., Any]]: + """Get all cached functions. + + Returns: + List of all cached agent functions, or empty list if cache is empty. + """ + return [cached["create_function"](cached["annotations"]) for cached in self._function_cache.values()] diff --git a/src/mcpd/mcpd_client.py b/src/mcpd/mcpd_client.py index 6fbd929..579d805 100644 --- a/src/mcpd/mcpd_client.py +++ b/src/mcpd/mcpd_client.py @@ -13,7 +13,7 @@ from collections.abc import Callable from enum import Enum from functools import wraps -from typing import Any, ParamSpec, TypeVar +from typing import Any, ParamSpec, Protocol, TypeVar import requests from cachetools import TTLCache, cached @@ -28,12 +28,26 @@ TimeoutError, ToolExecutionError, ) -from .function_builder import FunctionBuilder +from .function_builder import TOOL_SEPARATOR, FunctionBuilder P = ParamSpec("P") R = TypeVar("R") +class _AgentFunction(Protocol): + """Protocol for generated agent functions with metadata. + + Internal type for functions created by FunctionBuilder. + Not exposed in public API to maintain compatibility with AI frameworks. + """ + + __name__: str + _server_name: str + _tool_name: str + + def __call__(self, *args: Any, **kwargs: Any) -> Any: ... + + class HealthStatus(Enum): """Enumeration of possible MCP server health statuses.""" @@ -367,8 +381,14 @@ def _get_tool_definitions(self, server_name: str) -> list[dict[str, Any]]: except requests.exceptions.RequestException as e: raise McpdError(f"Error listing tool definitions for server '{server_name}': {e}") from e - def agent_tools(self, servers: list[str] | None = None, *, check_health: bool = True) -> list[Callable[..., Any]]: - """Generate callable Python functions for all available tools, suitable for AI agents. + def agent_tools( + self, + servers: list[str] | None = None, + tools: list[str] | None = None, + *, + refresh_cache: bool = False, + ) -> list[Callable[..., Any]]: + """Generate callable Python functions for available tools, suitable for AI agents. This method queries servers and creates self-contained, deepcopy-safe functions that can be passed to agentic frameworks like any-agent, LangChain, or custom AI @@ -379,8 +399,11 @@ def agent_tools(self, servers: list[str] | None = None, *, check_health: bool = their health status before fetching tools. Unhealthy servers are silently skipped to ensure the method returns quickly without waiting for timeouts on failed servers. - The generated functions are cached for performance. Use clear_agent_tools_cache() - to force regeneration if servers or tools have changed. + Generated functions are cached for performance. Once cached, subsequent calls return + the cached functions immediately without refetching schemas from healthy servers, + regardless of filter parameters. + Use clear_agent_tools_cache() to clear the cache, or set refresh_cache to true + to force regeneration when tool schemas have changed. Args: servers: Optional list of server names to filter by. @@ -388,29 +411,33 @@ def agent_tools(self, servers: list[str] | None = None, *, check_health: bool = If specified, only tools from the listed servers are included. Non-existent server names are silently ignored. - check_health: Whether to filter to healthy servers only. - If True (default), only returns tools from servers with 'ok' status. - If False, returns tools from all servers regardless of health. - Most users should leave this as True for best performance. + tools: Optional list of tool names to filter by. Supports both: + - Raw tool names: 'get_current_time' (matches across all servers) + - Fully qualified tool names: with server prefix and separator, + e.g. 'time__get_current_time' (matches specific {server}__{tool}) + If None, returns all tools from selected servers. + Empty list returns no tools. + + refresh_cache: When true, clears the cache and fetches fresh tool schemas from healthy servers. + When false or undefined, returns cached functions if available. + Defaults to false. Returns: - A list of callable functions, one for each tool from healthy servers (if check_health=True, the default) - or all servers (if check_health=False). + A list of callable functions, one for each matching tool from healthy servers. Each function has the following attributes: - __name__: The tool's qualified name (e.g., "time__get_current_time") - __doc__: The tool's description - - _schema: The original JSON Schema - - _server_name: The server hosting this tool - - _tool_name: The tool's name + - _server_name: The server hosting this tool (original name) + - _tool_name: The tool's name (original name) Raises: ConnectionError: If unable to connect to the mcpd daemon. TimeoutError: If requests to the daemon time out. AuthenticationError: If API key authentication fails. - McpdError: If unable to retrieve server health status (when check_health=True) - or retrieve tool definitions or generate functions. + McpdError: If unable to retrieve server health status or generate functions. + Servers for which tool schemas cannot be retrieved will be ignored. - Example: + Examples: >>> from any_agent import AnyAgent, AgentConfig >>> from mcpd import McpdClient >>> @@ -424,8 +451,24 @@ def agent_tools(self, servers: list[str] | None = None, *, check_health: bool = >>> time_tools = client.agent_tools(servers=['time']) >>> subset_tools = client.agent_tools(servers=['time', 'fetch']) >>> - >>> # Get tools from all servers regardless of health (keyword-only argument) - >>> all_tools = client.agent_tools(check_health=False) + >>> # Filter by tool names (cross-cutting) + >>> math_tools = client.agent_tools(tools=['add', 'multiply']) + >>> + >>> # Filter by qualified tool names + >>> specific = client.agent_tools(tools=['time__get_current_time']) + >>> + >>> # Combine server and tool filtering + >>> filtered = client.agent_tools( + ... servers=['time', 'math'], + ... tools=['add', 'get_current_time'] + ... ) + >>> + >>> # Force refresh of cached functions + >>> all_tools = client.agent_tools(refresh_cache=True) + >>> + >>> # Inspect metadata + >>> tool = client.agent_tools()[0] + >>> print(f"{tool._server_name}.{tool._tool_name}") >>> >>> # Use with an AI agent framework >>> agent_config = AgentConfig( @@ -443,21 +486,47 @@ def agent_tools(self, servers: list[str] | None = None, *, check_health: bool = same authentication and endpoint configuration. They are thread-safe but may not be suitable for pickling due to the embedded client state. """ - agent_tools = [] + if refresh_cache: + self.clear_agent_tools_cache() + + all_tools = self._agent_tools() + + return self._filter_agent_tools(all_tools, servers, tools) + + def _agent_tools(self) -> list[_AgentFunction]: + """Get or build cached agent tool functions from all healthy servers. - # Determine which servers to use. - servers_to_use = self.servers() if servers is None else servers + This internal method manages the agent tools cache. On first call, it builds + the cache by fetching tools from all healthy servers. On subsequent calls, + it returns the cached functions immediately without refetching schemas. + + Returns: + List of callable agent functions. + Returns cached functions if available, otherwise builds and caches functions from all healthy servers. + + Raises: + ConnectionError: If unable to connect to the mcpd daemon. + TimeoutError: If requests to the daemon time out. + AuthenticationError: If API key authentication fails. + McpdError: If unable to retrieve server health status. - # Filter to healthy servers if requested (one HTTP call for all servers). - if check_health: - servers_to_use = self._get_healthy_servers(servers_to_use) + Note: + Servers for which tool schemas cannot be retrieved will be ignored. + When logging is enabled a warning will be logged for these servers. + """ + # Return cached functions if available. + cached_functions = self._function_builder.get_cached_functions() + if cached_functions: + return cached_functions - # Fetch tools from selected servers only (avoids fetching from unhealthy servers). - for server_name in servers_to_use: + agent_tools = [] + healthy_servers = self._get_healthy_servers(self.servers()) + for server_name in healthy_servers: try: tool_schemas = self.tools(server_name=server_name) - except (ServerNotFoundError, ServerUnhealthyError): - # Server doesn't exist or became unhealthy - skip silently. + except (ConnectionError, TimeoutError, AuthenticationError, ServerNotFoundError, McpdError): + # These servers were reported as healthy, so failures for schemas would be unexpected. + # TODO: self._logger.warn(f"Failed to retrieve tool schema for server '{name}'") # include exception continue for tool_schema in tool_schemas: @@ -479,21 +548,75 @@ def _get_healthy_servers(self, server_names: list[str]) -> list[str]: McpdError: If unable to retrieve server health information. Note: - This method silently skips servers that don't exist or have - unhealthy status (timeout, unreachable, unknown). + When logging is enabled a warning will be logged for servers that don't exist + or have an unhealthy status (timeout, unreachable, unknown). """ - if not server_names: - return [] - health_map = self.server_health() - healthy_servers = [ - name - for name in server_names - if name in health_map and HealthStatus.is_healthy(health_map[name].get("status")) - ] + def is_valid(name: str) -> bool: + health = health_map.get(name) + + if not health: + # TODO: self._logger.warn(f"Skipping non-existent server '{name}'") + return False + + return HealthStatus.is_healthy(health.get("status")) + # TODO: self._logger.warn(f"Skipping unhealthy server '{name}' with status '{status}'") + + return [name for name in server_names if is_valid(name)] + + def _matches_tool_filter(self, func: _AgentFunction, tools: list[str]) -> bool: + """Check if a tool matches the tool filter. + + Supports two formats: + - Raw tool name: "get_current_time" (matches tool name only) + - Fully qualified tool names: with server prefix and separator, + e.g. 'time__get_current_time' (matches specific {server}__{tool}) + + When a filter contains TOOL_SEPARATOR (__), it's checked as prefixed (exact match against func.__name__); + then falls back to raw match. This handles tools whose names contain TOOL_SEPARATOR. + + Args: + func: The generated function to check. + tools: List of tool names to match against. + + Returns: + True if the tool matches any item in the filter. + """ + return any( + filter_item == func._tool_name + if TOOL_SEPARATOR not in filter_item + else filter_item == func.__name__ or filter_item == func._tool_name + for filter_item in tools + ) + + def _filter_agent_tools( + self, + functions: list[_AgentFunction], + servers: list[str] | None, + tools: list[str] | None, + ) -> list[_AgentFunction]: + """Filter agent tools by servers and/or tool names. + + Args: + functions: List of agent functions to filter. + servers: Optional list of server names to filter by. + tools: Optional list of tool names to filter by. + + Returns: + Filtered list of functions matching the criteria. + """ + result = functions + + # Filter by servers if specified. + if servers is not None: + result = [func for func in result if func._server_name in servers] + + # Filter by tools if specified. + if tools is not None: + result = [func for func in result if self._matches_tool_filter(func, tools)] - return healthy_servers + return result def has_tool(self, server_name: str, tool_name: str) -> bool: """Check if a specific tool exists on a given server. diff --git a/tests/unit/test_function_builder.py b/tests/unit/test_function_builder.py index b17274b..bcb79cc 100644 --- a/tests/unit/test_function_builder.py +++ b/tests/unit/test_function_builder.py @@ -141,6 +141,9 @@ def test_create_function_from_schema_caching(self, function_builder): # Should be different instances but same functionality assert func1 is not func2 assert func1.__name__ == func2.__name__ + # Cached instances should preserve metadata attributes. + assert func1._server_name == func2._server_name == "test_server" + assert func1._tool_name == func2._tool_name == "test_tool" def test_create_annotations_basic_types(self, function_builder): schema = { @@ -245,3 +248,33 @@ def test_create_function_compilation_error(self, function_builder): # Should not raise an error due to name sanitization func = function_builder.create_function_from_schema(schema, "test-server") assert func.__name__ == "test_server__test_tool" + + def test_function_has_server_name_attribute(self, function_builder): + """Test that generated function has _server_name attribute.""" + schema = {"name": "test_tool", "description": "Test", "inputSchema": {}} + + func = function_builder.create_function_from_schema(schema, "test_server") + + assert hasattr(func, "_server_name") + assert func._server_name == "test_server" + + def test_function_has_tool_name_attribute(self, function_builder): + """Test that generated function has _tool_name attribute.""" + schema = {"name": "test_tool", "description": "Test", "inputSchema": {}} + + func = function_builder.create_function_from_schema(schema, "test_server") + + assert hasattr(func, "_tool_name") + assert func._tool_name == "test_tool" + + def test_metadata_attributes_with_special_characters(self, function_builder): + """Test metadata attributes when names contain special characters.""" + schema = {"name": "test-tool.v2", "description": "Test", "inputSchema": {}} + + func = function_builder.create_function_from_schema(schema, "test-server") + + # Function name should be sanitized. + assert func.__name__ == "test_server__test_tool_v2" + # But metadata should have original names. + assert func._server_name == "test-server" + assert func._tool_name == "test-tool.v2" diff --git a/tests/unit/test_mcpd_client.py b/tests/unit/test_mcpd_client.py index 629b8ad..51dd5d0 100644 --- a/tests/unit/test_mcpd_client.py +++ b/tests/unit/test_mcpd_client.py @@ -154,10 +154,14 @@ def test_agent_tools(self, mock_health, mock_tools, mock_servers, client, tools_ assert result == [mock_func1, mock_func2] assert mock_create.call_count == 2 + @patch.object(McpdClient, "servers") @patch.object(McpdClient, "tools") @patch.object(McpdClient, "server_health") - def test_agent_tools_filter_by_single_server(self, mock_health, mock_tools, client, tools_side_effect): + def test_agent_tools_filter_by_single_server( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): """Test filtering tools by a single server name.""" + mock_servers.return_value = ["server1", "server2"] mock_tools.side_effect = tools_side_effect( { "server1": [{"name": "tool1", "description": "Test tool"}], @@ -172,18 +176,27 @@ def test_agent_tools_filter_by_single_server(self, mock_health, mock_tools, clie with patch.object(client._function_builder, "create_function_from_schema") as mock_create: mock_func1 = Mock() - mock_create.return_value = mock_func1 + mock_func1._server_name = "server1" + mock_func1._tool_name = "tool1" + mock_func2 = Mock() + mock_func2._server_name = "server2" + mock_func2._tool_name = "tool2" + mock_create.side_effect = [mock_func1, mock_func2] result = client.agent_tools(servers=["server1"]) + # Cache is built from both servers, but filtered to only server1. assert result == [mock_func1] - assert mock_create.call_count == 1 - mock_create.assert_called_once_with({"name": "tool1", "description": "Test tool"}, "server1") + assert mock_create.call_count == 2 + @patch.object(McpdClient, "servers") @patch.object(McpdClient, "tools") @patch.object(McpdClient, "server_health") - def test_agent_tools_filter_by_multiple_servers(self, mock_health, mock_tools, client, tools_side_effect): + def test_agent_tools_filter_by_multiple_servers( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): """Test filtering tools by multiple server names.""" + mock_servers.return_value = ["server1", "server2", "server3"] mock_tools.side_effect = tools_side_effect( { "server1": [{"name": "tool1", "description": "Test tool"}], @@ -200,40 +213,79 @@ def test_agent_tools_filter_by_multiple_servers(self, mock_health, mock_tools, c with patch.object(client._function_builder, "create_function_from_schema") as mock_create: mock_func1 = Mock() + mock_func1._server_name = "server1" + mock_func1._tool_name = "tool1" mock_func2 = Mock() - mock_create.side_effect = [mock_func1, mock_func2] + mock_func2._server_name = "server2" + mock_func2._tool_name = "tool2" + mock_func3 = Mock() + mock_func3._server_name = "server3" + mock_func3._tool_name = "tool3" + mock_create.side_effect = [mock_func1, mock_func2, mock_func3] result = client.agent_tools(servers=["server1", "server2"]) + # Cache is built from all 3 servers, but filtered to only server1 and server2. assert result == [mock_func1, mock_func2] - assert mock_create.call_count == 2 + assert mock_create.call_count == 3 + @patch.object(McpdClient, "servers") @patch.object(McpdClient, "tools") - def test_agent_tools_with_nonexistent_server(self, mock_tools, client, tools_side_effect): + @patch.object(McpdClient, "server_health") + def test_agent_tools_with_nonexistent_server( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): """Test filtering with server that doesn't exist.""" + mock_servers.return_value = ["server1"] mock_tools.side_effect = tools_side_effect( { "server1": [{"name": "tool1", "description": "Test tool"}], } ) + mock_health.return_value = { + "server1": {"status": "ok"}, + } with patch.object(client._function_builder, "create_function_from_schema") as mock_create: - result = client.agent_tools(servers=["nonexistent"], check_health=False) + mock_func1 = Mock() + mock_func1._server_name = "server1" + mock_func1._tool_name = "tool1" + mock_create.return_value = mock_func1 + result = client.agent_tools(servers=["nonexistent"]) + + # Cache is built from server1, but filtered to nonexistent server. assert result == [] - assert mock_create.call_count == 0 + assert mock_create.call_count == 1 + @patch.object(McpdClient, "servers") @patch.object(McpdClient, "tools") @patch.object(McpdClient, "server_health") - def test_agent_tools_with_empty_servers_list(self, mock_health, mock_tools, client): + def test_agent_tools_with_empty_servers_list( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): """Test filtering with empty server list.""" + mock_servers.return_value = ["server1"] + mock_tools.side_effect = tools_side_effect( + { + "server1": [{"name": "tool1", "description": "Test tool"}], + } + ) + mock_health.return_value = { + "server1": {"status": "ok"}, + } + with patch.object(client._function_builder, "create_function_from_schema") as mock_create: + mock_func1 = Mock() + mock_func1._server_name = "server1" + mock_func1._tool_name = "tool1" + mock_create.return_value = mock_func1 + result = client.agent_tools(servers=[]) + # Cache is built from all servers, but filtered to empty list. assert result == [] - mock_health.assert_not_called() - mock_tools.assert_not_called() - mock_create.assert_not_called() + assert mock_create.call_count == 1 @patch.object(McpdClient, "servers") @patch.object(McpdClient, "tools") @@ -296,30 +348,6 @@ def test_agent_tools_filters_by_health_default( assert mock_create.call_count == 1 mock_create.assert_called_with({"name": "tool1", "description": "Tool 1"}, "healthy_server") - @patch.object(McpdClient, "servers") - @patch.object(McpdClient, "tools") - def test_agent_tools_no_health_check_when_disabled(self, mock_tools, mock_servers, client, tools_side_effect): - """Test that health checking can be disabled.""" - mock_servers.return_value = ["server1", "server2"] - mock_tools.side_effect = tools_side_effect( - { - "server1": [{"name": "tool1", "description": "Tool 1"}], - "server2": [{"name": "tool2", "description": "Tool 2"}], - } - ) - - with ( - patch.object(client._function_builder, "create_function_from_schema") as mock_create, - patch.object(client, "server_health") as mock_health, - ): - mock_create.side_effect = [Mock(), Mock()] - - result = client.agent_tools(check_health=False) - - # Should include both tools without checking health. - assert len(result) == 2 - mock_health.assert_not_called() - @patch.object(McpdClient, "servers") @patch.object(McpdClient, "tools") @patch.object(McpdClient, "server_health") @@ -345,10 +373,14 @@ def test_agent_tools_all_servers_unhealthy(self, mock_health, mock_tools, mock_s assert result == [] mock_create.assert_not_called() + @patch.object(McpdClient, "servers") @patch.object(McpdClient, "tools") @patch.object(McpdClient, "server_health") - def test_agent_tools_with_servers_and_health_filtering(self, mock_health, mock_tools, client, tools_side_effect): + def test_agent_tools_with_servers_and_health_filtering( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): """Test combining server filtering and health filtering.""" + mock_servers.return_value = ["server1", "server2", "server3"] mock_tools.side_effect = tools_side_effect( { "server1": [{"name": "tool1", "description": "Tool 1"}], @@ -365,14 +397,20 @@ def test_agent_tools_with_servers_and_health_filtering(self, mock_health, mock_t with patch.object(client._function_builder, "create_function_from_schema") as mock_create: mock_func1 = Mock() - mock_create.return_value = mock_func1 + mock_func1._server_name = "server1" + mock_func1._tool_name = "tool1" + mock_func3 = Mock() + mock_func3._server_name = "server3" + mock_func3._tool_name = "tool3" + mock_create.side_effect = [mock_func1, mock_func3] # Request server1 and server2, but server2 is unhealthy. result = client.agent_tools(servers=["server1", "server2"]) - # Should only include tool from server1. + # Cache is built from server1 and server3 (server2 is unhealthy and skipped). + # Filtered to only server1 and server2, but server2 not in cache. assert result == [mock_func1] - assert mock_create.call_count == 1 + assert mock_create.call_count == 2 @patch.object(McpdClient, "servers") @patch.object(McpdClient, "tools") @@ -393,6 +431,317 @@ def test_agent_tools_health_check_exception(self, mock_health, mock_tools, mock_ with pytest.raises(Exception, match="Health check failed"): client.agent_tools() + @patch.object(McpdClient, "servers") + @patch.object(McpdClient, "tools") + @patch.object(McpdClient, "server_health") + def test_agent_tools_filter_by_raw_tool_name( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): + """Test filtering by raw tool name (cross-cutting).""" + mock_servers.return_value = ["math", "calc"] + mock_tools.side_effect = tools_side_effect( + { + "math": [ + {"name": "add", "description": "Add numbers"}, + {"name": "subtract", "description": "Subtract numbers"}, + ], + "calc": [ + {"name": "add", "description": "Calculator add"}, + {"name": "multiply", "description": "Multiply"}, + ], + } + ) + + mock_health.return_value = { + "math": {"status": "ok"}, + "calc": {"status": "ok"}, + } + + with patch.object(client._function_builder, "create_function_from_schema") as mock_create: + mock_func1 = Mock() + mock_func1.__name__ = "math__add" + mock_func1._tool_name = "add" + mock_func2 = Mock() + mock_func2.__name__ = "calc__add" + mock_func2._tool_name = "add" + mock_func3 = Mock() + mock_func3.__name__ = "math__subtract" + mock_func3._tool_name = "subtract" + mock_func4 = Mock() + mock_func4.__name__ = "calc__multiply" + mock_func4._tool_name = "multiply" + mock_create.side_effect = [mock_func1, mock_func3, mock_func2, mock_func4] + + result = client.agent_tools(tools=["add"]) + + # Should include 'add' from ALL servers that have it. + assert len(result) == 2 + assert mock_create.call_count == 4 # All functions created, but only 2 match filter + + @patch.object(McpdClient, "servers") + @patch.object(McpdClient, "tools") + @patch.object(McpdClient, "server_health") + def test_agent_tools_filter_by_prefixed_tool_name( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): + """Test filtering by prefixed tool name (specific server+tool).""" + mock_servers.return_value = ["time", "other"] + mock_tools.side_effect = tools_side_effect( + { + "time": [{"name": "get_current_time", "description": "Get time"}], + "other": [{"name": "get_current_time", "description": "Other time"}], + } + ) + + mock_health.return_value = { + "time": {"status": "ok"}, + "other": {"status": "ok"}, + } + + with patch.object(client._function_builder, "create_function_from_schema") as mock_create: + mock_func1 = Mock() + mock_func1.__name__ = "time__get_current_time" + mock_func1._tool_name = "get_current_time" + mock_func2 = Mock() + mock_func2.__name__ = "other__get_current_time" + mock_func2._tool_name = "get_current_time" + mock_create.side_effect = [mock_func1, mock_func2] + + result = client.agent_tools(tools=["time__get_current_time"]) + + # Should include only the exact match. + assert len(result) == 1 + assert result[0].__name__ == "time__get_current_time" + assert mock_create.call_count == 2 # Both created, only one matches + + @patch.object(McpdClient, "servers") + @patch.object(McpdClient, "tools") + @patch.object(McpdClient, "server_health") + def test_agent_tools_filter_by_mixed_formats( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): + """Test filtering with both raw and prefixed names.""" + mock_servers.return_value = ["math", "time"] + mock_tools.side_effect = tools_side_effect( + { + "math": [{"name": "add", "description": "Add"}, {"name": "subtract", "description": "Subtract"}], + "time": [{"name": "get_current_time", "description": "Get time"}], + } + ) + + mock_health.return_value = { + "math": {"status": "ok"}, + "time": {"status": "ok"}, + } + + with patch.object(client._function_builder, "create_function_from_schema") as mock_create: + mock_func1 = Mock() + mock_func1.__name__ = "math__add" + mock_func1._tool_name = "add" + mock_func2 = Mock() + mock_func2.__name__ = "math__subtract" + mock_func2._tool_name = "subtract" + mock_func3 = Mock() + mock_func3.__name__ = "time__get_current_time" + mock_func3._tool_name = "get_current_time" + mock_create.side_effect = [mock_func1, mock_func2, mock_func3] + + result = client.agent_tools(tools=["add", "time__get_current_time"]) + + # Should include all 'add' tools plus the specific time tool. + assert len(result) == 2 + assert mock_create.call_count == 3 # All created, only 2 match + + @patch.object(McpdClient, "servers") + @patch.object(McpdClient, "tools") + @patch.object(McpdClient, "server_health") + def test_agent_tools_filter_by_servers_and_tools( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): + """Test combining server and tool filtering.""" + mock_servers.return_value = ["time", "math", "fetch"] + mock_tools.side_effect = tools_side_effect( + { + "time": [{"name": "get_current_time", "description": "Get time"}], + "math": [{"name": "add", "description": "Add"}], + "fetch": [{"name": "fetch_url", "description": "Fetch"}], + } + ) + + mock_health.return_value = { + "time": {"status": "ok"}, + "math": {"status": "ok"}, + "fetch": {"status": "ok"}, + } + + with patch.object(client._function_builder, "create_function_from_schema") as mock_create: + mock_func1 = Mock() + mock_func1.__name__ = "time__get_current_time" + mock_func1._server_name = "time" + mock_func1._tool_name = "get_current_time" + mock_func2 = Mock() + mock_func2.__name__ = "math__add" + mock_func2._server_name = "math" + mock_func2._tool_name = "add" + mock_func3 = Mock() + mock_func3.__name__ = "fetch__fetch_url" + mock_func3._server_name = "fetch" + mock_func3._tool_name = "fetch_url" + mock_create.side_effect = [mock_func1, mock_func2, mock_func3] + + result = client.agent_tools(servers=["time", "math"], tools=["add", "get_current_time"]) + + # Cache is built from all 3 servers, filtered by both servers and tools. + assert len(result) == 2 + assert set(result) == {mock_func1, mock_func2} + assert mock_create.call_count == 3 + + @patch.object(McpdClient, "servers") + @patch.object(McpdClient, "tools") + @patch.object(McpdClient, "server_health") + def test_agent_tools_filter_with_nonexistent_tool( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): + """Test filtering with tool that doesn't exist.""" + mock_servers.return_value = ["time"] + mock_tools.side_effect = tools_side_effect( + { + "time": [{"name": "get_current_time", "description": "Get time"}], + } + ) + + mock_health.return_value = { + "time": {"status": "ok"}, + } + + with patch.object(client._function_builder, "create_function_from_schema") as mock_create: + mock_func = Mock() + mock_func.__name__ = "time__get_current_time" + mock_func._tool_name = "get_current_time" + mock_create.return_value = mock_func + + result = client.agent_tools(tools=["nonexistent_tool"]) + + # Should return empty list (function created but filtered out). + assert result == [] + assert mock_create.call_count == 1 + + @patch.object(McpdClient, "servers") + @patch.object(McpdClient, "tools") + @patch.object(McpdClient, "server_health") + def test_agent_tools_filter_with_empty_tools_list( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): + """Test filtering with empty tool list.""" + mock_servers.return_value = ["time"] + mock_tools.side_effect = tools_side_effect( + { + "time": [{"name": "get_current_time", "description": "Get time"}], + } + ) + + mock_health.return_value = { + "time": {"status": "ok"}, + } + + with patch.object(client._function_builder, "create_function_from_schema") as mock_create: + mock_func = Mock() + mock_func.__name__ = "time__get_current_time" + mock_func._tool_name = "get_current_time" + mock_create.return_value = mock_func + + result = client.agent_tools(tools=[]) + + # Should return empty list (function created but empty filter matches nothing). + assert result == [] + assert mock_create.call_count == 1 + + @patch.object(McpdClient, "servers") + @patch.object(McpdClient, "tools") + @patch.object(McpdClient, "server_health") + def test_agent_tools_handles_tool_name_with_double_underscore( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): + """Test that tool names containing __ are handled correctly.""" + mock_servers.return_value = ["test"] + mock_tools.side_effect = tools_side_effect( + { + "test": [{"name": "my__special__tool", "description": "Special tool"}], + } + ) + + mock_health.return_value = { + "test": {"status": "ok"}, + } + + with patch.object(client._function_builder, "create_function_from_schema") as mock_create: + mock_func = Mock() + mock_func.__name__ = "test__my__special__tool" + mock_func._tool_name = "my__special__tool" + mock_create.return_value = mock_func + + # Filter by raw name (should work). + result = client.agent_tools(tools=["my__special__tool"]) + assert len(result) == 1 + assert mock_create.call_count == 1 + + # Reset mock. + mock_create.reset_mock() + mock_create.return_value = mock_func + + # Filter by prefixed name (should work). + result = client.agent_tools(tools=["test__my__special__tool"]) + assert len(result) == 1 + assert mock_create.call_count == 1 + + @patch.object(McpdClient, "servers") + @patch.object(McpdClient, "tools") + @patch.object(McpdClient, "server_health") + def test_agent_tools_cache_hit_skips_discovery(self, mock_health, mock_tools, mock_servers, client): + """Test that cache hit returns cached functions without calling discovery methods.""" + # Create mock functions with required metadata. + mock_func1 = Mock() + mock_func1.__name__ = "time__get_current_time" + mock_func1._server_name = "time" + mock_func1._tool_name = "get_current_time" + + mock_func2 = Mock() + mock_func2.__name__ = "math__add" + mock_func2._server_name = "math" + mock_func2._tool_name = "add" + + # Create factory functions for the cache. + def create_time_func(annotations): + return mock_func1 + + def create_math_func(annotations): + return mock_func2 + + # Pre-populate the cache. + client._function_builder._function_cache["time__get_current_time"] = { + "compiled_code": None, + "annotations": {}, + "create_function": create_time_func, + } + client._function_builder._function_cache["math__add"] = { + "compiled_code": None, + "annotations": {}, + "create_function": create_math_func, + } + + # Call agent_tools - should return cached functions without calling discovery methods. + result = client.agent_tools() + + # Should return cached functions. + assert len(result) == 2 + assert mock_func1 in result + assert mock_func2 in result + + # Should NOT call any discovery methods. + mock_servers.assert_not_called() + mock_tools.assert_not_called() + mock_health.assert_not_called() + @patch.object(McpdClient, "tools") def test_has_tool_exists(self, mock_tools, client): mock_tools.return_value = [{"name": "existing_tool"}, {"name": "another_tool"}]