fix: pr comments

Author: jer96

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:

tests/multiagent/a2a/test_server.py

@@ -6,7 +6,6 @@
 from a2a.types import AgentCapabilities, AgentCard
 from fastapi import FastAPI
 from starlette.applications import Starlette
-
 from strands.multiagent.a2a.server import A2AServer
 
 
@@ -16,7 +15,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 +25,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 +36,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"
@@ -110,7 +107,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 +120,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 +143,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 +183,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)
@@ -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,9 +248,11 @@ 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):
@@ -262,9 +263,9 @@ def test_agent_skills_setter(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)
 
-    # 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 +294,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 +318,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,6 +335,70 @@ 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."""
+    from a2a.types import AgentSkill
+
+    # 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