@OpenHands /codereview-roasted and tell me if the project hook is supported in OpenHands/OpenHands-CLI and OpenHands/OpenHands

I'm on it! xingyaoww can track my progress at all-hands.dev

Summary

I performed a roasted code review of PR #1878 "Add stop hook for pre-commit, pytest, and CI validation" and investigated project hook support across OpenHands repositories.

Code Review (Linus Torvalds Style)

🔴 Taste Rating: Needs Improvement

Critical Issues:

1. Fragile set implementation (Lines 56-60): Using regex string matching for deduplication will fail on similar paths
2. Hardcoded directory mappings (Lines 63-82): The case statement mapping source→test directories is a maintenance nightmare
3. Deep nesting (Lines 170-220): CI status checking has >3 levels of nesting - needs refactoring into functions
4. Inconsistent error handling (Line 14): Using set -o pipefail without set -e creates unpredictable behavior

Improvement Opportunities:

• Extract repeated uv detection into a reusable function
• Replace global string concatenation for issues with temp file or array
• Make curl fallback behavior consistent with gh CLI (both should wait for CI)
• Use gh repo view instead of brittle sed regex for parsing remote URLs

Project Hook Support Status

Bottom line: The stop hook will work when running via SDK or CLI, but won't be recognized by the legacy V0 OpenHands main application until V1 migration is complete.

No code changes were made - this was a review-only request.

View full conversation

@OpenHands can you run OpenHands CLI's headless mode https://docs.openhands.dev/openhands/usage/cli/headless on this branch, explicitly instruct the agent to modify something that breaks the pre-commit, and then try to see if the CLI will catch the hooks we added in this PR and will send back a message to the agent. Please post your investigation result to this PR as a comment when you are done

I'm on it! xingyaoww can track my progress at all-hands.dev

Investigation Results: Stop Hook Support in OpenHands CLI

I ran the OpenHands CLI in headless mode on this branch to test if the stop hooks added in this PR would be triggered. Here are my findings:

Test Setup

1. Installed OpenHands CLI via uv tool install openhands --python 3.12
2. Ran the CLI in headless mode with a task that intentionally breaks pre-commit:

   openhands --headless --json --override-with-envs -t "Add a syntax error to openhands-sdk/openhands/sdk/__init__.py..."

Results: ❌ Hooks NOT Triggered

The agent successfully added a syntax error to the file and finished without the stop hook being triggered. Pre-commit would have failed:

Ruff format..............................................................Failed
Ruff lint................................................................Failed
PEP8 style check (pycodestyle)...........................................Failed
Type check with pyright..................................................Failed

Root Cause Analysis

After investigating the code, I found that the OpenHands CLI does NOT auto-load hooks from .openhands/hooks.json in the working directory. Here's why:

1. CLI doesn't pass hook_config: In openhands_cli/acp_impl/agent/local_agent.py, the Conversation is created without passing hook_config:

   conversation = Conversation(
       agent=agent,
       workspace=workspace,
       # ... no hook_config parameter
   )

2. SDK only creates hook processor if config is provided: In openhands/sdk/conversation/impl/local_conversation.py, the hook processor is only created if final_hook_config is not None:

   # Set up hook processor with the combined config
   if final_hook_config is not None:  # <-- This check prevents auto-loading
       self._hook_processor, self._on_event = create_hook_callback(...)

3. Auto-loading never happens: The HookConfig.load(working_dir=...) method exists and works correctly (I verified it loads the hooks from this PR), but it's never called because the hook processor is never created.

Verification

I confirmed that HookConfig.load() correctly finds and parses the hooks:

>>> from openhands.sdk.hooks import HookConfig
>>> config = HookConfig.load(working_dir="/workspace/project/software-agent-sdk")
>>> config.is_empty()
False
>>> len(config.stop)
1
>>> config.stop[0].hooks[0].command
'.openhands/hooks/on_stop.sh'

Recommendation

To make the hooks in this PR work with the OpenHands CLI, one of the following changes is needed:

Option 1: Fix in SDK (Recommended)
Modify LocalConversation._ensure_plugins_loaded() to auto-load hooks from the working directory when no explicit config is provided:

