Fix MCP tool bug by dropping unset parameters from input

python/packages/autogen-ext/pyproject.toml

@@ -135,7 +135,7 @@ semantic-kernel-all = [
 rich = ["rich>=13.9.4"]
 
 mcp = [
-    "mcp>=1.1.3",
+    "mcp>=1.5.0",
     "json-schema-to-pydantic>=0.2.2"
 ]
 

python/packages/autogen-ext/src/autogen_ext/models/azure/_azure_ai_client.py

@@ -603,11 +603,3 @@ def model_info(self) -> ModelInfo:
     @property
     def capabilities(self) -> ModelInfo:
         return self.model_info
-
-    def __del__(self) -> None:
-        # TODO: This is a hack to close the open client
-        if hasattr(self, "_client"):
-            try:
-                asyncio.get_running_loop().create_task(self._client.close())
-            except RuntimeError:
-                asyncio.run(self._client.close())

python/packages/autogen-ext/src/autogen_ext/models/openai/__init__.py

@@ -1,8 +1,8 @@
 from ._openai_client import (
+    AZURE_OPENAI_USER_AGENT,
     AzureOpenAIChatCompletionClient,
     BaseOpenAIChatCompletionClient,
     OpenAIChatCompletionClient,
-    AZURE_OPENAI_USER_AGENT,
 )
 from .config import (
     AzureOpenAIClientConfigurationConfigModel,

python/packages/autogen-ext/src/autogen_ext/models/openai/_openai_client.py

@@ -8,6 +8,7 @@
 import warnings
 from asyncio import Task
 from dataclasses import dataclass
+from importlib.metadata import PackageNotFoundError, version
 from typing import (
     Any,
     AsyncGenerator,
@@ -87,8 +88,6 @@
     OpenAIClientConfiguration,
     OpenAIClientConfigurationConfigModel,
 )
-from importlib.metadata import PackageNotFoundError, version
-
 
 logger = logging.getLogger(EVENT_LOGGER_NAME)
 trace_logger = logging.getLogger(TRACE_LOGGER_NAME)

python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py

@@ -1,3 +1,5 @@
+import asyncio
+import builtins
 from abc import ABC
 from typing import Any, Generic, Type, TypeVar
 
@@ -54,7 +56,10 @@
         Raises:
             Exception: If the operation is cancelled or the tool execution fails.
         """
-        kwargs = args.model_dump()
+        # Convert the input model to a dictionary
+        # Exclude unset values to avoid sending them to the MCP servers which may cause errors
+        # for many servers.
+        kwargs = args.model_dump(exclude_unset=True)
 
         try:
             async with create_mcp_server_session(self._server_params) as session:
@@ -63,13 +68,16 @@
                 if cancellation_token.is_cancelled():
                     raise Exception("Operation cancelled")
 
-                result = await session.call_tool(self._tool.name, kwargs)  # type: ignore
+                result_future = asyncio.ensure_future(session.call_tool(name=self._tool.name, arguments=kwargs))
+                cancellation_token.link_future(result_future)
+                result = await result_future
 
                 if result.isError:
                     raise Exception(f"MCP tool execution failed: {result.content}")
                 return result.content
         except Exception as e:
-            raise Exception(str(e)) from e
+            error_message = self._format_errors(e)
+            raise Exception(error_message) from e
 
     @classmethod
     async def from_server_params(cls, server_params: TServerParams, tool_name: str) -> "McpToolAdapter[TServerParams]":
@@ -98,3 +106,16 @@
                 )
 
         return cls(server_params=server_params, tool=matching_tool)
+
+    def _format_errors(self, error: Exception) -> str:
+        """Recursively format errors into a string."""
+
+        error_message = ""
+        if hasattr(builtins, "ExceptionGroup") and isinstance(error, builtins.ExceptionGroup):
+            # ExceptionGroup is available in Python 3.11+.
+            # TODO: how to make this compatible with Python 3.10?
+            for sub_exception in error.exceptions:  # type: ignore
+                error_message += self._format_errors(sub_exception)  # type: ignore
+        else:
+            error_message += f"{str(error)}\n"
+        return error_message

python/packages/autogen-ext/tests/tools/test_mcp_tools.py

@@ -1,4 +1,5 @@
 import logging
+import os
 from unittest.mock import AsyncMock, MagicMock
 
 import pytest
@@ -8,6 +9,7 @@
     SseServerParams,
     StdioMcpToolAdapter,
     StdioServerParams,
+    mcp_server_tools,
 )
 from json_schema_to_pydantic import create_model
 from mcp import ClientSession, Tool
@@ -280,3 +282,85 @@ async def test_sse_adapter_from_server_params(
         params_schema["properties"]["test_param"]["type"]
         == sample_sse_tool.inputSchema["properties"]["test_param"]["type"]
     )
+
+
+# TODO: why is this test not working in CI?
+@pytest.mark.skip(reason="Skipping test_mcp_server_fetch due to CI issues.")
+@pytest.mark.asyncio
+async def test_mcp_server_fetch() -> None:
+    params = StdioServerParams(
+        command="uvx",
+        args=["mcp-server-fetch"],
+        read_timeout_seconds=60,
+    )
+    tools = await mcp_server_tools(server_params=params)
+    assert tools is not None
+    assert tools[0].name == "fetch"
+    result = await tools[0].run_json({"url": "https://github.com/"}, CancellationToken())
+    assert result is not None
+
+
+# TODO: why is this test not working in CI?
+@pytest.mark.skip(reason="Skipping due to CI issues.")
+@pytest.mark.asyncio
+async def test_mcp_server_filesystem() -> None:
+    params = StdioServerParams(
+        command="npx",
+        args=[
+            "-y",
+            "@modelcontextprotocol/server-filesystem",
+            ".",
+        ],
+        read_timeout_seconds=60,
+    )
+    tools = await mcp_server_tools(server_params=params)
+    assert tools is not None
+    tools = [tool for tool in tools if tool.name == "read_file"]
+    assert len(tools) == 1
+    tool = tools[0]
+    result = await tool.run_json({"path": "README.md"}, CancellationToken())
+    assert result is not None
+
+
+# TODO: why is this test not working in CI?
+@pytest.mark.skip(reason="Skipping due to CI issues.")
+@pytest.mark.asyncio
+async def test_mcp_server_git() -> None:
+    params = StdioServerParams(
+        command="uvx",
+        args=["mcp-server-git"],
+        read_timeout_seconds=60,
+    )
+    tools = await mcp_server_tools(server_params=params)
+    assert tools is not None
+    tools = [tool for tool in tools if tool.name == "git_log"]
+    assert len(tools) == 1
+    tool = tools[0]
+    repo_path = os.path.join(os.path.dirname(__file__), "..", "..", "..", "..", "..")
+    result = await tool.run_json({"repo_path": repo_path}, CancellationToken())
+    assert result is not None
+
+
+@pytest.mark.asyncio
+async def test_mcp_server_github() -> None:
+    # Check if GITHUB_TOKEN is set.
+    if "GITHUB_TOKEN" not in os.environ:
+        pytest.skip("GITHUB_TOKEN environment variable is not set. Skipping test.")
+    params = StdioServerParams(
+        command="npx",
+        args=[
+            "-y",
+            "@modelcontextprotocol/server-github",
+        ],
+        env={"GITHUB_PERSONAL_ACCESS_TOKEN": os.environ["GITHUB_TOKEN"]},
+        read_timeout_seconds=60,
+    )
+    tools = await mcp_server_tools(server_params=params)
+    assert tools is not None
+    tools = [tool for tool in tools if tool.name == "get_file_contents"]
+    assert len(tools) == 1
+    tool = tools[0]
+    result = await tool.run_json(
+        {"owner": "microsoft", "repo": "autogen", "path": "python", "branch": "main"}, CancellationToken()
+    )
+    assert result is not None