From 41f49600d87322705ade4c1a362474ff319aec75 Mon Sep 17 00:00:00 2001 From: perfogic Date: Wed, 23 Apr 2025 15:18:48 +0700 Subject: [PATCH 1/7] update: override return_value_as_string for McpToolAdapter --- .../src/autogen_ext/tools/mcp/_base.py | 12 +++++ .../autogen-ext/tests/tools/test_mcp_tools.py | 48 ++++++++++++++++++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py b/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py index 488984ede072..4b15dffe94dc 100644 --- a/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py +++ b/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py @@ -7,6 +7,7 @@ from autogen_core.tools import BaseTool from autogen_core.utils import schema_to_pydantic_model from mcp import ClientSession, Tool +from mcp.types import EmbeddedResource, ImageContent, TextContent from pydantic import BaseModel from ._config import McpServerParams @@ -115,6 +116,17 @@ async def from_server_params(cls, server_params: TServerParams, tool_name: str) return cls(server_params=server_params, tool=matching_tool) + def return_value_as_string(self, value: Any) -> str: + if isinstance(value, list): + valid_types = (TextContent, ImageContent, EmbeddedResource) + return list( + map( + lambda t: (t.model_dump_json() if isinstance(t, valid_types) else str(t)), + value, + ) + ) + return str(value) + def _format_errors(self, error: Exception) -> str: """Recursively format errors into a string.""" diff --git a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py index 10f17c2c35e0..3671feac4909 100644 --- a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py +++ b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py @@ -16,6 +16,7 @@ mcp_server_tools, ) from mcp import ClientSession, Tool +from mcp.types import EmbeddedResource, ImageContent, TextContent, TextResourceContents @pytest.fixture @@ -182,6 +183,50 @@ async def test_adapter_from_server_params( ) +@pytest.mark.asyncio +async def test_adapter_from_server_params_with_return_value_as_string( + sample_tool: Tool, + sample_server_params: StdioServerParams, + mock_session: AsyncMock, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Test that adapter can be created from server parameters.""" + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_session + monkeypatch.setattr( + "autogen_ext.tools.mcp._base.create_mcp_server_session", + lambda *args, **kwargs: mock_context, # type: ignore + ) + + mock_session.list_tools.return_value.tools = [sample_tool] + + adapter = await StdioMcpToolAdapter.from_server_params(sample_server_params, "test_tool") + + assert ( + adapter.return_value_as_string( + [ + TextContent(text="this is a sample text", type="text"), + ImageContent( + data="this is a sample base64 encoded image", + mimeType="image/png", + type="image", + ), + EmbeddedResource( + type="resource", + resource=TextResourceContents( + text="this is a sample text", + uri="http://example.com/test", + ), + ), + ] + ) + ) == [ + '{"type":"text","text":"this is a sample text","annotations":null}', + '{"type":"image","data":"this is a sample base64 encoded image","mimeType":"image/png","annotations":null}', + '{"type":"resource","resource":{"uri":"http://example.com/test","mimeType":null,"text":"this is a sample text"},"annotations":null}', + ] + + @pytest.mark.asyncio async def test_adapter_from_factory( sample_tool: Tool, @@ -421,7 +466,8 @@ async def test_mcp_server_github() -> None: assert len(tools) == 1 tool = tools[0] result = await tool.run_json( - {"owner": "microsoft", "repo": "autogen", "path": "python", "branch": "main"}, CancellationToken() + {"owner": "microsoft", "repo": "autogen", "path": "python", "branch": "main"}, + CancellationToken(), ) assert result is not None From 35df1815c874efb6c648537fba3c71697bd216ba Mon Sep 17 00:00:00 2001 From: perfogic Date: Thu, 24 Apr 2025 02:40:47 +0700 Subject: [PATCH 2/7] fix: fix serialize case by case since EmbededResource has uri which is not JSON serialized --- .../src/autogen_ext/tools/mcp/_base.py | 30 ++++++++++++------- .../autogen-ext/tests/tools/test_mcp_tools.py | 29 +++++++++++------- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py b/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py index 4b15dffe94dc..a1e4845c3a12 100644 --- a/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py +++ b/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py @@ -7,8 +7,9 @@ from autogen_core.tools import BaseTool from autogen_core.utils import schema_to_pydantic_model from mcp import ClientSession, Tool -from mcp.types import EmbeddedResource, ImageContent, TextContent +from mcp.types import AnyUrl, EmbeddedResource, ImageContent, TextContent from pydantic import BaseModel +import json from ._config import McpServerParams from ._session import create_mcp_server_session @@ -116,16 +117,23 @@ async def from_server_params(cls, server_params: TServerParams, tool_name: str) return cls(server_params=server_params, tool=matching_tool) - def return_value_as_string(self, value: Any) -> str: - if isinstance(value, list): - valid_types = (TextContent, ImageContent, EmbeddedResource) - return list( - map( - lambda t: (t.model_dump_json() if isinstance(t, valid_types) else str(t)), - value, - ) - ) - return str(value) + def return_value_as_string(self, value: list) -> str: + """Return a string representation of the result.""" + + def serialize_item(item): + if isinstance(item, (TextContent, ImageContent)): + return item.model_dump() + elif isinstance(item, EmbeddedResource): + type = item.type + resource = {} + for key, val in item.resource.model_dump().items(): + if isinstance(val, AnyUrl): + resource[key] = str(val) + annotations = item.annotations.model_dump() if item.annotations else None + return {"type": type, "resource": resource, "annotations": annotations} + else: + return str(item) + return json.dumps([serialize_item(item) for item in value]) def _format_errors(self, error: Exception) -> str: """Recursively format errors into a string.""" diff --git a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py index 3671feac4909..f9da7af569a9 100644 --- a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py +++ b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py @@ -16,7 +16,14 @@ mcp_server_tools, ) from mcp import ClientSession, Tool -from mcp.types import EmbeddedResource, ImageContent, TextContent, TextResourceContents +from mcp.types import ( + AnyUrl, + Annotations, + EmbeddedResource, + ImageContent, + TextContent, + TextResourceContents, +) @pytest.fixture @@ -202,29 +209,29 @@ async def test_adapter_from_server_params_with_return_value_as_string( adapter = await StdioMcpToolAdapter.from_server_params(sample_server_params, "test_tool") - assert ( - adapter.return_value_as_string( + assert adapter.return_value_as_string( [ - TextContent(text="this is a sample text", type="text"), + TextContent( + text="this is a sample text", + type="text", + annotations=Annotations(audience=["user", "assistant"], priority=0.7), + ), ImageContent( data="this is a sample base64 encoded image", mimeType="image/png", type="image", + annotations=None, ), EmbeddedResource( type="resource", resource=TextResourceContents( text="this is a sample text", - uri="http://example.com/test", + uri=AnyUrl(url="http://example.com/test"), ), + annotations=Annotations(audience=["user"], priority=0.3), ), ] - ) - ) == [ - '{"type":"text","text":"this is a sample text","annotations":null}', - '{"type":"image","data":"this is a sample base64 encoded image","mimeType":"image/png","annotations":null}', - '{"type":"resource","resource":{"uri":"http://example.com/test","mimeType":null,"text":"this is a sample text"},"annotations":null}', - ] + ) == '[{"type": "text", "text": "this is a sample text", "annotations": {"audience": ["user", "assistant"], "priority": 0.7}}, {"type": "image", "data": "this is a sample base64 encoded image", "mimeType": "image/png", "annotations": null}, {"type": "resource", "resource": {"uri": "http://example.com/test"}, "annotations": {"audience": ["user"], "priority": 0.3}}]' @pytest.mark.asyncio From f8000299aa7c7a97849fea11f0f512a8ea5c9d27 Mon Sep 17 00:00:00 2001 From: perfogic Date: Thu, 24 Apr 2025 02:49:44 +0700 Subject: [PATCH 3/7] fix: add full items in resource on EmbededResource --- python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py | 2 ++ python/packages/autogen-ext/tests/tools/test_mcp_tools.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py b/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py index a1e4845c3a12..f721587b194a 100644 --- a/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py +++ b/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py @@ -129,6 +129,8 @@ def serialize_item(item): for key, val in item.resource.model_dump().items(): if isinstance(val, AnyUrl): resource[key] = str(val) + else: + resource[key] = val annotations = item.annotations.model_dump() if item.annotations else None return {"type": type, "resource": resource, "annotations": annotations} else: diff --git a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py index f9da7af569a9..f0a026495a75 100644 --- a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py +++ b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py @@ -231,7 +231,7 @@ async def test_adapter_from_server_params_with_return_value_as_string( annotations=Annotations(audience=["user"], priority=0.3), ), ] - ) == '[{"type": "text", "text": "this is a sample text", "annotations": {"audience": ["user", "assistant"], "priority": 0.7}}, {"type": "image", "data": "this is a sample base64 encoded image", "mimeType": "image/png", "annotations": null}, {"type": "resource", "resource": {"uri": "http://example.com/test"}, "annotations": {"audience": ["user"], "priority": 0.3}}]' + ) == '[{"type": "text", "text": "this is a sample text", "annotations": {"audience": ["user", "assistant"], "priority": 0.7}}, {"type": "image", "data": "this is a sample base64 encoded image", "mimeType": "image/png", "annotations": null}, {"type": "resource", "resource": {"uri": "http://example.com/test", "mimeType": null, "text": "this is a sample text"}, "annotations": {"audience": ["user"], "priority": 0.3}}]' @pytest.mark.asyncio From 2e6dcb9534e7652a1ea5855028148632a7b6f948 Mon Sep 17 00:00:00 2001 From: perfogic Date: Thu, 24 Apr 2025 02:51:21 +0700 Subject: [PATCH 4/7] chore: format code --- .../autogen-ext/src/autogen_ext/tools/mcp/_base.py | 1 + python/packages/autogen-ext/tests/tools/test_mcp_tools.py | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py b/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py index f721587b194a..a8a5e00e9790 100644 --- a/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py +++ b/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py @@ -135,6 +135,7 @@ def serialize_item(item): return {"type": type, "resource": resource, "annotations": annotations} else: return str(item) + return json.dumps([serialize_item(item) for item in value]) def _format_errors(self, error: Exception) -> str: diff --git a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py index f0a026495a75..5d9897ac6721 100644 --- a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py +++ b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py @@ -209,7 +209,8 @@ async def test_adapter_from_server_params_with_return_value_as_string( adapter = await StdioMcpToolAdapter.from_server_params(sample_server_params, "test_tool") - assert adapter.return_value_as_string( + assert ( + adapter.return_value_as_string( [ TextContent( text="this is a sample text", @@ -231,7 +232,9 @@ async def test_adapter_from_server_params_with_return_value_as_string( annotations=Annotations(audience=["user"], priority=0.3), ), ] - ) == '[{"type": "text", "text": "this is a sample text", "annotations": {"audience": ["user", "assistant"], "priority": 0.7}}, {"type": "image", "data": "this is a sample base64 encoded image", "mimeType": "image/png", "annotations": null}, {"type": "resource", "resource": {"uri": "http://example.com/test", "mimeType": null, "text": "this is a sample text"}, "annotations": {"audience": ["user"], "priority": 0.3}}]' + ) + == '[{"type": "text", "text": "this is a sample text", "annotations": {"audience": ["user", "assistant"], "priority": 0.7}}, {"type": "image", "data": "this is a sample base64 encoded image", "mimeType": "image/png", "annotations": null}, {"type": "resource", "resource": {"uri": "http://example.com/test", "mimeType": null, "text": "this is a sample text"}, "annotations": {"audience": ["user"], "priority": 0.3}}]' + ) @pytest.mark.asyncio From d4ded4dccf75161a88a9fd49b687ea520f9a3ddc Mon Sep 17 00:00:00 2001 From: perfogic Date: Fri, 25 Apr 2025 10:14:24 +0700 Subject: [PATCH 5/7] chore: fix by README --- .../autogen-ext/src/autogen_ext/tools/mcp/_base.py | 11 ++++++----- .../autogen-ext/tests/tools/test_mcp_tools.py | 3 +-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py b/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py index a8a5e00e9790..15fe73e94a1d 100644 --- a/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py +++ b/python/packages/autogen-ext/src/autogen_ext/tools/mcp/_base.py @@ -1,5 +1,6 @@ import asyncio import builtins +import json from abc import ABC from typing import Any, Dict, Generic, Type, TypeVar @@ -7,9 +8,9 @@ from autogen_core.tools import BaseTool from autogen_core.utils import schema_to_pydantic_model from mcp import ClientSession, Tool -from mcp.types import AnyUrl, EmbeddedResource, ImageContent, TextContent +from mcp.types import EmbeddedResource, ImageContent, TextContent from pydantic import BaseModel -import json +from pydantic.networks import AnyUrl from ._config import McpServerParams from ._session import create_mcp_server_session @@ -117,10 +118,10 @@ async def from_server_params(cls, server_params: TServerParams, tool_name: str) return cls(server_params=server_params, tool=matching_tool) - def return_value_as_string(self, value: list) -> str: + def return_value_as_string(self, value: list[Any]) -> str: """Return a string representation of the result.""" - def serialize_item(item): + def serialize_item(item: Any) -> dict[str, Any]: if isinstance(item, (TextContent, ImageContent)): return item.model_dump() elif isinstance(item, EmbeddedResource): @@ -134,7 +135,7 @@ def serialize_item(item): annotations = item.annotations.model_dump() if item.annotations else None return {"type": type, "resource": resource, "annotations": annotations} else: - return str(item) + return {} return json.dumps([serialize_item(item) for item in value]) diff --git a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py index 5d9897ac6721..0702e13167ab 100644 --- a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py +++ b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py @@ -17,13 +17,13 @@ ) from mcp import ClientSession, Tool from mcp.types import ( - AnyUrl, Annotations, EmbeddedResource, ImageContent, TextContent, TextResourceContents, ) +from pydantic.networks import AnyUrl @pytest.fixture @@ -204,7 +204,6 @@ async def test_adapter_from_server_params_with_return_value_as_string( "autogen_ext.tools.mcp._base.create_mcp_server_session", lambda *args, **kwargs: mock_context, # type: ignore ) - mock_session.list_tools.return_value.tools = [sample_tool] adapter = await StdioMcpToolAdapter.from_server_params(sample_server_params, "test_tool") From d18f62fb5d2906f062a174eff0d161b931db174d Mon Sep 17 00:00:00 2001 From: perfogic Date: Fri, 25 Apr 2025 12:35:36 +0700 Subject: [PATCH 6/7] chore: fix test --- .../autogen-ext/tests/tools/test_mcp_tools.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py index 0702e13167ab..5c08cbe3e312 100644 --- a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py +++ b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py @@ -79,7 +79,13 @@ def mock_session() -> AsyncMock: def mock_tool_response() -> MagicMock: response = MagicMock() response.isError = False - response.content = {"result": "test_output"} + response.content = [ + TextContent( + text="test_output", + type="text", + annotations=Annotations(audience=["user", "assistant"], priority=0.7), + ), + ] return response @@ -320,7 +326,13 @@ async def test_sse_tool_execution( mock_context = AsyncMock() mock_context.__aenter__.return_value = mock_sse_session - mock_sse_session.call_tool.return_value = MagicMock(isError=False, content={"result": "test_output"}) + mock_sse_session.call_tool.return_value = MagicMock(isError=False, content=[ + TextContent( + text="test_output", + type="text", + annotations=Annotations(audience=["user", "assistant"], priority=0.7), + ), + ]) monkeypatch.setattr( "autogen_ext.tools.mcp._base.create_mcp_server_session", From 260264d5fc273929033ef42497fa47aa9ec7c3ab Mon Sep 17 00:00:00 2001 From: perfogic Date: Fri, 25 Apr 2025 12:36:42 +0700 Subject: [PATCH 7/7] chore: format code --- .../autogen-ext/tests/tools/test_mcp_tools.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py index 5c08cbe3e312..778acd2d2fba 100644 --- a/python/packages/autogen-ext/tests/tools/test_mcp_tools.py +++ b/python/packages/autogen-ext/tests/tools/test_mcp_tools.py @@ -326,13 +326,16 @@ async def test_sse_tool_execution( mock_context = AsyncMock() mock_context.__aenter__.return_value = mock_sse_session - mock_sse_session.call_tool.return_value = MagicMock(isError=False, content=[ - TextContent( - text="test_output", - type="text", - annotations=Annotations(audience=["user", "assistant"], priority=0.7), - ), - ]) + mock_sse_session.call_tool.return_value = MagicMock( + isError=False, + content=[ + TextContent( + text="test_output", + type="text", + annotations=Annotations(audience=["user", "assistant"], priority=0.7), + ), + ], + ) monkeypatch.setattr( "autogen_ext.tools.mcp._base.create_mcp_server_session",