From 9bbe25fc101340da0b0a8edc7dc9dc886999b374 Mon Sep 17 00:00:00 2001 From: Chris Graf Date: Fri, 3 Jul 2026 09:41:35 -0500 Subject: [PATCH 1/2] Bound `_hook_hm_event`'s `hm` call with a short timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_011jWSryYVwQnNc6vsaWXd1J --- .gitignore | 1 + bin/agent-hook-session-end-claude | 9 +- lib/agentguard/hook-helpers.sh | 33 +++++++- test/suites/agent-hook-helpers-test | 123 ++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..e458ed5 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +.worktrees/ diff --git a/bin/agent-hook-session-end-claude b/bin/agent-hook-session-end-claude index cb4e6b2..1da8cf4 100755 --- a/bin/agent-hook-session-end-claude +++ b/bin/agent-hook-session-end-claude @@ -18,17 +18,12 @@ _agentguard_claude_session_end() { # Already titled — skip _agentguard_transcript_has_custom_title "$transcript_path" && return 0 - local timeout_prefix=() - if command -v timeout >/dev/null 2>&1; then - timeout_prefix=(timeout 60) - elif command -v gtimeout >/dev/null 2>&1; then - timeout_prefix=(gtimeout 60) - fi + _hook_timeout_prefix 60 # Fire-and-forget: nohup so naming survives hook exit, with a timeout when # the platform provides one. # shellcheck disable=SC2016 - nohup "${timeout_prefix[@]}" bash -c ' + nohup "${_HOOK_TIMEOUT_PREFIX[@]}" bash -c ' transcript_has_custom_title() { jq -e -R "$2" "$1" >/dev/null 2>&1 } diff --git a/lib/agentguard/hook-helpers.sh b/lib/agentguard/hook-helpers.sh index b89dab3..0a37415 100644 --- a/lib/agentguard/hook-helpers.sh +++ b/lib/agentguard/hook-helpers.sh @@ -585,6 +585,22 @@ _hook_hm_warn_once() { _hook_once_per_session "hm-warn-$key" && _hook_warn "$message" } +# Best-effort external timeout guard for a slow subprocess, general-purpose +# (not Hive Memory-specific). Not every platform ships GNU `timeout` — macOS +# needs `coreutils` from brew for `gtimeout` — so this silently no-ops there +# rather than ever blocking a caller when neither is present. Sets the +# _HOOK_TIMEOUT_PREFIX array (0, 1, or 2 words) to prepend to the guarded +# command; an empty array expands to nothing, leaving the command unguarded. +_hook_timeout_prefix() { + local seconds="$1" + _HOOK_TIMEOUT_PREFIX=() + if command -v timeout >/dev/null 2>&1; then + _HOOK_TIMEOUT_PREFIX=(timeout "$seconds") + elif command -v gtimeout >/dev/null 2>&1; then + _HOOK_TIMEOUT_PREFIX=(gtimeout "$seconds") + fi +} + _hook_hm_event() { _hook_hm_available || return 0 @@ -617,6 +633,15 @@ _hook_hm_event() { ;; esac hm_args+=(--json "$@") + + # `hm`'s canonical store for a given alias can be a network-backed mount + # (e.g. an rclone/cloud-synced Google Drive path) with no latency bound of + # its own. Guard the call with a short external timeout so a slow or + # unreachable store degrades to "skip memory context this turn" well + # inside the hook runner's own timeout budget, instead of risking the + # whole hook process getting killed later and its output discarded. + _hook_timeout_prefix "${AGENTGUARD_HIVE_MEMORY_TIMEOUT:-2}" + if [ "$project_infer" -eq 0 ]; then response=$( env -u HIVE_MEMORY_PROJECT \ @@ -624,7 +649,7 @@ _hook_hm_event() { HIVE_MEMORY_SESSION_ID="$_HOOK_SESSION_KEY" \ HIVE_MEMORY_PROJECT_INFER=0 \ HIVE_MEMORY_HOOK_ACTIVE=1 \ - hm "${hm_args[@]}" 2>"$err" + "${_HOOK_TIMEOUT_PREFIX[@]}" hm "${hm_args[@]}" 2>"$err" ) else response=$( @@ -633,12 +658,14 @@ _hook_hm_event() { HIVE_MEMORY_PROJECT="$project" \ HIVE_MEMORY_PROJECT_INFER="$project_infer" \ HIVE_MEMORY_HOOK_ACTIVE=1 \ - hm "${hm_args[@]}" 2>"$err" + "${_HOOK_TIMEOUT_PREFIX[@]}" hm "${hm_args[@]}" 2>"$err" ) fi rc=$? if [ "$rc" -ne 0 ]; then - if [ -s "$err" ]; then + if [ "$rc" -eq 124 ]; then + _hook_hm_warn_once "timeout" "Hive Memory hook timed out after ${AGENTGUARD_HIVE_MEMORY_TIMEOUT:-2}s — skipping memory context this turn" + elif [ -s "$err" ]; then _hook_hm_warn_once "failed" "Hive Memory hook failed: $(tr '\n' ' ' <"$err")" else _hook_hm_warn_once "failed" "Hive Memory hook failed with status $rc" diff --git a/test/suites/agent-hook-helpers-test b/test/suites/agent-hook-helpers-test index 46128a2..0f95413 100755 --- a/test/suites/agent-hook-helpers-test +++ b/test/suites/agent-hook-helpers-test @@ -504,6 +504,11 @@ case "${HM_MODE:-empty}" in printf 'backend offline\n' >&2 exit 42 ;; + hang) + # Simulates a canonical store that never responds (e.g. an unreachable + # network mount) so the timeout guard has something real to bound. + sleep 10 + ;; *) printf '%s\n' '{"warnings":[],"actions":[]}' ;; @@ -887,6 +892,124 @@ HOME="$HM_HOME" \ hm_fail_count=$(grep -c 'Hive Memory hook failed' "$hm_fail_err" || true) _assert_eq "hm hook: repeated failures warn once per session" "1" "$hm_fail_count" +# `hm`'s canonical store can be a network-backed mount with no bound of its +# own (a real incident: an rclone/cloud-synced Google Drive mount going +# through a slow revalidation caused prompt-submit to blow past the host +# agent's own hook timeout). `_hook_hm_event` must guard the call so a +# hung/unreachable store degrades to "skip memory context this turn" well +# inside a short, configurable budget instead of running unbounded. +# +# _hook_hm_prompt_submit returns early on an empty prompt, before ever +# calling _hook_hm_event — so, unlike the mocked-response cases above, this +# needs real prompt text in _HOOK_INPUT to actually reach the guarded `hm` +# call. +: >"$HM_LOG" +hm_hang_err="$HM_MOCK_ROOT/hang.err" +hm_hang_start=$(date +%s) +HOME="$HM_HOME" \ + PATH="$HM_BIN:$PATH" \ + TMPDIR="$HM_TMP" \ + HM_LOG="$HM_LOG" \ + HM_MODE=hang \ + AGENTGUARD_HIVE_MEMORY_HOOKS=1 \ + AGENTGUARD_HIVE_MEMORY_TIMEOUT=1 \ + AGENTGUARD_NAME=codex \ + CODEX_THREAD_ID="hm-hang-session" \ + bash -c ' + source "$1" + _HOOK_INPUT='"'"'{"prompt":"remember this"}'"'"' + _hook_hm_prompt_submit + ' _ "$HELPERS" >/dev/null 2>"$hm_hang_err" +hm_hang_elapsed=$(($(date +%s) - hm_hang_start)) +# The mock sleeps 10s; a working guard returns close to the 1s budget. Bound +# well below 10s so this only fails if the guard genuinely isn't cutting the +# call short, not on ordinary CI scheduling jitter around 1s. +if [ "$hm_hang_elapsed" -lt 6 ]; then + _pass "hm hook: a hung store is cut off near the configured timeout, not left to run" +else + _fail "hm hook: a hung store is cut off near the configured timeout, not left to run" +fi +_assert_contains "hm hook: a timeout is reported distinctly from a hard failure" \ + "Hive Memory hook timed out after 1s" "$(cat "$hm_hang_err")" + +# The timeout is advisory infrastructure, not a hard failure: the calling +# hook must still exit cleanly and simply proceed without memory context. +hm_hang_rc=0 +HOME="$HM_HOME" \ + PATH="$HM_BIN:$PATH" \ + TMPDIR="$HM_TMP" \ + HM_LOG="$HM_LOG" \ + HM_MODE=hang \ + AGENTGUARD_HIVE_MEMORY_HOOKS=1 \ + AGENTGUARD_HIVE_MEMORY_TIMEOUT=1 \ + AGENTGUARD_NAME=codex \ + CODEX_THREAD_ID="hm-hang-session-2" \ + bash -c ' + source "$1" + _HOOK_INPUT='"'"'{"prompt":"remember this"}'"'"' + _hook_hm_prompt_submit + ' _ "$HELPERS" >/dev/null 2>/dev/null || hm_hang_rc=$? +_assert_eq "hm hook: a timed-out call still returns success to the caller" "0" "$hm_hang_rc" + +# Lock in the actual default (AGENTGUARD_HIVE_MEMORY_TIMEOUT unset) rather +# than only ever exercising it overridden for test speed above — a future +# change to the hardcoded default in hook-helpers.sh should fail this +# assertion, not silently drift from what's documented/expected. Costs a +# real ~2s of wall clock since it deliberately does not override the value. +: >"$HM_LOG" +hm_hang_default_err="$HM_MOCK_ROOT/hang-default.err" +HOME="$HM_HOME" \ + PATH="$HM_BIN:$PATH" \ + TMPDIR="$HM_TMP" \ + HM_LOG="$HM_LOG" \ + HM_MODE=hang \ + AGENTGUARD_HIVE_MEMORY_HOOKS=1 \ + AGENTGUARD_NAME=codex \ + CODEX_THREAD_ID="hm-hang-default-session" \ + bash -c ' + unset AGENTGUARD_HIVE_MEMORY_TIMEOUT + source "$1" + _HOOK_INPUT='"'"'{"prompt":"remember this"}'"'"' + _hook_hm_prompt_submit + ' _ "$HELPERS" >/dev/null 2>"$hm_hang_default_err" +_assert_contains "hm hook: default timeout is 2s when unconfigured" \ + "Hive Memory hook timed out after 2s" "$(cat "$hm_hang_default_err")" + +# _hook_timeout_prefix (general-purpose, not Hive Memory-specific): absent +# timeout/gtimeout means an empty prefix, so a guarded command silently runs +# unguarded rather than the caller ever blocking on a missing dependency. +# PATH is narrowed *inside* the sourced script, after `bash` itself has +# already started with the ambient PATH — narrowing it on the outer `bash -c` +# invocation instead would make the outer shell unable to find `bash` at all. +timeout_prefix_absent_len=$( + bash -c ' + source "$1" + PATH="/nonexistent-agentguard-test-path" + _hook_timeout_prefix 5 + printf "%s" "${#_HOOK_TIMEOUT_PREFIX[@]}" + ' _ "$HELPERS" +) +_assert_eq "hook timeout prefix: no-ops without timeout/gtimeout on PATH" \ + "0" "$timeout_prefix_absent_len" + +FAKE_TIMEOUT_BIN=$(_tmpdir) +cat >"$FAKE_TIMEOUT_BIN/timeout" <<'SCRIPT' +#!/usr/bin/env bash +shift +exec "$@" +SCRIPT +chmod +x "$FAKE_TIMEOUT_BIN/timeout" +timeout_prefix_present=$( + bash -c ' + source "$1" + PATH="$2" + _hook_timeout_prefix 5 + printf "%s\n" "${_HOOK_TIMEOUT_PREFIX[*]}" + ' _ "$HELPERS" "$FAKE_TIMEOUT_BIN" +) +_assert_eq "hook timeout prefix: prepends the available timeout binary and duration" \ + "timeout 5" "$timeout_prefix_present" + # _hook_agent_name returns "codex" when CODEX_INTERNAL_ORIGINATOR_OVERRIDE is set # shellcheck disable=SC2034 # env vars consumed by sourced helpers, not direct use ( From 8d1f9e7386674b48aee1a04c9f01abdf5be9ab14 Mon Sep 17 00:00:00 2001 From: Chris Graf Date: Fri, 3 Jul 2026 10:01:55 -0500 Subject: [PATCH 2/2] Add a portable timeout fallback for stock macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 '