From 5fe3968e45157cd796a388e62008195fec5eadb9 Mon Sep 17 00:00:00 2001 From: openhands Date: Tue, 11 Nov 2025 14:51:29 +0000 Subject: [PATCH 1/3] feat: Implement dynamic tool registration for RemoteConversation - Add tool_utils.py with TOOL_MODULE_MAP and register_tools_by_name() for dynamic tool import - Update StartConversationRequest model to accept registered_tools field - Update RemoteConversation to send list of registered tools in payload - Update conversation_service to dynamically register tools from client - Remove hardcoded register_planning_tools() call from tool_router.py - Add comprehensive tests for dynamic tool registration - Maintain backward compatibility for existing code Fixes #1128 Co-authored-by: openhands --- .../agent_server/conversation_service.py | 15 +++ .../openhands/agent_server/models.py | 7 ++ .../openhands/agent_server/tool_router.py | 5 +- .../openhands/agent_server/tool_utils.py | 48 +++++++++ .../conversation/impl/remote_conversation.py | 5 + .../agent_server/test_conversation_router.py | 100 ++++++++++++++++++ tests/agent_server/test_tool_utils.py | 73 +++++++++++++ 7 files changed, 251 insertions(+), 2 deletions(-) create mode 100644 openhands-agent-server/openhands/agent_server/tool_utils.py create mode 100644 tests/agent_server/test_tool_utils.py diff --git a/openhands-agent-server/openhands/agent_server/conversation_service.py b/openhands-agent-server/openhands/agent_server/conversation_service.py index 611f0dbc51..10aa7fff41 100644 --- a/openhands-agent-server/openhands/agent_server/conversation_service.py +++ b/openhands-agent-server/openhands/agent_server/conversation_service.py @@ -189,6 +189,21 @@ async def start_conversation( ) return conversation_info, False + # Dynamically register tools from client's registry + if request.registered_tools: + from openhands.agent_server.tool_utils import register_tools_by_name + + try: + register_tools_by_name(request.registered_tools) + logger.info( + f"Dynamically registered {len(request.registered_tools)} tools " + f"for conversation {conversation_id}: {request.registered_tools}" + ) + except ValueError as e: + logger.error(f"Failed to register tools: {e}") + # Continue even if some tools fail to register + # The agent will fail gracefully if it tries to use unregistered tools + stored = StoredConversation(id=conversation_id, **request.model_dump()) event_service = await self._start_event_service(stored) initial_message = request.initial_message diff --git a/openhands-agent-server/openhands/agent_server/models.py b/openhands-agent-server/openhands/agent_server/models.py index a19080f5e9..816e1d1809 100644 --- a/openhands-agent-server/openhands/agent_server/models.py +++ b/openhands-agent-server/openhands/agent_server/models.py @@ -97,6 +97,13 @@ class StartConversationRequest(BaseModel): default_factory=dict, description="Secrets available in the conversation", ) + registered_tools: list[str] = Field( + default_factory=list, + description=( + "List of tool names registered on the client that should be " + "dynamically registered on the server for this conversation." + ), + ) class StoredConversation(StartConversationRequest): diff --git a/openhands-agent-server/openhands/agent_server/tool_router.py b/openhands-agent-server/openhands/agent_server/tool_router.py index b37a32e9e8..2583afd296 100644 --- a/openhands-agent-server/openhands/agent_server/tool_router.py +++ b/openhands-agent-server/openhands/agent_server/tool_router.py @@ -4,12 +4,13 @@ from openhands.sdk.tool.registry import list_registered_tools from openhands.tools.preset.default import register_default_tools -from openhands.tools.preset.planning import register_planning_tools tool_router = APIRouter(prefix="/tools", tags=["Tools"]) +# Register default tools for backward compatibility +# Planning tools and other custom tools are now dynamically registered +# when creating a RemoteConversation register_default_tools(enable_browser=True) -register_planning_tools() # Tool listing diff --git a/openhands-agent-server/openhands/agent_server/tool_utils.py b/openhands-agent-server/openhands/agent_server/tool_utils.py new file mode 100644 index 0000000000..d4ef75e2b4 --- /dev/null +++ b/openhands-agent-server/openhands/agent_server/tool_utils.py @@ -0,0 +1,48 @@ +"""Utility functions for dynamic tool registration on the server.""" + +import importlib + +from openhands.sdk.logger import get_logger + + +logger = get_logger(__name__) + +# Mapping of tool names to their module paths for dynamic import +TOOL_MODULE_MAP = { + "terminal": "openhands.tools.terminal", + "file_editor": "openhands.tools.file_editor", + "task_tracker": "openhands.tools.task_tracker", + "browser": "openhands.tools.browser_use", + "glob": "openhands.tools.glob", + "grep": "openhands.tools.grep", + "planning_file_editor": "openhands.tools.planning_file_editor", +} + + +def register_tools_by_name(tool_names: list[str]) -> None: + """Dynamically register tools by importing their modules. + + Args: + tool_names: List of tool names to register. + + Raises: + ValueError: If a tool name is not recognized or cannot be imported. + """ + for tool_name in tool_names: + if tool_name not in TOOL_MODULE_MAP: + logger.warning( + f"Tool '{tool_name}' not found in TOOL_MODULE_MAP. " + "Skipping registration." + ) + continue + + module_path = TOOL_MODULE_MAP[tool_name] + try: + # Import the module to trigger tool auto-registration + importlib.import_module(module_path) + logger.debug(f"Tool '{tool_name}' registered via module '{module_path}'") + except ImportError as e: + logger.error( + f"Failed to import module '{module_path}' for tool '{tool_name}': {e}" + ) + raise ValueError(f"Cannot register tool '{tool_name}': {e}") from e diff --git a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py index 9b53ee3ea7..cd0689b483 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py @@ -451,6 +451,9 @@ def __init__( self._client = workspace.client if conversation_id is None: + # Import here to avoid circular imports + from openhands.sdk.tool.registry import list_registered_tools + payload = { "agent": agent.model_dump( mode="json", context={"expose_secrets": True} @@ -462,6 +465,8 @@ def __init__( "workspace": LocalWorkspace( working_dir=self.workspace.working_dir ).model_dump(), + # Include list of registered tools for dynamic registration on server + "registered_tools": list_registered_tools(), } resp = _send_request( self._client, "POST", "/api/conversations", json=payload diff --git a/tests/agent_server/test_conversation_router.py b/tests/agent_server/test_conversation_router.py index dc3106083c..3773501424 100644 --- a/tests/agent_server/test_conversation_router.py +++ b/tests/agent_server/test_conversation_router.py @@ -1169,3 +1169,103 @@ def test_generate_conversation_title_invalid_params( assert response.status_code == 422 # Validation error finally: client.app.dependency_overrides.clear() + + +def test_start_conversation_with_registered_tools( + client, mock_conversation_service, sample_conversation_info +): + """Test start_conversation endpoint with registered_tools field.""" + + # Mock the service response + mock_conversation_service.start_conversation.return_value = ( + sample_conversation_info, + True, + ) + + # Override the dependency + client.app.dependency_overrides[get_conversation_service] = ( + lambda: mock_conversation_service + ) + + try: + request_data = { + "agent": { + "llm": { + "model": "gpt-4o", + "api_key": "test-key", + "usage_id": "test-llm", + }, + "tools": [ + {"name": "glob"}, + {"name": "grep"}, + {"name": "planning_file_editor"}, + ], + }, + "workspace": {"working_dir": "/tmp/test"}, + "registered_tools": ["glob", "grep", "planning_file_editor"], + } + + response = client.post("/api/conversations", json=request_data) + + assert response.status_code == 201 + data = response.json() + assert data["id"] == str(sample_conversation_info.id) + + # Verify service was called + mock_conversation_service.start_conversation.assert_called_once() + call_args = mock_conversation_service.start_conversation.call_args + request_arg = call_args[0][0] + assert hasattr(request_arg, "registered_tools") + assert request_arg.registered_tools == [ + "glob", + "grep", + "planning_file_editor", + ] + finally: + client.app.dependency_overrides.clear() + + +def test_start_conversation_without_registered_tools( + client, mock_conversation_service, sample_conversation_info +): + """Test start_conversation endpoint without registered_tools field.""" + + # Mock the service response + mock_conversation_service.start_conversation.return_value = ( + sample_conversation_info, + True, + ) + + # Override the dependency + client.app.dependency_overrides[get_conversation_service] = ( + lambda: mock_conversation_service + ) + + try: + request_data = { + "agent": { + "llm": { + "model": "gpt-4o", + "api_key": "test-key", + "usage_id": "test-llm", + }, + "tools": [{"name": "TerminalTool"}], + }, + "workspace": {"working_dir": "/tmp/test"}, + } + + response = client.post("/api/conversations", json=request_data) + + assert response.status_code == 201 + data = response.json() + assert data["id"] == str(sample_conversation_info.id) + + # Verify service was called + mock_conversation_service.start_conversation.assert_called_once() + call_args = mock_conversation_service.start_conversation.call_args + request_arg = call_args[0][0] + assert hasattr(request_arg, "registered_tools") + # Should default to empty list + assert request_arg.registered_tools == [] + finally: + client.app.dependency_overrides.clear() diff --git a/tests/agent_server/test_tool_utils.py b/tests/agent_server/test_tool_utils.py new file mode 100644 index 0000000000..e610341750 --- /dev/null +++ b/tests/agent_server/test_tool_utils.py @@ -0,0 +1,73 @@ +"""Tests for tool_utils.py dynamic tool registration.""" + +from openhands.agent_server.tool_utils import ( + TOOL_MODULE_MAP, + register_tools_by_name, +) +from openhands.sdk.tool.registry import list_registered_tools + + +def test_register_tools_by_name_glob(): + """Test that glob tool can be registered dynamically.""" + # Register the glob tool + register_tools_by_name(["glob"]) + + # Verify it's in the registry + registered_tools = list_registered_tools() + assert "glob" in registered_tools + + +def test_register_tools_by_name_grep(): + """Test that grep tool can be registered dynamically.""" + # Register the grep tool + register_tools_by_name(["grep"]) + + # Verify it's in the registry + registered_tools = list_registered_tools() + assert "grep" in registered_tools + + +def test_register_tools_by_name_planning_file_editor(): + """Test that planning_file_editor tool can be registered dynamically.""" + # Register the planning_file_editor tool + register_tools_by_name(["planning_file_editor"]) + + # Verify it's in the registry + registered_tools = list_registered_tools() + assert "planning_file_editor" in registered_tools + + +def test_register_tools_by_name_multiple(): + """Test that multiple tools can be registered at once.""" + # Register multiple tools + tools_to_register = ["glob", "grep", "planning_file_editor"] + register_tools_by_name(tools_to_register) + + # Verify all are in the registry + registered_tools = list_registered_tools() + for tool_name in tools_to_register: + assert tool_name in registered_tools + + +def test_register_tools_by_name_unknown_tool(): + """Test that unknown tool names are handled gracefully.""" + # This should log a warning but not raise an exception + register_tools_by_name(["unknown_tool"]) + + # Verify it's not in the registry + registered_tools = list_registered_tools() + assert "unknown_tool" not in registered_tools + + +def test_tool_module_map_contains_planning_tools(): + """Test that TOOL_MODULE_MAP contains all planning tools.""" + assert "glob" in TOOL_MODULE_MAP + assert "grep" in TOOL_MODULE_MAP + assert "planning_file_editor" in TOOL_MODULE_MAP + + +def test_tool_module_map_contains_default_tools(): + """Test that TOOL_MODULE_MAP contains default tools.""" + assert "terminal" in TOOL_MODULE_MAP + assert "file_editor" in TOOL_MODULE_MAP + assert "task_tracker" in TOOL_MODULE_MAP From e43369f9ec47f18b883260f4573b729d722bbb75 Mon Sep 17 00:00:00 2001 From: openhands Date: Tue, 11 Nov 2025 21:06:16 +0000 Subject: [PATCH 2/3] feat: implement dynamic tool registration with module qualnames Revised approach: Track module qualnames at registration time and dynamically import modules by qualname on server-side. Changes: - Modified tool registry to track module qualnames when tools are registered - Added _MODULE_QUALNAMES dict to store tool name -> module qualname mapping - Updated register_tool() to capture and store module qualname from factory - Added get_tool_module_qualnames() function to retrieve the mapping - Updated StartConversationRequest model to accept tool_module_qualnames dict - Updated RemoteConversation to send tool module qualnames in conversation payload - Updated conversation_service to dynamically import modules by qualname - Removed hardcoded planning tools registration from tool_router.py - Removed tool_utils.py and manual TOOL_MODULE_MAP (superseded by registry tracking) - Added comprehensive tests for dynamic tool registration This approach provides a single source of truth in the registry and works with any tool without predefined mappings. Fixes #1128 Co-authored-by: openhands --- .../agent_server/conversation_service.py | 31 +++++--- .../openhands/agent_server/models.py | 9 ++- .../openhands/agent_server/tool_utils.py | 48 ------------ .../conversation/impl/remote_conversation.py | 6 +- openhands-sdk/openhands/sdk/tool/registry.py | 23 ++++++ .../agent_server/test_conversation_router.py | 34 +++++---- tests/agent_server/test_tool_utils.py | 73 ------------------- tests/sdk/test_registry_qualnames.py | 68 +++++++++++++++++ 8 files changed, 140 insertions(+), 152 deletions(-) delete mode 100644 openhands-agent-server/openhands/agent_server/tool_utils.py delete mode 100644 tests/agent_server/test_tool_utils.py create mode 100644 tests/sdk/test_registry_qualnames.py diff --git a/openhands-agent-server/openhands/agent_server/conversation_service.py b/openhands-agent-server/openhands/agent_server/conversation_service.py index 10aa7fff41..9d5fbba60e 100644 --- a/openhands-agent-server/openhands/agent_server/conversation_service.py +++ b/openhands-agent-server/openhands/agent_server/conversation_service.py @@ -190,19 +190,30 @@ async def start_conversation( return conversation_info, False # Dynamically register tools from client's registry - if request.registered_tools: - from openhands.agent_server.tool_utils import register_tools_by_name + if request.tool_module_qualnames: + import importlib - try: - register_tools_by_name(request.registered_tools) + for tool_name, module_qualname in request.tool_module_qualnames.items(): + try: + # Import the module to trigger tool auto-registration + importlib.import_module(module_qualname) + logger.debug( + f"Tool '{tool_name}' registered via module '{module_qualname}'" + ) + except ImportError as e: + logger.warning( + f"Failed to import module '{module_qualname}' for tool " + f"'{tool_name}': {e}. Tool will not be available." + ) + # Continue even if some tools fail to register + # The agent will fail gracefully if it tries to use unregistered + # tools + if request.tool_module_qualnames: logger.info( - f"Dynamically registered {len(request.registered_tools)} tools " - f"for conversation {conversation_id}: {request.registered_tools}" + f"Dynamically registered {len(request.tool_module_qualnames)} " + f"tools for conversation {conversation_id}: " + f"{list(request.tool_module_qualnames.keys())}" ) - except ValueError as e: - logger.error(f"Failed to register tools: {e}") - # Continue even if some tools fail to register - # The agent will fail gracefully if it tries to use unregistered tools stored = StoredConversation(id=conversation_id, **request.model_dump()) event_service = await self._start_event_service(stored) diff --git a/openhands-agent-server/openhands/agent_server/models.py b/openhands-agent-server/openhands/agent_server/models.py index 816e1d1809..6258c95dc5 100644 --- a/openhands-agent-server/openhands/agent_server/models.py +++ b/openhands-agent-server/openhands/agent_server/models.py @@ -97,11 +97,12 @@ class StartConversationRequest(BaseModel): default_factory=dict, description="Secrets available in the conversation", ) - registered_tools: list[str] = Field( - default_factory=list, + tool_module_qualnames: dict[str, str] = Field( + default_factory=dict, description=( - "List of tool names registered on the client that should be " - "dynamically registered on the server for this conversation." + "Mapping of tool names to their module qualnames from the client's " + "registry. These modules will be dynamically imported on the server " + "to register the tools for this conversation." ), ) diff --git a/openhands-agent-server/openhands/agent_server/tool_utils.py b/openhands-agent-server/openhands/agent_server/tool_utils.py deleted file mode 100644 index d4ef75e2b4..0000000000 --- a/openhands-agent-server/openhands/agent_server/tool_utils.py +++ /dev/null @@ -1,48 +0,0 @@ -"""Utility functions for dynamic tool registration on the server.""" - -import importlib - -from openhands.sdk.logger import get_logger - - -logger = get_logger(__name__) - -# Mapping of tool names to their module paths for dynamic import -TOOL_MODULE_MAP = { - "terminal": "openhands.tools.terminal", - "file_editor": "openhands.tools.file_editor", - "task_tracker": "openhands.tools.task_tracker", - "browser": "openhands.tools.browser_use", - "glob": "openhands.tools.glob", - "grep": "openhands.tools.grep", - "planning_file_editor": "openhands.tools.planning_file_editor", -} - - -def register_tools_by_name(tool_names: list[str]) -> None: - """Dynamically register tools by importing their modules. - - Args: - tool_names: List of tool names to register. - - Raises: - ValueError: If a tool name is not recognized or cannot be imported. - """ - for tool_name in tool_names: - if tool_name not in TOOL_MODULE_MAP: - logger.warning( - f"Tool '{tool_name}' not found in TOOL_MODULE_MAP. " - "Skipping registration." - ) - continue - - module_path = TOOL_MODULE_MAP[tool_name] - try: - # Import the module to trigger tool auto-registration - importlib.import_module(module_path) - logger.debug(f"Tool '{tool_name}' registered via module '{module_path}'") - except ImportError as e: - logger.error( - f"Failed to import module '{module_path}' for tool '{tool_name}': {e}" - ) - raise ValueError(f"Cannot register tool '{tool_name}': {e}") from e diff --git a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py index cd0689b483..fbd15d81ca 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py @@ -452,7 +452,7 @@ def __init__( if conversation_id is None: # Import here to avoid circular imports - from openhands.sdk.tool.registry import list_registered_tools + from openhands.sdk.tool.registry import get_tool_module_qualnames payload = { "agent": agent.model_dump( @@ -465,8 +465,8 @@ def __init__( "workspace": LocalWorkspace( working_dir=self.workspace.working_dir ).model_dump(), - # Include list of registered tools for dynamic registration on server - "registered_tools": list_registered_tools(), + # Include tool module qualnames for dynamic registration on server + "tool_module_qualnames": get_tool_module_qualnames(), } resp = _send_request( self._client, "POST", "/api/conversations", json=payload diff --git a/openhands-sdk/openhands/sdk/tool/registry.py b/openhands-sdk/openhands/sdk/tool/registry.py index 0095b2b770..2d1942c66e 100644 --- a/openhands-sdk/openhands/sdk/tool/registry.py +++ b/openhands-sdk/openhands/sdk/tool/registry.py @@ -29,6 +29,7 @@ _LOCK = RLock() _REG: dict[str, Resolver] = {} +_MODULE_QUALNAMES: dict[str, str] = {} # Maps tool name to module qualname def _resolver_from_instance(name: str, tool: ToolDefinition) -> Resolver: @@ -137,11 +138,22 @@ def register_tool( "(3) a callable factory returning a Sequence[ToolDefinition]" ) + # Track the module qualname for this tool + module_qualname = None + if isinstance(factory, type): + module_qualname = factory.__module__ + elif callable(factory): + module_qualname = getattr(factory, "__module__", None) + elif isinstance(factory, ToolDefinition): + module_qualname = factory.__class__.__module__ + with _LOCK: # TODO: throw exception when registering duplicate name tools if name in _REG: logger.warning(f"Duplicate tool name registerd {name}") _REG[name] = resolver + if module_qualname: + _MODULE_QUALNAMES[name] = module_qualname def resolve_tool( @@ -159,3 +171,14 @@ def resolve_tool( def list_registered_tools() -> list[str]: with _LOCK: return list(_REG.keys()) + + +def get_tool_module_qualnames() -> dict[str, str]: + """Get a mapping of tool names to their module qualnames. + + Returns: + A dictionary mapping tool names to module qualnames (e.g., + {"glob": "openhands.tools.glob.definition"}). + """ + with _LOCK: + return dict(_MODULE_QUALNAMES) diff --git a/tests/agent_server/test_conversation_router.py b/tests/agent_server/test_conversation_router.py index 3773501424..9d332e35c2 100644 --- a/tests/agent_server/test_conversation_router.py +++ b/tests/agent_server/test_conversation_router.py @@ -1171,10 +1171,10 @@ def test_generate_conversation_title_invalid_params( client.app.dependency_overrides.clear() -def test_start_conversation_with_registered_tools( +def test_start_conversation_with_tool_module_qualnames( client, mock_conversation_service, sample_conversation_info ): - """Test start_conversation endpoint with registered_tools field.""" + """Test start_conversation endpoint with tool_module_qualnames field.""" # Mock the service response mock_conversation_service.start_conversation.return_value = ( @@ -1202,7 +1202,13 @@ def test_start_conversation_with_registered_tools( ], }, "workspace": {"working_dir": "/tmp/test"}, - "registered_tools": ["glob", "grep", "planning_file_editor"], + "tool_module_qualnames": { + "glob": "openhands.tools.glob.definition", + "grep": "openhands.tools.grep.definition", + "planning_file_editor": ( + "openhands.tools.planning_file_editor.definition" + ), + }, } response = client.post("/api/conversations", json=request_data) @@ -1215,20 +1221,20 @@ def test_start_conversation_with_registered_tools( mock_conversation_service.start_conversation.assert_called_once() call_args = mock_conversation_service.start_conversation.call_args request_arg = call_args[0][0] - assert hasattr(request_arg, "registered_tools") - assert request_arg.registered_tools == [ - "glob", - "grep", - "planning_file_editor", - ] + assert hasattr(request_arg, "tool_module_qualnames") + assert request_arg.tool_module_qualnames == { + "glob": "openhands.tools.glob.definition", + "grep": "openhands.tools.grep.definition", + "planning_file_editor": ("openhands.tools.planning_file_editor.definition"), + } finally: client.app.dependency_overrides.clear() -def test_start_conversation_without_registered_tools( +def test_start_conversation_without_tool_module_qualnames( client, mock_conversation_service, sample_conversation_info ): - """Test start_conversation endpoint without registered_tools field.""" + """Test start_conversation endpoint without tool_module_qualnames field.""" # Mock the service response mock_conversation_service.start_conversation.return_value = ( @@ -1264,8 +1270,8 @@ def test_start_conversation_without_registered_tools( mock_conversation_service.start_conversation.assert_called_once() call_args = mock_conversation_service.start_conversation.call_args request_arg = call_args[0][0] - assert hasattr(request_arg, "registered_tools") - # Should default to empty list - assert request_arg.registered_tools == [] + assert hasattr(request_arg, "tool_module_qualnames") + # Should default to empty dict + assert request_arg.tool_module_qualnames == {} finally: client.app.dependency_overrides.clear() diff --git a/tests/agent_server/test_tool_utils.py b/tests/agent_server/test_tool_utils.py deleted file mode 100644 index e610341750..0000000000 --- a/tests/agent_server/test_tool_utils.py +++ /dev/null @@ -1,73 +0,0 @@ -"""Tests for tool_utils.py dynamic tool registration.""" - -from openhands.agent_server.tool_utils import ( - TOOL_MODULE_MAP, - register_tools_by_name, -) -from openhands.sdk.tool.registry import list_registered_tools - - -def test_register_tools_by_name_glob(): - """Test that glob tool can be registered dynamically.""" - # Register the glob tool - register_tools_by_name(["glob"]) - - # Verify it's in the registry - registered_tools = list_registered_tools() - assert "glob" in registered_tools - - -def test_register_tools_by_name_grep(): - """Test that grep tool can be registered dynamically.""" - # Register the grep tool - register_tools_by_name(["grep"]) - - # Verify it's in the registry - registered_tools = list_registered_tools() - assert "grep" in registered_tools - - -def test_register_tools_by_name_planning_file_editor(): - """Test that planning_file_editor tool can be registered dynamically.""" - # Register the planning_file_editor tool - register_tools_by_name(["planning_file_editor"]) - - # Verify it's in the registry - registered_tools = list_registered_tools() - assert "planning_file_editor" in registered_tools - - -def test_register_tools_by_name_multiple(): - """Test that multiple tools can be registered at once.""" - # Register multiple tools - tools_to_register = ["glob", "grep", "planning_file_editor"] - register_tools_by_name(tools_to_register) - - # Verify all are in the registry - registered_tools = list_registered_tools() - for tool_name in tools_to_register: - assert tool_name in registered_tools - - -def test_register_tools_by_name_unknown_tool(): - """Test that unknown tool names are handled gracefully.""" - # This should log a warning but not raise an exception - register_tools_by_name(["unknown_tool"]) - - # Verify it's not in the registry - registered_tools = list_registered_tools() - assert "unknown_tool" not in registered_tools - - -def test_tool_module_map_contains_planning_tools(): - """Test that TOOL_MODULE_MAP contains all planning tools.""" - assert "glob" in TOOL_MODULE_MAP - assert "grep" in TOOL_MODULE_MAP - assert "planning_file_editor" in TOOL_MODULE_MAP - - -def test_tool_module_map_contains_default_tools(): - """Test that TOOL_MODULE_MAP contains default tools.""" - assert "terminal" in TOOL_MODULE_MAP - assert "file_editor" in TOOL_MODULE_MAP - assert "task_tracker" in TOOL_MODULE_MAP diff --git a/tests/sdk/test_registry_qualnames.py b/tests/sdk/test_registry_qualnames.py new file mode 100644 index 0000000000..2d691c3ec6 --- /dev/null +++ b/tests/sdk/test_registry_qualnames.py @@ -0,0 +1,68 @@ +"""Tests for tool registry module qualname tracking.""" + +from openhands.sdk.tool.registry import ( + get_tool_module_qualnames, + list_registered_tools, + register_tool, +) + + +def test_get_tool_module_qualnames_with_class(): + """Test that module qualnames are tracked when registering a class.""" + from openhands.tools.glob import GlobTool + + # Register the GlobTool class + register_tool("test_glob_class", GlobTool) + + # Get the module qualnames + qualnames = get_tool_module_qualnames() + + # Verify the tool is tracked with its module + assert "test_glob_class" in qualnames + assert qualnames["test_glob_class"] == "openhands.tools.glob.definition" + + +def test_get_tool_module_qualnames_with_callable(): + """Test that module qualnames are tracked when registering a callable.""" + + def test_factory(conv_state): + return [] + + # Register the callable + register_tool("test_callable", test_factory) + + # Get the module qualnames + qualnames = get_tool_module_qualnames() + + # Verify the tool is tracked with its module + assert "test_callable" in qualnames + assert "test_registry_qualnames" in qualnames["test_callable"] + + +def test_get_tool_module_qualnames_after_import(): + """Test that importing a tool module registers it with qualname.""" + # Import glob tool module to trigger auto-registration + import openhands.tools.glob.definition # noqa: F401 + + # Get registered tools + registered_tools = list_registered_tools() + + # Should have glob registered + assert "glob" in registered_tools + + # Get module qualnames + qualnames = get_tool_module_qualnames() + + # Verify glob has its module qualname tracked + if "glob" in qualnames: + assert qualnames["glob"] == "openhands.tools.glob.definition" + + +def test_get_tool_module_qualnames_returns_copy(): + """Test that get_tool_module_qualnames returns a copy, not the original dict.""" + qualnames1 = get_tool_module_qualnames() + qualnames2 = get_tool_module_qualnames() + + # Should be equal but not the same object + assert qualnames1 == qualnames2 + assert qualnames1 is not qualnames2 From 4a1bb26b985e71c51c4310a620df9c28022079fb Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 29 Nov 2025 20:32:52 +0000 Subject: [PATCH 3/3] fix: Move test_registry_qualnames.py to tests/cross SDK tests should not import from openhands.tools. Since this test uses GlobTool from openhands.tools, it should be in tests/cross which is for tests that use both SDK and tools packages. This fixes the CI failure where the sdk-tests job checks for and rejects any openhands.tools imports in tests/sdk/. Co-authored-by: openhands --- tests/{sdk => cross}/test_registry_qualnames.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{sdk => cross}/test_registry_qualnames.py (100%) diff --git a/tests/sdk/test_registry_qualnames.py b/tests/cross/test_registry_qualnames.py similarity index 100% rename from tests/sdk/test_registry_qualnames.py rename to tests/cross/test_registry_qualnames.py