fix: strip stop hooks from SDK to prevent conversation stall#651
Draft
fix: strip stop hooks from SDK to prevent conversation stall#651
Conversation
Stop hooks executed inside the SDK's run-loop state lock cause two issues: 1. A missing hook script exits with code 2 (Python's file-not-found), which the SDK interprets as 'block the stop' — creating an infinite LLM-calling loop. 2. The SDK holds its FIFO lock while running stop hooks, which blocks pause/ESC from acquiring the lock. Fix: Strip stop hooks from HookConfig before passing to the SDK's Conversation. Execute them at the CLI level after conversation.run() returns, with better error handling: - Missing scripts (exit code 2 with file-not-found stderr) → allow stop - Timeouts and crashes → allow stop - Only explicit blocks (deliberate exit code 2 or JSON deny) → deny stop Co-authored-by: openhands <openhands@all-hands.dev>
Contributor
Coverage Report •
|
||||||||||||||||||||||||||||||||||||||||||||||||||
- Use stored conversation_id and get_work_dir() instead of reaching through conversation.state (ConversationStateProtocol doesn't expose conversation_id, BaseConversation doesn't expose workspace directly) - Fix E303 too many blank lines in test file Co-authored-by: openhands <openhands@all-hands.dev>
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
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.
What Changed
Fixes #649 — Stop hooks cause conversation to stall in 'Working' state after FinishAction.
Problem
When a user configures a
Stophook in.openhands/hooks.json, the conversation stalls in the "Working" state after the agent completes (FinishAction). Two issues in how the SDK's run loop handles stop hooks cause this:Exit code 2 treated as "block" → infinite loop: Python exits with code 2 when it can't find a script file (
python /nonexistent.py). The SDK's hook executor interprets exit code 2 as "deny the stop", causing the run loop to set status back to RUNNING, callagent.step()again (which triggers another FinishAction), and repeat indefinitely — each iteration involving an LLM call.Stop hooks run while holding the state lock → pause blocked: The SDK's run loop executes stop hooks inside
with self._state:(holding the FIFO lock), sopause()cannot acquire the lock while hooks run. Combined with the infinite loop, the UI becomes permanently unresponsive.Solution
Strip stop hooks from the
HookConfigbefore passing to the SDK'sConversation, and handle them at the CLI level afterconversation.run()returns. This:{"decision": "deny"}responses still block the stopChanges
openhands_cli/stop_hooks.pyopenhands_cli/setup.pystrip_stop_hooks()helper;setup_conversation()now returns(conversation, stop_matchers)openhands_cli/tui/core/conversation_runner.pyconversation.run()returns with FINISHED statusopenhands_cli/acp_impl/agent/local_agent.pyopenhands_cli/acp_impl/agent/remote_agent.pytests/test_stop_hooks.pytests/tui/test_headless_mode.pysetup_conversation()return typeVerification
This PR was created by an AI assistant (OpenHands) on behalf of the user.
@xingyaoww can click here to continue refining the PR
🚀 Try this PR