diff --git a/README.md b/README.md index aa76292..7d828e1 100644 --- a/README.md +++ b/README.md @@ -90,15 +90,18 @@ from mcpd import McpdClient # Assumes the mcpd daemon is running client = McpdClient(api_endpoint="http://localhost:8090") -# Get all tools from all servers +# Get all tools from healthy servers (default - filters out unhealthy servers) all_tools = client.agent_tools() -# Get tools from specific servers only +# Get tools from specific servers, only if healthy time_tools = client.agent_tools(servers=['time']) -# Get tools from multiple servers +# 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) + agent_config = AgentConfig( tools=client.agent_tools(), model_id="gpt-4.1-nano", # Requires OPENAI_API_KEY to be set @@ -142,7 +145,7 @@ 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() -> list[Callable]` - Returns a list of self-contained, callable functions suitable for agentic frameworks. +* `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.clear_agent_tools_cache()` - Clears cached generated callable functions that are created when calling agent_tools(). diff --git a/src/mcpd/mcpd_client.py b/src/mcpd/mcpd_client.py index 2795483..6fbd929 100644 --- a/src/mcpd/mcpd_client.py +++ b/src/mcpd/mcpd_client.py @@ -367,13 +367,17 @@ 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) -> list[Callable[..., Any]]: + 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. - This method queries all servers via `tools()` and creates self-contained, - deepcopy-safe functions that can be passed to agentic frameworks like any-agent, - LangChain, or custom AI systems. Each function includes its schema as metadata - and handles the MCP communication internally. + 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 + systems. Each function includes its schema as metadata and handles the MCP + communication internally. + + By default, this method automatically filters out unhealthy servers by checking + 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. @@ -384,8 +388,14 @@ def agent_tools(self, servers: list[str] | None = None) -> list[Callable[..., An 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. + Returns: - A list of callable functions, one for each tool across all servers. + 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). Each function has the following attributes: - __name__: The tool's qualified name (e.g., "time__get_current_time") - __doc__: The tool's description @@ -397,8 +407,8 @@ def agent_tools(self, servers: list[str] | None = None) -> list[Callable[..., An ConnectionError: If unable to connect to the mcpd daemon. TimeoutError: If requests to the daemon time out. AuthenticationError: If API key authentication fails. - ServerNotFoundError: If a server becomes unavailable during tool retrieval. - McpdError: If unable to retrieve tool definitions or generate functions. + McpdError: If unable to retrieve server health status (when check_health=True) + or retrieve tool definitions or generate functions. Example: >>> from any_agent import AnyAgent, AgentConfig @@ -406,14 +416,17 @@ def agent_tools(self, servers: list[str] | None = None) -> list[Callable[..., An >>> >>> client = McpdClient(api_endpoint="http://localhost:8090") >>> - >>> # Get all tools as callable functions + >>> # Get all tools from healthy servers (default) >>> tools = client.agent_tools() >>> print(f"Generated {len(tools)} callable tools") >>> - >>> # Get tools from specific servers only + >>> # Get tools from specific servers, only if healthy >>> 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) + >>> >>> # Use with an AI agent framework >>> agent_config = AgentConfig( ... tools=tools, @@ -431,24 +444,57 @@ def agent_tools(self, servers: list[str] | None = None) -> list[Callable[..., An but may not be suitable for pickling due to the embedded client state. """ agent_tools = [] - all_tools = self.tools() # Determine which servers to use. - servers_to_use = all_tools.keys() if servers is None else servers + servers_to_use = self.servers() if servers is None else servers - # Fetch tools from selected servers. + # 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) + + # Fetch tools from selected servers only (avoids fetching from unhealthy servers). for server_name in servers_to_use: - if server_name not in all_tools: - # Server doesn't exist or has no tools - skip silently. + try: + tool_schemas = self.tools(server_name=server_name) + except (ServerNotFoundError, ServerUnhealthyError): + # Server doesn't exist or became unhealthy - skip silently. continue - tool_schemas = all_tools[server_name] for tool_schema in tool_schemas: func = self._function_builder.create_function_from_schema(tool_schema, server_name) agent_tools.append(func) return agent_tools + def _get_healthy_servers(self, server_names: list[str]) -> list[str]: + """Filter server names to only those that are healthy. + + Args: + server_names: List of server names to filter. + + Returns: + List of server names that have health status 'ok'. + + Raises: + 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). + """ + 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")) + ] + + return healthy_servers + 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/conftest.py b/tests/conftest.py index 2a7a6b9..ae3af10 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,4 @@ +from collections.abc import Callable from unittest.mock import Mock import pytest @@ -55,3 +56,28 @@ def client(fqdn): @pytest.fixture(scope="function") def client_with_auth(fqdn): return McpdClient(api_endpoint=fqdn, api_key="test-key") # pragma: allowlist secret + + +@pytest.fixture +def tools_side_effect(): + """Factory for creating tools() mock side effects. + + Returns a function that creates side_effect functions for mocking tools(). + The side_effect returns the appropriate tool list based on server_name parameter. + + Usage: + def test_something(tools_side_effect): + tools_map = { + "server1": [{"name": "tool1", "description": "Tool 1"}], + "server2": [{"name": "tool2", "description": "Tool 2"}], + } + mock_tools.side_effect = tools_side_effect(tools_map) + """ + + def _create_side_effect(tools_map: dict[str, list[dict]]) -> Callable[[str | None], list[dict]]: + def side_effect(server_name: str | None = None) -> list[dict]: + return tools_map.get(server_name, []) + + return side_effect + + return _create_side_effect diff --git a/tests/unit/test_mcpd_client.py b/tests/unit/test_mcpd_client.py index 8d819a1..629b8ad 100644 --- a/tests/unit/test_mcpd_client.py +++ b/tests/unit/test_mcpd_client.py @@ -126,11 +126,22 @@ def test_perform_call_request_error(self, mock_post, client): with pytest.raises(McpdError, match="Error calling tool 'test_tool' on server 'test_server'"): client._perform_call("test_server", "test_tool", {"param": "value"}) + @patch.object(McpdClient, "servers") @patch.object(McpdClient, "tools") - def test_agent_tools(self, mock_tools, client): - mock_tools.return_value = { - "server1": [{"name": "tool1", "description": "Test tool"}], - "server2": [{"name": "tool2", "description": "Another tool"}], + @patch.object(McpdClient, "server_health") + def test_agent_tools(self, mock_health, mock_tools, mock_servers, client, tools_side_effect): + mock_servers.return_value = ["server1", "server2"] + + mock_tools.side_effect = tools_side_effect( + { + "server1": [{"name": "tool1", "description": "Test tool"}], + "server2": [{"name": "tool2", "description": "Another tool"}], + } + ) + + mock_health.return_value = { + "server1": {"status": "ok"}, + "server2": {"status": "ok"}, } with patch.object(client._function_builder, "create_function_from_schema") as mock_create: @@ -144,11 +155,19 @@ def test_agent_tools(self, mock_tools, client): assert mock_create.call_count == 2 @patch.object(McpdClient, "tools") - def test_agent_tools_filter_by_single_server(self, mock_tools, client): + @patch.object(McpdClient, "server_health") + def test_agent_tools_filter_by_single_server(self, mock_health, mock_tools, client, tools_side_effect): """Test filtering tools by a single server name.""" - mock_tools.return_value = { - "server1": [{"name": "tool1", "description": "Test tool"}], - "server2": [{"name": "tool2", "description": "Another tool"}], + mock_tools.side_effect = tools_side_effect( + { + "server1": [{"name": "tool1", "description": "Test tool"}], + "server2": [{"name": "tool2", "description": "Another tool"}], + } + ) + + mock_health.return_value = { + "server1": {"status": "ok"}, + "server2": {"status": "ok"}, } with patch.object(client._function_builder, "create_function_from_schema") as mock_create: @@ -162,12 +181,21 @@ def test_agent_tools_filter_by_single_server(self, mock_tools, client): mock_create.assert_called_once_with({"name": "tool1", "description": "Test tool"}, "server1") @patch.object(McpdClient, "tools") - def test_agent_tools_filter_by_multiple_servers(self, mock_tools, client): + @patch.object(McpdClient, "server_health") + def test_agent_tools_filter_by_multiple_servers(self, mock_health, mock_tools, client, tools_side_effect): """Test filtering tools by multiple server names.""" - mock_tools.return_value = { - "server1": [{"name": "tool1", "description": "Test tool"}], - "server2": [{"name": "tool2", "description": "Another tool"}], - "server3": [{"name": "tool3", "description": "Third tool"}], + mock_tools.side_effect = tools_side_effect( + { + "server1": [{"name": "tool1", "description": "Test tool"}], + "server2": [{"name": "tool2", "description": "Another tool"}], + "server3": [{"name": "tool3", "description": "Third tool"}], + } + ) + + mock_health.return_value = { + "server1": {"status": "ok"}, + "server2": {"status": "ok"}, + "server3": {"status": "ok"}, } with patch.object(client._function_builder, "create_function_from_schema") as mock_create: @@ -181,37 +209,50 @@ def test_agent_tools_filter_by_multiple_servers(self, mock_tools, client): assert mock_create.call_count == 2 @patch.object(McpdClient, "tools") - def test_agent_tools_with_nonexistent_server(self, mock_tools, client): + def test_agent_tools_with_nonexistent_server(self, mock_tools, client, tools_side_effect): """Test filtering with server that doesn't exist.""" - mock_tools.return_value = { - "server1": [{"name": "tool1", "description": "Test tool"}], - } + mock_tools.side_effect = tools_side_effect( + { + "server1": [{"name": "tool1", "description": "Test tool"}], + } + ) with patch.object(client._function_builder, "create_function_from_schema") as mock_create: - result = client.agent_tools(servers=["nonexistent"]) + result = client.agent_tools(servers=["nonexistent"], check_health=False) assert result == [] assert mock_create.call_count == 0 @patch.object(McpdClient, "tools") - def test_agent_tools_with_empty_servers_list(self, mock_tools, client): + @patch.object(McpdClient, "server_health") + def test_agent_tools_with_empty_servers_list(self, mock_health, mock_tools, client): """Test filtering with empty server list.""" - mock_tools.return_value = { - "server1": [{"name": "tool1", "description": "Test tool"}], - } - with patch.object(client._function_builder, "create_function_from_schema") as mock_create: result = client.agent_tools(servers=[]) assert result == [] - assert mock_create.call_count == 0 + mock_health.assert_not_called() + mock_tools.assert_not_called() + mock_create.assert_not_called() + @patch.object(McpdClient, "servers") @patch.object(McpdClient, "tools") - def test_agent_tools_without_servers_parameter(self, mock_tools, client): + @patch.object(McpdClient, "server_health") + def test_agent_tools_without_servers_parameter( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): """Test existing behavior - returns all tools when servers parameter not provided.""" - mock_tools.return_value = { - "server1": [{"name": "tool1", "description": "Test tool"}], - "server2": [{"name": "tool2", "description": "Another tool"}], + mock_servers.return_value = ["server1", "server2"] + mock_tools.side_effect = tools_side_effect( + { + "server1": [{"name": "tool1", "description": "Test tool"}], + "server2": [{"name": "tool2", "description": "Another tool"}], + } + ) + + mock_health.return_value = { + "server1": {"status": "ok"}, + "server2": {"status": "ok"}, } with patch.object(client._function_builder, "create_function_from_schema") as mock_create: @@ -224,6 +265,134 @@ def test_agent_tools_without_servers_parameter(self, mock_tools, client): 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_filters_by_health_default( + self, mock_health, mock_tools, mock_servers, client, tools_side_effect + ): + """Test that agent_tools filters to healthy servers by default.""" + mock_servers.return_value = ["healthy_server", "unhealthy_server"] + mock_tools.side_effect = tools_side_effect( + { + "healthy_server": [{"name": "tool1", "description": "Tool 1"}], + "unhealthy_server": [{"name": "tool2", "description": "Tool 2"}], + } + ) + + mock_health.return_value = { + "healthy_server": {"status": "ok"}, + "unhealthy_server": {"status": "timeout"}, + } + + with patch.object(client._function_builder, "create_function_from_schema") as mock_create: + mock_func = Mock() + mock_create.return_value = mock_func + + result = client.agent_tools() + + # Should only include tool from healthy server. + assert result == [mock_func] + 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") + def test_agent_tools_all_servers_unhealthy(self, mock_health, mock_tools, mock_servers, client, tools_side_effect): + """Test behavior when all servers are unhealthy.""" + 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"}], + } + ) + + mock_health.return_value = { + "server1": {"status": "timeout"}, + "server2": {"status": "unreachable"}, + } + + with patch.object(client._function_builder, "create_function_from_schema") as mock_create: + result = client.agent_tools() + + # Should return empty list. + assert result == [] + mock_create.assert_not_called() + + @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): + """Test combining server filtering and health filtering.""" + mock_tools.side_effect = tools_side_effect( + { + "server1": [{"name": "tool1", "description": "Tool 1"}], + "server2": [{"name": "tool2", "description": "Tool 2"}], + "server3": [{"name": "tool3", "description": "Tool 3"}], + } + ) + + mock_health.return_value = { + "server1": {"status": "ok"}, + "server2": {"status": "timeout"}, + "server3": {"status": "ok"}, + } + + with patch.object(client._function_builder, "create_function_from_schema") as mock_create: + mock_func1 = Mock() + mock_create.return_value = mock_func1 + + # Request server1 and server2, but server2 is unhealthy. + result = client.agent_tools(servers=["server1", "server2"]) + + # Should only include tool from server1. + assert result == [mock_func1] + assert mock_create.call_count == 1 + + @patch.object(McpdClient, "servers") + @patch.object(McpdClient, "tools") + @patch.object(McpdClient, "server_health") + def test_agent_tools_health_check_exception(self, mock_health, mock_tools, mock_servers, client, tools_side_effect): + """Test behavior when health check raises exception.""" + mock_servers.return_value = ["server1"] + mock_tools.side_effect = tools_side_effect( + { + "server1": [{"name": "tool1", "description": "Tool 1"}], + } + ) + + # Health check raises exception. + mock_health.side_effect = Exception("Health check failed") + + # Exception should propagate. + with pytest.raises(Exception, match="Health check failed"): + client.agent_tools() + @patch.object(McpdClient, "tools") def test_has_tool_exists(self, mock_tools, client): mock_tools.return_value = [{"name": "existing_tool"}, {"name": "another_tool"}]