From 1d63e882bc1c46e9af8484647d5091b296d8fa3f Mon Sep 17 00:00:00 2001 From: Godopu Date: Wed, 28 Oct 2026 04:10:15 +0900 Subject: [PATCH] fix(update): the legacy start_watchdog matches the monitor exactly too; a restart is reported only when the monitor runs (Issue #72, review round 4) With MAM_DELEGATION_PY=0 the legacy helper still found the monitor with an unanchored pgrep -f, so a process that only named the monitor path was reported as the restarted monitor and none was started. lib.sh _monitor_pids is the shell twin of daemon.find_running; update.sh and post_update.sh check the PID start_watchdog printed with mam_monitor_started before saying "monitor: restarted". Co-Authored-By: Claude Opus 5.5 --- .agents/skills/lib.sh | 37 +++++++++- .agents/skills/lib_py/delegation/daemon.py | 4 +- .../scripts/post_update.sh | 3 +- .../multi-agent-mux-update/scripts/update.sh | 2 + tests/test_issue72_update.py | 69 ++++++++++++++++--- 5 files changed, 103 insertions(+), 12 deletions(-) diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index 7b862ff..8f8375c 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -2235,6 +2235,41 @@ _delegate_publish_event_legacy() { "$py_bin" "$pub" --job "$job_id" --event "$event" --detail "$detail" ${extra[@]+"${extra[@]}"} || true } +# _monitor_pids +# Issue #72: PIDs whose command line is exactly ` +# --subscribe[ ...]` - the shell twin of lib_py.delegation.daemon.find_running, +# so both start_watchdog branches recognise the same daemons. Not a `pgrep -f` +# regex: that also matched another workspace's daemon (an unescaped `.`) and a +# process that only names the path in an argument. +_monitor_pids() { + ps -A -ww -o pid= -o args= | MAM_MONITOR_WANT="$1 --subscribe" awk ' + { line = $0; sub(/^ +/, "", line) + i = index(line, " "); if (!i) next + pid = substr(line, 1, i - 1); args = substr(line, i + 1) + j = index(args, " "); if (!j) next + n = split(substr(args, 1, j - 1), a0, "/"); rest = substr(args, j + 1) + want = ENVIRON["MAM_MONITOR_WANT"] + if (a0[n] == "bash" && (rest == want || index(rest, want " ") == 1)) print pid }' +} + +# mam_monitor_started +# Issue #72: 0 once one of the PIDs start_watchdog printed is WORKSPACE's +# monitor (_monitor_pids), so a restart is only reported when it happened. +# Polled for up to 5 s: a daemon just started from the shell carries its +# parent's command line until it execs bash. +mam_monitor_started() { + local script="$1/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh" n=0 p + shift + while [ "$n" -lt 50 ]; do + for p in $(_monitor_pids "$script"); do + case " $* " in *" $p "*) return 0 ;; esac + done + sleep 0.1 + n=$((n + 1)) + done + return 1 +} + # start_watchdog [workdir] # Spawns a watchdog process to monitor a delegate-job JOB in the background. # The watchdog re-spawns the subscriber every 2 minutes (or whatever hard @@ -2253,7 +2288,7 @@ _start_watchdog_legacy() { # Check if reconcile.sh --subscribe is already running on this workspace local pid - pid=$(pgrep -f "bash $monitor_script --subscribe" || true) + pid=$(_monitor_pids "$monitor_script" || true) # Issue #19: MAM_NO_SUBSCRIBER_DAEMON=1 reports a running daemon but never # starts one (test sandboxes; externally supervised monitors). diff --git a/.agents/skills/lib_py/delegation/daemon.py b/.agents/skills/lib_py/delegation/daemon.py index 96e51de..f84f1b2 100644 --- a/.agents/skills/lib_py/delegation/daemon.py +++ b/.agents/skills/lib_py/delegation/daemon.py @@ -42,8 +42,8 @@ def find_running(monitor_script: str) -> List[str]: A subscriber is a process whose command line is ` --subscribe[ ...]`, argv0 a bash, as both this module and the shell helper - start it. Issue #72: matched exactly, not as the shell helper's `pgrep -f` - regex, because the update skill signals what this returns - a daemon of + start it. Issue #72: matched exactly (lib.sh _monitor_pids is the shell + twin the legacy helper uses), not as a `pgrep -f` regex, because the update skill signals what this returns - a daemon of another workspace the unescaped path also matched, or a process that only names the path in an argument, is not this workspace's monitor. Multiple matches are returned in full: the legacy helper echoed every line and diff --git a/.agents/skills/multi-agent-mux-update/scripts/post_update.sh b/.agents/skills/multi-agent-mux-update/scripts/post_update.sh index 5d4ef7b..8fe6f14 100755 --- a/.agents/skills/multi-agent-mux-update/scripts/post_update.sh +++ b/.agents/skills/multi-agent-mux-update/scripts/post_update.sh @@ -37,7 +37,8 @@ fi if [ "$MON" = "1" ]; then if [ "${MAM_NO_SUBSCRIBER_DAEMON:-0}" = "1" ]; then echo "monitor: not restarted (MAM_NO_SUBSCRIBER_DAEMON=1)" - elif pid="$(start_watchdog update "$WS")"; then + # shellcheck disable=SC2086 + elif pid="$(start_watchdog update "$WS")" && mam_monitor_started "$WS" $pid; then echo "monitor: restarted (PID $pid)" else echo "ERROR: could not restart the monitor daemon" >&2 diff --git a/.agents/skills/multi-agent-mux-update/scripts/update.sh b/.agents/skills/multi-agent-mux-update/scripts/update.sh index 60da74e..ba903a1 100755 --- a/.agents/skills/multi-agent-mux-update/scripts/update.sh +++ b/.agents/skills/multi-agent-mux-update/scripts/update.sh @@ -104,6 +104,8 @@ _restart_monitor() { fi local pid pid=$(start_watchdog update "$WS") || { echo "ERROR: could not restart the monitor daemon" >&2; return 1; } + # shellcheck disable=SC2086 + mam_monitor_started "$WS" $pid || { echo "ERROR: could not restart the monitor daemon (PID ${pid:-none} is not its monitor)" >&2; return 1; } echo "monitor: restarted (PID $pid)" } diff --git a/tests/test_issue72_update.py b/tests/test_issue72_update.py index dfcd00f..cbce937 100644 --- a/tests/test_issue72_update.py +++ b/tests/test_issue72_update.py @@ -146,9 +146,12 @@ class Box: _until(lambda: _lock_held(self.mam / "monitor.lock")) def monitor_pids(self): - r = subprocess.run(["pgrep", "-f", "bash %s --subscribe" % (self.ws / MONITOR_REL)], - capture_output=True, text=True) - return r.stdout.split() + return _exact_monitor_pids(self.ws / MONITOR_REL) + + def decoy(self): + """A process that only names this workspace's monitor in an argument.""" + return self.helper("/bin/bash", "-c", "sleep 120 & wait $!", + "bash %s --subscribe" % (self.ws / MONITOR_REL)) def monitor_starts(self): p = self.mam / "stub-monitor.log" @@ -164,6 +167,20 @@ def _until(cond, timeout=15.0): raise AssertionError("condition not met within %ss" % timeout) +def _exact_monitor_pids(script) -> list: + """`bash