Files
hermes-agent/tests/hermes_cli/test_git_probe_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

134 lines
4.5 KiB
Python

"""POSIX process-tree cleanup for ``bounded_git_probe`` (port of openai/codex#36793).
Timing out a git probe must not leave helper descendants (credential helpers,
``git-remote-https``, hook children) running after the probe returns. The
probe spawns the child in its own process group (``process_group=0``) and
``kill_process_tree`` signals the whole group with ``os.killpg`` — but
only when the child actually leads its own group, so a shared-group spawn can
never take down unrelated processes.
These tests use REAL subprocesses (no mocks): a mock cannot reproduce group
membership or survival semantics.
"""
import os
import subprocess
import sys
import textwrap
import time
import pytest
from hermes_cli import _subprocess_compat
from hermes_cli._subprocess_compat import bounded_git_probe, kill_process_tree
pytestmark = pytest.mark.platforms("posix") # POSIX process-group semantics
def _write_forking_script(tmp_path, marker_name="child.pid"):
"""A fake ``git`` that forks a long-lived descendant, then stalls."""
marker = tmp_path / marker_name
script = tmp_path / "fakegit.sh"
script.write_text(
textwrap.dedent(
f"""\
#!/usr/bin/env bash
sleep 300 &
echo $! > {marker}
sleep 300
"""
)
)
script.chmod(0o755)
return script, marker
def _pid_alive(pid: int) -> bool:
try:
os.kill(pid, 0)
except ProcessLookupError:
return False
return True
def _wait_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("forking script never wrote its descendant pid")
def test_timeout_kills_descendants(tmp_path):
"""A probe timeout must take the descendant down with the launcher."""
script, marker = _write_forking_script(tmp_path)
out = bounded_git_probe([str(script)], timeout=1.0)
assert out == ""
child_pid = _wait_marker(marker)
# Precondition sanity: the descendant existed (marker written) — now it
# must be gone shortly after the probe returned.
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 probe timeout"
def test_posix_spawn_uses_own_process_group(tmp_path):
"""The probe child must lead its own process group (killpg precondition)."""
script = tmp_path / "pgid.sh"
script.write_text("#!/usr/bin/env bash\necho \"$$ $(ps -o pgid= -p $$ | tr -d ' ')\"\n")
script.chmod(0o755)
out = bounded_git_probe([str(script)], timeout=5.0)
pid, pgid = out.split()
assert pid == pgid, f"probe child pid={pid} does not lead its group pgid={pgid}"
assert int(pgid) != os.getpgid(0), "probe child must not share our group"
def test_group_kill_skipped_when_child_shares_our_group():
"""kill_process_tree must never killpg a group the child doesn't lead.
Spawn WITHOUT process_group=0 (child inherits OUR group): the ownership
check (pgid == pid) must skip the group signal, or the test process itself
would die here.
"""
proc = subprocess.Popen(
["sleep", "60"],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
stdin=subprocess.DEVNULL,
)
assert os.getpgid(proc.pid) == os.getpgid(0) # shared group precondition
kill_process_tree(proc)
proc.wait(timeout=5)
# We are alive to make this assertion — killpg on our own group would have
# taken the test runner down. The direct child is still killed.
assert proc.returncode is not None
def test_fast_path_unaffected(tmp_path):
"""Successful probes behave exactly as before the group-kill port."""
subprocess.run(["git", "init", "-q", str(tmp_path / "repo")], check=True)
out = bounded_git_probe(
["git", "-C", str(tmp_path / "repo"), "rev-parse", "--is-inside-work-tree"],
timeout=10,
)
assert out == "true"
def test_kill_helper_swallow_all_failures():
"""Cleanup on the fail-open path must never raise, even for a reaped pid."""
class _Dead:
pid = 2**22 # extremely unlikely to exist
def kill(self):
raise OSError("already reaped")
kill_process_tree(_Dead()) # must not raise