From ba55e5f2cf9a7e3e0677adf0e78c5f1858589d84 Mon Sep 17 00:00:00 2001 From: Mackenzie Zastrow Date: Mon, 9 Jun 2025 15:01:59 -0400 Subject: [PATCH 1/2] refactor: Disallow similar tool names in the tool registry Per follow-up to #178, where we discussed preventing similar_tool and similar-tool from both being added to the tool registry, to avoid ambiguity in direct-method invocations --- src/strands/agent/agent.py | 8 ++------ src/strands/tools/registry.py | 15 +++++++++++++++ tests/strands/agent/test_agent.py | 22 ---------------------- tests/strands/tools/test_registry.py | 20 ++++++++++++++++++++ 4 files changed, 37 insertions(+), 28 deletions(-) diff --git a/src/strands/agent/agent.py b/src/strands/agent/agent.py index 5854fba68..56f5b92e7 100644 --- a/src/strands/agent/agent.py +++ b/src/strands/agent/agent.py @@ -108,14 +108,10 @@ def find_normalized_tool_name() -> Optional[str]: # all tools that can be represented with the normalized name if "_" in name: filtered_tools = [ - tool_name - for (tool_name, tool) in tool_registry.items() - if tool_name.replace("-", "_") == name + tool_name for (tool_name, tool) in tool_registry.items() if tool_name.replace("-", "_") == name ] - if len(filtered_tools) > 1: - raise AttributeError(f"Multiple tools matching '{name}' found: {', '.join(filtered_tools)}") - + # The registry itself defends against similar names, so we can just take the first match if filtered_tools: return filtered_tools[0] diff --git a/src/strands/tools/registry.py b/src/strands/tools/registry.py index 2efdd6002..e56ee999c 100644 --- a/src/strands/tools/registry.py +++ b/src/strands/tools/registry.py @@ -189,6 +189,21 @@ def register_tool(self, tool: AgentTool) -> None: tool.is_dynamic, ) + if self.registry.get(tool.tool_name) is None: + normalized_name = tool.tool_name.replace("-", "_") + + matching_tools = [ + tool_name + for (tool_name, tool) in self.registry.items() + if tool_name.replace("-", "_") == normalized_name + ] + + if matching_tools: + raise ValueError( + f"Tool name '{tool.tool_name}' already exists as '{matching_tools[0]}'." + " Cannot add a duplicate tool which differs by a '-' or '_'" + ) + # Register in main registry self.registry[tool.tool_name] = tool diff --git a/tests/strands/agent/test_agent.py b/tests/strands/agent/test_agent.py index 60b38ffab..d6f47be04 100644 --- a/tests/strands/agent/test_agent.py +++ b/tests/strands/agent/test_agent.py @@ -739,28 +739,6 @@ def function(system_prompt: str) -> str: } -def test_agent_tool_with_multiple_normalized_matches(agent, tool_registry, mock_randint): - agent.tool_handler = unittest.mock.Mock() - - @strands.tools.tool(name="system-prompter_1") - def function1(system_prompt: str) -> str: - return system_prompt - - @strands.tools.tool(name="system-prompter-1") - def function2(system_prompt: str) -> str: - return system_prompt - - agent.tool_registry.register_tool(strands.tools.tools.FunctionTool(function1)) - agent.tool_registry.register_tool(strands.tools.tools.FunctionTool(function2)) - - mock_randint.return_value = 1 - - with pytest.raises(AttributeError) as err: - agent.tool.system_prompter_1(system_prompt="tool prompt") - - assert str(err.value) == "Multiple tools matching 'system_prompter_1' found: system-prompter_1, system-prompter-1" - - def test_agent_tool_with_no_normalized_match(agent, tool_registry, mock_randint): agent.tool_handler = unittest.mock.Mock() diff --git a/tests/strands/tools/test_registry.py b/tests/strands/tools/test_registry.py index 3dca5371c..1b274f46b 100644 --- a/tests/strands/tools/test_registry.py +++ b/tests/strands/tools/test_registry.py @@ -2,8 +2,11 @@ Tests for the SDK tool registry module. """ +from unittest.mock import MagicMock + import pytest +from strands.tools import PythonAgentTool from strands.tools.registry import ToolRegistry @@ -23,3 +26,20 @@ def test_process_tools_with_invalid_path(): with pytest.raises(ValueError, match=f"Failed to load tool {invalid_path.split('.')[0]}: Tool file not found:.*"): tool_registry.process_tools([invalid_path]) + + +def test_register_tool_with_similar_name_raises(): + tool_1 = PythonAgentTool(tool_name="tool-like-this", tool_spec=MagicMock(), callback=lambda: None) + tool_2 = PythonAgentTool(tool_name="tool_like_this", tool_spec=MagicMock(), callback=lambda: None) + + tool_registry = ToolRegistry() + + tool_registry.register_tool(tool_1) + + with pytest.raises(ValueError) as err: + tool_registry.register_tool(tool_2) + + assert ( + str(err.value) == "Tool name 'tool_like_this' already exists as 'tool-like-this'. " + "Cannot add a duplicate tool which differs by a '-' or '_'" + ) From f9aec4b305a552ad67054ea7191c79666ef44081 Mon Sep 17 00:00:00 2001 From: Mackenzie Zastrow Date: Wed, 11 Jun 2025 16:02:30 -0400 Subject: [PATCH 2/2] fix: Remove comment that is no longer needed --- src/strands/telemetry/tracer.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/strands/telemetry/tracer.py b/src/strands/telemetry/tracer.py index 9f731996e..34eb7bed8 100644 --- a/src/strands/telemetry/tracer.py +++ b/src/strands/telemetry/tracer.py @@ -13,9 +13,7 @@ from opentelemetry import trace from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter - -# See https://github.com/open-telemetry/opentelemetry-python/issues/4615 for the type ignore -from opentelemetry.sdk.resources import Resource # type: ignore[attr-defined] +from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.trace import TracerProvider from opentelemetry.sdk.trace.export import BatchSpanProcessor, ConsoleSpanExporter, SimpleSpanProcessor from opentelemetry.trace import StatusCode