From 9768b2db6183bd6b54722a5afb671bb686796f48 Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 3 Nov 2025 21:48:48 +0000 Subject: [PATCH 1/2] Add conv_state parameter to ToolDefinition.create() and update all downstream usage - Add conv_state: ConversationState parameter to ToolDefinition.create() base method - Update all tool subclasses to accept conv_state parameter: - FinishTool, ThinkTool, MCPToolDefinition, BrowserUseTool - Update all call sites that invoke .create() method throughout the codebase - Fix all test files to pass conv_state parameter correctly - All 1061 SDK tests pass (excluding browser and e2e tests) Co-authored-by: openhands --- openhands-sdk/openhands/sdk/agent/base.py | 2 +- openhands-sdk/openhands/sdk/mcp/tool.py | 5 +- openhands-sdk/openhands/sdk/mcp/utils.py | 16 ++- .../openhands/sdk/tool/builtins/finish.py | 4 +- .../openhands/sdk/tool/builtins/think.py | 4 +- openhands-sdk/openhands/sdk/tool/tool.py | 6 +- .../openhands/tools/browser_use/definition.py | 97 ++++++++++++++++--- tests/sdk/agent/test_agent_serialization.py | 10 +- tests/sdk/mcp/test_create_mcp_tool.py | 32 +++--- tests/sdk/mcp/test_mcp_security_risk.py | 18 ++-- tests/sdk/mcp/test_mcp_tool.py | 17 +++- tests/sdk/mcp/test_mcp_tool_immutability.py | 12 ++- tests/sdk/mcp/test_mcp_tool_kind_field.py | 4 +- tests/sdk/mcp/test_mcp_tool_serialization.py | 26 +++-- tests/sdk/mcp/test_mcp_tool_validation.py | 14 +-- tests/sdk/tool/test_builtins.py | 4 +- tests/sdk/tool/test_tool_serialization.py | 30 +++--- .../tools/browser_use/test_browser_toolset.py | 38 ++++---- 18 files changed, 233 insertions(+), 106 deletions(-) diff --git a/openhands-sdk/openhands/sdk/agent/base.py b/openhands-sdk/openhands/sdk/agent/base.py index cc47e5ed68..655b2f8a9d 100644 --- a/openhands-sdk/openhands/sdk/agent/base.py +++ b/openhands-sdk/openhands/sdk/agent/base.py @@ -208,7 +208,7 @@ def _initialize(self, state: "ConversationState"): # Add MCP tools if configured if self.mcp_config: - mcp_tools = create_mcp_tools(self.mcp_config, timeout=30) + mcp_tools = create_mcp_tools(state, self.mcp_config, timeout=30) tools.extend(mcp_tools) logger.info( diff --git a/openhands-sdk/openhands/sdk/mcp/tool.py b/openhands-sdk/openhands/sdk/mcp/tool.py index 3ee9ff5ff6..07fad047b6 100644 --- a/openhands-sdk/openhands/sdk/mcp/tool.py +++ b/openhands-sdk/openhands/sdk/mcp/tool.py @@ -6,7 +6,7 @@ if TYPE_CHECKING: - from openhands.sdk.conversation import LocalConversation + from openhands.sdk.conversation import ConversationState, LocalConversation import mcp.types from litellm import ChatCompletionToolParam @@ -188,8 +188,11 @@ def action_from_arguments(self, arguments: dict[str, Any]) -> MCPToolAction: @classmethod def create( cls, + conv_state: "ConversationState", # noqa: ARG003 + *, mcp_tool: mcp.types.Tool, mcp_client: MCPClient, + **kwargs, # noqa: ARG003 ) -> Sequence["MCPToolDefinition"]: try: annotations = ( diff --git a/openhands-sdk/openhands/sdk/mcp/utils.py b/openhands-sdk/openhands/sdk/mcp/utils.py index 7c3252c37c..30dc1fdb31 100644 --- a/openhands-sdk/openhands/sdk/mcp/utils.py +++ b/openhands-sdk/openhands/sdk/mcp/utils.py @@ -1,6 +1,7 @@ """Utility functions for MCP integration.""" import logging +from typing import TYPE_CHECKING import mcp.types from fastmcp.client.logging import LogMessage @@ -11,6 +12,10 @@ from openhands.sdk.tool.tool import ToolDefinition +if TYPE_CHECKING: + from openhands.sdk.conversation import ConversationState + + logger = get_logger(__name__) LOGGING_LEVEL_MAP = logging.getLevelNamesMapping() @@ -30,7 +35,9 @@ async def log_handler(message: LogMessage): logger.log(level, msg, extra=extra) -async def _list_tools(client: MCPClient) -> list[ToolDefinition]: +async def _list_tools( + conv_state: "ConversationState", client: MCPClient +) -> list[ToolDefinition]: """List tools from an MCP client.""" tools: list[ToolDefinition] = [] @@ -39,7 +46,7 @@ async def _list_tools(client: MCPClient) -> list[ToolDefinition]: mcp_type_tools: list[mcp.types.Tool] = await client.list_tools() for mcp_tool in mcp_type_tools: tool_sequence = MCPToolDefinition.create( - mcp_tool=mcp_tool, mcp_client=client + conv_state=conv_state, mcp_tool=mcp_tool, mcp_client=client ) tools.extend(tool_sequence) # Flatten sequence into list assert not client.is_connected(), ( @@ -49,6 +56,7 @@ async def _list_tools(client: MCPClient) -> list[ToolDefinition]: def create_mcp_tools( + conv_state: "ConversationState", config: dict | MCPConfig, timeout: float = 30.0, ) -> list[MCPToolDefinition]: @@ -57,7 +65,9 @@ def create_mcp_tools( if isinstance(config, dict): config = MCPConfig.model_validate(config) client = MCPClient(config, log_handler=log_handler) - tools = client.call_async_from_sync(_list_tools, timeout=timeout, client=client) + tools = client.call_async_from_sync( + _list_tools, timeout=timeout, conv_state=conv_state, client=client + ) logger.info(f"Created {len(tools)} MCP tools: {[t.name for t in tools]}") return tools diff --git a/openhands-sdk/openhands/sdk/tool/builtins/finish.py b/openhands-sdk/openhands/sdk/tool/builtins/finish.py index 0b0aea9a29..27e8b1ee91 100644 --- a/openhands-sdk/openhands/sdk/tool/builtins/finish.py +++ b/openhands-sdk/openhands/sdk/tool/builtins/finish.py @@ -74,13 +74,13 @@ class FinishTool(ToolDefinition[FinishAction, FinishObservation]): @classmethod def create( cls, - conv_state: "ConversationState | None" = None, # noqa: ARG003 + conv_state: "ConversationState", # noqa: ARG003 **params, ) -> Sequence[Self]: """Create FinishTool instance. Args: - conv_state: Optional conversation state (not used by FinishTool). + conv_state: Conversation state (not used by FinishTool). **params: Additional parameters (none supported). Returns: diff --git a/openhands-sdk/openhands/sdk/tool/builtins/think.py b/openhands-sdk/openhands/sdk/tool/builtins/think.py index 090f66c23d..812626e897 100644 --- a/openhands-sdk/openhands/sdk/tool/builtins/think.py +++ b/openhands-sdk/openhands/sdk/tool/builtins/think.py @@ -90,13 +90,13 @@ class ThinkTool(ToolDefinition[ThinkAction, ThinkObservation]): @classmethod def create( cls, - conv_state: "ConversationState | None" = None, # noqa: ARG003 + conv_state: "ConversationState", # noqa: ARG003 **params, ) -> Sequence[Self]: """Create ThinkTool instance. Args: - conv_state: Optional conversation state (not used by ThinkTool). + conv_state: Conversation state (not used by ThinkTool). **params: Additional parameters (none supported). Returns: diff --git a/openhands-sdk/openhands/sdk/tool/tool.py b/openhands-sdk/openhands/sdk/tool/tool.py index eb93ef9c1e..39ae83d48a 100644 --- a/openhands-sdk/openhands/sdk/tool/tool.py +++ b/openhands-sdk/openhands/sdk/tool/tool.py @@ -34,7 +34,7 @@ if TYPE_CHECKING: - from openhands.sdk.conversation import LocalConversation + from openhands.sdk.conversation import ConversationState, LocalConversation ActionT = TypeVar("ActionT", bound=Action) @@ -179,7 +179,7 @@ def create(cls, conv_state, **params): @classmethod @abstractmethod - def create(cls, *args, **kwargs) -> Sequence[Self]: + def create(cls, conv_state: "ConversationState", **kwargs) -> Sequence[Self]: """Create a sequence of Tool instances. This method must be implemented by all subclasses to provide custom @@ -187,7 +187,7 @@ def create(cls, *args, **kwargs) -> Sequence[Self]: from conv_state and other optional parameters. Args: - *args: Variable positional arguments (typically conv_state as first arg). + conv_state: Conversation state containing workspace and other context. **kwargs: Optional parameters for tool initialization. Returns: diff --git a/openhands-tools/openhands/tools/browser_use/definition.py b/openhands-tools/openhands/tools/browser_use/definition.py index 8ee077d66e..1a415f1d23 100644 --- a/openhands-tools/openhands/tools/browser_use/definition.py +++ b/openhands-tools/openhands/tools/browser_use/definition.py @@ -17,7 +17,7 @@ # Lazy import to avoid hanging during module import if TYPE_CHECKING: - from openhands.tools.browser_use.impl import BrowserToolExecutor + from openhands.sdk.conversation.state import ConversationState # Maximum output size for browser observations @@ -102,7 +102,14 @@ class BrowserNavigateTool(ToolDefinition[BrowserNavigateAction, BrowserObservati """Tool for browser navigation.""" @classmethod - def create(cls, executor: "BrowserToolExecutor") -> Sequence[Self]: + def create( + cls, + conv_state: "ConversationState", # noqa: ARG003 + **kwargs, + ) -> Sequence[Self]: + from openhands.tools.browser_use.impl import BrowserToolExecutor + + executor = kwargs.get("executor") or BrowserToolExecutor(**kwargs) return [ cls( name="browser_navigate", @@ -153,7 +160,14 @@ class BrowserClickTool(ToolDefinition[BrowserClickAction, BrowserObservation]): """Tool for clicking browser elements.""" @classmethod - def create(cls, executor: "BrowserToolExecutor") -> Sequence[Self]: + def create( + cls, + conv_state: "ConversationState", # noqa: ARG003 + **kwargs, + ) -> Sequence[Self]: + from openhands.tools.browser_use.impl import BrowserToolExecutor + + executor = kwargs.get("executor") or BrowserToolExecutor(**kwargs) return [ cls( name="browser_click", @@ -201,7 +215,14 @@ class BrowserTypeTool(ToolDefinition[BrowserTypeAction, BrowserObservation]): """Tool for typing text into browser elements.""" @classmethod - def create(cls, executor: "BrowserToolExecutor") -> Sequence[Self]: + def create( + cls, + conv_state: "ConversationState", # noqa: ARG003 + **kwargs, + ) -> Sequence[Self]: + from openhands.tools.browser_use.impl import BrowserToolExecutor + + executor = kwargs.get("executor") or BrowserToolExecutor(**kwargs) return [ cls( name="browser_type", @@ -246,7 +267,14 @@ class BrowserGetStateTool(ToolDefinition[BrowserGetStateAction, BrowserObservati """Tool for getting browser state.""" @classmethod - def create(cls, executor: "BrowserToolExecutor") -> Sequence[Self]: + def create( + cls, + conv_state: "ConversationState", # noqa: ARG003 + **kwargs, + ) -> Sequence[Self]: + from openhands.tools.browser_use.impl import BrowserToolExecutor + + executor = kwargs.get("executor") or BrowserToolExecutor(**kwargs) return [ cls( name="browser_get_state", @@ -294,7 +322,14 @@ class BrowserGetContentTool( """Tool for getting page content in markdown.""" @classmethod - def create(cls, executor: "BrowserToolExecutor") -> Sequence[Self]: + def create( + cls, + conv_state: "ConversationState", # noqa: ARG003 + **kwargs, + ) -> Sequence[Self]: + from openhands.tools.browser_use.impl import BrowserToolExecutor + + executor = kwargs.get("executor") or BrowserToolExecutor(**kwargs) return [ cls( name="browser_get_content", @@ -339,7 +374,14 @@ class BrowserScrollTool(ToolDefinition[BrowserScrollAction, BrowserObservation]) """Tool for scrolling the browser page.""" @classmethod - def create(cls, executor: "BrowserToolExecutor") -> Sequence[Self]: + def create( + cls, + conv_state: "ConversationState", # noqa: ARG003 + **kwargs, + ) -> Sequence[Self]: + from openhands.tools.browser_use.impl import BrowserToolExecutor + + executor = kwargs.get("executor") or BrowserToolExecutor(**kwargs) return [ cls( name="browser_scroll", @@ -378,7 +420,14 @@ class BrowserGoBackTool(ToolDefinition[BrowserGoBackAction, BrowserObservation]) """Tool for going back in browser history.""" @classmethod - def create(cls, executor: "BrowserToolExecutor") -> Sequence[Self]: + def create( + cls, + conv_state: "ConversationState", # noqa: ARG003 + **kwargs, + ) -> Sequence[Self]: + from openhands.tools.browser_use.impl import BrowserToolExecutor + + executor = kwargs.get("executor") or BrowserToolExecutor(**kwargs) return [ cls( name="browser_go_back", @@ -417,7 +466,14 @@ class BrowserListTabsTool(ToolDefinition[BrowserListTabsAction, BrowserObservati """Tool for listing browser tabs.""" @classmethod - def create(cls, executor: "BrowserToolExecutor") -> Sequence[Self]: + def create( + cls, + conv_state: "ConversationState", # noqa: ARG003 + **kwargs, + ) -> Sequence[Self]: + from openhands.tools.browser_use.impl import BrowserToolExecutor + + executor = kwargs.get("executor") or BrowserToolExecutor(**kwargs) return [ cls( name="browser_list_tabs", @@ -461,7 +517,14 @@ class BrowserSwitchTabTool(ToolDefinition[BrowserSwitchTabAction, BrowserObserva """Tool for switching browser tabs.""" @classmethod - def create(cls, executor: "BrowserToolExecutor") -> Sequence[Self]: + def create( + cls, + conv_state: "ConversationState", # noqa: ARG003 + **kwargs, + ) -> Sequence[Self]: + from openhands.tools.browser_use.impl import BrowserToolExecutor + + executor = kwargs.get("executor") or BrowserToolExecutor(**kwargs) return [ cls( name="browser_switch_tab", @@ -504,7 +567,14 @@ class BrowserCloseTabTool(ToolDefinition[BrowserCloseTabAction, BrowserObservati """Tool for closing browser tabs.""" @classmethod - def create(cls, executor: "BrowserToolExecutor") -> Sequence[Self]: + def create( + cls, + conv_state: "ConversationState", # noqa: ARG003 + **kwargs, + ) -> Sequence[Self]: + from openhands.tools.browser_use.impl import BrowserToolExecutor + + executor = kwargs.get("executor") or BrowserToolExecutor(**kwargs) return [ cls( name="browser_close_tab", @@ -536,13 +606,16 @@ class BrowserToolSet(ToolDefinition[BrowserAction, BrowserObservation]): @classmethod def create( cls, + conv_state: "ConversationState", **executor_config, ) -> list[ToolDefinition[BrowserAction, BrowserObservation]]: # Import executor only when actually needed to # avoid hanging during module import from openhands.tools.browser_use.impl import BrowserToolExecutor + # Create a single shared executor for all tools executor = BrowserToolExecutor(**executor_config) + # Each tool.create() returns a Sequence[Self], so we flatten the results tools: list[ToolDefinition[BrowserAction, BrowserObservation]] = [] for tool_class in [ @@ -557,5 +630,5 @@ def create( BrowserSwitchTabTool, BrowserCloseTabTool, ]: - tools.extend(tool_class.create(executor)) + tools.extend(tool_class.create(conv_state, executor=executor)) return tools diff --git a/tests/sdk/agent/test_agent_serialization.py b/tests/sdk/agent/test_agent_serialization.py index 0636c42978..2100af1182 100644 --- a/tests/sdk/agent/test_agent_serialization.py +++ b/tests/sdk/agent/test_agent_serialization.py @@ -17,7 +17,7 @@ from openhands.sdk.utils.models import OpenHandsModel -def create_mock_mcp_tool(name: str) -> MCPToolDefinition: +def create_mock_mcp_tool(conv_state, name: str) -> MCPToolDefinition: # Create mock MCP tool and client mock_mcp_tool = mcp.types.Tool( name=name, @@ -31,7 +31,9 @@ def create_mock_mcp_tool(name: str) -> MCPToolDefinition: }, ) mock_client = Mock(spec=MCPClient) - tools = MCPToolDefinition.create(mock_mcp_tool, mock_client) + tools = MCPToolDefinition.create( + conv_state, mcp_tool=mock_mcp_tool, mcp_client=mock_client + ) return tools[0] # Extract single tool from sequence @@ -52,8 +54,8 @@ def test_agent_supports_polymorphic_json_serialization() -> None: assert deserialized_agent.model_dump() == agent.model_dump() -def test_mcp_tool_serialization(): - tool = create_mock_mcp_tool("test_mcp_tool_serialization") +def test_mcp_tool_serialization(mock_conversation_state): + tool = create_mock_mcp_tool(mock_conversation_state, "test_mcp_tool_serialization") dumped = tool.model_dump_json() loaded = ToolDefinition.model_validate_json(dumped) assert loaded.model_dump_json() == dumped diff --git a/tests/sdk/mcp/test_create_mcp_tool.py b/tests/sdk/mcp/test_create_mcp_tool.py index 9b2dd8b59f..dd6d7c5d04 100644 --- a/tests/sdk/mcp/test_create_mcp_tool.py +++ b/tests/sdk/mcp/test_create_mcp_tool.py @@ -5,7 +5,7 @@ from openhands.sdk.mcp import create_mcp_tools -def test_mock_create_mcp_tools_empty_config(): +def test_mock_create_mcp_tools_empty_config(mock_conversation_state): """Test creating MCP tools with empty configuration.""" config = {} @@ -17,12 +17,12 @@ def test_mock_create_mcp_tools_empty_config(): # Mock call_async_from_sync to return empty list mock_client.call_async_from_sync.return_value = [] - tools = create_mcp_tools(config) + tools = create_mcp_tools(mock_conversation_state, config) assert len(tools) == 0 -def test_mock_create_mcp_tools_stdio_server(): +def test_mock_create_mcp_tools_stdio_server(mock_conversation_state): """Test creating MCP tools with stdio server configuration.""" config = { "mcpServers": { @@ -48,13 +48,13 @@ def test_mock_create_mcp_tools_stdio_server(): # Mock call_async_from_sync to return the tools mock_client.call_async_from_sync.return_value = [mock_tool] - tools = create_mcp_tools(config) + tools = create_mcp_tools(mock_conversation_state, config) assert len(tools) == 1 assert tools[0] == mock_tool -def test_mock_create_mcp_tools_http_server(): +def test_mock_create_mcp_tools_http_server(mock_conversation_state): """Test creating MCP tools with HTTP server configuration.""" config = { "mcpServers": { @@ -79,13 +79,13 @@ def test_mock_create_mcp_tools_http_server(): # Mock call_async_from_sync to return the tools mock_client.call_async_from_sync.return_value = [mock_tool] - tools = create_mcp_tools(config) + tools = create_mcp_tools(mock_conversation_state, config) assert len(tools) == 1 assert tools[0] == mock_tool -def test_mock_create_mcp_tools_mixed_servers(): +def test_mock_create_mcp_tools_mixed_servers(mock_conversation_state): """Test creating MCP tools with mixed server configurations.""" config = { "mcpServers": { @@ -115,14 +115,14 @@ def test_mock_create_mcp_tools_mixed_servers(): # Mock call_async_from_sync to return the tools mock_client.call_async_from_sync.return_value = [mock_tool1, mock_tool2] - tools = create_mcp_tools(config) + tools = create_mcp_tools(mock_conversation_state, config) assert len(tools) == 2 assert tools[0] == mock_tool1 assert tools[1] == mock_tool2 -def test_mock_create_mcp_tools_with_timeout(): +def test_mock_create_mcp_tools_with_timeout(mock_conversation_state): """Test creating MCP tools with custom timeout.""" config = { "mcpServers": { @@ -142,7 +142,7 @@ def test_mock_create_mcp_tools_with_timeout(): # Mock call_async_from_sync to return empty list mock_client.call_async_from_sync.return_value = [] - tools = create_mcp_tools(config, timeout=60.0) + tools = create_mcp_tools(mock_conversation_state, config, timeout=60.0) # Verify timeout was passed to call_async_from_sync mock_client.call_async_from_sync.assert_called_once() @@ -152,7 +152,7 @@ def test_mock_create_mcp_tools_with_timeout(): assert len(tools) == 0 -def test_mock_create_mcp_tools_connection_error(): +def test_mock_create_mcp_tools_connection_error(mock_conversation_state): """Test creating MCP tools with connection error.""" config = { "mcpServers": { @@ -173,12 +173,12 @@ def test_mock_create_mcp_tools_connection_error(): mock_client.call_async_from_sync.return_value = [] # Should not raise exception, but return empty list - tools = create_mcp_tools(config) + tools = create_mcp_tools(mock_conversation_state, config) assert len(tools) == 0 -def test_mock_create_mcp_tools_dict_config(): +def test_mock_create_mcp_tools_dict_config(mock_conversation_state): """Test creating MCP tools with dict configuration (not MCPConfig object).""" config = { "mcpServers": { @@ -202,19 +202,19 @@ def test_mock_create_mcp_tools_dict_config(): # Mock call_async_from_sync to return the tools mock_client.call_async_from_sync.return_value = [mock_tool] - tools = create_mcp_tools(config) + tools = create_mcp_tools(mock_conversation_state, config) assert len(tools) == 1 assert tools[0] == mock_tool -def test_real_create_mcp_tools_dict_config(): +def test_real_create_mcp_tools_dict_config(mock_conversation_state): """Test creating MCP tools with dict configuration (not MCPConfig object).""" mcp_config = { "mcpServers": {"fetch": {"command": "uvx", "args": ["mcp-server-fetch"]}} } - tools = create_mcp_tools(mcp_config) + tools = create_mcp_tools(mock_conversation_state, mcp_config) assert len(tools) == 1 assert tools[0].name == "fetch" diff --git a/tests/sdk/mcp/test_mcp_security_risk.py b/tests/sdk/mcp/test_mcp_security_risk.py index aa0649c411..ad05f624a6 100644 --- a/tests/sdk/mcp/test_mcp_security_risk.py +++ b/tests/sdk/mcp/test_mcp_security_risk.py @@ -43,7 +43,7 @@ async def __aexit__(self, *args): pass -def test_mcp_tool_to_openai_with_security_risk(): +def test_mcp_tool_to_openai_with_security_risk(mock_conversation_state): """Test that MCP tool schema includes security_risk field correctly. This test reproduces the bug where MCP tools with security_risk enabled @@ -62,7 +62,9 @@ def test_mcp_tool_to_openai_with_security_risk(): ) mock_client = MockMCPClient() - tools = MCPToolDefinition.create(mcp_tool=mcp_tool_def, mcp_client=mock_client) + tools = MCPToolDefinition.create( + mock_conversation_state, mcp_tool=mcp_tool_def, mcp_client=mock_client + ) tool = tools[0] # Generate OpenAI tool schema WITH security risk prediction @@ -94,7 +96,7 @@ def test_mcp_tool_to_openai_with_security_risk(): ) -def test_mcp_tool_action_from_arguments_with_security_risk(): +def test_mcp_tool_action_from_arguments_with_security_risk(mock_conversation_state): """Test that action_from_arguments works correctly with security_risk popped. This test simulates what happens in Agent._get_action_event where @@ -112,7 +114,9 @@ def test_mcp_tool_action_from_arguments_with_security_risk(): ) mock_client = MockMCPClient() - tools = MCPToolDefinition.create(mcp_tool=mcp_tool_def, mcp_client=mock_client) + tools = MCPToolDefinition.create( + mock_conversation_state, mcp_tool=mcp_tool_def, mcp_client=mock_client + ) tool = tools[0] # Simulate LLM providing arguments with security_risk @@ -132,7 +136,7 @@ def test_mcp_tool_action_from_arguments_with_security_risk(): assert action.data == {"url": "https://google.com"} -def test_mcp_tool_validates_correctly_after_security_risk_pop(): +def test_mcp_tool_validates_correctly_after_security_risk_pop(mock_conversation_state): """Test that MCP tool validation works after security_risk is popped. This is the full integration test that reproduces the bug scenario: @@ -153,7 +157,9 @@ def test_mcp_tool_validates_correctly_after_security_risk_pop(): ) mock_client = MockMCPClient() - tools = MCPToolDefinition.create(mcp_tool=mcp_tool_def, mcp_client=mock_client) + tools = MCPToolDefinition.create( + mock_conversation_state, mcp_tool=mcp_tool_def, mcp_client=mock_client + ) tool = tools[0] # Simulate what Agent does: diff --git a/tests/sdk/mcp/test_mcp_tool.py b/tests/sdk/mcp/test_mcp_tool.py index 3ca4f9c8bf..b142568c2a 100644 --- a/tests/sdk/mcp/test_mcp_tool.py +++ b/tests/sdk/mcp/test_mcp_tool.py @@ -4,6 +4,7 @@ from unittest.mock import MagicMock, Mock import mcp.types +import pytest from openhands.sdk.llm import TextContent from openhands.sdk.mcp.client import MCPClient @@ -212,9 +213,11 @@ def mock_call_async_from_sync(coro_func, **kwargs): class TestMCPTool: """Test MCPTool functionality.""" - def setup_method(self): + @pytest.fixture(autouse=True) + def setup(self, mock_conversation_state): """Set up test fixtures.""" self.mock_client: MockMCPClient = MockMCPClient() + self.mock_conversation_state = mock_conversation_state # Create mock MCP tool self.mock_mcp_tool: Mock = MagicMock(spec=mcp.types.Tool) @@ -228,7 +231,9 @@ def setup_method(self): self.mock_mcp_tool.meta = None tools = MCPToolDefinition.create( - mcp_tool=self.mock_mcp_tool, mcp_client=self.mock_client + self.mock_conversation_state, + mcp_tool=self.mock_mcp_tool, + mcp_client=self.mock_client, ) self.tool: MCPToolDefinition = tools[0] # Extract single tool from sequence @@ -264,7 +269,9 @@ def test_mcp_tool_with_annotations(self): mock_tool_with_annotations.meta = {"version": "1.0"} tools = MCPToolDefinition.create( - mcp_tool=mock_tool_with_annotations, mcp_client=self.mock_client + self.mock_conversation_state, + mcp_tool=mock_tool_with_annotations, + mcp_client=self.mock_client, ) tool = tools[0] # Extract single tool from sequence @@ -283,7 +290,9 @@ def test_mcp_tool_no_description(self): mock_tool_no_desc.meta = None tools = MCPToolDefinition.create( - mcp_tool=mock_tool_no_desc, mcp_client=self.mock_client + self.mock_conversation_state, + mcp_tool=mock_tool_no_desc, + mcp_client=self.mock_client, ) tool = tools[0] # Extract single tool from sequence diff --git a/tests/sdk/mcp/test_mcp_tool_immutability.py b/tests/sdk/mcp/test_mcp_tool_immutability.py index 5472f08ad8..8c1e368b01 100644 --- a/tests/sdk/mcp/test_mcp_tool_immutability.py +++ b/tests/sdk/mcp/test_mcp_tool_immutability.py @@ -21,9 +21,11 @@ def __init__(self): class TestMCPToolImmutability: """Test suite for MCPTool immutability features.""" - def setup_method(self): + @pytest.fixture(autouse=True) + def setup(self, mock_conversation_state): """Set up test environment.""" self.mock_client: MockMCPClient = MockMCPClient() + self.mock_conversation_state = mock_conversation_state # Create a mock MCP tool self.mock_mcp_tool: Mock = MagicMock(spec=mcp.types.Tool) @@ -37,7 +39,9 @@ def setup_method(self): self.mock_mcp_tool.meta = {"version": "1.0"} tools = MCPToolDefinition.create( - mcp_tool=self.mock_mcp_tool, mcp_client=self.mock_client + self.mock_conversation_state, + mcp_tool=self.mock_mcp_tool, + mcp_client=self.mock_client, ) self.tool: MCPToolDefinition = tools[0] # Extract single tool from sequence @@ -112,7 +116,9 @@ def test_mcp_tool_create_immutable_instance(self): mock_tool2.meta = None tools2 = MCPToolDefinition.create( - mcp_tool=mock_tool2, mcp_client=self.mock_client + self.mock_conversation_state, + mcp_tool=mock_tool2, + mcp_client=self.mock_client, ) tool2 = tools2[0] # Extract single tool from sequence diff --git a/tests/sdk/mcp/test_mcp_tool_kind_field.py b/tests/sdk/mcp/test_mcp_tool_kind_field.py index a62444615f..fec2463bc7 100644 --- a/tests/sdk/mcp/test_mcp_tool_kind_field.py +++ b/tests/sdk/mcp/test_mcp_tool_kind_field.py @@ -10,12 +10,12 @@ @pytest.fixture -def fetch_tool(): +def fetch_tool(mock_conversation_state): """Create a real MCP fetch tool using the mcp-server-fetch package.""" mcp_config = { "mcpServers": {"fetch": {"command": "uvx", "args": ["mcp-server-fetch"]}} } - tools = create_mcp_tools(mcp_config) + tools = create_mcp_tools(mock_conversation_state, mcp_config) assert len(tools) == 1 return tools[0] diff --git a/tests/sdk/mcp/test_mcp_tool_serialization.py b/tests/sdk/mcp/test_mcp_tool_serialization.py index a8f7e0d037..5f63414351 100644 --- a/tests/sdk/mcp/test_mcp_tool_serialization.py +++ b/tests/sdk/mcp/test_mcp_tool_serialization.py @@ -31,13 +31,17 @@ def create_mock_mcp_tool(name: str) -> mcp.types.Tool: ) -def test_mcp_tool_json_serialization_deserialization() -> None: +def test_mcp_tool_json_serialization_deserialization( + mock_conversation_state, +) -> None: # Create mock MCP tool and client mock_mcp_tool = create_mock_mcp_tool( "test_mcp_tool_json_serialization_deserialization" ) mock_client = Mock(spec=MCPClient) - tools = MCPToolDefinition.create(mock_mcp_tool, mock_client) + tools = MCPToolDefinition.create( + mock_conversation_state, mcp_tool=mock_mcp_tool, mcp_client=mock_client + ) mcp_tool = tools[0] # Extract single tool from sequence tool_json = mcp_tool.model_dump_json() @@ -47,14 +51,16 @@ def test_mcp_tool_json_serialization_deserialization() -> None: assert deserialized_tool.model_dump() == mcp_tool.model_dump() -def test_mcp_tool_polymorphic_behavior() -> None: +def test_mcp_tool_polymorphic_behavior(mock_conversation_state) -> None: """Test MCPTool polymorphic behavior using Tool base class.""" # Create mock MCP tool and client mock_mcp_tool = create_mock_mcp_tool("test_mcp_tool_polymorphic_behavior") mock_client = Mock(spec=MCPClient) # Create MCPTool instance - tools = MCPToolDefinition.create(mock_mcp_tool, mock_client) + tools = MCPToolDefinition.create( + mock_conversation_state, mcp_tool=mock_mcp_tool, mcp_client=mock_client + ) mcp_tool = tools[0] # Extract single tool from sequence # Should be instance of ToolDefinition @@ -67,14 +73,16 @@ def test_mcp_tool_polymorphic_behavior() -> None: assert hasattr(mcp_tool, "mcp_tool") -def test_mcp_tool_kind_field() -> None: +def test_mcp_tool_kind_field(mock_conversation_state) -> None: """Test that MCPTool kind field is correctly set.""" # Create mock MCP tool and client mock_mcp_tool = create_mock_mcp_tool("test_mcp_tool_kind_field") mock_client = Mock(spec=MCPClient) # Create MCPTool instance - tools = MCPToolDefinition.create(mock_mcp_tool, mock_client) + tools = MCPToolDefinition.create( + mock_conversation_state, mcp_tool=mock_mcp_tool, mcp_client=mock_client + ) mcp_tool = tools[0] # Extract single tool from sequence # Check kind field @@ -108,7 +116,7 @@ def test_mcp_tool_fallback_behavior() -> None: ) -def test_mcp_tool_essential_properties() -> None: +def test_mcp_tool_essential_properties(mock_conversation_state) -> None: """Test that MCPTool maintains essential properties after creation.""" # Create mock MCP tool with specific properties mock_mcp_tool = mcp.types.Tool( @@ -123,7 +131,9 @@ def test_mcp_tool_essential_properties() -> None: mock_client = Mock(spec=MCPClient) # Create MCPTool instance - tools = MCPToolDefinition.create(mock_mcp_tool, mock_client) + tools = MCPToolDefinition.create( + mock_conversation_state, mcp_tool=mock_mcp_tool, mcp_client=mock_client + ) mcp_tool = tools[0] # Extract single tool from sequence # Verify essential properties are preserved diff --git a/tests/sdk/mcp/test_mcp_tool_validation.py b/tests/sdk/mcp/test_mcp_tool_validation.py index 90a0c11e7b..70df7ffc56 100644 --- a/tests/sdk/mcp/test_mcp_tool_validation.py +++ b/tests/sdk/mcp/test_mcp_tool_validation.py @@ -8,18 +8,19 @@ from openhands.sdk.mcp.tool import MCPToolDefinition -def _make_tool_with_schema(schema: dict): +def _make_tool_with_schema(conv_state, schema: dict): mcp_tool = mcp.types.Tool( name="fetch", description="Fetch a URL", inputSchema=schema, ) client = Mock(spec=MCPClient) - return MCPToolDefinition.create(mcp_tool, client)[0] + return MCPToolDefinition.create(conv_state, mcp_tool=mcp_tool, mcp_client=client)[0] -def test_mcp_action_from_arguments_validates_and_sanitizes(): +def test_mcp_action_from_arguments_validates_and_sanitizes(mock_conversation_state): tool = _make_tool_with_schema( + mock_conversation_state, { "type": "object", "properties": { @@ -27,7 +28,7 @@ def test_mcp_action_from_arguments_validates_and_sanitizes(): "timeout": {"type": "number"}, }, "required": ["url"], - } + }, ) # includes a None that should be dropped @@ -39,15 +40,16 @@ def test_mcp_action_from_arguments_validates_and_sanitizes(): assert action.data == {"url": "https://example.com"} -def test_mcp_action_from_arguments_raises_on_invalid(): +def test_mcp_action_from_arguments_raises_on_invalid(mock_conversation_state): tool = _make_tool_with_schema( + mock_conversation_state, { "type": "object", "properties": { "url": {"type": "string"}, }, "required": ["url"], - } + }, ) # missing required url diff --git a/tests/sdk/tool/test_builtins.py b/tests/sdk/tool/test_builtins.py index d69f0d589d..6df9e01318 100644 --- a/tests/sdk/tool/test_builtins.py +++ b/tests/sdk/tool/test_builtins.py @@ -1,11 +1,11 @@ from openhands.sdk.tool.builtins import BUILT_IN_TOOLS -def test_all_tools_property(): +def test_all_tools_property(mock_conversation_state): # BUILT_IN_TOOLS contains tool classes, so we need to instantiate them for tool_class in BUILT_IN_TOOLS: # Create tool instances using .create() method - tool_instances = tool_class.create() + tool_instances = tool_class.create(mock_conversation_state) assert len(tool_instances) > 0, ( f"{tool_class.__name__}.create() should return at least one tool" ) diff --git a/tests/sdk/tool/test_tool_serialization.py b/tests/sdk/tool/test_tool_serialization.py index aa8c861cde..df7169cdf4 100644 --- a/tests/sdk/tool/test_tool_serialization.py +++ b/tests/sdk/tool/test_tool_serialization.py @@ -9,10 +9,10 @@ from openhands.sdk.tool.builtins import FinishTool, ThinkTool -def test_tool_serialization_deserialization() -> None: +def test_tool_serialization_deserialization(mock_conversation_state) -> None: """Test that Tool supports polymorphic JSON serialization/deserialization.""" # Use FinishTool which is a simple built-in tool - tool_instances = FinishTool.create() + tool_instances = FinishTool.create(mock_conversation_state) tool = tool_instances[0] # Serialize to JSON @@ -26,14 +26,16 @@ def test_tool_serialization_deserialization() -> None: assert tool.model_dump() == deserialized_tool.model_dump() -def test_tool_supports_polymorphic_field_json_serialization() -> None: +def test_tool_supports_polymorphic_field_json_serialization( + mock_conversation_state, +) -> None: """Test that Tool supports polymorphic JSON serialization when used as a field.""" class Container(BaseModel): tool: ToolDefinition # Create container with tool - tool_instances = FinishTool.create() + tool_instances = FinishTool.create(mock_conversation_state) tool = tool_instances[0] container = Container(tool=tool) @@ -48,16 +50,18 @@ class Container(BaseModel): assert tool.model_dump() == deserialized_container.tool.model_dump() -def test_tool_supports_nested_polymorphic_json_serialization() -> None: +def test_tool_supports_nested_polymorphic_json_serialization( + mock_conversation_state, +) -> None: """Test that Tool supports nested polymorphic JSON serialization.""" class NestedContainer(BaseModel): tools: list[ToolDefinition] # Create container with multiple tools - tool1_instances = FinishTool.create() + tool1_instances = FinishTool.create(mock_conversation_state) tool1 = tool1_instances[0] - tool2_instances = ThinkTool.create() + tool2_instances = ThinkTool.create(mock_conversation_state) tool2 = tool2_instances[0] container = NestedContainer(tools=[tool1, tool2]) @@ -75,10 +79,10 @@ class NestedContainer(BaseModel): assert tool2.model_dump() == deserialized_container.tools[1].model_dump() -def test_tool_model_validate_json_dict() -> None: +def test_tool_model_validate_json_dict(mock_conversation_state) -> None: """Test that Tool.model_validate works with dict from JSON.""" # Create tool - tool_instances = FinishTool.create() + tool_instances = FinishTool.create(mock_conversation_state) tool = tool_instances[0] # Serialize to JSON, then parse to dict @@ -109,10 +113,10 @@ def test_tool_no_fallback_behavior_json() -> None: ToolDefinition.model_validate_json(tool_json) -def test_tool_type_annotation_works_json() -> None: +def test_tool_type_annotation_works_json(mock_conversation_state) -> None: """Test that ToolType annotation works correctly with JSON.""" # Create tool - tool_instances = FinishTool.create() + tool_instances = FinishTool.create(mock_conversation_state) tool = tool_instances[0] # Use ToolType annotation @@ -132,10 +136,10 @@ class TestModel(BaseModel): assert tool.model_dump() == deserialized_model.tool.model_dump() -def test_tool_kind_field_json() -> None: +def test_tool_kind_field_json(mock_conversation_state) -> None: """Test Tool kind field is correctly set and preserved through JSON.""" # Create tool - tool_instances = FinishTool.create() + tool_instances = FinishTool.create(mock_conversation_state) tool = tool_instances[0] # Check kind field diff --git a/tests/tools/browser_use/test_browser_toolset.py b/tests/tools/browser_use/test_browser_toolset.py index ec2c3221a7..d58313e341 100644 --- a/tests/tools/browser_use/test_browser_toolset.py +++ b/tests/tools/browser_use/test_browser_toolset.py @@ -5,9 +5,9 @@ from openhands.tools.browser_use.impl import BrowserToolExecutor -def test_browser_toolset_create_returns_list(): +def test_browser_toolset_create_returns_list(mock_conversation_state): """Test that BrowserToolSet.create() returns a list of tools.""" - tools = BrowserToolSet.create() + tools = BrowserToolSet.create(mock_conversation_state) assert isinstance(tools, list) assert len(tools) == 10 # All browser tools @@ -17,9 +17,9 @@ def test_browser_toolset_create_returns_list(): assert isinstance(tool, ToolDefinition) -def test_browser_toolset_create_includes_all_browser_tools(): +def test_browser_toolset_create_includes_all_browser_tools(mock_conversation_state): """Test that BrowserToolSet.create() includes all expected browser tools.""" - tools = BrowserToolSet.create() + tools = BrowserToolSet.create(mock_conversation_state) # Get tool names tool_names = [tool.name for tool in tools] @@ -46,9 +46,9 @@ def test_browser_toolset_create_includes_all_browser_tools(): assert len(tool_names) == len(expected_names) -def test_browser_toolset_create_tools_have_shared_executor(): +def test_browser_toolset_create_tools_have_shared_executor(mock_conversation_state): """Test that all tools from BrowserToolSet.create() share the same executor.""" - tools = BrowserToolSet.create() + tools = BrowserToolSet.create(mock_conversation_state) # Get the executor from the first tool first_executor = tools[0].executor @@ -60,9 +60,9 @@ def test_browser_toolset_create_tools_have_shared_executor(): assert tool.executor is first_executor -def test_browser_toolset_create_tools_are_properly_configured(): +def test_browser_toolset_create_tools_are_properly_configured(mock_conversation_state): """Test that tools from BrowserToolSet.create() are properly configured.""" - tools = BrowserToolSet.create() + tools = BrowserToolSet.create(mock_conversation_state) # Find a specific tool to test (e.g., navigate tool) navigate_tool = None @@ -78,10 +78,12 @@ def test_browser_toolset_create_tools_are_properly_configured(): assert navigate_tool.executor is not None -def test_browser_toolset_create_multiple_calls_create_separate_executors(): +def test_browser_toolset_create_multiple_calls_create_separate_executors( + mock_conversation_state, +): """Test that multiple calls to BrowserToolSet.create() create separate executors.""" - tools1 = BrowserToolSet.create() - tools2 = BrowserToolSet.create() + tools1 = BrowserToolSet.create(mock_conversation_state) + tools2 = BrowserToolSet.create(mock_conversation_state) # Executors should be different instances executor1 = tools1[0].executor @@ -92,9 +94,9 @@ def test_browser_toolset_create_multiple_calls_create_separate_executors(): assert isinstance(executor2, BrowserToolExecutor) -def test_browser_toolset_create_tools_can_generate_mcp_schema(): +def test_browser_toolset_create_tools_can_generate_mcp_schema(mock_conversation_state): """Test that tools from BrowserToolSet.create() can generate MCP schemas.""" - tools = BrowserToolSet.create() + tools = BrowserToolSet.create(mock_conversation_state) for tool in tools: mcp_tool = tool.to_mcp_tool() @@ -112,20 +114,20 @@ def test_browser_toolset_create_tools_can_generate_mcp_schema(): assert "properties" in input_schema -def test_browser_toolset_create_no_parameters(): - """Test that BrowserToolSet.create() works without parameters.""" +def test_browser_toolset_create_no_parameters(mock_conversation_state): + """Test that BrowserToolSet.create() works with conv_state parameter.""" # Should not raise any exceptions - tools = BrowserToolSet.create() + tools = BrowserToolSet.create(mock_conversation_state) assert len(tools) > 0 -def test_browser_toolset_inheritance(): +def test_browser_toolset_inheritance(mock_conversation_state): """Test that BrowserToolSet properly inherits from Tool.""" assert issubclass(BrowserToolSet, ToolDefinition) # BrowserToolSet should not be instantiable directly (it's a factory) # The create method returns a list, not an instance of BrowserToolSet - tools = BrowserToolSet.create() + tools = BrowserToolSet.create(mock_conversation_state) for tool in tools: assert not isinstance(tool, BrowserToolSet) assert isinstance(tool, ToolDefinition) From 96ede4c766978912e7206e9113294a12478ff25a Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 3 Nov 2025 23:37:44 +0000 Subject: [PATCH 2/2] Fix pre-commit checks: add **kwargs to create() methods and export ConversationState - Add **kwargs with noqa: ARG003 comment to all tool create() methods to match base class signature - Export ConversationState from SDK __init__.py for type checking - Fix example file (02_custom_tools.py) and test file (test_registry.py) to match base class signature - All pre-commit checks now pass (ruff format, ruff lint, pycodestyle, pyright) Co-authored-by: openhands --- examples/01_standalone_sdk/02_custom_tools.py | 6 +++++- openhands-sdk/openhands/sdk/__init__.py | 2 ++ openhands-tools/openhands/tools/delegate/definition.py | 1 + openhands-tools/openhands/tools/execute_bash/definition.py | 1 + openhands-tools/openhands/tools/file_editor/definition.py | 1 + openhands-tools/openhands/tools/glob/definition.py | 1 + openhands-tools/openhands/tools/grep/definition.py | 1 + .../openhands/tools/planning_file_editor/definition.py | 1 + openhands-tools/openhands/tools/task_tracker/definition.py | 6 +++++- tests/sdk/tool/test_registry.py | 1 + 10 files changed, 19 insertions(+), 2 deletions(-) diff --git a/examples/01_standalone_sdk/02_custom_tools.py b/examples/01_standalone_sdk/02_custom_tools.py index 62292a55ba..44fa6492fa 100644 --- a/examples/01_standalone_sdk/02_custom_tools.py +++ b/examples/01_standalone_sdk/02_custom_tools.py @@ -11,6 +11,7 @@ Action, Agent, Conversation, + ConversationState, Event, ImageContent, LLMConvertibleEvent, @@ -124,7 +125,10 @@ class GrepTool(ToolDefinition[GrepAction, GrepObservation]): @classmethod def create( - cls, conv_state, bash_executor: BashExecutor | None = None + cls, + conv_state: ConversationState, + bash_executor: BashExecutor | None = None, + **kwargs, # noqa: ARG003 ) -> Sequence[ToolDefinition]: """Create GrepTool instance with a GrepExecutor. diff --git a/openhands-sdk/openhands/sdk/__init__.py b/openhands-sdk/openhands/sdk/__init__.py index a07bf60c25..c2a8c7cc25 100644 --- a/openhands-sdk/openhands/sdk/__init__.py +++ b/openhands-sdk/openhands/sdk/__init__.py @@ -9,6 +9,7 @@ BaseConversation, Conversation, ConversationCallbackType, + ConversationState, LocalConversation, RemoteConversation, ) @@ -81,6 +82,7 @@ "LocalConversation", "RemoteConversation", "ConversationCallbackType", + "ConversationState", "Event", "LLMConvertibleEvent", "AgentContext", diff --git a/openhands-tools/openhands/tools/delegate/definition.py b/openhands-tools/openhands/tools/delegate/definition.py index f23d87de74..10647daa83 100644 --- a/openhands-tools/openhands/tools/delegate/definition.py +++ b/openhands-tools/openhands/tools/delegate/definition.py @@ -85,6 +85,7 @@ def create( cls, conv_state: "ConversationState", max_children: int = 5, + **kwargs, # noqa: ARG003 ) -> Sequence["DelegateTool"]: """Initialize DelegateTool with a DelegateExecutor. diff --git a/openhands-tools/openhands/tools/execute_bash/definition.py b/openhands-tools/openhands/tools/execute_bash/definition.py index d1a39d18ea..6dca3ae450 100644 --- a/openhands-tools/openhands/tools/execute_bash/definition.py +++ b/openhands-tools/openhands/tools/execute_bash/definition.py @@ -229,6 +229,7 @@ def create( no_change_timeout_seconds: int | None = None, terminal_type: Literal["tmux", "subprocess"] | None = None, executor: ToolExecutor | None = None, + **kwargs, # noqa: ARG003 ) -> Sequence["BashTool"]: """Initialize BashTool with executor parameters. diff --git a/openhands-tools/openhands/tools/file_editor/definition.py b/openhands-tools/openhands/tools/file_editor/definition.py index f3d96f9152..60d288ced8 100644 --- a/openhands-tools/openhands/tools/file_editor/definition.py +++ b/openhands-tools/openhands/tools/file_editor/definition.py @@ -194,6 +194,7 @@ class FileEditorTool(ToolDefinition[FileEditorAction, FileEditorObservation]): def create( cls, conv_state: "ConversationState", + **kwargs, # noqa: ARG003 ) -> Sequence["FileEditorTool"]: """Initialize FileEditorTool with a FileEditorExecutor. diff --git a/openhands-tools/openhands/tools/glob/definition.py b/openhands-tools/openhands/tools/glob/definition.py index c18d86425b..b7765f87e3 100644 --- a/openhands-tools/openhands/tools/glob/definition.py +++ b/openhands-tools/openhands/tools/glob/definition.py @@ -89,6 +89,7 @@ class GlobTool(ToolDefinition[GlobAction, GlobObservation]): def create( cls, conv_state: "ConversationState", + **kwargs, # noqa: ARG003 ) -> Sequence["GlobTool"]: """Initialize GlobTool with a GlobExecutor. diff --git a/openhands-tools/openhands/tools/grep/definition.py b/openhands-tools/openhands/tools/grep/definition.py index 4913fde795..d18c1dde56 100644 --- a/openhands-tools/openhands/tools/grep/definition.py +++ b/openhands-tools/openhands/tools/grep/definition.py @@ -102,6 +102,7 @@ class GrepTool(ToolDefinition[GrepAction, GrepObservation]): def create( cls, conv_state: "ConversationState", + **kwargs, # noqa: ARG003 ) -> Sequence["GrepTool"]: """Initialize GrepTool with a GrepExecutor. diff --git a/openhands-tools/openhands/tools/planning_file_editor/definition.py b/openhands-tools/openhands/tools/planning_file_editor/definition.py index 2368d82304..1d370b2b95 100644 --- a/openhands-tools/openhands/tools/planning_file_editor/definition.py +++ b/openhands-tools/openhands/tools/planning_file_editor/definition.py @@ -61,6 +61,7 @@ class PlanningFileEditorTool( def create( cls, conv_state: "ConversationState", + **kwargs, # noqa: ARG003 ) -> Sequence["PlanningFileEditorTool"]: """Initialize PlanningFileEditorTool. diff --git a/openhands-tools/openhands/tools/task_tracker/definition.py b/openhands-tools/openhands/tools/task_tracker/definition.py index 82ad0529b6..d7e74c32e7 100644 --- a/openhands-tools/openhands/tools/task_tracker/definition.py +++ b/openhands-tools/openhands/tools/task_tracker/definition.py @@ -395,7 +395,11 @@ class TaskTrackerTool(ToolDefinition[TaskTrackerAction, TaskTrackerObservation]) """A ToolDefinition subclass that automatically initializes a TaskTrackerExecutor.""" # noqa: E501 @classmethod - def create(cls, conv_state: "ConversationState") -> Sequence["TaskTrackerTool"]: + def create( + cls, + conv_state: "ConversationState", + **kwargs, # noqa: ARG003 + ) -> Sequence["TaskTrackerTool"]: """Initialize TaskTrackerTool with a TaskTrackerExecutor. Args: diff --git a/tests/sdk/tool/test_registry.py b/tests/sdk/tool/test_registry.py index 8192544952..ea11efbb6a 100644 --- a/tests/sdk/tool/test_registry.py +++ b/tests/sdk/tool/test_registry.py @@ -45,6 +45,7 @@ def create( conv_state: ConversationState, greeting: str = "Hello", punctuation: str = "!", + **kwargs, # noqa: ARG003 ): class _ConfigurableExec(ToolExecutor[_HelloAction, _HelloObservation]): def __init__(self, greeting: str, punctuation: str) -> None: