mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
ci(process): no new detached spawn outside spawn_owned without a reason
A child started in its own session or group (start_new_session, os.setsid /
setpgid / setpgrp, windows_detach_popen_kwargs) does not die with a SIGKILLed
Hermes: it reparents to init and runs forever. That is how every family this PR
migrates leaked. With the families on spawn_owned, this keeps the class closed.
scripts/check_detached_spawns.py (AST) flags those shapes in the production trees
unless the line, or the line above it, says `detached-spawn: ok — <why>`. It runs
in scripts/check ("detached-spawns") and as a blocking lint.yml step. The baseline
is empty: every remaining site is on spawn_owned / own_process_groups, or carries
its reason. The reasons this commit adds are the census verdicts:
- must outlive the starter on purpose: restart watchers, /update and the updater
hand-offs, dashboard/desktop/service launches, cron and kanban workers
(restart-safe), the update-custody watchdog itself;
- awaited with a group kill on timeout: cron scripts, rg search, the Browser Use
CLI, update completion (holds the checkout lock fd);
- covered by its own parent guard: the compute host (compute_host._parent_guard_loop
flushes sessions; a supervisor KILL could cut that short);
- not a spawn: the wake-word `start_new_session` config key and event field.
tests/scripts/test_check_detached_spawns.py: each shape flagged, non-detached and
commented/string shapes not, the marker opts out on its line or the line above
and nowhere else, tests and the helper are out of scope, main() fails on a new
unmarked spawn, and the repository scan is empty. Teeth: appending an unmarked
start_new_session Popen to tools/file_operations_search.py turns
`scripts/check --only detached-spawns` red (rc 1); restoring it turns it green.
This commit is contained in:
1 parent
bcf275a675
commit
d1cb5d5cbd
27 files changed
+310
-8
No files matched your search
@@ -247,6 +247,12 @@ jobs:
|
||||
- name: Forbid literal /tmp paths outside the baseline
|
||||
run: python scripts/check_no_tmp_literals.py
|
||||
|
||||
# A child in its own session/group (start_new_session, os.setsid/setpgid) does not die with
|
||||
# a SIGKILLed Hermes. Long-lived children start through process_identity.spawn_owned; a line
|
||||
# that must stay detached says why with `detached-spawn: ok — <why>`.
|
||||
- name: Forbid detached spawns outside the owned-child helper
|
||||
run: python scripts/check_detached_spawns.py
|
||||
|
||||
# config.yaml is hand-edited and commented; a PyYAML dump of it destroys every comment
|
||||
# (#92554, regressed repeatedly). All writers go through the hermes_cli.config writer seam.
|
||||
- name: Forbid config.yaml writers that bypass the comment-preserving writer
|
||||
|
||||
@@ -3735,6 +3735,7 @@ def _launch_external_cron_worker(job: dict) -> bool:
|
||||
stdin=subprocess.DEVNULL,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=stderr_fd,
|
||||
# detached-spawn: ok — restart-safe cron worker, must survive a gateway restart (#101940)
|
||||
start_new_session=True,
|
||||
creationflags=windows_hide_flags(),
|
||||
)
|
||||
|
||||
@@ -459,6 +459,7 @@ def _run_job_script(
|
||||
# or stderr raises UnicodeDecodeError in communicate() and fails the whole run,
|
||||
# discarding the output (#105582; the Windows branch decodes lossily per #45099).
|
||||
popen_kwargs: dict[str, Any] = {
|
||||
# detached-spawn: ok — awaited; cancel and timeout tree-kill the group
|
||||
"start_new_session": True,
|
||||
"errors": "replace",
|
||||
}
|
||||
|
||||
@@ -1434,7 +1434,7 @@ class GatewayShutdownMixin(GatewaySessionEndMixin):
|
||||
# Break away from the parent CLI's job object or be reaped when the CLI exits; a job without
|
||||
# BREAKAWAY_OK rejects CREATE_BREAKAWAY_FROM_JOB (OSError) — retry once without the bit.
|
||||
try:
|
||||
subprocess.Popen(watcher_argv, **popen_kwargs, **windows_detach_popen_kwargs())
|
||||
subprocess.Popen(watcher_argv, **popen_kwargs, **windows_detach_popen_kwargs()) # detached-spawn: ok — restart watcher, must outlive the gateway it restarts
|
||||
except OSError:
|
||||
try:
|
||||
subprocess.Popen(
|
||||
@@ -1478,7 +1478,7 @@ class GatewayShutdownMixin(GatewaySessionEndMixin):
|
||||
argv = [setsid_bin, "bash", "-lc", shell_cmd] if setsid_bin else ["bash", "-lc", shell_cmd]
|
||||
subprocess.Popen( # noqa: ASYNC220 -- fire-and-forget spawn; Popen returns immediately and callers need the Popen object
|
||||
argv, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
|
||||
env=GatewayShutdownMixin._restart_watcher_env(), start_new_session=True,
|
||||
env=GatewayShutdownMixin._restart_watcher_env(), start_new_session=True, # detached-spawn: ok — restart watcher, must outlive the gateway it restarts
|
||||
)
|
||||
|
||||
def _wedged_agent_count(self) -> int:
|
||||
|
||||
@@ -160,6 +160,7 @@ def spawn_async_diagnostic(log_path: Path, signal_name: str, *,
|
||||
try: # start_new_session: outlive systemd killing our cgroup (KillMode=control-group) to flush
|
||||
return subprocess.Popen(
|
||||
[*bound, "bash", "-c", script], stdout=fd,
|
||||
# detached-spawn: ok — forensics dump runs while the gateway dies; bounded by timeout(1)
|
||||
stderr=subprocess.STDOUT, stdin=subprocess.DEVNULL, start_new_session=True,
|
||||
close_fds=True).pid
|
||||
except OSError:
|
||||
|
||||
@@ -139,6 +139,7 @@ def _spawn_detached_update(hermes_cmd, output_path, exit_code_path) -> None:
|
||||
subprocess.Popen(
|
||||
[sys.executable, "-c", _WINDOWS_UPDATE_HELPER, str(output_path), str(exit_code_path),
|
||||
sys.executable, "-m", "hermes_cli.main", "update", "--gateway"],
|
||||
# detached-spawn: ok — /update runs `hermes update`, which must outlive the gateway it restarts
|
||||
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, **windows_detach_popen_kwargs())
|
||||
return
|
||||
hermes_cmd_str = " ".join(shlex.quote(part) for part in hermes_cmd)
|
||||
@@ -154,6 +155,7 @@ def _spawn_detached_update(hermes_cmd, output_path, exit_code_path) -> None:
|
||||
# calls os.setsid() in the child.
|
||||
setsid_bin = shutil.which("setsid")
|
||||
argv = [setsid_bin, "bash", "-c", update_cmd] if setsid_bin else ["bash", "-c", update_cmd]
|
||||
# detached-spawn: ok — /update runs `hermes update`, which must outlive the gateway it restarts
|
||||
subprocess.Popen(argv, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, start_new_session=True)
|
||||
|
||||
|
||||
|
||||
@@ -271,6 +271,7 @@ def windows_detach_popen_kwargs() -> dict:
|
||||
"""
|
||||
if IS_WINDOWS:
|
||||
return {"creationflags": windows_detach_flags()}
|
||||
# detached-spawn: ok — the detach helper itself; every call site is checked on its own
|
||||
return {"start_new_session": True}
|
||||
|
||||
|
||||
|
||||
@@ -174,6 +174,7 @@ def launch_detached(
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
close_fds=True,
|
||||
# detached-spawn: ok — the desktop app launch, meant to outlive the CLI that opens it
|
||||
**windows_detach_popen_kwargs(),
|
||||
)
|
||||
return child.pid
|
||||
@@ -1261,7 +1261,7 @@ DEFAULT_CONFIG = {
|
||||
# openWakeWord/pyopen-wakeword only: consecutive over-threshold frames to fire (higher = fewer
|
||||
# false triggers, more latency; 1 = single-frame)
|
||||
"confirmation_frames": 3,
|
||||
"start_new_session": True, # fresh session on wake vs. continue the current one
|
||||
"start_new_session": True, # fresh session on wake vs. continue the current one (detached-spawn: ok — a config key)
|
||||
# sherpa only: listen for every wake-enabled profile's phrase and route to it
|
||||
"profile_routing": True,
|
||||
"openwakeword": {
|
||||
|
||||
@@ -1177,13 +1177,13 @@ def _spawn_gateway_restart_watcher(old_pid: int, run_argv: list[str], *, host: b
|
||||
popen_env = {"env": watcher_env} if watcher_env is not None else {}
|
||||
# Same detach for the watcher itself, so closing the terminal doesn't kill it.
|
||||
try:
|
||||
subprocess.Popen(watcher_argv, **devnull, **popen_env, **windows_detach_popen_kwargs())
|
||||
subprocess.Popen(watcher_argv, **devnull, **popen_env, **windows_detach_popen_kwargs()) # detached-spawn: ok — restart watcher, must outlive the process that asked for the restart
|
||||
except OSError:
|
||||
# Parent job object rejected CREATE_BREAKAWAY_FROM_JOB; retry without it (Windows only —
|
||||
# ``start_new_session=True`` cannot raise OSError on POSIX).
|
||||
fallback_kwargs: dict = (
|
||||
{"creationflags": windows_detach_flags_without_breakaway()} if sys.platform == "win32"
|
||||
else {"start_new_session": True}
|
||||
else {"start_new_session": True} # detached-spawn: ok — restart watcher, must outlive the process that asked for the restart
|
||||
)
|
||||
try:
|
||||
subprocess.Popen(watcher_argv, **devnull, **popen_env, **fallback_kwargs)
|
||||
|
||||
@@ -309,6 +309,7 @@ def _spawn_detached_gateway() -> bool:
|
||||
_timestamped_stderr_gateway_command(log_dir / "gateway.error.log"),
|
||||
stdin=subprocess.DEVNULL, stdout=out, stderr=subprocess.DEVNULL,
|
||||
env=child_env,
|
||||
# detached-spawn: ok — foreground-less gateway launch, a service meant to outlive the CLI
|
||||
**windows_detach_popen_kwargs(),
|
||||
)
|
||||
except OSError:
|
||||
|
||||
@@ -2882,7 +2882,7 @@ def _default_spawn(task: Task, workspace: str, *, board: Optional[str] = None) -
|
||||
stdout=log_f,
|
||||
stderr=subprocess.STDOUT,
|
||||
env=env,
|
||||
start_new_session=True,
|
||||
start_new_session=True, # detached-spawn: ok — restart-safe kanban worker, must survive the dispatcher
|
||||
creationflags=subprocess.CREATE_NO_WINDOW if _kb._IS_WINDOWS else 0,
|
||||
)
|
||||
except FileNotFoundError:
|
||||
|
||||
@@ -443,6 +443,7 @@ def _respawn_dashboard_processes(commands: list[list[str]]) -> list[list[str]]:
|
||||
with open(log_path, "ab") as log_f:
|
||||
proc = subprocess.Popen(
|
||||
command, stdin=subprocess.DEVNULL, stdout=log_f, stderr=subprocess.STDOUT,
|
||||
# detached-spawn: ok — dashboard service launch, meant to outlive the CLI that starts it
|
||||
start_new_session=True, close_fds=True)
|
||||
spawned.append((original, command, proc))
|
||||
except (OSError, ValueError) as exc:
|
||||
|
||||
@@ -543,6 +543,7 @@ def _relaunch_bare(entry: dict) -> int:
|
||||
env.pop(leaked, None)
|
||||
with open(logs / "gateway-update-resume.log", "ab") as log:
|
||||
proc = subprocess.Popen(list(entry["argv"]), env=env, stdin=subprocess.DEVNULL, stdout=log,
|
||||
# detached-spawn: ok — resumes the paused gateway after an update; it must outlive the updater
|
||||
stderr=subprocess.STDOUT, start_new_session=True, close_fds=True)
|
||||
deadline = time.monotonic() + _READY_TIMEOUT_S
|
||||
while time.monotonic() < deadline:
|
||||
|
||||
@@ -91,6 +91,7 @@ def run_completion(request: dict) -> dict:
|
||||
# starts runs outside the job), so the lock is never free while it runs.
|
||||
proc = subprocess.Popen(
|
||||
command, cwd=root, env=env, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
|
||||
# detached-spawn: ok — awaited; holds the checkout lock fd so an update finishes atomically even if this parent dies
|
||||
**({"start_new_session": True, "pass_fds": checkout_lock_fds(root)} if os.name == "posix" else
|
||||
{"creationflags": subprocess.CREATE_NO_WINDOW | CREATE_SUSPENDED}))
|
||||
decoder = codecs.getincrementaldecoder("utf-8")("replace")
|
||||
|
||||
@@ -362,6 +362,7 @@ def _owner_watch(pid: int):
|
||||
read_end, write_end = os.pipe() # non-inheritable: no other child keeps the owner's end open
|
||||
try:
|
||||
watcher = subprocess.Popen([sys.executable, "-I", "-S", "-c", _OWNER_WATCH, str(pid)], stdin=read_end,
|
||||
# detached-spawn: ok — the owner-death watchdog itself: it must outlive the owner to kill the fetch
|
||||
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, start_new_session=True)
|
||||
except OSError as exc:
|
||||
os.close(write_end)
|
||||
|
||||
@@ -131,6 +131,7 @@ def ensure_panel(update_root: Path) -> None:
|
||||
subprocess.Popen(
|
||||
["/usr/bin/osascript", "-l", "JavaScript", str(panel), str(status_file())],
|
||||
stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL,
|
||||
# detached-spawn: ok — update progress panel, self-exits on the terminal state
|
||||
stderr=subprocess.DEVNULL, start_new_session=True,
|
||||
)
|
||||
os.environ[UI_SPAWNED_ENV] = "1" # inherited: never spawn twice per chain
|
||||
|
||||
@@ -543,6 +543,7 @@ def _spawn_hermes_action(
|
||||
)
|
||||
proc = _popen(creationflags=windows_detach_flags_without_breakaway())
|
||||
else:
|
||||
# detached-spawn: ok — dashboard gateway start/restart actions, which must outlive the dashboard
|
||||
proc = _popen(start_new_session=True)
|
||||
except OSError as exc:
|
||||
# Without this the action log would hold only the started-header for a spawn that never
|
||||
|
||||
@@ -105,6 +105,7 @@ _GUARDS = {
|
||||
"footguns": ["scripts/check-windows-footguns.py", "--all"],
|
||||
"shebangs": ["scripts/check_bash_shebangs.py"],
|
||||
"no-tmp": ["scripts/check_no_tmp_literals.py"],
|
||||
"detached-spawns": ["scripts/check_detached_spawns.py"],
|
||||
"config-writers": ["scripts/check_config_yaml_writers.py"],
|
||||
"config-readers": ["scripts/check_config_yaml_readers.py"],
|
||||
"os-marker-fakes": ["scripts/ci/check_os_marker_fakes.py"],
|
||||
|
||||
@@ -0,0 +1,184 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Fail when production code starts a detached child outside the owned-child helper.
|
||||
|
||||
A child started in its own session/process group (``start_new_session=True``, ``os.setsid``,
|
||||
``os.setpgid``/``os.setpgrp`` in a ``preexec_fn``) no longer dies with the Hermes process that
|
||||
started it. When that process is SIGKILLed, OOM-killed or crashes, the child reparents to init
|
||||
(or a ``systemd --user`` subreaper) and runs forever. Every long-lived child that should die with
|
||||
its owner goes through ``hermes_cli.process_identity.spawn_owned`` (or hands its group to
|
||||
``own_process_groups`` where something else spawned it), which registers the group with the
|
||||
per-process owner-death supervisor.
|
||||
|
||||
Flags, in Python files of the production trees (tests, evals, scripts and nix checks excluded):
|
||||
|
||||
start_new_session=True / start_new_session=<anything but False>, or a ``"start_new_session"``
|
||||
dict key with a non-False value
|
||||
any reference to os.setsid / os.setpgid / os.setpgrp (call or ``preexec_fn=os.setsid``)
|
||||
a call to ``windows_detach_popen_kwargs()`` (``start_new_session=True`` on POSIX)
|
||||
|
||||
Opt out of one line with ``detached-spawn: ok — <why>`` on that line or the line directly above
|
||||
it. Legitimate reasons are the ones that stay valid after the owner dies: the child must OUTLIVE
|
||||
its owner on purpose (an updater relaunching the gateway, a restart watcher, a detached service
|
||||
launch), or the child is short-lived and awaited with a hard timeout that kills its group (the
|
||||
group exists only so that kill reaches grandchildren).
|
||||
|
||||
``_BASELINE`` maps a file to a tolerated count of unmarked hits. It is empty: every detached
|
||||
spawn site is on ``spawn_owned`` or marked with its reason. Keep it that way; an entry is a
|
||||
temporary burn-down, and an overstated one is an advisory (``--strict-baseline`` fails on it).
|
||||
|
||||
Run: python scripts/check_detached_spawns.py [--all] [--print-baseline] [--strict-baseline] [paths...]
|
||||
Exit 1 on any violation, 0 when clean.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import ast
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
MARKER = "detached-spawn: ok"
|
||||
# The helper itself: the one place a detached spawn is the point.
|
||||
HELPER_FILES = {"hermes_cli/process_identity.py"}
|
||||
SCAN_TREES = ("agent", "cron", "gateway", "hermes_cli", "plugins", "tools", "tui_gateway", "acp_adapter", "pm")
|
||||
SCAN_ROOT_FILES = ("cli.py", "run_agent.py", "hermes_state.py", "utils.py", "hermes_constants.py")
|
||||
SKIP_PARTS = {"tests", "__pycache__", "evals", "node_modules"}
|
||||
_DETACH_OS_ATTRS = {"setsid", "setpgid", "setpgrp"}
|
||||
# Helpers that return detaching Popen kwargs (``start_new_session=True`` on POSIX).
|
||||
_DETACH_HELPERS = {"windows_detach_popen_kwargs"}
|
||||
# Cheap text prefilter: only files mentioning one of these are parsed.
|
||||
_NEEDLES = ("start_new_session", "setsid", "setpgid", "setpgrp", *_DETACH_HELPERS)
|
||||
|
||||
|
||||
def _is_false(node: ast.AST) -> bool:
|
||||
return isinstance(node, ast.Constant) and node.value in (False, None, 0)
|
||||
|
||||
|
||||
def _is_os_detach(node: ast.AST) -> bool:
|
||||
return (isinstance(node, ast.Attribute) and node.attr in _DETACH_OS_ATTRS
|
||||
and isinstance(node.value, ast.Name) and node.value.id == "os")
|
||||
|
||||
|
||||
def _is_detach_helper_call(node: ast.AST) -> bool:
|
||||
return isinstance(node, ast.Call) and (getattr(node.func, "id", None) in _DETACH_HELPERS
|
||||
or getattr(node.func, "attr", None) in _DETACH_HELPERS)
|
||||
|
||||
|
||||
def detach_lines(text: str) -> list[int]:
|
||||
"""Line numbers of detached-spawn shapes in ``text`` (a Python module)."""
|
||||
if not any(needle in text for needle in _NEEDLES):
|
||||
return []
|
||||
try:
|
||||
tree = ast.parse(text)
|
||||
except (SyntaxError, ValueError):
|
||||
return []
|
||||
lines: set[int] = set()
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, ast.keyword) and node.arg == "start_new_session" and not _is_false(node.value):
|
||||
lines.add(node.value.lineno)
|
||||
elif isinstance(node, ast.Dict):
|
||||
for key, value in zip(node.keys, node.values):
|
||||
if isinstance(key, ast.Constant) and key.value == "start_new_session" and not _is_false(value):
|
||||
lines.add(key.lineno)
|
||||
elif _is_os_detach(node) or _is_detach_helper_call(node):
|
||||
lines.add(node.lineno)
|
||||
return sorted(lines)
|
||||
|
||||
|
||||
def _marked(src_lines: list[str], lineno: int) -> bool:
|
||||
here = src_lines[lineno - 1] if lineno - 1 < len(src_lines) else ""
|
||||
above = src_lines[lineno - 2] if lineno >= 2 else ""
|
||||
return MARKER in here or MARKER in above
|
||||
|
||||
|
||||
def _tracked_files(root: Path) -> list[Path]:
|
||||
try:
|
||||
out = subprocess.run(["git", "-C", str(root), "ls-files", "-z", "--", *SCAN_TREES, *SCAN_ROOT_FILES],
|
||||
capture_output=True, text=True, check=True, stdin=subprocess.DEVNULL, timeout=60).stdout
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
return []
|
||||
return [root / rel for rel in out.split("\0") if rel.endswith(".py")]
|
||||
|
||||
|
||||
def _in_scope(rel: str) -> bool:
|
||||
parts = Path(rel).parts
|
||||
return not (SKIP_PARTS & set(parts)) and not Path(rel).name.startswith("test_") and rel not in HELPER_FILES
|
||||
|
||||
|
||||
def scan(paths=None, root: Path | None = None) -> dict[str, list[tuple[int, str]]]:
|
||||
"""Repo-relative path -> [(lineno, line)] of unmarked detached spawns."""
|
||||
root = (root or ROOT).resolve()
|
||||
hits: dict[str, list[tuple[int, str]]] = {}
|
||||
for path in (list(paths) if paths else _tracked_files(root)):
|
||||
path = Path(path).resolve()
|
||||
try:
|
||||
rel = path.relative_to(root).as_posix()
|
||||
except ValueError:
|
||||
rel = str(path)
|
||||
if path.suffix != ".py" or not _in_scope(rel):
|
||||
continue
|
||||
try:
|
||||
text = path.read_text(encoding="utf-8-sig", errors="ignore")
|
||||
except OSError:
|
||||
continue
|
||||
src = text.splitlines()
|
||||
found = [(n, src[n - 1]) for n in detach_lines(text) if not _marked(src, n)]
|
||||
if found:
|
||||
hits[rel] = found
|
||||
return hits
|
||||
|
||||
|
||||
# Files that spawned unmarked detached children when this check landed. Burn-down list:
|
||||
# migrate to spawn_owned / own_process_groups, or mark the line with a reason, then drop the entry.
|
||||
_BASELINE: dict[str, int] = {
|
||||
}
|
||||
|
||||
|
||||
def _format_baseline(hits: dict[str, list]) -> str:
|
||||
body = "".join(f' "{rel}": {len(found)},\n' for rel, found in sorted(hits.items()))
|
||||
return "_BASELINE: dict[str, int] = {\n" + body + "}"
|
||||
|
||||
|
||||
def main(argv: list[str] | None = None) -> int:
|
||||
ap = argparse.ArgumentParser(description=(__doc__ or "").splitlines()[0])
|
||||
ap.add_argument("paths", nargs="*", help="files to check (default: the production trees)")
|
||||
ap.add_argument("--all", action="store_true", help="ignore _BASELINE; report every hit")
|
||||
ap.add_argument("--print-baseline", action="store_true", help="print a _BASELINE for the current tree")
|
||||
ap.add_argument("--strict-baseline", action="store_true", help="also fail when _BASELINE overstates a file")
|
||||
args = ap.parse_args(argv)
|
||||
|
||||
hits = scan(args.paths or None)
|
||||
if args.print_baseline:
|
||||
print(_format_baseline(hits))
|
||||
return 0
|
||||
baseline = {} if (args.all or args.paths) else _BASELINE
|
||||
problems: list[str] = []
|
||||
advisories: list[str] = []
|
||||
for rel in sorted(hits):
|
||||
allowed = baseline.get(rel)
|
||||
if allowed is not None and len(hits[rel]) <= allowed:
|
||||
if len(hits[rel]) < allowed:
|
||||
advisories.append(f"{rel}: {len(hits[rel])} detached spawn(s) left, _BASELINE says {allowed}")
|
||||
continue
|
||||
problems.extend(f"{rel}:{n}: {line.strip()[:160]}" for n, line in hits[rel])
|
||||
advisories.extend(f"{rel}: listed in _BASELINE but clean (or gone)" for rel in sorted(baseline) if rel not in hits)
|
||||
if advisories:
|
||||
print("advisory — _BASELINE in scripts/check_detached_spawns.py is stale; regenerate it with "
|
||||
"--print-baseline (fewer hits than listed is progress, not a failure):")
|
||||
print("\n".join(" " + a for a in advisories))
|
||||
if args.strict_baseline:
|
||||
problems.extend(advisories)
|
||||
if not problems:
|
||||
print("no detached spawns outside the owned-child helper and the baseline")
|
||||
return 0
|
||||
print("\n".join(problems))
|
||||
print(f"\n{len(problems)} detached spawn(s) flagged. A child in its own session/group outlives a "
|
||||
"SIGKILLed Hermes. Start long-lived children with hermes_cli.process_identity.spawn_owned "
|
||||
"(or hand the group to own_process_groups), or mark a child that must outlive its owner / "
|
||||
f"is awaited with a group-killing timeout with `{MARKER} — <why>` (same line or the line above).")
|
||||
return 1
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
@@ -0,0 +1,92 @@
|
||||
"""scripts/check_detached_spawns.py: detached spawns outside spawn_owned are flagged unless marked."""
|
||||
|
||||
import importlib.util
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
SCRIPT = Path(__file__).resolve().parents[2] / "scripts" / "check_detached_spawns.py"
|
||||
|
||||
|
||||
def _load():
|
||||
spec = importlib.util.spec_from_file_location("check_detached_spawns", SCRIPT)
|
||||
mod = importlib.util.module_from_spec(spec)
|
||||
spec.loader.exec_module(mod)
|
||||
return mod
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"src",
|
||||
[
|
||||
"subprocess.Popen(argv, start_new_session=True)",
|
||||
"subprocess.Popen(argv, start_new_session=flag)",
|
||||
'kwargs = {"start_new_session": True}',
|
||||
"subprocess.Popen(argv, preexec_fn=os.setsid)",
|
||||
"subprocess.Popen(argv, preexec_fn=lambda: os.setpgid(0, 0))",
|
||||
"os.setpgrp()",
|
||||
"subprocess.Popen(argv, **windows_detach_popen_kwargs())",
|
||||
"subprocess.Popen(argv, **_subprocess_compat.windows_detach_popen_kwargs())",
|
||||
],
|
||||
)
|
||||
def test_detached_shapes_are_flagged(src):
|
||||
assert _load().detach_lines(src) == [1]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"src",
|
||||
[
|
||||
"subprocess.Popen(argv)",
|
||||
"subprocess.Popen(argv, start_new_session=False)",
|
||||
'kwargs = {"start_new_session": False}',
|
||||
"spawn_owned(argv, purpose='x')",
|
||||
"# start_new_session=True in a comment",
|
||||
'doc = "os.setsid in a string"',
|
||||
],
|
||||
)
|
||||
def test_non_detached_shapes_are_not_flagged(src):
|
||||
assert _load().detach_lines(src) == []
|
||||
|
||||
|
||||
def test_marker_on_the_line_or_above_opts_out_and_nothing_else_does(tmp_path):
|
||||
mod = _load()
|
||||
pkg = tmp_path / "tools"
|
||||
pkg.mkdir()
|
||||
f = pkg / "x.py"
|
||||
f.write_text(textwrap.dedent("""\
|
||||
import os, subprocess
|
||||
subprocess.Popen(a, start_new_session=True) # detached-spawn: ok — awaited, group-killed
|
||||
# detached-spawn: ok — outlives its owner on purpose
|
||||
subprocess.Popen(b, start_new_session=True)
|
||||
# an unrelated comment
|
||||
subprocess.Popen(c, start_new_session=True)
|
||||
"""), encoding="utf-8")
|
||||
hits = mod.scan([f], root=tmp_path)
|
||||
assert [n for n, _ in hits["tools/x.py"]] == [6]
|
||||
|
||||
|
||||
def test_tests_and_the_helper_are_out_of_scope(tmp_path):
|
||||
mod = _load()
|
||||
for rel in ("tests/tools/test_x.py", "tools/test_y.py", "hermes_cli/process_identity.py"):
|
||||
path = tmp_path / rel
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text("subprocess.Popen(a, start_new_session=True)\n", encoding="utf-8")
|
||||
assert mod.scan(list(tmp_path.rglob("*.py")), root=tmp_path) == {}
|
||||
|
||||
|
||||
def test_a_new_unmarked_spawn_fails_main_and_a_marked_one_passes(tmp_path, monkeypatch, capsys):
|
||||
mod = _load()
|
||||
f = tmp_path / "tools" / "new.py"
|
||||
f.parent.mkdir()
|
||||
f.write_text("subprocess.Popen(a, start_new_session=True)\n", encoding="utf-8")
|
||||
monkeypatch.setattr(mod, "ROOT", tmp_path)
|
||||
assert mod.main([str(f)]) == 1
|
||||
assert "tools/new.py:1" in capsys.readouterr().out
|
||||
f.write_text("subprocess.Popen(a, start_new_session=True) # detached-spawn: ok — test\n", encoding="utf-8")
|
||||
assert mod.main([str(f)]) == 0
|
||||
|
||||
|
||||
def test_the_repository_has_no_unmarked_detached_spawn():
|
||||
mod = _load()
|
||||
assert mod.scan() == {}, "a detached spawn outside spawn_owned lacks a `detached-spawn: ok — <why>` marker"
|
||||
assert mod._BASELINE == {}
|
||||
@@ -24,8 +24,8 @@ from scripts.code_health.config import ENFORCEMENT
|
||||
REPO = Path(__file__).resolve().parents[2]
|
||||
_GUARD_SCRIPTS = (
|
||||
"scripts/check-windows-footguns.py", "scripts/check_bash_shebangs.py",
|
||||
"scripts/check_no_tmp_literals.py", "scripts/check_config_yaml_writers.py",
|
||||
"scripts/check_config_yaml_readers.py",
|
||||
"scripts/check_no_tmp_literals.py", "scripts/check_detached_spawns.py",
|
||||
"scripts/check_config_yaml_writers.py", "scripts/check_config_yaml_readers.py",
|
||||
"scripts/ci/check_os_marker_fakes.py", "scripts/check-case-collisions.py",
|
||||
"scripts/ci/check_lazy_deps_imports.py", "scripts/ci/check_profile_archive_boundary.py",
|
||||
"scripts/ci/check_agents_md_size.py", "scripts/ci/check_blob_size.py",
|
||||
|
||||
@@ -556,6 +556,7 @@ def _group_popen_kwargs() -> dict:
|
||||
si.dwFlags |= subprocess.STARTF_USESHOWWINDOW
|
||||
return {"creationflags": windows_hide_flags() | getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0),
|
||||
"startupinfo": si}
|
||||
# detached-spawn: ok — awaited CLI; a timeout kills its whole group (_run_cli_killing_process_group)
|
||||
return _quiet(_flags, {}, "Windows hide-flags unavailable") if os.name == "nt" else {"start_new_session": True}
|
||||
|
||||
|
||||
|
||||
@@ -613,6 +613,7 @@ class SearchMixin:
|
||||
proc = subprocess.Popen(
|
||||
args, cwd=cwd, env=_make_run_env(self.env.env), stdin=subprocess.DEVNULL,
|
||||
stdout=subprocess.PIPE, stderr=subprocess.PIPE if merge_stderr else subprocess.DEVNULL,
|
||||
# detached-spawn: ok — awaited rg under a deadline; the group is killed on timeout or interrupt
|
||||
start_new_session=True)
|
||||
except OSError as exc:
|
||||
return ExecuteResult(stdout=f"rg: {exc}", exit_code=2)
|
||||
|
||||
@@ -77,6 +77,7 @@ class WakeWordInUse(RuntimeError):
|
||||
_DEFAULTS: dict[str, Any] = {
|
||||
"enabled": False, "surface": "auto", "input_device": None, "capture": "auto",
|
||||
"provider": "auto", "phrase": "hey hermes", "sensitivity": 0.6,
|
||||
# detached-spawn: ok — not a spawn: the wake-word config key (fresh chat session on wake)
|
||||
"confirmation_frames": _DEFAULT_CONFIRMATION_FRAMES, "start_new_session": True,
|
||||
}
|
||||
|
||||
|
||||
@@ -351,6 +351,7 @@ class HostSupervisor:
|
||||
proc = subprocess.Popen(
|
||||
self.argv, cwd=str(self.cwd), env=env, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE, text=True, encoding="utf-8", errors="replace", bufsize=1,
|
||||
# detached-spawn: ok — compute host exits on its own parent-guard (compute_host._parent_guard_loop) and flushes sessions; a supervisor KILL could cut that flush short
|
||||
start_new_session=True)
|
||||
self._proc = proc
|
||||
for target, name in ((self._drain_stdout, "compute-host-stdout"),
|
||||
|
||||
@@ -504,6 +504,7 @@ def _wake_detect_handler(transport, sid: str, phrase: str, new_session: bool):
|
||||
try:
|
||||
_emit("wake.detected", sid, {
|
||||
"phrase": matched_phrase or phrase, "profile": matched_profile or None,
|
||||
# detached-spawn: ok — not a spawn: a wake.detected event field
|
||||
"start_new_session": new_session})
|
||||
finally:
|
||||
reset_transport(token)
|
||||
|
||||
Reference in new issue
Block a user