fix(hooks): #545 launcher watchdog pipe stall + flaky test margins (#565)

Two-layer root cause behind the rotating launcher-test failures:

1) Production: on timeout-less hosts (stock macOS) the watchdog subshell
   and its sleep inherited the caller's $(...) capture pipe; after the
   parent killed the subshell the orphaned sleep held the write end, so
   every healthy probe/guard run blocked for the full bound (~2x bound
   per PreToolUse call, measured 6.1s at the default). Redirect the
   watchdog to /dev/null — healthy watchdog path drops to ~0.15s,
   decisions unchanged.

2) Tests: the harness pinned ARS_PROBE_BOUND=1 for every test, so under
   machine load real interpreter+guard runs overran the bound and the
   launcher's designed pass-through degradation read as deny-test
   failures. Default is now a generous 30 (only the hanging-candidate
   tests wait a bound out; they override to 5), and a new
   LauncherSlowInterpreterTest pins the regression.

Suite: 21 tests / 70-80s -> 22 tests / ~23s, green under 8-way CPU load.

Closes #545


Claude-Session: https://claude.ai/code/session_01EA3EvegVqKrkM62u7k9PHF

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Edward Cheng-I Wu
2026-07-20 06:46:17 +08:00
committed by GitHub
parent 276afa1d05
commit 640d8384f9
3 changed files with 50 additions and 8 deletions
+1
View File
@@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file.
### Fixed
- **SETUP Method 4a description-length figure de-drifted.** `docs/SETUP.md` / `docs/SETUP.zh-TW.md` stated the four skill `description` fields "currently sit in the 440-842 range"; the actual lengths have since grown to 566-986 characters. Replaced the hardcoded range with the durable comparative statement (each exceeds claude.ai's 200-character upload cap while staying under Claude Code's 1,024-character allowance), so the sentence cannot silently drift again as descriptions evolve. Rationale for not trimming the descriptions themselves is unchanged.
- **Write-scope guard launcher: watchdog-path stall removed; #545 flaky launcher tests fixed (#545).** Two-layer root cause. (1) Production: on hosts without a `timeout` binary (stock macOS), `hooks/run_guard.sh`'s watchdog subshell — and the `sleep` it forks — inherited the caller's `$(...)` capture pipe; after the parent killed the subshell, the orphaned `sleep` kept the write end open, so every healthy probe/guard run blocked for the full wall-clock bound (~2× bound per PreToolUse call; measured 6.1 s at the default bound). The watchdog now redirects to `/dev/null`, cutting the healthy watchdog path to ~0.15 s with decisions unchanged. (2) Tests: the harness pinned `ARS_PROBE_BOUND=1`, leaving so little margin that loaded machines pushed real interpreter + guard runs past the bound into the launcher's designed pass-through degradation — which every deny-expecting test read as a rotating, load-dependent failure. The harness default is now a generous 30 s (pure margin; only the hanging-candidate tests wait a bound out, and they override to 5 s), and a new `LauncherSlowInterpreterTest` pins the regression (a python3 that takes ~1.5 s per invocation must still forward the real deny). Suite: 21 tests / 70-80 s → 22 tests / ~23 s, green under 8-way CPU load.
## [3.18.0] - 2026-07-18 — Self-improvement survey integration: advisory quality layers, risk-stratified claim gate, cross-model reviewer & judge tracks, cache re-validation
+10 -2
View File
@@ -1,5 +1,5 @@
#!/bin/sh
# version: 1.0.0
# version: 1.0.1
#
# ARS write-scope guard LAUNCHER — PreToolUse hook (#454 Windows portability fix).
#
@@ -132,6 +132,14 @@ run_bounded() {
{ "$@" >"$_rb_out" <&3 3<&- & } 3<&0
fi
_cmd_pid=$!
# The watchdog subshell (and the `sleep` it forks) must NOT inherit our stdout: when the
# caller captures run_bounded via `$(...)`, an inherited fd1 IS that pipe, and after we
# kill the subshell its orphaned `sleep` keeps the write end open — the caller's `$(...)`
# then blocks until the FULL bound elapses even though the job finished in milliseconds.
# That stall happened on every healthy probe/guard run on every timeout-less host (~2x
# bound per hook call, measured 6.1s at the default bound on stock macOS) and is the
# load-margin flake #545 documents. Same reasoning as note (2) above for the job itself;
# the watchdog writes nothing to stdout, so /dev/null loses nothing.
( sleep "$PROBE_BOUND"
# If the parent already signalled completion, the child finished within the bound: do NOT
# kill (its pid may have been recycled to an innocent process) and do NOT flag a timeout.
@@ -145,7 +153,7 @@ run_bounded() {
kill -KILL "-$_cmd_pid" 2>/dev/null || kill -KILL "$_cmd_pid" 2>/dev/null
fi
fi
) &
) >/dev/null 2>&1 &
_watch_pid=$!
wait "$_cmd_pid" 2>/dev/null
_st=$?
+39 -6
View File
@@ -70,6 +70,19 @@ _SYS_PATH = "/usr/bin:/bin:/usr/sbin:/sbin"
_SH = shutil.which("sh") or "/bin/sh"
_LAUNCHER_TIMEOUT = 45 # generous so a (bounded) hanging-candidate test can't false-fail
# #545: GENEROUS default bound. For decision-forwarding tests the bound is pure safety
# margin — it only ever elapses when a candidate actually hangs — and the old pinned "1"
# left so little margin that interpreter startup + the guard run could overrun it under
# machine load (worse on hosts without a `timeout` binary, where the watchdog's done-file
# handshake races completions near the bound). The launcher then degraded BY DESIGN to
# pass-through and deny-expecting tests failed in rotation. Hanging-candidate tests are
# the ONLY ones whose runtime scales with the bound (they wait it out to kill the hung
# probe), so they override with _HANG_PROBE_BOUND — small for speed, but not the flaky
# "1": the same bound also clocks the real python3 probe and the guard run inside them,
# so it still needs load headroom.
_DEFAULT_PROBE_BOUND = "30"
_HANG_PROBE_BOUND = "5"
def _run_launcher(bin_dir, payload, extra_env=None, launcher=LAUNCHER):
"""Run the launcher with PATH = bin_dir + system dirs (bin_dir first, so injected
@@ -82,8 +95,7 @@ def _run_launcher(bin_dir, payload, extra_env=None, launcher=LAUNCHER):
path = bin_dir + os.pathsep + _SYS_PATH
env = {
"PATH": path,
# Short probe bound keeps the suite fast; a hanging-candidate is killed in ~1s.
"ARS_PROBE_BOUND": "1",
"ARS_PROBE_BOUND": _DEFAULT_PROBE_BOUND, # #545: see the constant's comment
# The launcher must resolve the guard from its OWN path, not any var (codex P1).
# We deliberately do NOT set CLAUDE_PLUGIN_ROOT in most tests.
}
@@ -289,7 +301,7 @@ class LauncherSelfResolveTest(unittest.TestCase):
_fake_real_python(os.path.join(bin_dir, "python3"))
payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md"))
env = {"PATH": bin_dir + os.pathsep + _SYS_PATH, "CLAUDE_PROJECT_DIR": ws,
"ARS_PROBE_BOUND": "1"}
"ARS_PROBE_BOUND": _DEFAULT_PROBE_BOUND}
proc = subprocess.run([_SH, os.path.join(spaced, "hooks", "run_guard.sh")],
input=json.dumps(payload), env=env, capture_output=True,
text=True, timeout=_LAUNCHER_TIMEOUT)
@@ -364,14 +376,17 @@ class LauncherHangingCandidateTest(unittest.TestCase):
"""A candidate interpreter that hangs must be killed within a bound, then move on —
the hot path must never hang (spec §3.3 watchdog)."""
_HANG_BOUND = {"ARS_PROBE_BOUND": _HANG_PROBE_BOUND} # #545: see _DEFAULT_PROBE_BOUND
def test_hanging_py_then_real_python3(self):
with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws:
# py hangs on the marker probe; python3 is real. Launcher must kill py's probe
# and fall through to python3 within the test's 30s subprocess timeout.
# and fall through to python3 within the test's _LAUNCHER_TIMEOUT.
_write_exec(os.path.join(bin_dir, "py"), "#!/bin/sh\nsleep 60\n")
_fake_real_python(os.path.join(bin_dir, "python3"))
payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md"))
code, out, err = _run_launcher(bin_dir, payload, extra_env={"CLAUDE_PROJECT_DIR": ws})
code, out, err = _run_launcher(bin_dir, payload,
extra_env={"CLAUDE_PROJECT_DIR": ws, **self._HANG_BOUND})
self.assertEqual(code, 0, f"must not hang/error on a hanging candidate; err={err!r}")
self.assertEqual(json.loads(out)["hookSpecificOutput"].get("permissionDecision"), "deny",
"must kill the hanging py and use the real python3")
@@ -391,12 +406,30 @@ class LauncherHangingCandidateTest(unittest.TestCase):
payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md"))
code, out, err = _run_launcher(bin_dir, payload,
extra_env={"CLAUDE_PROJECT_DIR": ws,
"ARS_GUARD_FORCE_WATCHDOG": "1"})
"ARS_GUARD_FORCE_WATCHDOG": "1",
**self._HANG_BOUND})
self.assertEqual(code, 0, f"watchdog must reap the hung probe; err={err!r}")
self.assertEqual(json.loads(out)["hookSpecificOutput"].get("permissionDecision"), "deny",
"watchdog must kill the hanging py and fall through to python3")
class LauncherSlowInterpreterTest(unittest.TestCase):
"""#545 regression pin: a SLOW but working interpreter (what a loaded machine
produces) must not be false-degraded into pass-through. Under the harness default
bound a python3 that takes ~1.5s per invocation must still produce the real deny
decision — see _DEFAULT_PROBE_BOUND for the full mechanism."""
def test_slow_but_working_python_forwards_deny(self):
with tempfile.TemporaryDirectory() as bin_dir, tempfile.TemporaryDirectory() as ws:
_write_exec(os.path.join(bin_dir, "python3"),
f'#!/bin/sh\nsleep 1.5\nexec "{REAL_PY}" "$@"\n')
payload = _bucket_a_payload(ws, os.path.join(ws, "out_of_scope", "x.md"))
code, out, err = _run_launcher(bin_dir, payload, extra_env={"CLAUDE_PROJECT_DIR": ws})
self.assertEqual(code, 0, f"err={err!r}")
self.assertEqual(json.loads(out)["hookSpecificOutput"].get("permissionDecision"), "deny",
"a slow-but-real interpreter must not be false-degraded to pass-through")
class LauncherWatchdogRobustnessTest(unittest.TestCase):
"""Round-6 gemini track: the no-`timeout` watchdog must not fail OPEN under (a) a private-
temp allocation failure or (b) the pid-reuse race that would false-report a timeout."""