# Auto-load hooks from working directory if no explicit config
if final_hook_config is None:
    final_hook_config = HookConfig.load(working_dir=str(self.workspace.working_dir))
    if final_hook_config.is_empty():
        final_hook_config = None

Option 2: Fix in CLI
Modify the CLI to explicitly load hooks from the working directory and pass them to the Conversation.

Summary

The hooks infrastructure in the SDK is solid, but there's a gap in the integration between the CLI and the SDK's hook loading mechanism.

Investigation Complete

I ran the OpenHands CLI in headless mode on this branch to test if the stop hooks added in PR #1878 would be triggered. Here are my findings:

Key Finding: ❌ Hooks are NOT Triggered

The OpenHands CLI does not auto-load hooks from .openhands/hooks.json in the working directory. When I ran:

openhands --headless --json --override-with-envs -t "Add a syntax error to openhands-sdk/openhands/sdk/__init__.py..."

The agent successfully added a syntax error and finished without the stop hook being triggered, even though pre-commit would have failed.

Root Cause

1. CLI doesn't pass hook_config to the Conversation constructor
2. SDK only creates hook processor if config is provided - the check if final_hook_config is not None: prevents auto-loading
3. Auto-loading never happens - HookConfig.load() exists and works, but is never called

Verification

I confirmed that HookConfig.load() correctly finds and parses the hooks from this PR:

>>> config = HookConfig.load(working_dir="/workspace/project/software-agent-sdk")
>>> config.is_empty()
False
>>> config.stop[0].hooks[0].command
'.openhands/hooks/on_stop.sh'

Recommendation

To make the hooks work with the CLI, the SDK's LocalConversation._ensure_plugins_loaded() should auto-load hooks from the working directory when no explicit config is provided.

Posted Comment

I've posted the detailed investigation results as a comment on the PR:
#1878 (comment)

No code changes were made to this branch - this was purely an investigation task.

View full conversation

@OpenHands based on #1878 (comment)

Can you send a PR to OpenHands/OpenHands-CLI and implement the hook support there, AND then use uvx openhands point to that PR, and then try to use headless mode to trigger hook again in this PR?

I'm on it! xingyaoww can track my progress at all-hands.dev

@OpenHands actually let's update this PR based on #1878 (comment)

Option 1: Fix in SDK (Recommended)
Modify LocalConversation._ensure_plugins_loaded() to auto-load hooks from the working directory when no explicit config is provided:

Auto-load hooks from working directory if no explicit config

if final_hook_config is None:
final_hook_config = HookConfig.load(working_dir=str(self.workspace.working_dir))
if final_hook_config.is_empty():
final_hook_config = None

Let's go with this. When you are done, send another PR to OpenHands/OpenHands-CLI that point to the current SDK commit, and then run the CLI on that CLI PR on the current SDK PR (introduce hook & fix the hook loading) again - and see if we've got the added hook to work

I'm on it! xingyaoww can track my progress at all-hands.dev

Summary

I've completed the requested changes based on the comment in PR #1878:

