fix(update): the legacy start_watchdog starts the monitor in its own process group; tests clean up only the processes they started (Issue #72, review round 5)
With MAM_DELEGATION_PY=0 the monitor was left in the caller's process group (decision 2), on every restart the update makes. The legacy body now spawns it under job control (`set -m`, macOS has no setsid(1)) and restores the caller's option. The update tests assert the restarted monitor's PGID in both modes, and their teardown kills helper groups and this test's exact monitors instead of `pkill -f <ws>`, then reports anything left that names the workspace. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1d63e882bc
commit
ec7f5c4e57
2 files changed
+39
-4
No files matched your search
@@ -2297,10 +2297,16 @@ _start_watchdog_legacy() {
|
||||
elif [ -z "$pid" ]; then
|
||||
# Start the wildcard monitor subscriber daemon with --idle-timeout 0 (never idle out)
|
||||
# and ensure it runs with $workdir as cwd to anchor relative log paths.
|
||||
local orig_pwd="$PWD"
|
||||
# Issue #72 (decision 2): job control (`set -m`) starts it in its own
|
||||
# process group, out of the caller's, as the python body's new session
|
||||
# does; macOS has no setsid(1). The caller's option is restored.
|
||||
local orig_pwd="$PWD" had_m=0
|
||||
case $- in *m*) had_m=1 ;; esac
|
||||
cd "$workdir" || return 1
|
||||
set -m
|
||||
nohup bash "$monitor_script" --subscribe --idle-timeout 0 </dev/null >> "$log_file" 2>&1 &
|
||||
pid=$!
|
||||
[ "$had_m" = 1 ] || set +m
|
||||
cd "$orig_pwd" || return 1
|
||||
fi
|
||||
|
||||
|
||||
@@ -181,6 +181,18 @@ def _exact_monitor_pids(script) -> list:
|
||||
return pids
|
||||
|
||||
|
||||
def _descendants(pid) -> list:
|
||||
kids = subprocess.run(["pgrep", "-P", str(pid)], capture_output=True, text=True).stdout.split()
|
||||
return [d for k in kids for d in [k] + _descendants(k)]
|
||||
|
||||
|
||||
def _processes_naming(path) -> list:
|
||||
"""Live processes whose command line names PATH (read only, to report leftovers)."""
|
||||
out = subprocess.run(["ps", "-A", "-ww", "-o", "pid=", "-o", "args="], capture_output=True, text=True).stdout
|
||||
return [line.strip() for line in out.splitlines()
|
||||
if str(path) in line and line.split(None, 1)[0] != str(os.getpid())]
|
||||
|
||||
|
||||
def _lock_held(path: Path) -> bool:
|
||||
if not path.exists():
|
||||
return False
|
||||
@@ -206,12 +218,26 @@ def box(template, tmp_path):
|
||||
try:
|
||||
yield b
|
||||
finally:
|
||||
# Only what this test started: its helpers' process groups, and the
|
||||
# monitors whose exact command line is this test's own workspace
|
||||
# (detached, so in a group of their own), with their children. No
|
||||
# pattern kill: the host runs other workspaces' monitors.
|
||||
for p in b.helpers:
|
||||
try:
|
||||
os.killpg(p.pid, signal.SIGKILL)
|
||||
except OSError:
|
||||
pass
|
||||
subprocess.run(["pkill", "-KILL", "-f", str(ws)], capture_output=True)
|
||||
p.wait()
|
||||
for pid in _exact_monitor_pids(ws / MONITOR_REL):
|
||||
for q in [pid] + _descendants(pid):
|
||||
try:
|
||||
os.kill(int(q), signal.SIGKILL)
|
||||
except OSError:
|
||||
pass
|
||||
try:
|
||||
_until(lambda: not _processes_naming(ws), timeout=10)
|
||||
except AssertionError:
|
||||
raise AssertionError("processes of this test outlived it: %s" % _processes_naming(ws))
|
||||
|
||||
|
||||
# --- helpers ---------------------------------------------------------------------
|
||||
@@ -673,8 +699,9 @@ def test_only_this_workspaces_exact_monitor_command_line_is_its_monitor(tmp_path
|
||||
def test_every_restart_starts_a_real_monitor_in_both_delegation_modes(box, mode, case):
|
||||
"""MAM_DELEGATION_PY=0 keeps the legacy start_watchdog, the switch the
|
||||
delegate skill documents. Its restart, after an install, after a stop
|
||||
timeout or after a rollback, must start this workspace's monitor again and
|
||||
not report a process that only names the monitor path in an argument."""
|
||||
timeout or after a rollback, must start this workspace's monitor again, out
|
||||
of the caller's process group (decision 2), and not report a process that
|
||||
only names the monitor path in an argument."""
|
||||
env = {"MAM_DELEGATION_PY": mode}
|
||||
box.stub_monitor()
|
||||
box.start_monitor(env=dict(env, STUB_MONITOR_IGNORE_TERM="1") if case == "killed" else env)
|
||||
@@ -693,6 +720,8 @@ def test_every_restart_starts_a_real_monitor_in_both_delegation_modes(box, mode,
|
||||
assert "PID %s)" % decoy.pid not in res.stdout, "the decoy is reported as the monitor"
|
||||
_until(lambda: bool(box.monitor_pids()))
|
||||
assert not (set(box.monitor_pids()) & first) and box.monitor_starts() == 2
|
||||
# box.update runs update.sh in this test's process group: the caller's.
|
||||
assert all(os.getpgid(int(p)) != os.getpgrp() for p in box.monitor_pids()), "left in the caller's group"
|
||||
assert decoy.poll() is None, "the decoy was not signalled"
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user