From 426f411d922e5cd6ddbf4c092970753c7913b11b Mon Sep 17 00:00:00 2001 From: jer Date: Wed, 25 Jun 2025 15:44:47 +0000 Subject: [PATCH 1/2] feat(a2a): tools as skills --- src/strands/multiagent/a2a/__init__.py | 4 +- .../multiagent/a2a/{agent.py => server.py} | 30 +- tests/multiagent/a2a/conftest.py | 5 + tests/multiagent/a2a/test_agent.py | 165 ------- tests/multiagent/a2a/test_server.py | 431 ++++++++++++++++++ 5 files changed, 462 insertions(+), 173 deletions(-) rename src/strands/multiagent/a2a/{agent.py => server.py} (86%) delete mode 100644 tests/multiagent/a2a/test_agent.py create mode 100644 tests/multiagent/a2a/test_server.py diff --git a/src/strands/multiagent/a2a/__init__.py b/src/strands/multiagent/a2a/__init__.py index c54256187..56c1c2900 100644 --- a/src/strands/multiagent/a2a/__init__.py +++ b/src/strands/multiagent/a2a/__init__.py @@ -9,6 +9,6 @@ A2AAgent: A wrapper that adapts a Strands Agent to be A2A-compatible. """ -from .agent import A2AAgent +from .server import A2AServer -__all__ = ["A2AAgent"] +__all__ = ["A2AServer"] diff --git a/src/strands/multiagent/a2a/agent.py b/src/strands/multiagent/a2a/server.py similarity index 86% rename from src/strands/multiagent/a2a/agent.py rename to src/strands/multiagent/a2a/server.py index 4359100de..f49ddb603 100644 --- a/src/strands/multiagent/a2a/agent.py +++ b/src/strands/multiagent/a2a/server.py @@ -21,7 +21,7 @@ logger = logging.getLogger(__name__) -class A2AAgent: +class A2AServer: """A2A-compatible wrapper for Strands Agent.""" def __init__( @@ -32,6 +32,7 @@ def __init__( host: str = "0.0.0.0", port: int = 9000, version: str = "0.0.1", + skills: list[AgentSkill] | None, ): """Initialize an A2A-compatible agent from a Strands agent. @@ -42,6 +43,7 @@ def __init__( host: The hostname or IP address to bind the A2A server to. Defaults to "0.0.0.0". port: The port to bind the A2A server to. Defaults to 9000. version: The version of the agent. Defaults to "0.0.1". + skills: The list of capabilities or functions the agent can perform. """ self.host = host self.port = port @@ -56,6 +58,7 @@ def __init__( agent_executor=StrandsA2AExecutor(self.strands_agent), task_store=InMemoryTaskStore(), ) + self._agent_skills = skills or self._get_skills_from_tools() logger.info("Strands' integration with A2A is experimental. Be aware of frequent breaking changes.") @property @@ -88,9 +91,8 @@ def public_agent_card(self) -> AgentCard: capabilities=self.capabilities, ) - @property - def agent_skills(self) -> list[AgentSkill]: - """Get the list of skills this agent provides. + def _get_skills_from_tools(self) -> list[AgentSkill]: + """Get the list of skills from Strands agent tools. Skills represent specific capabilities that the agent can perform. Strands agent tools are adapted to A2A skills. @@ -98,8 +100,24 @@ def agent_skills(self) -> list[AgentSkill]: Returns: list[AgentSkill]: A list of skills this agent provides. """ - # TODO: translate Strands tools (native & MCP) to skills - return [] + return [ + AgentSkill(name=config["name"], id=config["name"], description=config["description"], tags=[]) + for config in self.strands_agent.tool_registry.get_all_tools_config().values() + ] + + @property + def agent_skills(self) -> list[AgentSkill]: + """Get the list of skills this agent provides.""" + return self._agent_skills + + @agent_skills.setter + def agent_skills(self, skills: list[AgentSkill]) -> None: + """Set the list of skills this agent provides. + + Args: + skills: A list of AgentSkill objects to set for this agent. + """ + self._agent_skills = skills def to_starlette_app(self) -> Starlette: """Create a Starlette application for serving this agent via HTTP. diff --git a/tests/multiagent/a2a/conftest.py b/tests/multiagent/a2a/conftest.py index 558a45948..a9730eacb 100644 --- a/tests/multiagent/a2a/conftest.py +++ b/tests/multiagent/a2a/conftest.py @@ -22,6 +22,11 @@ def mock_strands_agent(): mock_result.message = {"content": [{"text": "Test response"}]} agent.return_value = mock_result + # Setup mock tool registry + mock_tool_registry = MagicMock() + mock_tool_registry.get_all_tools_config.return_value = {} + agent.tool_registry = mock_tool_registry + return agent diff --git a/tests/multiagent/a2a/test_agent.py b/tests/multiagent/a2a/test_agent.py deleted file mode 100644 index 5558c2af7..000000000 --- a/tests/multiagent/a2a/test_agent.py +++ /dev/null @@ -1,165 +0,0 @@ -"""Tests for the A2AAgent class.""" - -from unittest.mock import patch - -import pytest -from a2a.types import AgentCapabilities, AgentCard -from fastapi import FastAPI -from starlette.applications import Starlette - -from strands.multiagent.a2a.agent import A2AAgent - - -def test_a2a_agent_initialization(mock_strands_agent): - """Test that A2AAgent initializes correctly with default values.""" - a2a_agent = A2AAgent(mock_strands_agent) - - assert a2a_agent.strands_agent == mock_strands_agent - assert a2a_agent.name == "Test Agent" - assert a2a_agent.description == "A test agent for unit testing" - assert a2a_agent.host == "0.0.0" - assert a2a_agent.port == 9000 - assert a2a_agent.http_url == "http://0.0.0:9000/" - assert a2a_agent.version == "0.0.1" - assert isinstance(a2a_agent.capabilities, AgentCapabilities) - - -def test_a2a_agent_initialization_with_custom_values(mock_strands_agent): - """Test that A2AAgent initializes correctly with custom values.""" - a2a_agent = A2AAgent( - mock_strands_agent, - host="127.0.0.1", - port=8080, - version="1.0.0", - ) - - assert a2a_agent.host == "127.0.0.1" - assert a2a_agent.port == 8080 - assert a2a_agent.http_url == "http://127.0.0.1:8080/" - assert a2a_agent.version == "1.0.0" - - -def test_public_agent_card(mock_strands_agent): - """Test that public_agent_card returns a valid AgentCard.""" - a2a_agent = A2AAgent(mock_strands_agent) - - card = a2a_agent.public_agent_card - - assert isinstance(card, AgentCard) - assert card.name == "Test Agent" - assert card.description == "A test agent for unit testing" - assert card.url == "http://0.0.0:9000/" - assert card.version == "0.0.1" - assert card.defaultInputModes == ["text"] - assert card.defaultOutputModes == ["text"] - assert card.skills == [] - assert card.capabilities == a2a_agent.capabilities - - -def test_public_agent_card_with_missing_name(mock_strands_agent): - """Test that public_agent_card raises ValueError when name is missing.""" - mock_strands_agent.name = "" - a2a_agent = A2AAgent(mock_strands_agent) - - with pytest.raises(ValueError, match="A2A agent name cannot be None or empty"): - _ = a2a_agent.public_agent_card - - -def test_public_agent_card_with_missing_description(mock_strands_agent): - """Test that public_agent_card raises ValueError when description is missing.""" - mock_strands_agent.description = "" - a2a_agent = A2AAgent(mock_strands_agent) - - with pytest.raises(ValueError, match="A2A agent description cannot be None or empty"): - _ = a2a_agent.public_agent_card - - -def test_agent_skills(mock_strands_agent): - """Test that agent_skills returns an empty list (current implementation).""" - a2a_agent = A2AAgent(mock_strands_agent) - - skills = a2a_agent.agent_skills - - assert isinstance(skills, list) - assert len(skills) == 0 - - -def test_to_starlette_app(mock_strands_agent): - """Test that to_starlette_app returns a Starlette application.""" - a2a_agent = A2AAgent(mock_strands_agent) - - app = a2a_agent.to_starlette_app() - - assert isinstance(app, Starlette) - - -def test_to_fastapi_app(mock_strands_agent): - """Test that to_fastapi_app returns a FastAPI application.""" - a2a_agent = A2AAgent(mock_strands_agent) - - app = a2a_agent.to_fastapi_app() - - assert isinstance(app, FastAPI) - - -@patch("uvicorn.run") -def test_serve_with_starlette(mock_run, mock_strands_agent): - """Test that serve starts a Starlette server by default.""" - a2a_agent = A2AAgent(mock_strands_agent) - - a2a_agent.serve() - - mock_run.assert_called_once() - args, kwargs = mock_run.call_args - assert isinstance(args[0], Starlette) - assert kwargs["host"] == "0.0.0" - assert kwargs["port"] == 9000 - - -@patch("uvicorn.run") -def test_serve_with_fastapi(mock_run, mock_strands_agent): - """Test that serve starts a FastAPI server when specified.""" - a2a_agent = A2AAgent(mock_strands_agent) - - a2a_agent.serve(app_type="fastapi") - - mock_run.assert_called_once() - args, kwargs = mock_run.call_args - assert isinstance(args[0], FastAPI) - assert kwargs["host"] == "0.0.0" - assert kwargs["port"] == 9000 - - -@patch("uvicorn.run") -def test_serve_with_custom_kwargs(mock_run, mock_strands_agent): - """Test that serve passes additional kwargs to uvicorn.run.""" - a2a_agent = A2AAgent(mock_strands_agent) - - a2a_agent.serve(log_level="debug", reload=True) - - mock_run.assert_called_once() - _, kwargs = mock_run.call_args - assert kwargs["log_level"] == "debug" - assert kwargs["reload"] is True - - -@patch("uvicorn.run", side_effect=KeyboardInterrupt) -def test_serve_handles_keyboard_interrupt(mock_run, mock_strands_agent, caplog): - """Test that serve handles KeyboardInterrupt gracefully.""" - a2a_agent = A2AAgent(mock_strands_agent) - - a2a_agent.serve() - - assert "Strands A2A server shutdown requested (KeyboardInterrupt)" in caplog.text - assert "Strands A2A server has shutdown" in caplog.text - - -@patch("uvicorn.run", side_effect=Exception("Test exception")) -def test_serve_handles_general_exception(mock_run, mock_strands_agent, caplog): - """Test that serve handles general exceptions gracefully.""" - a2a_agent = A2AAgent(mock_strands_agent) - - a2a_agent.serve() - - assert "Strands A2A server encountered exception" in caplog.text - assert "Strands A2A server has shutdown" in caplog.text diff --git a/tests/multiagent/a2a/test_server.py b/tests/multiagent/a2a/test_server.py new file mode 100644 index 000000000..7323797dd --- /dev/null +++ b/tests/multiagent/a2a/test_server.py @@ -0,0 +1,431 @@ +"""Tests for the A2AAgent class.""" + +from unittest.mock import patch + +import pytest +from a2a.types import AgentCapabilities, AgentCard +from fastapi import FastAPI +from starlette.applications import Starlette + +from strands.multiagent.a2a.server import A2AServer + + +def test_a2a_agent_initialization(mock_strands_agent): + """Test that A2AAgent initializes correctly with default values.""" + # Mock tool registry for default skills + mock_tool_config = {"test_tool": {"name": "test_tool", "description": "A test tool"}} + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + a2a_agent = A2AServer(mock_strands_agent, skills=None) + + assert a2a_agent.strands_agent == mock_strands_agent + assert a2a_agent.name == "Test Agent" + assert a2a_agent.description == "A test agent for unit testing" + assert a2a_agent.host == "0.0.0.0" + assert a2a_agent.port == 9000 + assert a2a_agent.http_url == "http://0.0.0.0:9000/" + assert a2a_agent.version == "0.0.1" + assert isinstance(a2a_agent.capabilities, AgentCapabilities) + # Should have skills from tools + assert len(a2a_agent.agent_skills) == 1 + assert a2a_agent.agent_skills[0].name == "test_tool" + + +def test_a2a_agent_initialization_with_custom_values(mock_strands_agent): + """Test that A2AAgent initializes correctly with custom values.""" + a2a_agent = A2AServer( + mock_strands_agent, + host="127.0.0.1", + port=8080, + version="1.0.0", + skills=None, + ) + + assert a2a_agent.host == "127.0.0.1" + assert a2a_agent.port == 8080 + assert a2a_agent.http_url == "http://127.0.0.1:8080/" + assert a2a_agent.version == "1.0.0" + + +def test_a2a_agent_initialization_with_custom_skills(mock_strands_agent): + """Test that A2AAgent initializes correctly with custom skills.""" + from a2a.types import AgentSkill + + custom_skills = [ + AgentSkill(name="custom_skill", id="custom_skill", description="A custom skill", tags=["test"]), + AgentSkill(name="another_skill", id="another_skill", description="Another custom skill", tags=[]), + ] + + a2a_agent = A2AServer( + mock_strands_agent, + skills=custom_skills, + ) + + assert a2a_agent.agent_skills == custom_skills + assert len(a2a_agent.agent_skills) == 2 + assert a2a_agent.agent_skills[0].name == "custom_skill" + assert a2a_agent.agent_skills[1].name == "another_skill" + + +def test_public_agent_card(mock_strands_agent): + """Test that public_agent_card returns a valid AgentCard.""" + # Mock empty tool registry for this test + mock_strands_agent.tool_registry.get_all_tools_config.return_value = {} + + a2a_agent = A2AServer(mock_strands_agent, skills=[]) + + card = a2a_agent.public_agent_card + + assert isinstance(card, AgentCard) + assert card.name == "Test Agent" + assert card.description == "A test agent for unit testing" + assert card.url == "http://0.0.0.0:9000/" + assert card.version == "0.0.1" + assert card.defaultInputModes == ["text"] + assert card.defaultOutputModes == ["text"] + assert card.skills == [] + assert card.capabilities == a2a_agent.capabilities + + +def test_public_agent_card_with_missing_name(mock_strands_agent): + """Test that public_agent_card raises ValueError when name is missing.""" + mock_strands_agent.name = "" + a2a_agent = A2AServer(mock_strands_agent, skills=[]) + + with pytest.raises(ValueError, match="A2A agent name cannot be None or empty"): + _ = a2a_agent.public_agent_card + + +def test_public_agent_card_with_missing_description(mock_strands_agent): + """Test that public_agent_card raises ValueError when description is missing.""" + mock_strands_agent.description = "" + a2a_agent = A2AServer(mock_strands_agent, skills=[]) + + with pytest.raises(ValueError, match="A2A agent description cannot be None or empty"): + _ = a2a_agent.public_agent_card + + +def test_agent_skills_empty_registry(mock_strands_agent): + """Test that agent_skills returns an empty list when no tools are registered.""" + # Mock empty tool registry + mock_strands_agent.tool_registry.get_all_tools_config.return_value = {} + + a2a_agent = A2AServer(mock_strands_agent, skills=None) + skills = a2a_agent.agent_skills + + assert isinstance(skills, list) + assert len(skills) == 0 + + +def test_agent_skills_with_single_tool(mock_strands_agent): + """Test that agent_skills returns correct skills for a single tool.""" + # Mock tool registry with one tool + mock_tool_config = {"calculator": {"name": "calculator", "description": "Performs basic mathematical calculations"}} + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + a2a_agent = A2AServer(mock_strands_agent, skills=None) + skills = a2a_agent.agent_skills + + assert isinstance(skills, list) + assert len(skills) == 1 + + skill = skills[0] + assert skill.name == "calculator" + assert skill.id == "calculator" + assert skill.description == "Performs basic mathematical calculations" + assert skill.tags == [] + + +def test_agent_skills_with_multiple_tools(mock_strands_agent): + """Test that agent_skills returns correct skills for multiple tools.""" + # Mock tool registry with multiple tools + mock_tool_config = { + "calculator": {"name": "calculator", "description": "Performs basic mathematical calculations"}, + "weather": {"name": "weather", "description": "Gets current weather information"}, + "file_reader": {"name": "file_reader", "description": "Reads and processes files"}, + } + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + a2a_agent = A2AServer(mock_strands_agent, skills=None) + skills = a2a_agent.agent_skills + + assert isinstance(skills, list) + assert len(skills) == 3 + + # Check that all tools are converted to skills + skill_names = [skill.name for skill in skills] + assert "calculator" in skill_names + assert "weather" in skill_names + assert "file_reader" in skill_names + + # Check specific skill properties + for skill in skills: + assert skill.name == skill.id # id should match name + assert isinstance(skill.description, str) + assert len(skill.description) > 0 + assert skill.tags == [] + + +def test_agent_skills_with_complex_tool_config(mock_strands_agent): + """Test that agent_skills handles complex tool configurations correctly.""" + # Mock tool registry with complex tool configuration + mock_tool_config = { + "advanced_calculator": { + "name": "advanced_calculator", + "description": "Advanced mathematical operations including trigonometry and statistics", + "inputSchema": { + "json": { + "type": "object", + "properties": { + "operation": {"type": "string", "description": "The operation to perform"}, + "values": {"type": "array", "description": "Array of numbers"}, + }, + } + }, + } + } + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + a2a_agent = A2AServer(mock_strands_agent, skills=None) + skills = a2a_agent.agent_skills + + assert isinstance(skills, list) + assert len(skills) == 1 + + skill = skills[0] + assert skill.name == "advanced_calculator" + assert skill.id == "advanced_calculator" + assert skill.description == "Advanced mathematical operations including trigonometry and statistics" + assert skill.tags == [] + + +def test_agent_skills_preserves_tool_order(mock_strands_agent): + """Test that agent_skills preserves the order of tools from the registry.""" + # Mock tool registry with ordered tools + from collections import OrderedDict + + mock_tool_config = OrderedDict( + [ + ("tool_a", {"name": "tool_a", "description": "First tool"}), + ("tool_b", {"name": "tool_b", "description": "Second tool"}), + ("tool_c", {"name": "tool_c", "description": "Third tool"}), + ] + ) + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + a2a_agent = A2AServer(mock_strands_agent, skills=None) + skills = a2a_agent.agent_skills + + assert len(skills) == 3 + assert skills[0].name == "tool_a" + assert skills[1].name == "tool_b" + assert skills[2].name == "tool_c" + + +def test_agent_skills_handles_missing_description(mock_strands_agent): + """Test that agent_skills handles tools with missing description gracefully.""" + # Mock tool registry with tool missing description + mock_tool_config = { + "incomplete_tool": { + "name": "incomplete_tool" + # Missing description + } + } + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + # This should raise a KeyError during initialization when trying to access missing description + with pytest.raises(KeyError): + A2AServer(mock_strands_agent, skills=None) + + +def test_agent_skills_handles_missing_name(mock_strands_agent): + """Test that agent_skills handles tools with missing name gracefully.""" + # Mock tool registry with tool missing name + mock_tool_config = { + "incomplete_tool": { + "description": "A tool without a name" + # Missing name + } + } + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + # This should raise a KeyError during initialization when trying to access missing name + with pytest.raises(KeyError): + A2AServer(mock_strands_agent, skills=None) + + +def test_agent_skills_setter(mock_strands_agent): + """Test that agent_skills setter works correctly.""" + from a2a.types import AgentSkill + + # Mock tool registry for initial setup + mock_tool_config = {"test_tool": {"name": "test_tool", "description": "A test tool"}} + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + a2a_agent = A2AServer(mock_strands_agent, skills=None) + + # Initially should get skills from tools + initial_skills = a2a_agent.agent_skills + assert len(initial_skills) == 1 + assert initial_skills[0].name == "test_tool" + + # Set new skills using setter + new_skills = [ + AgentSkill(name="new_skill", id="new_skill", description="A new skill", tags=["new"]), + AgentSkill(name="another_new_skill", id="another_new_skill", description="Another new skill", tags=[]), + ] + + a2a_agent.agent_skills = new_skills + + # Verify skills were updated + assert a2a_agent.agent_skills == new_skills + assert len(a2a_agent.agent_skills) == 2 + assert a2a_agent.agent_skills[0].name == "new_skill" + assert a2a_agent.agent_skills[1].name == "another_new_skill" + + +def test_get_skills_from_tools_method(mock_strands_agent): + """Test the _get_skills_from_tools method directly.""" + # Mock tool registry with multiple tools + mock_tool_config = { + "calculator": {"name": "calculator", "description": "Performs basic mathematical calculations"}, + "weather": {"name": "weather", "description": "Gets current weather information"}, + } + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + a2a_agent = A2AServer(mock_strands_agent, skills=None) + skills = a2a_agent._get_skills_from_tools() + + assert isinstance(skills, list) + assert len(skills) == 2 + + skill_names = [skill.name for skill in skills] + assert "calculator" in skill_names + assert "weather" in skill_names + + for skill in skills: + assert skill.name == skill.id + assert isinstance(skill.description, str) + assert len(skill.description) > 0 + assert skill.tags == [] + + +def test_initialization_with_none_skills_uses_tools(mock_strands_agent): + """Test that passing skills=None uses tools from the agent.""" + mock_tool_config = {"test_tool": {"name": "test_tool", "description": "A test tool"}} + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + a2a_agent = A2AServer(mock_strands_agent, skills=None) + + # Should get skills from tools + skills = a2a_agent.agent_skills + assert len(skills) == 1 + assert skills[0].name == "test_tool" + assert skills[0].description == "A test tool" + + +def test_initialization_with_empty_skills_list(mock_strands_agent): + """Test that passing an empty skills list works correctly.""" + a2a_agent = A2AServer(mock_strands_agent, skills=[]) + + # Should have empty skills list + skills = a2a_agent.agent_skills + assert isinstance(skills, list) + assert len(skills) == 0 + + +def test_public_agent_card_with_custom_skills(mock_strands_agent): + """Test that public_agent_card includes custom skills.""" + from a2a.types import AgentSkill + + custom_skills = [ + AgentSkill(name="custom_skill", id="custom_skill", description="A custom skill", tags=["test"]), + ] + + a2a_agent = A2AServer(mock_strands_agent, skills=custom_skills) + card = a2a_agent.public_agent_card + + assert card.skills == custom_skills + assert len(card.skills) == 1 + assert card.skills[0].name == "custom_skill" + + +def test_to_starlette_app(mock_strands_agent): + """Test that to_starlette_app returns a Starlette application.""" + a2a_agent = A2AServer(mock_strands_agent, skills=[]) + + app = a2a_agent.to_starlette_app() + + assert isinstance(app, Starlette) + + +def test_to_fastapi_app(mock_strands_agent): + """Test that to_fastapi_app returns a FastAPI application.""" + a2a_agent = A2AServer(mock_strands_agent, skills=[]) + + app = a2a_agent.to_fastapi_app() + + assert isinstance(app, FastAPI) + + +@patch("uvicorn.run") +def test_serve_with_starlette(mock_run, mock_strands_agent): + """Test that serve starts a Starlette server by default.""" + a2a_agent = A2AServer(mock_strands_agent, skills=[]) + + a2a_agent.serve() + + mock_run.assert_called_once() + args, kwargs = mock_run.call_args + assert isinstance(args[0], Starlette) + assert kwargs["host"] == "0.0.0.0" + assert kwargs["port"] == 9000 + + +@patch("uvicorn.run") +def test_serve_with_fastapi(mock_run, mock_strands_agent): + """Test that serve starts a FastAPI server when specified.""" + a2a_agent = A2AServer(mock_strands_agent, skills=[]) + + a2a_agent.serve(app_type="fastapi") + + mock_run.assert_called_once() + args, kwargs = mock_run.call_args + assert isinstance(args[0], FastAPI) + assert kwargs["host"] == "0.0.0.0" + assert kwargs["port"] == 9000 + + +@patch("uvicorn.run") +def test_serve_with_custom_kwargs(mock_run, mock_strands_agent): + """Test that serve passes additional kwargs to uvicorn.run.""" + a2a_agent = A2AServer(mock_strands_agent, skills=[]) + + a2a_agent.serve(log_level="debug", reload=True) + + mock_run.assert_called_once() + _, kwargs = mock_run.call_args + assert kwargs["log_level"] == "debug" + assert kwargs["reload"] is True + + +@patch("uvicorn.run", side_effect=KeyboardInterrupt) +def test_serve_handles_keyboard_interrupt(mock_run, mock_strands_agent, caplog): + """Test that serve handles KeyboardInterrupt gracefully.""" + a2a_agent = A2AServer(mock_strands_agent, skills=[]) + + a2a_agent.serve() + + assert "Strands A2A server shutdown requested (KeyboardInterrupt)" in caplog.text + assert "Strands A2A server has shutdown" in caplog.text + + +@patch("uvicorn.run", side_effect=Exception("Test exception")) +def test_serve_handles_general_exception(mock_run, mock_strands_agent, caplog): + """Test that serve handles general exceptions gracefully.""" + a2a_agent = A2AServer(mock_strands_agent, skills=[]) + + a2a_agent.serve() + + assert "Strands A2A server encountered exception" in caplog.text + assert "Strands A2A server has shutdown" in caplog.text From 249206781b1e87b06e53f575bd94e975cd562400 Mon Sep 17 00:00:00 2001 From: jer Date: Thu, 26 Jun 2025 20:40:15 +0000 Subject: [PATCH 2/2] fix: pr comments --- src/strands/multiagent/a2a/server.py | 6 +- tests/multiagent/a2a/test_server.py | 104 +++++++++++++++++++++------ 2 files changed, 86 insertions(+), 24 deletions(-) diff --git a/src/strands/multiagent/a2a/server.py b/src/strands/multiagent/a2a/server.py index f49ddb603..0e271b1cf 100644 --- a/src/strands/multiagent/a2a/server.py +++ b/src/strands/multiagent/a2a/server.py @@ -32,7 +32,7 @@ def __init__( host: str = "0.0.0.0", port: int = 9000, version: str = "0.0.1", - skills: list[AgentSkill] | None, + skills: list[AgentSkill] | None = None, ): """Initialize an A2A-compatible agent from a Strands agent. @@ -58,7 +58,7 @@ def __init__( agent_executor=StrandsA2AExecutor(self.strands_agent), task_store=InMemoryTaskStore(), ) - self._agent_skills = skills or self._get_skills_from_tools() + self._agent_skills = skills logger.info("Strands' integration with A2A is experimental. Be aware of frequent breaking changes.") @property @@ -108,7 +108,7 @@ def _get_skills_from_tools(self) -> list[AgentSkill]: @property def agent_skills(self) -> list[AgentSkill]: """Get the list of skills this agent provides.""" - return self._agent_skills + return self._agent_skills if self._agent_skills is not None else self._get_skills_from_tools() @agent_skills.setter def agent_skills(self, skills: list[AgentSkill]) -> None: diff --git a/tests/multiagent/a2a/test_server.py b/tests/multiagent/a2a/test_server.py index 7323797dd..a851c8c7d 100644 --- a/tests/multiagent/a2a/test_server.py +++ b/tests/multiagent/a2a/test_server.py @@ -1,9 +1,10 @@ """Tests for the A2AAgent class.""" +from collections import OrderedDict from unittest.mock import patch import pytest -from a2a.types import AgentCapabilities, AgentCard +from a2a.types import AgentCapabilities, AgentCard, AgentSkill from fastapi import FastAPI from starlette.applications import Starlette @@ -16,7 +17,7 @@ def test_a2a_agent_initialization(mock_strands_agent): mock_tool_config = {"test_tool": {"name": "test_tool", "description": "A test tool"}} mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config - a2a_agent = A2AServer(mock_strands_agent, skills=None) + a2a_agent = A2AServer(mock_strands_agent) assert a2a_agent.strands_agent == mock_strands_agent assert a2a_agent.name == "Test Agent" @@ -26,7 +27,6 @@ def test_a2a_agent_initialization(mock_strands_agent): assert a2a_agent.http_url == "http://0.0.0.0:9000/" assert a2a_agent.version == "0.0.1" assert isinstance(a2a_agent.capabilities, AgentCapabilities) - # Should have skills from tools assert len(a2a_agent.agent_skills) == 1 assert a2a_agent.agent_skills[0].name == "test_tool" @@ -38,7 +38,6 @@ def test_a2a_agent_initialization_with_custom_values(mock_strands_agent): host="127.0.0.1", port=8080, version="1.0.0", - skills=None, ) assert a2a_agent.host == "127.0.0.1" @@ -49,7 +48,6 @@ def test_a2a_agent_initialization_with_custom_values(mock_strands_agent): def test_a2a_agent_initialization_with_custom_skills(mock_strands_agent): """Test that A2AAgent initializes correctly with custom skills.""" - from a2a.types import AgentSkill custom_skills = [ AgentSkill(name="custom_skill", id="custom_skill", description="A custom skill", tags=["test"]), @@ -110,7 +108,7 @@ def test_agent_skills_empty_registry(mock_strands_agent): # Mock empty tool registry mock_strands_agent.tool_registry.get_all_tools_config.return_value = {} - a2a_agent = A2AServer(mock_strands_agent, skills=None) + a2a_agent = A2AServer(mock_strands_agent) skills = a2a_agent.agent_skills assert isinstance(skills, list) @@ -123,7 +121,7 @@ def test_agent_skills_with_single_tool(mock_strands_agent): mock_tool_config = {"calculator": {"name": "calculator", "description": "Performs basic mathematical calculations"}} mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config - a2a_agent = A2AServer(mock_strands_agent, skills=None) + a2a_agent = A2AServer(mock_strands_agent) skills = a2a_agent.agent_skills assert isinstance(skills, list) @@ -146,7 +144,7 @@ def test_agent_skills_with_multiple_tools(mock_strands_agent): } mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config - a2a_agent = A2AServer(mock_strands_agent, skills=None) + a2a_agent = A2AServer(mock_strands_agent) skills = a2a_agent.agent_skills assert isinstance(skills, list) @@ -186,7 +184,7 @@ def test_agent_skills_with_complex_tool_config(mock_strands_agent): } mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config - a2a_agent = A2AServer(mock_strands_agent, skills=None) + a2a_agent = A2AServer(mock_strands_agent) skills = a2a_agent.agent_skills assert isinstance(skills, list) @@ -202,7 +200,6 @@ def test_agent_skills_with_complex_tool_config(mock_strands_agent): def test_agent_skills_preserves_tool_order(mock_strands_agent): """Test that agent_skills preserves the order of tools from the registry.""" # Mock tool registry with ordered tools - from collections import OrderedDict mock_tool_config = OrderedDict( [ @@ -213,7 +210,7 @@ def test_agent_skills_preserves_tool_order(mock_strands_agent): ) mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config - a2a_agent = A2AServer(mock_strands_agent, skills=None) + a2a_agent = A2AServer(mock_strands_agent) skills = a2a_agent.agent_skills assert len(skills) == 3 @@ -233,9 +230,11 @@ def test_agent_skills_handles_missing_description(mock_strands_agent): } mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config - # This should raise a KeyError during initialization when trying to access missing description + a2a_agent = A2AServer(mock_strands_agent) + + # This should raise a KeyError when accessing agent_skills due to missing description with pytest.raises(KeyError): - A2AServer(mock_strands_agent, skills=None) + _ = a2a_agent.agent_skills def test_agent_skills_handles_missing_name(mock_strands_agent): @@ -249,22 +248,23 @@ def test_agent_skills_handles_missing_name(mock_strands_agent): } mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config - # This should raise a KeyError during initialization when trying to access missing name + a2a_agent = A2AServer(mock_strands_agent) + + # This should raise a KeyError when accessing agent_skills due to missing name with pytest.raises(KeyError): - A2AServer(mock_strands_agent, skills=None) + _ = a2a_agent.agent_skills def test_agent_skills_setter(mock_strands_agent): """Test that agent_skills setter works correctly.""" - from a2a.types import AgentSkill # Mock tool registry for initial setup mock_tool_config = {"test_tool": {"name": "test_tool", "description": "A test tool"}} mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config - a2a_agent = A2AServer(mock_strands_agent, skills=None) + a2a_agent = A2AServer(mock_strands_agent) - # Initially should get skills from tools + # Initially should get skills from tools (lazy loaded) initial_skills = a2a_agent.agent_skills assert len(initial_skills) == 1 assert initial_skills[0].name == "test_tool" @@ -293,7 +293,7 @@ def test_get_skills_from_tools_method(mock_strands_agent): } mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config - a2a_agent = A2AServer(mock_strands_agent, skills=None) + a2a_agent = A2AServer(mock_strands_agent) skills = a2a_agent._get_skills_from_tools() assert isinstance(skills, list) @@ -317,7 +317,7 @@ def test_initialization_with_none_skills_uses_tools(mock_strands_agent): a2a_agent = A2AServer(mock_strands_agent, skills=None) - # Should get skills from tools + # Should get skills from tools (lazy loaded) skills = a2a_agent.agent_skills assert len(skills) == 1 assert skills[0].name == "test_tool" @@ -334,9 +334,71 @@ def test_initialization_with_empty_skills_list(mock_strands_agent): assert len(skills) == 0 +def test_lazy_loading_behavior(mock_strands_agent): + """Test that skills are only loaded from tools when accessed and no explicit skills are provided.""" + mock_tool_config = {"test_tool": {"name": "test_tool", "description": "A test tool"}} + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + # Create agent without explicit skills + a2a_agent = A2AServer(mock_strands_agent) + + # Verify that _agent_skills is None initially (not loaded yet) + assert a2a_agent._agent_skills is None + + # Access agent_skills property - this should trigger lazy loading + skills = a2a_agent.agent_skills + + # Verify skills were loaded from tools + assert len(skills) == 1 + assert skills[0].name == "test_tool" + + # Verify that _agent_skills is still None (lazy loading doesn't cache) + assert a2a_agent._agent_skills is None + + +def test_explicit_skills_override_tools(mock_strands_agent): + """Test that explicitly provided skills override tool-based skills.""" + + # Mock tool registry with tools + mock_tool_config = {"test_tool": {"name": "test_tool", "description": "A test tool"}} + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + # Provide explicit skills + explicit_skills = [AgentSkill(name="explicit_skill", id="explicit_skill", description="An explicit skill", tags=[])] + + a2a_agent = A2AServer(mock_strands_agent, skills=explicit_skills) + + # Should use explicit skills, not tools + skills = a2a_agent.agent_skills + assert len(skills) == 1 + assert skills[0].name == "explicit_skill" + assert skills[0].description == "An explicit skill" + + +def test_skills_not_loaded_during_initialization(mock_strands_agent): + """Test that skills are not loaded from tools during initialization.""" + # Create a mock that would raise an exception if called + mock_strands_agent.tool_registry.get_all_tools_config.side_effect = Exception("Should not be called during init") + + # This should not raise an exception because tools are not accessed during initialization + a2a_agent = A2AServer(mock_strands_agent) + + # Verify that _agent_skills is None + assert a2a_agent._agent_skills is None + + # Reset the mock to return proper data for when skills are actually accessed + mock_tool_config = {"test_tool": {"name": "test_tool", "description": "A test tool"}} + mock_strands_agent.tool_registry.get_all_tools_config.side_effect = None + mock_strands_agent.tool_registry.get_all_tools_config.return_value = mock_tool_config + + # Now accessing skills should work + skills = a2a_agent.agent_skills + assert len(skills) == 1 + assert skills[0].name == "test_tool" + + def test_public_agent_card_with_custom_skills(mock_strands_agent): """Test that public_agent_card includes custom skills.""" - from a2a.types import AgentSkill custom_skills = [ AgentSkill(name="custom_skill", id="custom_skill", description="A custom skill", tags=["test"]),