Bound _hook_hm_event's hm call with a short timeout#41
Merged
Conversation
Summary `hm`'s canonical store for a given alias can be a network-backed mount (an rclone/cloud-synced Google Drive path, for one real machine) with no latency bound of its own. `_hook_hm_event` called `hm` unguarded, relying entirely on the host agent's own outer hook timeout to eventually kill it — a real incident traced a "UserPromptSubmit hook timed out after 10s" complaint back to exactly this, compounded by an unrelated hook-array duplication bug in the calling dotfiles repo. - add `_hook_timeout_prefix`, a general-purpose (not Hive Memory specific) portable `timeout`/`gtimeout` wrapper that no-ops when neither binary is present, matching the platform-support level this repo already targets (macOS ships neither by default) - guard the `hm` call in `_hook_hm_event` with it, defaulting to 2s via `AGENTGUARD_HIVE_MEMORY_TIMEOUT` — normal warm calls measured around 60-130ms, so 2s is generous headroom while still keeping the worst case tight and well inside any host agent's own hook timeout - a `rc -eq 124` (the guard firing) now warns distinctly from a hard `hm` failure, so "the store is slow" and "the store is broken" are distinguishable in hook stderr/warnings - either way, `_hook_hm_event` still returns 0: the timeout is advisory infrastructure, not a reason to fail the calling hook — the caller just proceeds without memory context that turn - consolidate `agent-hook-session-end-claude`'s own inline timeout/gtimeout duplication (second use of the same four-line pattern) onto the new shared helper Testing - full suite (`test/agentguard-test`): all suites, 0 failed - `shellcheck` and `checkrun lint` clean on all changed files - new tests: a real hung mock `hm` (not just a mocked exit code) is cut off near the configured budget rather than left to run, the timeout is reported distinctly from a hard failure, a timed-out call still returns success to the caller, the *default* (env var unset) is locked at 2s so a future change to the hardcoded default fails this test instead of silently drifting, and `_hook_timeout_prefix` itself is tested directly for both the guarded and no-op-fallback cases Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011jWSryYVwQnNc6vsaWXd1J
Summary CI caught a real gap in the previous commit: GitHub's macOS runners ship neither `timeout` nor `gtimeout`, so `_HOOK_TIMEOUT_PREFIX` fell back to empty and the guard did nothing — three tests failed after running the mock's full 10s unguarded instead of being cut off near the configured budget. This isn't just a CI quirk; stock macOS (no Homebrew coreutils) has the same gap, so any Mac in the fleet without coreutils installed would have been just as unprotected as before this whole change. - add a portable bash-only timeout implementation (background the command, poll every 0.1s, TERM then KILL past budget, exit 124) used only when neither real binary is available, so the guard actually works everywhere bash runs instead of depending on an undocumented/optional dependency - it has to be a real executable, not a shell function: `env -u HIVE_MEMORY_PROJECT ...` (used on one of _hook_hm_event's two code paths) can only exec real binaries, not the calling shell's functions, so the fallback is a `bash -c '<script>' _ "$seconds"` invocation instead — no new dependency since bash is already required - fix the test suite's own mock `hm hang` case to `exec sleep` instead of forking it: a plain `sleep 10` child left an orphaned grandchild holding a captured-output pipe open for the full 10s even after the timeout guard correctly fired at the configured budget, which is a test-mock artifact (the real hm binary is a single process, not a shell wrapping a subprocess) that masked the fallback actually working correctly end to end Testing - full suite (`test/agentguard-test`): all suites, 0 failed - `shellcheck` and `checkrun lint` clean on all changed files - new tests: the portable fallback itself is proven against a real hanging `sleep`, and a second end-to-end test goes through the real `_hook_hm_prompt_submit` path with only bash/dirname/basename/ps/sed /tr/cat/jq/mktemp/sleep on PATH (everything hook-helpers.sh and detect.sh need, minus timeout/gtimeout specifically) against a genuinely hung mock `hm` — the closest local reproduction of the exact scenario that failed on macOS CI Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011jWSryYVwQnNc6vsaWXd1J
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
1 participant
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
hm's canonical store for a given alias can be a network-backed mount (an rclone/cloud-synced Google Drive path, for one real machine) with no latency bound of its own;_hook_hm_eventcalledhmfully unguarded, relying entirely on the host agent's own outer hook timeout to eventually kill it_hook_timeout_prefix, a general-purpose portabletimeout/gtimeoutwrapper (no-ops when neither is present), guards thehmcall with it (default 2s viaAGENTGUARD_HIVE_MEMORY_TIMEOUT), distinguishes a timeout warning from a hard failure, and still returns success either way since this is advisory infrastructure, not a reason to fail the calling hookagent-hook-session-end-claude's own inline duplicate of the same timeout/gtimeout pattern onto the new shared helperTest plan
test/agentguard-test): all suites, 0 failedshellcheckandcheckrun lintclean on all changed fileshmis cut off near budget, timeout vs hard-failure messaging is distinct, a timed-out call still returns success, the default (2s, env var unset) is locked in by its own test, and_hook_timeout_prefixis tested directly for both guarded and no-op-fallback cases🤖 Generated with Claude Code
https://claude.ai/code/session_011jWSryYVwQnNc6vsaWXd1J