remove max_tools, add kwargs

Author: dbschmigelski

src/strands/agent/agent.py

@@ -524,6 +524,9 @@ def cleanup(self) -> None:
         Note: This method uses a "belt and braces" approach with automatic cleanup
         through __del__ as a fallback, but explicit cleanup is recommended.
         """
+        if self._cleanup_called:
+            return
+
         run_async(self.cleanup_async)
 
     async def cleanup_async(self) -> None:

src/strands/experimental/tools/mcp/mcp_tool_provider.py

@@ -1,7 +1,7 @@
 """MCP Tool Provider implementation."""
 
 import logging
-from typing import Callable, Optional, Pattern, Sequence, Union
+from typing import Any, Callable, Optional, Pattern, Sequence, Union
 
 from typing_extensions import TypedDict
 
@@ -23,37 +23,39 @@ class ToolFilters(TypedDict, total=False):
     Tools are filtered in this order:
     1. If 'allowed' is specified, only tools matching these patterns are included
     2. Tools matching 'rejected' patterns are then excluded
-    3. If the result exceeds 'max_tools', it's truncated
     """
 
     allowed: list[_ToolFilterPattern]
     rejected: list[_ToolFilterPattern]
-    max_tools: int
 
 
 class MCPToolProvider(ToolProvider):
     """Tool provider for MCP clients with managed lifecycle."""
 
     def __init__(
-        self, *, client: MCPClient, tool_filters: Optional[ToolFilters] = None, disambiguator: Optional[str] = None
+        self,
+        *,
+        client: MCPClient,
+        tool_filters: Optional[ToolFilters] = None,
+        prefix: Optional[str] = None,
+        **kwargs: Any,
     ) -> None:
         """Initialize with an MCP client.
 
         Args:
             client: The MCP client to manage.
             tool_filters: Optional filters to apply to tools.
-            disambiguator: Optional prefix for tool names.
+            prefix: Optional prefix for tool names.
+            **kwargs: Additional arguments for future compatibility.
         """
-        logger.debug(
-            "tool_filters=<%s>, disambiguator=<%s> | initializing MCPToolProvider", tool_filters, disambiguator
-        )
+        logger.debug("tool_filters=<%s>, prefix=<%s> | initializing MCPToolProvider", tool_filters, prefix)
         self._client = client
         self._tool_filters = tool_filters
-        self._disambiguator = disambiguator
+        self._prefix = prefix
         self._tools: Optional[list[MCPAgentTool]] = None  # None = not loaded yet, [] = loaded but empty
         self._started = False
 
-    async def load_tools(self) -> Sequence[AgentTool]:
+    async def load_tools(self, **kwargs: Any) -> Sequence[AgentTool]:
         """Load and return tools from the MCP client.
 
         Returns:
@@ -77,12 +79,6 @@ async def load_tools(self) -> Sequence[AgentTool]:
             pagination_token = None
             page_count = 0
 
-            # Determine max_tools limit for early termination
-            max_tools_limit = None
-            if self._tool_filters and "max_tools" in self._tool_filters:
-                max_tools_limit = self._tool_filters["max_tools"]
-                logger.debug("max_tools_limit=<%d> | will stop when reached", max_tools_limit)
-
             while True:
                 logger.debug("page=<%d>, token=<%s> | fetching tools page", page_count, pagination_token)
                 paginated_tools = self._client.list_tools_sync(pagination_token)
@@ -91,15 +87,10 @@ async def load_tools(self) -> Sequence[AgentTool]:
                 for tool in paginated_tools:
                     # Apply filters
                     if self._should_include_tool(tool):
-                        # Apply disambiguation if needed
-                        processed_tool = self._apply_disambiguation(tool)
+                        # Apply prefix if needed
+                        processed_tool = self._apply_prefix(tool)
                         self._tools.append(processed_tool)
 
-                        # Check if we've reached max_tools limit
-                        if max_tools_limit is not None and len(self._tools) >= max_tools_limit:
-                            logger.debug("max_tools_reached=<%d> | stopping pagination early", len(self._tools))
-                            return self._tools
-
                 logger.debug(
                     "page=<%d>, page_tools=<%d>, total_filtered=<%d> | processed page",
                     page_count,
@@ -134,14 +125,14 @@ def _should_include_tool(self, tool: MCPAgentTool) -> bool:
 
         return True
 
-    def _apply_disambiguation(self, tool: MCPAgentTool) -> MCPAgentTool:
-        """Apply disambiguation to a single tool if needed."""
-        if not self._disambiguator:
+    def _apply_prefix(self, tool: MCPAgentTool) -> MCPAgentTool:
+        """Apply prefix to a single tool if needed."""
+        if not self._prefix:
             return tool
 
-        # Create new tool with disambiguated agent name but preserve original MCP name
+        # Create new tool with prefixed agent name but preserve original MCP name
         old_name = tool.tool_name
-        new_agent_name = f"{self._disambiguator}_{tool.mcp_tool.name}"
+        new_agent_name = f"{self._prefix}_{tool.mcp_tool.name}"
         new_tool = MCPAgentTool(tool.mcp_tool, tool.mcp_client, agent_facing_tool_name=new_agent_name)
         logger.debug("tool_rename=<%s->%s> | renamed tool", old_name, new_agent_name)
         return new_tool
@@ -160,7 +151,7 @@ def _matches_patterns(self, tool: MCPAgentTool, patterns: list[_ToolFilterPatter
                     return True
         return False
 
-    async def cleanup(self) -> None:
+    async def cleanup(self, **kwargs: Any) -> None:
         """Clean up the MCP client connection."""
         if not self._started:
             return

src/strands/experimental/tools/tool_provider.py

@@ -1,7 +1,7 @@
 """Tool provider interface."""
 
 from abc import ABC, abstractmethod
-from typing import TYPE_CHECKING, Sequence
+from typing import TYPE_CHECKING, Any, Sequence
 
 if TYPE_CHECKING:
     from ...types.tools import AgentTool
@@ -15,18 +15,24 @@ class ToolProvider(ABC):
     """
 
     @abstractmethod
