Implement automatic tool registration on import #862
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add register_tool imports and automatic registration calls to all tool definition files - Tools now automatically register themselves when their definition modules are imported - Update preset configurations to remove manual registration calls - Add comprehensive unit tests to verify automatic registration works correctly - Simplifies tool usage from manual registration to just importing the tool Fixes #861 Co-authored-by: openhands <[email protected]>
- Move test_automatic_registration.py from tests/sdk/tool/ to tests/cross/ - SDK tests should only import from openhands.sdk, cross tests can import from openhands.tools - All tests still pass in their new location Co-authored-by: openhands <[email protected]>
Coverage Report •
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Co-authored-by: openhands <[email protected]>
Co-authored-by: openhands <[email protected]>
- Updated register_default_tools() to use ToolClass.tool_name instead of ToolClass.name - This fixes AttributeError when importing tools that use ClassVar[str] for tool_name - All automatic registration tests now pass Co-authored-by: openhands <[email protected]>
- Add _camel_to_snake utility function to convert CamelCase to snake_case - Add __init_subclass__ method to ToolBase for automatic tool naming - Remove hardcoded tool_name attributes from all 7 tool classes - Update preset files to use .tool_name attributes instead of hardcoded strings - Add comprehensive test suite for automatic naming functionality - Update existing tests to expect snake_case tool names - All tools now automatically get snake_case names (e.g., BashTool -> bash_tool) This eliminates the need for manual tool_name declarations while maintaining backward compatibility through the .tool_name class attribute. Co-authored-by: openhands <[email protected]>
enyst
reviewed
Oct 22, 2025
enyst
reviewed
Oct 22, 2025
xingyaoww
reviewed
Oct 23, 2025
|
|
||
|
|
||
| # Automatically register the tool when this module is imported | ||
| register_tool(BrowserToolSet.tool_name, BrowserToolSet) |
There was a problem hiding this comment.
We should add a check to make sure there's no tool name conflicts in the tool registry
There was a problem hiding this comment.
I'm thinking of doing this in a separate PR so we can better track the changes
I'll need to look into reseting the tool registry so that unit tests aren't complaining
|
[Automatic Post]: It has been a while since there was any activity on this PR. @malhotra5, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. |
|
Found 9 test failures on Blacksmith runners:
|
- Changed .tool_name to .name in examples, tests, and tool registration - Removed invalid name= parameters from tool constructors since name is a ClassVar - Updated browser tool tests to expect correct auto-generated names - All ToolDefinition subclasses now consistently use .name attribute Co-authored-by: openhands <[email protected]>
- Remove name= parameters from mock tool constructors since name is now a ClassVar - Update test expectations to use auto-generated tool names (e.g., mock_test_tool) - Fix regex patterns in error message tests to match new tool names - Remove test for assigning to name attribute since it's now a ClassVar Co-authored-by: openhands <[email protected]>
- Set name as ClassVar[str] instead of instance variable to properly override automatic naming - Update test expectations to use correct tool names (finish_tool, think_tool) - Add proper ClassVar import for type checking Co-authored-by: openhands <[email protected]>
- Update mock tool call from 'think' to 'think_tool' to match automatic naming - Ensures test passes with new ClassVar name attribute system Co-authored-by: openhands <[email protected]>
…override - Fixed test_llm_completion.py: Added ClassVar name and removed name= parameter - Fixed test_to_responses_tool.py: Added ClassVar name and removed name= parameter - Fixed test_to_responses_tool_security.py: Created separate tool classes with ClassVar names - Fixed MCPToolDefinition: Used property with type: ignore[override] to override ClassVar name - All 171 tests passing, all type checker issues resolved Co-authored-by: openhands <[email protected]>
- Updated BashTool tests to use 'bash_tool' instead of 'execute_bash' - Fixed FileEditorTool tests to expect 'file_editor_tool' instead of 'str_replace_editor' - Corrected hello world test tool registration and mock LLM responses - All core test suites now passing: bash tool (5/5), file editor (8/8), hello world (3/3), automatic registration (9/9), bash auto detection (7/7) Co-authored-by: openhands <[email protected]>
The test imports from openhands.tools which violates the SDK test isolation rule. SDK tests should only import from openhands.sdk, while cross tests can import from both openhands.sdk and openhands.tools. This fixes the CI failure about openhands.tools imports in tests/sdk/. Co-authored-by: openhands <[email protected]>
- Fixed agent.py to check for 'finish_tool' instead of 'finish' when setting agent status to FINISHED - Fixed response_utils.py to check for 'finish_tool' instead of 'finish' when extracting final response - Updated test_agent_final_response.py to use correct tool name 'finish_tool' in test cases - Updated test_confirmation_mode.py to use correct tool names 'finish_tool' and 'think_tool' This resolves the issue where agents would get stuck in loops because they couldn't recognize finish tool calls due to name mismatch. The FinishTool class name gets converted to 'finish_tool' via _camel_to_snake() function, so all references should use this consistent naming. Fixes failing tests: - test_single_finish_action_skips_confirmation_entirely - test_think_and_finish_action_skips_confirmation_entirely Co-authored-by: openhands <[email protected]>
Update test expectations to match actual tool names: - GlobTool.name: 'glob' -> 'glob_tool' - GrepTool.name: 'grep' -> 'grep_tool' This aligns with the _camel_to_snake() naming convention used in tool registration system. Co-authored-by: openhands <[email protected]>
malhotra5
commented
Nov 4, 2025
|
Verified that this works in the CLI |
xingyaoww
reviewed
Nov 4, 2025
| mcp_tool: mcp.types.Tool = Field(description="The MCP tool definition.") | ||
|
|
||
| @property | ||
| def name(self) -> str: # type: ignore[override] |
There was a problem hiding this comment.
can we don't do "# type: ignore[override]"?
There was a problem hiding this comment.
Yeah this one is a bit hacky. Essentially every tool name is on a class level. However, for MCPTool the name of the tool is instantiated later based on the instance level details. Without the type ignore we get type error
Instance variable "name" overrides class variable of same name in class "ToolDefinition"PylancereportIncompatibleVariableOverride
There was a problem hiding this comment.
Maybe we introduce a separate field for mcp tool name and get rid of this hacky override?
@property
def mcp_tool_name(self):
return self.mcp_tool.name
xingyaoww
reviewed
Nov 4, 2025
There was a problem hiding this comment.
Final few things left:
- we should add a check at tool registry when we try to register the same tool name multiple time (either throw an exception, or log a warning message)
- do string search for explicit "execute_bash" mention in prompts and update it with the updated tool name
- (optional), remove "tool" suffix from automatic tool names
I'll throw a warning log for now, but wanted to do this in a follow up PR to isolate the changes, particularly with the test fixtures etc (because running all the unit tests will be registering the same tools) |
- Changed agent_status to execution_status to match ConversationState field - Fixed import issue with AgentExecutionStatus Co-authored-by: openhands <[email protected]>
Updated all test files to use the new tool naming convention: - think_tool → think - execute_bash → bash - str_replace_editor → file_editor - mock_test_tool → mock_test - mock_immutable_tool → mock_immutable - __simple_hello_tool → __simple_hello All SDK tests now pass with the new automatic tool naming logic. Co-authored-by: openhands <[email protected]>
Updated all tool tests to expect new naming convention where '_tool' suffix is removed: - BashTool: 'bash_tool' -> 'bash' - FileEditorTool: 'file_editor_tool' -> 'file_editor' - GlobTool: 'glob_tool' -> 'glob' - GrepTool: 'grep_tool' -> 'grep' - PlanningFileEditorTool: 'planning_file_editor_tool' -> 'planning_file_editor' Updated test assertions and expected values to match the new automatic naming logic. Co-authored-by: openhands <[email protected]>
….com/OpenHands/software-agent-sdk into openhands/simplify-tool-registration
xingyaoww
added
integration-test
|
Hi! I started running the integration tests on your PR. You will receive a comment with the results shortly. |
🔄 Running Examples with
|
| Example | Status | Duration | Cost |
|---|---|---|---|
| 01_standalone_sdk/01_hello_world.py | ✅ PASS | 22s | $0.02 |
| 01_standalone_sdk/02_custom_tools.py | ✅ PASS | 27s | $0.03 |
| 01_standalone_sdk/03_activate_skill.py | ✅ PASS | 12s | $0.01 |
| 01_standalone_sdk/05_use_llm_registry.py | ✅ PASS | 10s | $0.01 |
| 01_standalone_sdk/07_mcp_integration.py | ✅ PASS | 57s | $0.04 |
| 01_standalone_sdk/09_pause_example.py | ✅ PASS | 13s | $0.01 |
| 01_standalone_sdk/10_persistence.py | ✅ PASS | 39s | $0.03 |
| 01_standalone_sdk/11_async.py | ✅ PASS | 35s | $0.03 |
| 01_standalone_sdk/12_custom_secrets.py | ✅ PASS | 14s | $0.01 |
| 01_standalone_sdk/13_get_llm_metrics.py | ✅ PASS | 33s | $0.01 |
| 01_standalone_sdk/14_context_condenser.py | ✅ PASS | 162s | $0.30 |
| 01_standalone_sdk/17_image_input.py | ✅ PASS | 19s | $0.02 |
| 01_standalone_sdk/18_send_message_while_processing.py | ✅ PASS | 22s | $0.01 |
| 01_standalone_sdk/19_llm_routing.py | ✅ PASS | 17s | $0.01 |
| 01_standalone_sdk/20_stuck_detector.py | ✅ PASS | 19s | $0.01 |
| 01_standalone_sdk/21_generate_extraneous_conversation_costs.py | ✅ PASS | 14s | $0.01 |
| 01_standalone_sdk/22_anthropic_thinking.py | ✅ PASS | 20s | $0.02 |
| 01_standalone_sdk/23_responses_reasoning.py | ✅ PASS | 73s | $0.01 |
| 01_standalone_sdk/24_planning_agent_workflow.py | ✅ PASS | 225s | $0.25 |
| 02_remote_agent_server/01_convo_with_local_agent_server.py | ✅ PASS | 71s | $0.00 |
| 02_remote_agent_server/02_convo_with_docker_sandboxed_server.py | ✅ PASS | 51s | $0.00 |
✅ All tests passed!
Total: 21 | Passed: 21 | Failed: 0 | Total Cost: $0.85
🧪 Integration Tests ResultsOverall Success Rate: 100.0% 📁 Detailed Logs & ArtifactsClick the links below to access detailed agent/LLM logs showing the complete reasoning process for each model. On the GitHub Actions page, scroll down to the 'Artifacts' section to download the logs.
📊 Summary
📋 Detailed Resultslitellm_proxy_claude_sonnet_4_5_20250929
litellm_proxy_deepseek_deepseek_chat
litellm_proxy_gpt_5_mini_2025_08_07
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements automatic tool registration as requested in issue #861. Tools now automatically register themselves when their definition modules are imported, eliminating the need for manual registration calls.
Changes Made
Tool Definition Files
openhands/tools/execute_bash/definition.py- BashToolopenhands/tools/file_editor/definition.py- FileEditorToolopenhands/tools/task_tracker/definition.py- TaskTrackerToolopenhands/tools/browser_use/definition.py- BrowserToolSetopenhands/tools/grep/definition.py- GrepToolopenhands/tools/glob/definition.py- GlobToolopenhands/tools/planning_file_editor/definition.py- PlanningFileEditorToolPreset Configurations
openhands/tools/preset/default.py- Updated to rely on automatic registrationopenhands/tools/preset/planning.py- Updated to rely on automatic registrationTesting
tests/sdk/tool/test_automatic_registration.py:__init__.pyfilesBefore and After
Before (Manual Registration Required)
After (Automatic Registration)
Implementation Details
Each tool definition file now includes:
register_toolfunctionregister_tool("ToolName", ToolClass)The registration happens when the module is imported, making tool usage much simpler and more intuitive.
Testing
Fixes
Closes #861
Breaking Changes
None. This change is backward compatible - existing code that manually registers tools will continue to work, but the manual registration is now redundant.
@malhotra5 can click here to continue refining the PR
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:0292c6e-pythonRun
All tags pushed for this build
About Multi-Architecture Support
0292c6e-python) is a multi-arch manifest supporting both amd64 and arm640292c6e-python-amd64) are also available if needed