Files
hermes-agent/tests/agent/test_shell_hooks_tree_kill.py
ethernet 59624b7ace tests: replace every bare host skipif with platforms()
The OS lanes are marker-driven: list_os_marked_tests.py picks the files
a lane imports from their platforms() specs and the lane selects with
-m platforms. A test gated with skipif(sys.platform != "win32") is
therefore never imported on the Windows lane and skipped everywhere else
— it runs on no host. skipif(sys.platform == "win32") tests were merely
invisible to the lane bookkeeping, but the rule the tree now follows is
one host marker, never a bare skipif.

Mechanical mapping, semantics preserved: skip-on-Windows → "posix",
skip-off-Windows → "windows", skip-off-Linux → "linux", skip-on-macOS →
"not macos". The former skip reasons stay as trailing comments. A
non-host condition (os.geteuid() == 0) stays a separate skipif beside
the marker, spelled getattr(os, "geteuid", ...) so the decorator still
imports on Windows.

Where the conversion would stack two platforms() marks on one test (the
conftest rejects that at collection) the narrower mark wins:
- test_update_wedged_gateway: the class is already platforms("linux");
  its per-test "needs UNIX sockets" marks were redundant and are gone.
- test_process_registry.TestSystemdCgroupIsolation: the class-level
  skip-on-Windows moves onto the 11 methods that had no host mark; the
  11 platforms("linux") methods keep theirs.
- test_file_ops_single_roundtrip: the two fifo tests drop their
  platforms("linux") in favour of the module's "posix" (mkfifo exists on
  macOS; both tests already skip when it does not).
- test_linux_desktop_entry / test_gateway_job_teardown_live: duplicate
  or wider marks removed.
2026-09-21 19:18:15 -04:00

197 lines
6.6 KiB
Python