-    async def load_tools(self) -> Sequence["AgentTool"]:
+    async def load_tools(self, **kwargs: Any) -> Sequence["AgentTool"]:
         """Load and return the tools in this provider.
 
+        Args:
+            **kwargs: Additional arguments for future compatibility.
+
         Returns:
             List of tools that are ready to use.
         """
         pass
 
     @abstractmethod
-    async def cleanup(self) -> None:
+    async def cleanup(self, **kwargs: Any) -> None:
         """Clean up resources used by the tools in this provider.
 
+        Args:
+            **kwargs: Additional arguments for future compatibility.
+
         Should be called when the tools are no longer needed.
         """
         pass

tests/strands/agent/test_agent.py

@@ -1012,6 +1012,18 @@ def test_agent_cleanup_idempotent(agent):
     mock_provider.cleanup.assert_called_once()
 
 
+def test_agent_cleanup_early_return_avoids_thread_spawn(agent):
+    """Test that cleanup returns early when already called, avoiding thread spawn cost."""
+    # Mark cleanup as already called
+    agent._cleanup_called = True
+
+    with unittest.mock.patch("strands.agent.agent.run_async") as mock_run_async:
+        agent.cleanup()
+
+        # Verify run_async was not called since cleanup already happened
+        mock_run_async.assert_not_called()
+
+
 def test_agent__del__emits_warning_for_automatic_cleanup(agent):
     """Test that __del__ emits warning when cleanup wasn't called manually."""
     # Add a mock tool provider so cleanup will be called

tests/strands/experimental/tools/mcp/test_mcp_tool_provider.py

@@ -55,21 +55,21 @@ def test_init_with_client_only(mock_mcp_client):
 
     assert provider._client is mock_mcp_client
     assert provider._tool_filters is None
-    assert provider._disambiguator is None
+    assert provider._prefix is None
     assert provider._tools is None
     assert provider._started is False
 
 
 def test_init_with_all_parameters(mock_mcp_client):
     """Test initialization with all parameters."""
-    filters = {"allowed": ["tool1"], "max_tools": 5}
-    disambiguator = "test_prefix"
+    filters = {"allowed": ["tool1"]}
+    prefix = "test_prefix"
 
-    provider = MCPToolProvider(client=mock_mcp_client, tool_filters=filters, disambiguator=disambiguator)
+    provider = MCPToolProvider(client=mock_mcp_client, tool_filters=filters, prefix=prefix)
 
     assert provider._client is mock_mcp_client
     assert provider._tool_filters == filters
-    assert provider._disambiguator == disambiguator
+    assert provider._prefix == prefix
     assert provider._tools is None
     assert provider._started is False
 