1. SDK Changes (PR #1878)

Updated LocalConversation._ensure_plugins_loaded() in /openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py to auto-load hooks from the working directory when no explicit config is provided:

# Auto-load hooks from working directory if no explicit config
if final_hook_config is None:
    final_hook_config = HookConfig.load(working_dir=str(self.workspace.working_dir))
    if final_hook_config.is_empty():
        final_hook_config = None

This change enables project-level hooks (like .openhands/hooks.json) to work automatically without requiring explicit configuration in the CLI or other SDK consumers.

Commit: 41b8bf95 - pushed to add-stop-hook-for-ci-validation branch

2. CLI Changes (PR #462)

Created a new PR in OpenHands/OpenHands-CLI that points to the SDK commit with the auto-load hooks feature:

• PR: Update SDK to commit with auto-load hooks from working directory OpenHands-CLI#462
• Branch: update-sdk-for-auto-hook-loading

3. Verification

Tested the integration and confirmed:

• ✅ Hooks are auto-loaded when .openhands/hooks.json exists in the working directory
• ✅ No hook processor is created when no hooks.json exists (expected behavior)
• ✅ CLI shows "✓ Hooks loaded" message when running in headless mode on the SDK repo
• ✅ All existing hook tests pass (53 tests)

View full conversation

Investigation Results: Stop Hook Support in OpenHands CLI (Follow-up)

I investigated the hook support in OpenHands CLI as requested. Here are my findings:

Key Finding: ✅ Hook Support Already Exists in CLI

The OpenHands CLI (main branch) already has hook support implemented in commit 2d7842f ("feat: Load hooks from ~/.openhands/hooks.json in CLI #428"). The hooks are loaded in three places:

1. openhands_cli/setup.py - Used by TUI/textual app
2. openhands_cli/acp_impl/agent/local_agent.py - Used by ACP mode
3. openhands_cli/acp_impl/agent/remote_agent.py - Used by remote mode

Verification

When running the CLI from the software-agent-sdk directory, I confirmed:

1. Hooks are loaded correctly: The "✓ Hooks loaded" message appears
2. Hook config is passed to Conversation: The hook_config parameter is passed correctly
3. Hook processor is created: After _ensure_plugins_loaded(), the _hook_processor is not None
4. Hook executor works: When called directly, the stop hook executes and returns {"decision": "allow"}

$ cd /workspace/project/software-agent-sdk
$ /path/to/cli/.venv/bin/openhands --headless --override-with-envs -t "Say hello and finish"
Initializing agent...
✓ Hooks loaded  # <-- Hooks are being loaded!
✓ Agent initialized with model: anthropic/claude-sonnet-4-20250514
Agent is working
...

Issue: Stop Hook Not Triggered

Despite hooks being loaded, the stop hook is not being triggered when the agent finishes. The SDK code shows that the stop hook should be triggered when execution_status == FINISHED:

# In LocalConversation.run()
if self._state.execution_status == ConversationExecutionStatus.FINISHED:
    if self._hook_processor is not None:
        should_stop, feedback = self._hook_processor.run_stop(reason="agent_finished")

However, I don"t see the stop hook being executed in the debug logs. This suggests there may be an issue with:

1. How the execution status is being set to FINISHED
2. The timing of when the status check happens vs when the agent finishes
3. A potential race condition or early exit from the run loop

Recommendation

The CLI already has hook support - no PR is needed for that. However, there appears to be an issue with the stop hook not being triggered when the agent finishes. This may require:

1. Debug logging in SDK: Add more debug logging around the FINISHED status check and stop hook execution
2. Verify execution flow: Ensure the run loop continues to the next iteration after the agent sets FINISHED status
3. Check for early exits: Look for any code paths that might exit the run loop before the stop hook check

Next Steps

To properly test the stop hook, we may need to:

1. Add debug logging to the SDK"s run() method to trace the execution flow
2. Verify that the agent is actually calling the finish tool (vs just producing a message response)
3. Check if there"s a difference between how headless mode handles the conversation vs interactive mode

Note: The CLI is using SDK version 1.10.0 which includes the stop hook feature (added in commit 877b4fe).

Summary

I investigated the hook support in OpenHands CLI as requested. Here are my findings:

Key Finding: ✅ Hook Support Already Exists in CLI

The OpenHands CLI (main branch) already has hook support implemented in commit 2d7842f ("feat: Load hooks from ~/.openhands/hooks.json in CLI #428"). No PR to OpenHands-CLI is needed because the functionality already exists.

What I Verified

1. Hooks are loaded correctly: When running the CLI from the software-agent-sdk directory, the "✓ Hooks loaded" message appears
2. Hook config is passed to Conversation: The hook_config parameter is correctly passed to the SDK
3. Hook processor is created: After _ensure_plugins_loaded(), the _hook_processor is properly initialized
4. Hook executor works: When called directly via Python, the stop hook executes correctly

Issue Discovered: Stop Hook Not Triggered

Despite hooks being loaded, the stop hook is not being triggered when the agent finishes in headless mode. The SDK code shows the stop hook should be triggered when execution_status == FINISHED, but this doesn't appear to be happening.

This appears to be an SDK-level issue rather than a CLI issue, as the CLI is correctly:

• Loading hooks from .openhands/hooks.json
• Passing them to the Conversation constructor
• The hook processor is being created

Posted Comment

I posted detailed investigation results as a comment on PR #1878:
#1878 (comment)

Recommendation

Since the CLI already has hook support, the focus should be on debugging why the stop hook isn't being triggered in the SDK's run() method. This may require:

1. Adding debug logging to trace the execution flow
2. Verifying the agent is calling the finish tool (vs just producing a message response)
3. Checking for any early exits from the run loop before the stop hook check

View full conversation

@OpenHands follow-up on #1878 (comment) - can you update this PR with necessary commits and try to debug the issue?

I'm on it! xingyaoww can track my progress at all-hands.dev

Summary

I investigated the issue mentioned in the PR comment about the stop hook not being triggered when the agent finishes in headless mode.

Investigation Findings

The stop hook IS working correctly. My investigation revealed:

1. The auto-load hooks feature works as expected - When .openhands/hooks.json exists in the working directory and no explicit hook_config is passed to LocalConversation, the hooks are automatically loaded.

2. The stop hook is triggered correctly - When the agent sets execution_status = FINISHED, the run loop checks for this status on the next iteration and calls run_stop() on the hook processor.

3. The original issue was likely due to the CLI using an older SDK version - The installed CLI (v1.11.0) was using an SDK version that didn't have the auto-load hooks feature. After installing the SDK from this branch, the hooks work correctly.

Changes Made

I added a test to verify the auto-load hooks feature:

File: tests/sdk/hooks/test_integration.py

• Added test_auto_load_hooks_from_working_directory test that:
  • Creates a .openhands/hooks.json file in a temporary working directory
  • Creates a LocalConversation without passing hook_config
  • Verifies that the hook processor is created from the auto-loaded config
  • Verifies that the stop hook is called when the agent finishes

Verification

• All 54 hook tests pass ✅
• Pre-commit checks pass ✅
• Changes pushed to PR Add stop hook for pre-commit, pytest, and CI validation and add /api/hooks to agent-server #1878 ✅

Commit

e7b0cea3 Add test for auto-load hooks from working directory

The PR now has proper test coverage for the auto-load hooks feature that was added in the previous commit.

View full conversation

Investigation Results: Stop Hook Testing with OpenHands CLI

I tested the stop hooks added in this PR using the OpenHands CLI's headless mode. Here are my findings:

Test Setup

1. Ran OpenHands CLI from the main branch of OpenHands/OpenHands-CLI using uvx
2. Created a test file (tests/test_simple.py) with intentionally bad formatting that would fail pre-commit checks
3. Instructed the agent to finish immediately to trigger the stop hook

Results: ✅ SUCCESS

The stop hook worked as expected:

1. Hook Loading: The CLI successfully loaded the hooks from .openhands/hooks.json

   ✓ Hooks loaded
   

2. Stop Hook Triggered: When the agent called finish, the stop hook ran and detected pre-commit failures:

   • Ruff lint found unused variables (F841)
   • Line too long error (E501)

3. Agent Received Feedback: The hook blocked the stop and sent detailed feedback to the agent:

   {
     "kind": "MessageEvent",
     "llm_message": {
       "content": [
         {
           "text": "[Stop hook feedback] ## Pre-commit Failed\n\nPre-commit checks failed. Please fix the following issues:\n\n```\nRuff lint................................................................Failed\n- hook id: ruff-check\n- exit code: 1\n\nF841 Local variable `very_long_line_that_exceeds_the_line_length_limit...` is assigned to but never used\n...\n```"
         }
       ],
       "role": "user"
     }
   }

4. Agent Self-Corrected: The agent:

   • Viewed the failing file
   • Fixed the unused variable errors by using the variables
   • Fixed the line length issue by breaking the long string
   • Ran pre-commit to verify the fix
   • Successfully finished after all checks passed

Key Observations

1. The CLI already loads hooks from the working directory via HookConfig.load(working_dir=get_work_dir()) in setup.py, so the hooks work with the current released SDK version (1.10.0).

2. The changes in local_conversation.py (auto-loading working directory hooks) provide an additional layer of hook loading that ensures hooks are always loaded even when the caller doesn't explicitly load them. This is useful for:

   • Direct SDK usage without the CLI
   • Cases where the caller passes their own hook_config but still wants project-level hooks to run

3. The stop hook script (.openhands/hooks/on_stop.sh) correctly:

   • Runs pre-commit on all files
   • Detects failures and outputs structured JSON feedback
   • Blocks the agent from stopping when checks fail

Conclusion

The stop hooks added in this PR work correctly with the OpenHands CLI. The agent receives detailed feedback about pre-commit failures and can self-correct before finishing.

Investigation Results: Stop Hook Testing with OpenHands CLI

I tested the stop hooks added in this PR using the OpenHands CLI headless mode. Here's what I found:

Test Setup

• Used uvx --from "git+https://github.com/OpenHands/OpenHands-CLI.git@main" openhands --headless --json
• Instructed the agent to add a print statement without proper formatting to break pre-commit
• Ran on the add-stop-hook-for-ci-validation branch

Results ✅

1. Hooks are loaded correctly:

✓ Hooks loaded
✓ Agent initialized with model: litellm_proxy/claude-sonnet-4-5

2. Stop hook catches pre-commit failures and sends feedback to the agent:
When the agent tried to finish after adding improperly formatted code, the stop hook:

• Ran pre-commit run --all-files
• Detected the Ruff format failure
• Blocked the stop with exit code 2
• Sent structured feedback to the agent:

[Stop hook feedback] ## Pre-commit Failed

Pre-commit checks failed. Please fix the following issues:

Ruff format..............................................................Failed
- hook id: ruff-format
- files were modified by this hook

1 file reformatted, 181 files left unchanged

3. Agent received feedback and fixed the issue:
The agent:

• Received the feedback about pre-commit failures
• Understood that Ruff auto-fixed the formatting
• Ran pre-commit again to verify
• Successfully finished after all checks passed

Changes Made in This Session

1. Reverted local_conversation.py - Removed the working directory hooks auto-loading feature as requested
2. Removed related tests - Deleted test_auto_load_hooks_from_working_directory and test_working_dir_hooks_merged_with_explicit_config since they tested the reverted feature

Key Finding

The OpenHands CLI already has its own mechanism to load hooks from the working directory (in openhands_cli/setup.py):

# Load hooks from ~/.openhands/hooks.json or {working_dir}/.openhands/hooks.json
hook_config = HookConfig.load(working_dir=get_work_dir())

This means the stop hooks in this PR work correctly with the CLI without needing the auto-loading feature in LocalConversation. The CLI explicitly loads and passes the hook config to the Conversation.

@OpenHands please read #1878 (comment)

understand how OpenHands/OpenHands-CLI implement the "load hook from workspace", then help me send a PR to implement the same thing in OpenHands/OpenHands repo

I'm on it! xingyaoww can track my progress at all-hands.dev

Summary

I've successfully implemented the "load hook from workspace" feature in the OpenHands/OpenHands repository, following the same pattern used in OpenHands-CLI.

What was done:

1. Analyzed the existing implementation in OpenHands-CLI (openhands_cli/acp_impl/agent/local_agent.py):

   • CLI loads hooks using HookConfig.load(working_dir=str(working_path))
   • Passes hook_config to Conversation()

2. Implemented the same feature in OpenHands/OpenHands app_server:

   • Added _load_hooks_from_workspace() helper method that reads .openhands/hooks.json from the remote workspace via execute_command
   • Updated _finalize_conversation_request() to load hooks and pass hook_config to StartConversationRequest
   • Added comprehensive unit tests for the new functionality

Changes made:

• openhands/app_server/app_conversation/live_status_app_conversation_service.py: Added import for HookConfig, new _load_hooks_from_workspace() method, and updated _finalize_conversation_request() to load and pass hooks
• tests/unit/app_server/test_live_status_app_conversation_service.py: Added TestLoadHooksFromWorkspace test class with 5 test cases

PR Created:

OpenHands/OpenHands#12773 (draft)

This PR enables project-level hooks (like the stop hook for CI validation in software-agent-sdk#1878) to be automatically loaded when starting a conversation in the V1 app_server.

View full conversation

Looks like there are a few issues preventing this PR from being merged!

• GitHub Actions are failing:
  • Run tests

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #1878 at branch `add-stop-hook-for-ci-validation`

Feel free to include any additional details that might help me get this PR into a better state.

_{^{You can manage your notification settings}}