"""Process-tree cleanup for timed-out shell hooks (port of openai/codex#37527).
Timing out a shell hook must not leave descendant processes running after the
hook itself is stopped. ``_spawn`` places the hook in its own process group on
POSIX (``process_group=0``) and, on timeout, ``kill_process_tree`` signals the
whole group — gated on the child actually leading its own group so a
shared-group spawn can never take down unrelated processes. Hooks that
complete within their timeout keep their descendants (intentionally detached
helpers survive a successful run).
These tests use REAL subprocesses (no mocks): group membership and survival
semantics cannot be mocked.
"""
import os
import signal
import sys
import textwrap
import threading
import time
import pytest
from agent.shell_hooks import ShellHookSpec, _spawn
pytestmark = pytest.mark.platforms("posix") # POSIX process-group semantics
def _spec(command: str, timeout: int = 2) -> ShellHookSpec:
return ShellHookSpec(
event="post_tool_call",
command=command,
matcher=None,
timeout=timeout,
fail_closed=False,
)
def _write_forking_script(tmp_path, stall_after: bool):
"""A hook that forks a long-lived descendant, then stalls or exits."""
marker = tmp_path / "child.pid"
script = tmp_path / "hook.sh"
tail = "sleep 300" if stall_after else "exit 0"
script.write_text(
textwrap.dedent(
f"""\
#!/usr/bin/env bash
sleep 300 &
echo $! > {marker}
{tail}
"""
)
)
script.chmod(0o755)
return script, marker
def _pid_alive(pid: int) -> bool:
try:
os.kill(pid, 0)
except ProcessLookupError:
return False
return True
def _read_marker(marker, timeout=5.0) -> int:
deadline = time.monotonic() + timeout
while time.monotonic() < deadline:
if marker.exists() and marker.read_text().strip():
return int(marker.read_text().strip())
time.sleep(0.05)
raise AssertionError("hook script never wrote its descendant pid")
def test_timeout_kills_descendants(tmp_path):
"""A hook timeout must take the forked descendant down with the hook."""
script, marker = _write_forking_script(tmp_path, stall_after=True)
t0 = time.monotonic()
r = _spawn(_spec(str(script), timeout=1), "{}")
elapsed = time.monotonic() - t0
assert r["timed_out"] is True
assert elapsed < 30, "timeout cleanup must not hang on held pipes"
child_pid = _read_marker(marker)
deadline = time.monotonic() + 3.0
while time.monotonic() < deadline and _pid_alive(child_pid):
time.sleep(0.05)
alive = _pid_alive(child_pid)
if alive: # cleanup so a failure doesn't leak a 300s sleeper
os.kill(child_pid, 9)
assert not alive, f"descendant {child_pid} survived hook timeout"
@pytest.mark.live_system_guard_bypass # cleanup-kills a helper reparented to init
def test_successful_hook_preserves_detached_helpers(tmp_path):
"""A hook that completes in time keeps its intentionally detached helpers.
Mirrors codex#37527's "preserve descendants on success" semantics: tree
cleanup fires only on the timeout/error path.
"""
script, marker = _write_forking_script(tmp_path, stall_after=False)
# Detach the helper's stdio so the pipes reach EOF despite the survivor.
script.write_text(
textwrap.dedent(
f"""\
#!/usr/bin/env bash
sleep 300 > /dev/null 2>&1 < /dev/null &
echo $! > {marker}
exit 0
"""
)
)
r = _spawn(_spec(str(script), timeout=10), "{}")
assert r["timed_out"] is False
assert r["returncode"] == 0
child_pid = _read_marker(marker)
time.sleep(0.3)
alive = _pid_alive(child_pid)
if alive:
os.kill(child_pid, 9)
assert alive, "successful hook's detached helper must survive"
def test_hook_child_leads_own_process_group(tmp_path):
"""The hook child must lead its own group (killpg ownership precondition)."""
script = tmp_path / "pgid.sh"
script.write_text("#!/usr/bin/env bash\necho \"$$ $(ps -o pgid= -p $$ | tr -d ' ')\"\n")
script.chmod(0o755)
r = _spawn(_spec(str(script), timeout=10), "{}")
assert r["returncode"] == 0
pid, pgid = r["stdout"].split()
assert pid == pgid, f"hook child pid={pid} does not lead its group pgid={pgid}"
assert int(pgid) != os.getpgid(0), "hook child must not share our group"
def test_fast_path_contract_unchanged(tmp_path):
"""stdin JSON delivery, stdout/stderr capture, and exit codes still work."""
script = tmp_path / "echoer.sh"
script.write_text(
"#!/usr/bin/env bash\ncat\necho errline >&2\nexit 3\n"
)
script.chmod(0o755)
r = _spawn(_spec(str(script), timeout=10), '{"tool_name": "terminal"}')
assert r["returncode"] == 3
assert r["stdout"] == '{"tool_name": "terminal"}'
assert "errline" in r["stderr"]
assert r["error"] is None
assert r["timed_out"] is False
def test_interrupt_kills_hook_and_propagates(tmp_path):
"""Ctrl+C during a hook must reap the hook, then let KeyboardInterrupt through.
The hook leads its own process group, so the terminal's SIGINT never
reaches it: only ``_spawn``'s own cleanup can stop it.
"""
marker = tmp_path / "hook.pid"
script = tmp_path / "hook.sh"
script.write_text(f'#!/usr/bin/env bash\necho $$ > "{marker}"\nsleep 300\n', encoding="utf-8")
script.chmod(0o755)
def interrupt_once_running():
"""Interrupt only once the hook is up, so the signal lands inside _spawn."""
deadline = time.monotonic() + 5.0
while time.monotonic() < deadline:
if marker.exists() and marker.read_text().strip():
os.kill(os.getpid(), signal.SIGINT)
return
time.sleep(0.05)
interrupter = threading.Thread(target=interrupt_once_running, daemon=True)
interrupter.start()
with pytest.raises(KeyboardInterrupt):
_spawn(_spec(str(script), timeout=300), "{}")
interrupter.join(timeout=5)
hook_pid = _read_marker(marker)
deadline = time.monotonic() + 3.0
while time.monotonic() < deadline and _pid_alive(hook_pid):
time.sleep(0.05)
alive = _pid_alive(hook_pid)
if alive: # cleanup so a failure doesn't leak a 300s sleeper
os.kill(hook_pid, 9)
assert not alive, f"hook {hook_pid} survived the interrupt"
def test_missing_command_still_fails_open():
r = _spawn(_spec("/nonexistent/hook-command-xyz", timeout=2), "{}")
assert r["error"] == "command not found"
assert r["returncode"] is None