@@ -232,24 +232,8 @@ async def test_rejected_filter(mock_mcp_client):
 
 
 @pytest.mark.asyncio
-async def test_max_tools_filter(mock_mcp_client):
-    """Test max_tools filter functionality."""
-    tools_list = [create_mock_tool(f"tool_{i}") for i in range(5)]
-
-    mock_mcp_client.list_tools_sync.return_value = PaginatedList(tools_list)
-
-    filters: ToolFilters = {"max_tools": 3}
-    provider = MCPToolProvider(client=mock_mcp_client, tool_filters=filters)
-
-    tools = await provider.load_tools()
-
-    assert len(tools) == 3
-    assert all(tool.tool_name.startswith("tool_") for tool in tools)
-
-
-@pytest.mark.asyncio
-async def test_disambiguator_renames_tools(mock_mcp_client):
-    """Test that disambiguator properly renames tools."""
+async def test_prefix_renames_tools(mock_mcp_client):
+    """Test that prefix properly renames tools."""
     original_tool = MagicMock(spec=MCPAgentTool)
     original_tool.tool_name = "original_name"
     original_tool.mcp_tool = MagicMock()
@@ -263,7 +247,7 @@ async def test_disambiguator_renames_tools(mock_mcp_client):
         new_tool.tool_name = "prefix_original_name"
         mock_agent_tool_class.return_value = new_tool
 
-        provider = MCPToolProvider(client=mock_mcp_client, disambiguator="prefix")
+        provider = MCPToolProvider(client=mock_mcp_client, prefix="prefix")
 
         tools = await provider.load_tools()
 

tests_integ/test_mcp_tool_provider.py

@@ -22,7 +22,7 @@ def test_mcp_tool_provider_filters():
         lambda: stdio_client(StdioServerParameters(command="python", args=["tests_integ/echo_server.py"]))
     )
 
-    # Test string filter, regex filter, callable filter, max_tools, and disambiguator
+    # Test string filter, regex filter, callable filter, and prefix
     def short_names_only(tool) -> bool:
         return len(tool.tool_name) <= 20  # Allow most tools
 
@@ -32,7 +32,7 @@ def short_names_only(tool) -> bool:
         "max_tools": 2,
     }
 
-    provider = MCPToolProvider(client=stdio_mcp_client, tool_filters=filters, disambiguator="test")
+    provider = MCPToolProvider(client=stdio_mcp_client, tool_filters=filters, prefix="test")
     agent = Agent(tools=[provider])
     tool_names = agent.tool_names
 
@@ -51,7 +51,7 @@ def test_mcp_tool_provider_execution():
     )
 
     filters: ToolFilters = {"allowed": ["echo"]}
-    provider = MCPToolProvider(client=stdio_mcp_client, tool_filters=filters, disambiguator="filtered")
+    provider = MCPToolProvider(client=stdio_mcp_client, tool_filters=filters, prefix="filtered")
     agent = Agent(
         tools=[provider],
     )
@@ -80,7 +80,7 @@ def test_mcp_tool_provider_reuse():
     )
 
     filters: ToolFilters = {"allowed": ["echo"]}
-    provider = MCPToolProvider(client=stdio_mcp_client, tool_filters=filters, disambiguator="shared")
+    provider = MCPToolProvider(client=stdio_mcp_client, tool_filters=filters, prefix="shared")
 
     # Create first agent with the provider
     agent1 = Agent(tools=[provider])
@@ -116,11 +116,11 @@ def test_mcp_tool_provider_multiple_servers():
         lambda: stdio_client(StdioServerParameters(command="python", args=["tests_integ/echo_server.py"]))
     )
 
-    # Create providers with different disambiguators
-    provider1 = MCPToolProvider(client=client1, tool_filters={"allowed": ["echo"]}, disambiguator="server1")
+    # Create providers with different prefixes
+    provider1 = MCPToolProvider(client=client1, tool_filters={"allowed": ["echo"]}, prefix="server1")
     # Use correct tool name from echo_server.py
     provider2 = MCPToolProvider(
-        client=client2, tool_filters={"allowed": ["echo_with_structured_content"]}, disambiguator="server2"
+        client=client2, tool_filters={"allowed": ["echo_with_structured_content"]}, prefix="server2"
     )
 
     # Create agent with both providers