From ce315d8c5e7d05e9c9e63cbc204b8d6c07822d88 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 13 Sep 2026 15:34:56 +0900 Subject: [PATCH 1/7] fix(issue-19): bound the subscriber daemon to its workspace, forward SIGTERM, keep sandboxes from leaking it create_session.sh detaches `reconcile.sh --subscribe --idle-timeout 0` per workspace; test sandboxes never reaped it and the daemon neither noticed its workspace vanish nor honoured SIGTERM (bash deferred the signal behind the foreground python child). 202 orphans were found holding public-broker wildcard subscriptions for deleted pytest tmpdirs. - daemon.py / lib.sh: MAM_NO_SUBSCRIBER_DAEMON=1 (literal) skips the spawn but still reports a running daemon; cli prints a stderr note, stdout stays empty. - reconcile.sh: python subscriber runs as a waited background child with a TERM/INT trap (exit 143); the MQTT loop exits when the workspace dir is gone or .mam/monitor.lock is no longer the held inode; SIGTERM/SIGINT handled via the stop event so loop_stop/disconnect run; the polling fallback checks the same and sleeps in a waited child. - conftest: sandbox default MAM_NO_SUBSCRIBER_DAEMON=1, per-sandbox reaper finalizer, session-scope leak guard over the pytest basetemp. - tests: 11 lifecycle tests (tests/test_issue19_subscriber_daemon_lifecycle.py); phase-1 watchdog/daemon characterization tests opt back in. - docs: monitor SKILL.md lifecycle contract + pitfalls, create SKILL.md note. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- .agents/skills/lib.sh | 6 +- .agents/skills/lib_py/delegation/cli.py | 3 + .agents/skills/lib_py/delegation/daemon.py | 20 +- .../skills/multi-agent-mux-create/SKILL.md | 2 + .../skills/multi-agent-mux-monitor/SKILL.md | 19 ++ .../scripts/reconcile.sh | 70 +++- tests/conftest.py | 99 +++++- ...est_issue19_subscriber_daemon_lifecycle.py | 316 ++++++++++++++++++ tests/test_issue5_phase1_delegation.py | 13 +- 9 files changed, 536 insertions(+), 12 deletions(-) create mode 100644 tests/test_issue19_subscriber_daemon_lifecycle.py diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index fb8d439..a2fe749 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -2007,7 +2007,11 @@ _start_watchdog_legacy() { local pid pid=$(pgrep -f "bash $monitor_script --subscribe" || true) - if [ -z "$pid" ]; then + # Issue #19: MAM_NO_SUBSCRIBER_DAEMON=1 reports a running daemon but never + # starts one (test sandboxes; externally supervised monitors). + if [ -z "$pid" ] && [ "${MAM_NO_SUBSCRIBER_DAEMON:-0}" = "1" ]; then + echo "subscriber daemon not started (MAM_NO_SUBSCRIBER_DAEMON=1)" >&2 + 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" diff --git a/.agents/skills/lib_py/delegation/cli.py b/.agents/skills/lib_py/delegation/cli.py index 6844a09..83a6d6f 100644 --- a/.agents/skills/lib_py/delegation/cli.py +++ b/.agents/skills/lib_py/delegation/cli.py @@ -85,6 +85,9 @@ def main(argv: Optional[List[str]] = None) -> int: # Byte-identical to the legacy shell helper's failure path. print("ERROR: monitor script not found: {}".format(exc.args[0]), file=sys.stderr) return 1 + if not pids and daemon.daemon_disabled(): + print("subscriber daemon not started ({}=1)".format(daemon.NO_DAEMON_ENV), + file=sys.stderr) for pid in pids: print(pid) return 0 diff --git a/.agents/skills/lib_py/delegation/daemon.py b/.agents/skills/lib_py/delegation/daemon.py index 999c94b..630ee9d 100644 --- a/.agents/skills/lib_py/delegation/daemon.py +++ b/.agents/skills/lib_py/delegation/daemon.py @@ -20,6 +20,17 @@ MONITOR_RELPATH = os.path.join( ) LOG_RELPATH = os.path.join(".mam", "multi-agent-mux-monitor.log") +# Issue #19: opt-out switch for the per-workspace subscriber daemon. Test +# sandboxes set it so that `create_session.sh` under test never leaves a +# detached `reconcile.sh --subscribe --idle-timeout 0` behind; an operator can +# set it too when the monitor is supervised elsewhere. Only the literal "1" +# disables the spawn - any other value keeps the normal behaviour. +NO_DAEMON_ENV = "MAM_NO_SUBSCRIBER_DAEMON" + + +def daemon_disabled() -> bool: + return os.environ.get(NO_DAEMON_ENV, "") == "1" + def monitor_script_path(workdir: str) -> str: return os.path.join(workdir, MONITOR_RELPATH) @@ -80,12 +91,19 @@ def ensure_subscriber_daemon(job_id: str, workdir: Optional[str] = None) -> List `job_id` is accepted for signature compatibility and is deliberately unused: the daemon is per-workspace (log paths anchor on `workdir`), exactly as in the shell original. Raises FileNotFoundError when the monitor is absent so - the CLI can reproduce the legacy stderr line and exit 1. + the CLI can reproduce the legacy stderr line and exit 1. With + MAM_NO_SUBSCRIBER_DAEMON=1 (issue #19) nothing is spawned and only the PIDs + of a daemon that already runs are returned. """ workdir = workdir or os.getcwd() monitor_script = monitor_script_path(workdir) if not os.path.isfile(monitor_script): raise FileNotFoundError(monitor_script) + if daemon_disabled(): + # Issue #19: report an already-running daemon if there is one, but + # never start a new one. Callers echo the (possibly empty) PID list. + logger.info("%s=1: not starting the subscriber daemon", NO_DAEMON_ENV) + return find_running(monitor_script) running = find_running(monitor_script) if running: return running diff --git a/.agents/skills/multi-agent-mux-create/SKILL.md b/.agents/skills/multi-agent-mux-create/SKILL.md index 5af6232..e9d51d4 100644 --- a/.agents/skills/multi-agent-mux-create/SKILL.md +++ b/.agents/skills/multi-agent-mux-create/SKILL.md @@ -68,6 +68,8 @@ If any check fails → abort with a non-zero exit and report the reason (automat When running multiple agent sessions alongside other workflows (e.g., cmux, background workers, manual herdr sessions), sharing the default herdr server can lead to session name conflicts, monitoring clutter, and accidental destruction of user sessions via global commands. To prevent this, you can run this skill inside an **isolated herdr session** using the `HERDR_SESSION_NAME` environment variable or the `--herdr-session ` flag (opt-in; alias: `--herdr-server`; legacy env alias: `HERDR_SERVER_NAME`). +On success the script also starts the per-workspace monitor daemon through `start_watchdog` (`reconcile.sh --subscribe --idle-timeout 0`, one per workspace, see the lifecycle contract in [multi-agent-mux-monitor/SKILL.md](../multi-agent-mux-monitor/SKILL.md)). Set `MAM_NO_SUBSCRIBER_DAEMON=1` to skip that spawn when the monitor is supervised elsewhere or the workspace is ephemeral (Issue #19); the `watchdog PID:` line is then empty. + Additionally, you can specify `--herdr-workspace ` (default: workspace slug without `mam-` prefix) to name and group the agent panes inside a dedicated Herdr workspace tab in the Herdr runtime (`herdr workspace create --label` / `rename`) as well as recording it in `.mam/agent-sessions.yaml`. Note that `--herdr-workspace` configures the workspace tab label within the session, whereas `--herdr-session` selects the daemon socket itself. Under the hood this maps to a real, separate herdr **session** (`herdr --session ` — its own socket, its own `agent list`/`workspace list`, completely invisible to the default session and vice versa), not just a workspace label inside the same server. `lib.sh`'s shim bootstraps the named session's server headlessly (`herdr --session server`, backgrounded) the first time it's needed, and scopes every subsequent herdr call to it automatically — this headless bootstrap is what lets it work even when the skill itself is running from inside another herdr-managed pane (a plain interactive `herdr --session ` launch is blocked there by herdr's "nested herdr is disabled" guard; headless `server` mode isn't). diff --git a/.agents/skills/multi-agent-mux-monitor/SKILL.md b/.agents/skills/multi-agent-mux-monitor/SKILL.md index 7c39a27..554dda2 100644 --- a/.agents/skills/multi-agent-mux-monitor/SKILL.md +++ b/.agents/skills/multi-agent-mux-monitor/SKILL.md @@ -69,6 +69,23 @@ Run it under whatever supervisor you already use (a dedicated herdr pane, `nohup or a background job). Nothing else needs to be running for the monitor to work — it reconciles YAML ↔ herdr ↔ disk on its own. +### Daemon lifecycle contract (Issue #19) + +`multi-agent-mux-create` starts one detached `reconcile.sh --subscribe --idle-timeout 0` +per workspace through `lib.sh::start_watchdog` (`lib_py.delegation.daemon`). That daemon: + +- is **one per workspace** — `.mam/monitor.lock` (flock) rejects a second instance, and + `start_watchdog` returns the running PID instead of spawning again; +- **ends on its own only when its workspace is gone** — every wake-up (≤ 5 s on the MQTT + loop, every poll on the broker-down fallback) it checks that the workspace directory + still exists and that `.mam/monitor.lock` is still the inode it holds. A deleted + workspace, a purged `.mam/`, or a re-created lock file makes it disconnect and exit 0; +- **stops promptly on `SIGTERM`/`SIGINT`** — the bash wrapper forwards the signal to the + python subscriber and exits 143. `SIGKILL` is never required; +- is **not started when `MAM_NO_SUBSCRIBER_DAEMON=1`** — the literal `1` only. Use it when + the monitor is supervised elsewhere; a daemon that already runs is still reported. Test + sandboxes set it by default and additionally reap any daemon anchored in the sandbox. + The herdr commands the script issues (`herdr agent list`, `herdr agent get `) are real native herdr commands — do not substitute tmux-era names like `herdr ls` / `herdr list-panes` outside a shell that has sourced `.agents/skills/lib.sh`. @@ -141,6 +158,8 @@ YAML: no such session - **Don't expect `--once` to stay alive** — it does a single pass and exits. Use `--subscribe` for continuous monitoring. - **`--idle-timeout` defaults to 3600s** — a monitor meant to run indefinitely needs `--idle-timeout 0` explicitly, or it will quietly exit after an hour of broker silence. +- **`--idle-timeout 0` is bounded by the workspace, not by time** — the daemon exits when its workspace directory or `.mam/monitor.lock` disappears (Issue #19). Do not delete `.mam/monitor.lock` under a running monitor unless you mean to stop it. +- **Spawning from tests or ephemeral checkouts** — set `MAM_NO_SUBSCRIBER_DAEMON=1` so `create_session.sh` does not detach a daemon that outlives the checkout; the pytest sandbox does this for you. - **The poll interval is a default** — `RECONCILE_POLL_INTERVAL` (15s) is what the broker-down fallback uses. A workspace with 5+ agent sessions can bump it to reduce noise. - **Coalesce repeated drifts** — the same drift re-appears on every pass until it is resolved. A caller that acts on `drifts[]` should compare against the previous pass and act only on *new* entries; the script does not deduplicate for you. - **Don't fight the user's explicit action** — if `multi-agent-mux-stop` is mid-flight and the monitor sees the same session in two states within 5s, prefer the user's most recent action. The monitor should not auto-revert a fresh `terminated` to `running` because of a stale `herdr has-session` check. diff --git a/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh b/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh index 0e7b55d..7a3a288 100755 --- a/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh +++ b/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh @@ -72,12 +72,27 @@ if [ "$SUBSCRIBE" = "1" ]; then # no single interpreter needs both paho and PyYAML (review items 4/5/6). PYBIN="$(_delegate_py_bin)" + # Issue #19: the subscriber runs as a background child and this shell waits + # on it, so SIGTERM/SIGINT reach the daemon promptly. With the child in the + # foreground bash deferred the signal until the child exited on its own, + # which for --idle-timeout 0 was never - only SIGKILL worked. + _sub_pid="" + _sub_stop() { + trap - TERM INT + if [ -n "$_sub_pid" ]; then + kill -TERM "$_sub_pid" 2>/dev/null || true + wait "$_sub_pid" 2>/dev/null || true + fi + exit 143 + } + trap _sub_stop TERM INT + # The MQTT subscribe loop exits 3 to signal "broker unavailable → poll instead". set +e YAML_PATH="$AGENT_SESSIONS_YAML" HOME_DIR="$HOME_DIR" CLAUDE_PROJECT_DIR="$CLAUDE_PROJECT_DIR" LOCAL_BIN="$LOCAL_BIN" \ WORKSPACE_ROOT="$WORKSPACE_ROOT" SUB_TIMEOUT="$SUB_TIMEOUT" SUB_IDLE_TIMEOUT="$SUB_IDLE_TIMEOUT" \ SKILLS_DIR="$SKILLS_DIR" LIB_SH="$LIB_SH" \ - "$PYBIN" - <<'PYEOF' + "$PYBIN" - <<'PYEOF' & import os, sys, json, time, subprocess lib_sh = os.environ.get('LIB_SH', '') @@ -100,6 +115,27 @@ except BlockingIOError: except Exception as e: print(f"MQTT Monitor: failed to acquire monitor lock ({e}). Exiting.", flush=True) sys.exit(1) +lock_ino = os.fstat(lock_file.fileno()).st_ino + + +def _workspace_gone(): + """Issue #19: the daemon serves exactly one workspace. Once that workspace + is deleted (a purged sandbox, an rm -rf'd project) or its `.mam/monitor.lock` + is no longer the inode we hold (a re-created `.mam/`, where a fresh daemon + could now take a new lock), there is nothing left to reconcile - exit + instead of holding a broker subscription forever.""" + try: + root = os.path.abspath(workspace_root) if workspace_root else os.getcwd() + except OSError: + return "cwd no longer exists" + if not os.path.isdir(root): + return f"workspace {root} no longer exists" + try: + if os.stat(lock_file_path).st_ino != lock_ino: + return f"{lock_file_path} was replaced" + except OSError: + return f"{lock_file_path} no longer exists" + return None # Locate skills/multi-agent-mux-delegate-job/scripts to import mqtt_common — relative first, then # an upward walk from cwd. No hardcoded absolute path (review item 6). @@ -271,11 +307,28 @@ if not state['connected']: client.loop_stop() sys.exit(3) -import threading +import threading, signal stop_event = threading.Event() +state['stop_signal'] = None + + +def _on_stop_signal(signum, _frame): + state['stop_signal'] = signum + stop_event.set() + + +signal.signal(signal.SIGTERM, _on_stop_signal) +signal.signal(signal.SIGINT, _on_stop_signal) start = time.time() try: while True: + if state['stop_signal'] is not None: + print(f"MQTT Monitor: signal {state['stop_signal']} received, exiting", flush=True) + break + gone = _workspace_gone() + if gone: + print(f"MQTT Monitor: {gone}; exiting", flush=True) + break now = time.time() wall_left = (timeout - (now - start)) if timeout else None idle_left = (idle_timeout - (now - state['last_msg'])) if idle_timeout else None @@ -302,19 +355,30 @@ finally: pass sys.exit(0) PYEOF + _sub_pid=$! + wait "$_sub_pid" sub_rc=$? + _sub_pid="" set -e if [ "$sub_rc" = "3" ]; then echo "MQTT Monitor: broker unavailable — falling back to polling (interval ${POLL_INTERVAL}s)" >&2 _self="$SKILLS_DIR/multi-agent-mux-monitor/scripts/reconcile.sh" _start=$(date +%s) + # Issue #19: same lifecycle as the MQTT loop - stop when the workspace is + # gone, and sleep in a waited child so SIGTERM/SIGINT are not deferred. + trap 'trap - TERM INT; exit 143' TERM INT while :; do + if [ ! -d "$WORKSPACE_ROOT" ] || [ ! -f "$WORKSPACE_ROOT/.mam/monitor.lock" ]; then + echo "MQTT Monitor: workspace $WORKSPACE_ROOT or its monitor.lock is gone; exiting" >&2 + break + fi bash "$_self" --once --emit-diff >/dev/null 2>&1 || true if [ "$SUB_TIMEOUT" != "0" ] && [ "$(( $(date +%s) - _start ))" -ge "$SUB_TIMEOUT" ]; then break fi - sleep "$POLL_INTERVAL" + sleep "$POLL_INTERVAL" & + wait $! || true done fi exit 0 diff --git a/tests/conftest.py b/tests/conftest.py index 93f144d..6b1798c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,13 +11,89 @@ SKILLS_DIR = str(Path(__file__).resolve().parent.parent / ".agents" / "skills") if SKILLS_DIR not in sys.path: sys.path.insert(0, SKILLS_DIR) +def _subscriber_daemon_pids(root): + """PIDs of `reconcile.sh --subscribe` daemons whose command line lives under + `root`, plus their children (the python subscriber the bash wrapper waits on). + + Issue #19: `create_session.sh` starts one such daemon per workspace, + detached into its own session with `--idle-timeout 0`. A sandbox that ran + it must reap it, or every suite run leaves ~30 daemons holding wildcard + subscriptions on the default public broker behind.""" + import subprocess as _sp + root = str(root) + try: + out = _sp.run(["pgrep", "-f", "reconcile.sh --subscribe"], + capture_output=True, text=True).stdout.split() + except OSError: + return [] + found = [] + for pid in out: + try: + args = _sp.run(["ps", "-o", "args=", "-p", pid], + capture_output=True, text=True).stdout + except OSError: + continue + if root in args: + found.append(int(pid)) + kids = [] + for pid in found: + try: + kids += [int(k) for k in _sp.run(["pgrep", "-P", str(pid)], + capture_output=True, text=True).stdout.split()] + except (OSError, ValueError): + pass + return found + kids + + +def _reap_subscriber_daemons(root): + """Terminate (then kill) every subscriber daemon anchored under `root`. + Returns the PIDs that were still alive before the sweep.""" + import signal as _sig + import time as _t + pids = _subscriber_daemon_pids(root) + for sig in (_sig.SIGTERM, _sig.SIGKILL): + for pid in pids: + try: + os.kill(pid, sig) + except OSError: + pass + deadline = _t.time() + 3 + while _t.time() < deadline and any(_pid_alive(p) for p in pids): + _t.sleep(0.1) + if not any(_pid_alive(p) for p in pids): + break + return pids + + +def _pid_alive(pid): + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + except PermissionError: + return True + # a zombie still answers kill(0); ps state Z means it is gone for our purposes + import subprocess as _sp + try: + state = _sp.run(["ps", "-o", "stat=", "-p", str(pid)], + capture_output=True, text=True).stdout.strip() + except OSError: + return True + return bool(state) and not state.startswith("Z") + + @pytest.fixture -def mam_sandbox(tmp_path, monkeypatch): +def mam_sandbox(tmp_path, monkeypatch, request): """ Manages a temporary sandboxed directory for .mam/ configuration files and copies the scripts from .agents/skills to prevent contaminating the real codebase. Also overrides environments (HOME, PATH, AGENT_SESSIONS_YAML, etc.) for isolation. """ + # Issue #19: reap any subscriber daemon a test spawned under this sandbox, + # whatever path it came from (create_session.sh, start_watchdog, a direct + # `reconcile.sh --subscribe`). Registered first so it runs after every + # other finalizer of the test. + request.addfinalizer(lambda: _reap_subscriber_daemons(tmp_path)) # 1. Copy the skill scripts to sandboxed tmp_path/skills and tmp_path/.agents/skills src_skills = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".agents", "skills")) shutil.copytree(src_skills, tmp_path / "skills") @@ -52,6 +128,10 @@ def mam_sandbox(tmp_path, monkeypatch): # exist, so importing or running delegation code inside a sandbox never injects the # real .mam.env into this process's os.environ. monkeypatch.setenv("MAM_ENV_FILE", str(tmp_path / ".mam.env.absent")) + # Issue #19: never let code under test detach a real subscriber daemon + # (`reconcile.sh --subscribe --idle-timeout 0`) from a sandbox. Tests that + # exercise the daemon itself override this with "0" or delenv. + monkeypatch.setenv("MAM_NO_SUBSCRIBER_DAEMON", "1") monkeypatch.delenv("HERDR_SESSION_NAME", raising=False) monkeypatch.delenv("HERDR_SERVER_NAME", raising=False) monkeypatch.delenv("HERDR_WORKSPACE_ID", raising=False) @@ -1274,6 +1354,23 @@ def _drop_ambient_mam_version(): os.environ["MAM_VERSION"] = previous +@pytest.fixture(scope="session", autouse=True) +def subscriber_daemon_leak_guard(tmp_path_factory): + """Issue #19: no `reconcile.sh --subscribe` daemon anchored in this session's + pytest basetemp may survive the suite. Mirrors the #15 registry guard: the + per-sandbox finalizer is the fix, this is the regression alarm. Leftovers + are reaped after being reported so a failing run still cleans up.""" + yield + basetemp = str(Path(tmp_path_factory.getbasetemp()).resolve()) + leaked = _subscriber_daemon_pids(basetemp) + if leaked: + _reap_subscriber_daemons(basetemp) + assert not leaked, ( + "Issue #19: {} subscriber daemon process(es) anchored under {} survived the " + "suite (reaped now): {}".format(len(leaked), basetemp, leaked) + ) + + @pytest.fixture(scope="session", autouse=True) def delegation_tree_provenance_guard(tmp_path_factory): yield diff --git a/tests/test_issue19_subscriber_daemon_lifecycle.py b/tests/test_issue19_subscriber_daemon_lifecycle.py new file mode 100644 index 0000000..af7ba4b --- /dev/null +++ b/tests/test_issue19_subscriber_daemon_lifecycle.py @@ -0,0 +1,316 @@ +#!/usr/bin/env python3 +"""Issue #19 — the per-workspace subscriber daemon must not outlive its workspace. + +`create_session.sh` detaches `reconcile.sh --subscribe --idle-timeout 0` into +its own session for every workspace. Three gaps let ~30 of them pile up per +suite run, each holding a wildcard subscription on the default public broker: + + * nothing in the test sandbox stopped the spawn or reaped the daemon; + * the daemon never noticed its workspace (a deleted pytest tmpdir) was gone; + * SIGTERM did not work - bash kept the python child in the foreground and + deferred the signal until the child exited, which with idle-timeout 0 was + never. + +This file pins the fix from all three sides: the MAM_NO_SUBSCRIBER_DAEMON +opt-out that sandboxes set by default, the daemon's self-termination when its +workspace or `.mam/monitor.lock` disappears, and prompt SIGTERM handling on +both the MQTT loop and the broker-down polling fallback. +""" +from __future__ import annotations + +import os +import re +import signal +import subprocess +import sys +import time +from pathlib import Path + +import pytest + +FIXTURES = Path(__file__).parent / "fixtures" / "issue_5" / "phase1" +DELEGATION_MODES = ("0", "1") + + +def _skills(sandbox: Path) -> Path: + return sandbox / ".agents" / "skills" + + +def _reconcile(sandbox: Path) -> Path: + return _skills(sandbox) / "multi-agent-mux-monitor" / "scripts" / "reconcile.sh" + + +def _install_fake_monitor(sandbox: Path) -> Path: + target = _reconcile(sandbox) + target.write_text((FIXTURES / "fake_reconcile.sh").read_text(encoding="utf-8"), encoding="utf-8") + target.chmod(0o755) + return target + + +def _run_helper(sandbox: Path, snippet: str, env=None) -> subprocess.CompletedProcess: + run_env = dict(os.environ) + if env: + run_env.update(env) + return subprocess.run( + ["bash", "-c", 'source "{}" >/dev/null 2>&1; {}'.format(_skills(sandbox) / "lib.sh", snippet)], + capture_output=True, text=True, env=run_env, timeout=60, + ) + + +def _daemons_for(sandbox: Path) -> list: + out = subprocess.run(["pgrep", "-f", "reconcile.sh --subscribe"], capture_output=True, text=True).stdout.split() + hits = [] + for pid in out: + args = subprocess.run(["ps", "-o", "args=", "-p", pid], capture_output=True, text=True).stdout + if str(sandbox) in args: + hits.append(int(pid)) + return hits + + +def _alive(pid: int) -> bool: + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + state = subprocess.run(["ps", "-o", "stat=", "-p", str(pid)], capture_output=True, text=True).stdout.strip() + return bool(state) and not state.startswith("Z") + + +def _kill(pids) -> None: + for pid in pids: + try: + os.kill(int(pid), signal.SIGKILL) + except (OSError, ValueError): + pass + + +def _start_subscriber(sandbox: Path, log: Path, extra_env: dict) -> subprocess.Popen: + env = dict(os.environ) + env.update(extra_env) + handle = open(log, "ab") + try: + return subprocess.Popen( + ["bash", str(_reconcile(sandbox)), "--subscribe", "--idle-timeout", "0"], + cwd=str(sandbox), env=env, stdin=subprocess.DEVNULL, stdout=handle, stderr=subprocess.STDOUT, + start_new_session=True, + ) + finally: + handle.close() + + +def _wait_for(log: Path, pattern: str, timeout: float) -> bool: + deadline = time.time() + timeout + while time.time() < deadline: + if log.exists() and re.search(pattern, log.read_text(errors="replace")): + return True + time.sleep(0.1) + return False + + +def _wait_exit(proc: subprocess.Popen, timeout: float): + try: + return proc.wait(timeout=timeout) + except subprocess.TimeoutExpired: + return None + + +def _unreachable_broker_env() -> dict: + # Port 9 (discard) is closed on a stock macOS/Linux host: connect() is + # refused at once, so the daemon drops into its polling fallback quickly. + return {"MQTT_BROKER": "127.0.0.1", "MQTT_PORT": "9", "MQTT_TLS": "0", "RECONCILE_POLL_INTERVAL": "1"} + + +# --------------------------------------------------------------------------- +# 1. the sandbox opt-out +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize("mode", DELEGATION_MODES) +def test_sandbox_default_never_spawns_a_daemon(mam_sandbox, mode): + """With the conftest default MAM_NO_SUBSCRIBER_DAEMON=1, start_watchdog + prints no PID and leaves no process behind, in both helper bodies.""" + assert os.environ.get("MAM_NO_SUBSCRIBER_DAEMON") == "1" + _install_fake_monitor(mam_sandbox) + res = _run_helper(mam_sandbox, 'start_watchdog job1 "{}"'.format(mam_sandbox), env={"MAM_DELEGATION_PY": mode}) + assert res.returncode == 0, res.stderr + assert res.stdout.strip() == "", res.stdout + assert "MAM_NO_SUBSCRIBER_DAEMON=1" in res.stderr + assert _daemons_for(mam_sandbox) == [] + + +def test_opt_out_still_reports_a_daemon_that_already_runs(mam_sandbox, monkeypatch): + """Opting out means "do not start", not "pretend nothing runs": an existing + daemon's PID is still returned so callers keep their dedup contract.""" + from lib_py.delegation import daemon + + monitor = _install_fake_monitor(mam_sandbox) + spawned = [] + try: + monkeypatch.delenv("MAM_NO_SUBSCRIBER_DAEMON", raising=False) + first = daemon.ensure_subscriber_daemon("job1", str(mam_sandbox)) + spawned += first + assert len(first) == 1 and _alive(int(first[0])) + + monkeypatch.setenv("MAM_NO_SUBSCRIBER_DAEMON", "1") + assert daemon.daemon_disabled() + assert daemon.ensure_subscriber_daemon("job1", str(mam_sandbox)) == first + assert daemon.find_running(str(monitor)) == first + finally: + _kill(spawned) + + +def test_only_the_literal_one_disables_the_spawn(mam_sandbox, monkeypatch): + from lib_py.delegation import daemon + + for value in ("0", "", "yes", "true"): + monkeypatch.setenv("MAM_NO_SUBSCRIBER_DAEMON", value) + assert not daemon.daemon_disabled(), value + monkeypatch.setenv("MAM_NO_SUBSCRIBER_DAEMON", "1") + assert daemon.daemon_disabled() + + +def _create_session(sandbox: Path, name: str) -> subprocess.CompletedProcess: + script = _skills(sandbox) / "multi-agent-mux-create" / "scripts" / "create_session.sh" + return subprocess.run( + ["bash", str(script), "--workspace", str(sandbox), "--agent", "claude", "--role", "Creator", "--session", name], + capture_output=True, text=True, cwd=str(sandbox), timeout=180, + ) + + +def test_create_session_under_sandbox_leaves_no_daemon(mam_sandbox, mock_herdr, mock_agents): + """The leaking path itself: a full create_session.sh run in a sandbox.""" + res = _create_session(mam_sandbox, "issue19-creator-claude") + assert res.returncode == 0, res.stderr + res.stdout + assert "watchdog PID:" in res.stdout + assert _daemons_for(mam_sandbox) == [] + + +def test_create_session_without_opt_out_spawns_a_daemon_the_reaper_removes(mam_sandbox, mock_herdr, mock_agents, monkeypatch): + """Belt and braces: with the opt-out lifted the historic leak is reproduced + (create_session.sh does detach a daemon), and the sandbox reaper takes it + down - so a test that forgets the opt-out still cannot leak.""" + from conftest import _reap_subscriber_daemons + + monkeypatch.delenv("MAM_NO_SUBSCRIBER_DAEMON", raising=False) + res = _create_session(mam_sandbox, "issue19-leaky-creator-claude") + assert res.returncode == 0, res.stderr + res.stdout + pids = _daemons_for(mam_sandbox) + try: + assert pids, "expected create_session.sh to detach a subscriber daemon" + reaped = _reap_subscriber_daemons(mam_sandbox) + assert set(pids) <= set(reaped) + deadline = time.time() + 5 + while time.time() < deadline and any(_alive(p) for p in pids): + time.sleep(0.1) + assert not any(_alive(p) for p in pids) + finally: + _kill(pids) + + +# --------------------------------------------------------------------------- +# 2. the sandbox reaper +# --------------------------------------------------------------------------- + +def test_sandbox_reaper_terminates_daemons_anchored_in_the_sandbox(mam_sandbox, monkeypatch): + from conftest import _reap_subscriber_daemons, _subscriber_daemon_pids + from lib_py.delegation import daemon + + _install_fake_monitor(mam_sandbox) + monkeypatch.delenv("MAM_NO_SUBSCRIBER_DAEMON", raising=False) + pid = int(daemon.ensure_subscriber_daemon("job1", str(mam_sandbox))[0]) + try: + assert pid in _subscriber_daemon_pids(mam_sandbox) + reaped = _reap_subscriber_daemons(mam_sandbox) + assert pid in reaped + deadline = time.time() + 5 + while time.time() < deadline and _alive(pid): + time.sleep(0.1) + assert not _alive(pid), "reaper left the daemon running" + assert _subscriber_daemon_pids(mam_sandbox) == [] + finally: + _kill([pid]) + + +# --------------------------------------------------------------------------- +# 3. self-termination and signals - polling fallback (no broker needed) +# --------------------------------------------------------------------------- + +def test_poll_fallback_exits_when_monitor_lock_disappears(mam_sandbox, mock_herdr, monkeypatch): + monkeypatch.delenv("MAM_NO_SUBSCRIBER_DAEMON", raising=False) + log = mam_sandbox / "sub.log" + proc = _start_subscriber(mam_sandbox, log, _unreachable_broker_env()) + try: + assert _wait_for(log, r"falling back to polling", 30), log.read_text() + lock = mam_sandbox / ".mam" / "monitor.lock" + assert lock.exists() + lock.unlink() + rc = _wait_exit(proc, 15) + assert rc == 0, (rc, log.read_text()) + assert "monitor.lock is gone" in log.read_text() + finally: + if proc.poll() is None: + proc.kill() + + +def test_poll_fallback_sigterm_exits_promptly(mam_sandbox, mock_herdr, monkeypatch): + monkeypatch.delenv("MAM_NO_SUBSCRIBER_DAEMON", raising=False) + monkeypatch.setenv("RECONCILE_POLL_INTERVAL", "60") # a foreground sleep would defer the signal for a minute + log = mam_sandbox / "sub.log" + proc = _start_subscriber(mam_sandbox, log, _unreachable_broker_env() | {"RECONCILE_POLL_INTERVAL": "60"}) + try: + assert _wait_for(log, r"falling back to polling", 30), log.read_text() + time.sleep(1.0) # let the loop reach its sleep + t0 = time.time() + proc.send_signal(signal.SIGTERM) + rc = _wait_exit(proc, 10) + assert rc is not None, "SIGTERM was deferred: daemon still running after 10s" + assert time.time() - t0 < 5, "SIGTERM took too long to act" + assert rc == 143, rc + finally: + if proc.poll() is None: + proc.kill() + + +# --------------------------------------------------------------------------- +# 4. self-termination and signals - MQTT loop (needs a reachable broker) +# --------------------------------------------------------------------------- + +def _start_connected(mam_sandbox, log): + proc = _start_subscriber(mam_sandbox, log, {}) + if not _wait_for(log, r"MQTT Monitor: subscribed to ", 45): + proc.kill() + proc.wait(timeout=10) + pytest.skip("broker unreachable from this host: " + log.read_text()[-300:]) + return proc + + +def test_mqtt_loop_exits_when_workspace_lock_is_replaced(mam_sandbox, monkeypatch): + monkeypatch.delenv("MAM_NO_SUBSCRIBER_DAEMON", raising=False) + log = mam_sandbox / "sub.log" + proc = _start_connected(mam_sandbox, log) + try: + lock = mam_sandbox / ".mam" / "monitor.lock" + lock.unlink() + lock.write_text("") # a re-created .mam/: a fresh daemon could now take this lock + rc = _wait_exit(proc, 15) + assert rc == 0, (rc, log.read_text()) + assert re.search(r"monitor\.lock (was replaced|no longer exists); exiting", log.read_text()), log.read_text() + finally: + if proc.poll() is None: + proc.kill() + + +def test_mqtt_loop_sigterm_exits_cleanly(mam_sandbox, monkeypatch): + monkeypatch.delenv("MAM_NO_SUBSCRIBER_DAEMON", raising=False) + log = mam_sandbox / "sub.log" + proc = _start_connected(mam_sandbox, log) + try: + t0 = time.time() + proc.send_signal(signal.SIGTERM) + rc = _wait_exit(proc, 10) + assert rc is not None, "SIGTERM was deferred: daemon still running after 10s" + assert time.time() - t0 < 6 + assert rc == 143, (rc, log.read_text()) + assert "signal 15 received, exiting" in log.read_text(), log.read_text() + finally: + if proc.poll() is None: + proc.kill() diff --git a/tests/test_issue5_phase1_delegation.py b/tests/test_issue5_phase1_delegation.py index dd08c8e..d5f410e 100644 --- a/tests/test_issue5_phase1_delegation.py +++ b/tests/test_issue5_phase1_delegation.py @@ -194,7 +194,7 @@ def test_c07_watchdog_spawns_daemon_with_workdir_and_log(mam_sandbox, mode): res = _run_helper( mam_sandbox, 'start_watchdog job1 "{}"'.format(workdir), - env={"MAM_DELEGATION_PY": mode}, + env={"MAM_DELEGATION_PY": mode, "MAM_NO_SUBSCRIBER_DAEMON": "0"}, ) assert res.returncode == 0, res.stderr pid = res.stdout.strip() @@ -219,12 +219,12 @@ def test_c08_watchdog_second_call_reuses_running_daemon(mam_sandbox, mode): spawned = [] try: first = _run_helper(mam_sandbox, 'start_watchdog job1 "{}"'.format(workdir), - env={"MAM_DELEGATION_PY": mode}) + env={"MAM_DELEGATION_PY": mode, "MAM_NO_SUBSCRIBER_DAEMON": "0"}) pid1 = first.stdout.strip() spawned.append(pid1) assert _alive(pid1) second = _run_helper(mam_sandbox, 'start_watchdog job1 "{}"'.format(workdir), - env={"MAM_DELEGATION_PY": mode}) + env={"MAM_DELEGATION_PY": mode, "MAM_NO_SUBSCRIBER_DAEMON": "0"}) assert second.returncode == 0, second.stderr assert second.stdout.strip() == pid1, (first.stdout, second.stdout) matches = subprocess.run( @@ -259,9 +259,9 @@ def test_c09_start_watchdog_and_ensure_subscriber_daemon_agree(mam_sandbox, mode spawned = [] try: a = _run_helper(mam_sandbox, 'start_watchdog job1 "{}"'.format(workdir), - env={"MAM_DELEGATION_PY": mode}) + env={"MAM_DELEGATION_PY": mode, "MAM_NO_SUBSCRIBER_DAEMON": "0"}) b = _run_helper(mam_sandbox, 'ensure_subscriber_daemon job1 "{}"'.format(workdir), - env={"MAM_DELEGATION_PY": mode}) + env={"MAM_DELEGATION_PY": mode, "MAM_NO_SUBSCRIBER_DAEMON": "0"}) spawned += [a.stdout.strip(), b.stdout.strip()] assert a.returncode == b.returncode == 0, (a.stderr, b.stderr) assert a.stdout.strip() == b.stdout.strip(), (a.stdout, b.stdout) @@ -649,10 +649,11 @@ def test_daemon_missing_monitor_raises_with_the_legacy_path(mam_sandbox): assert exc.value.args[0] == str(daemon.monitor_script_path(str(workdir))) -def test_daemon_spawn_dedup_and_survives_parent_exit(mam_sandbox): +def test_daemon_spawn_dedup_and_survives_parent_exit(mam_sandbox, monkeypatch): """The daemon must outlive the shell that started it and never double-spawn.""" from lib_py.delegation import daemon + monkeypatch.delenv("MAM_NO_SUBSCRIBER_DAEMON", raising=False) # issue #19 sandbox default monitor = _install_fake_monitor(mam_sandbox) workdir = str(mam_sandbox) spawned = [] From 38d8ccb316665f60fbb761443143c1ffde31cbe8 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 13 Sep 2026 15:35:33 +0900 Subject: [PATCH 2/7] release(v4.4.5): bump framework and eight skills to 4.4.5 in lockstep Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- .agents/skills/lib.sh | 2 +- .agents/skills/lib_py/loop/eager.py | 2 +- .../skills/multi-agent-mux-create/SKILL.md | 2 +- .../multi-agent-mux-delegate-job/SKILL.md | 2 +- .agents/skills/multi-agent-mux-loop/SKILL.md | 2 +- .../skills/multi-agent-mux-monitor/SKILL.md | 2 +- .../multi-agent-mux-orc-onboard/SKILL.md | 2 +- .../skills/multi-agent-mux-resume/SKILL.md | 2 +- .../skills/multi-agent-mux-status/SKILL.md | 2 +- .agents/skills/multi-agent-mux-stop/SKILL.md | 2 +- BOOTSTRAP.ko.md | 2 +- BOOTSTRAP.md | 2 +- MESSAGING.md | 2 +- README.ko.md | 2 +- README.md | 2 +- VERSIONS.md | 35 +++++++++++++------ 16 files changed, 40 insertions(+), 25 deletions(-) diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index a2fe749..8889f54 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -29,7 +29,7 @@ export WORKSPACE_ROOT # Framework semantic version. Single runtime source of truth; kept in lockstep # with VERSIONS.md and the 8 SKILL.md frontmatters by tests/test_version_consistency.py. # NOTE: unlike other MAM_* variables this one is intentionally NOT env-overridable. -MAM_VERSION="4.4.4" +MAM_VERSION="4.4.5" export MAM_VERSION AGENT_SESSIONS_YAML="${AGENT_SESSIONS_YAML:-$WORKSPACE_ROOT/.mam/agent-sessions.yaml}" diff --git a/.agents/skills/lib_py/loop/eager.py b/.agents/skills/lib_py/loop/eager.py index 8ffa4bb..109c27f 100644 --- a/.agents/skills/lib_py/loop/eager.py +++ b/.agents/skills/lib_py/loop/eager.py @@ -13,7 +13,7 @@ import sys from typing import List, Sequence # Framework version locked with framework constants -FRAMEWORK_VERSION = "4.4.4" +FRAMEWORK_VERSION = "4.4.5" CORE_MODULES: Sequence[str] = ( # Standard library modules diff --git a/.agents/skills/multi-agent-mux-create/SKILL.md b/.agents/skills/multi-agent-mux-create/SKILL.md index e9d51d4..79df6b2 100644 --- a/.agents/skills/multi-agent-mux-create/SKILL.md +++ b/.agents/skills/multi-agent-mux-create/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-create description: "Create a new agent session (claude, antigravity/agy) in a dedicated herdr session for context-preserving long-running work. Always creates a herdr session — never backgrounds with nohup/disown. Writes the new session to .mam/agent-sessions.yaml. Use when you want to start a fresh agent (no prior UUID) for a new project workspace." -version: 4.4.4 +version: 4.4.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-delegate-job/SKILL.md b/.agents/skills/multi-agent-mux-delegate-job/SKILL.md index 048bb57..fb78042 100644 --- a/.agents/skills/multi-agent-mux-delegate-job/SKILL.md +++ b/.agents/skills/multi-agent-mux-delegate-job/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-delegate-job description: "Delegate a unit of work to any autonomous agent (claude-code, hermes, agy, grok-build, codex, or a human) and observe it asynchronously over an MQTT event channel. Supported roles include orchestrator, worker, and reviewer." -version: 4.4.4 +version: 4.4.5 author: godopu license: MIT platforms: [linux, macos, windows] diff --git a/.agents/skills/multi-agent-mux-loop/SKILL.md b/.agents/skills/multi-agent-mux-loop/SKILL.md index 2c07186..2807cd5 100644 --- a/.agents/skills/multi-agent-mux-loop/SKILL.md +++ b/.agents/skills/multi-agent-mux-loop/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-loop description: "Run an autonomous planning-execution-review loop using multiple agents (Planner, Creator, Reviewers) in the workspace. Automatically orchestrates plan discussion, code changes, and peer reviews until a unanimous PASS is achieved or the maximum iteration limit is reached." -version: 4.4.4 +version: 4.4.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-monitor/SKILL.md b/.agents/skills/multi-agent-mux-monitor/SKILL.md index 554dda2..75a197d 100644 --- a/.agents/skills/multi-agent-mux-monitor/SKILL.md +++ b/.agents/skills/multi-agent-mux-monitor/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-monitor description: "Run a long-lived reconciler that watches .mam/agent-sessions.yaml against the actual herdr/agent runtime state and reconciles them. Use when you want live visibility into which agent sessions are running, which are dead, which have stale YAML entries, and which have new session ids that haven't been recorded yet. Runs as a persistent loop (`reconcile.sh --subscribe`) that keeps going until it times out, idles out, or is interrupted." -version: 4.4.4 +version: 4.4.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md b/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md index a3cc1e4..a2458fd 100644 --- a/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md +++ b/.agents/skills/multi-agent-mux-orc-onboard/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-orc-onboard description: "Register current or specified orchestrator session UUID into agent-sessions.yaml orchestrator_uuids list to prevent sub-agent discovery capture." -version: 4.4.4 +version: 4.4.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-resume/SKILL.md b/.agents/skills/multi-agent-mux-resume/SKILL.md index b18d204..a30a938 100644 --- a/.agents/skills/multi-agent-mux-resume/SKILL.md +++ b/.agents/skills/multi-agent-mux-resume/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-resume description: "Resume an existing agent (claude, antigravity/agy) conversation by UUID into a herdr session. Reads .mam/agent-sessions.yaml for the saved session/conversation id, spawns (or reuses) a herdr session of the matching name, and runs `claude -r ` or `agy --conversation ` inside. Use when you want to reattach to a previous session's context, or revive a session whose herdr died but the agent's conversation is still on disk." -version: 4.4.4 +version: 4.4.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-status/SKILL.md b/.agents/skills/multi-agent-mux-status/SKILL.md index 3b13993..3600c35 100644 --- a/.agents/skills/multi-agent-mux-status/SKILL.md +++ b/.agents/skills/multi-agent-mux-status/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-status description: "Read-only instant snapshot of all agent herdr sessions — name, YAML status, herdr alive, pane cmd/cwd, resume UUID on disk, and any drift. No mutation. Reuses reconcile.sh --dry-run for the diff logic. Use when you want to know 'what's running RIGHT NOW' without spinning up the monitor loop." -version: 4.4.4 +version: 4.4.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/.agents/skills/multi-agent-mux-stop/SKILL.md b/.agents/skills/multi-agent-mux-stop/SKILL.md index 9a4f0fc..106e82f 100644 --- a/.agents/skills/multi-agent-mux-stop/SKILL.md +++ b/.agents/skills/multi-agent-mux-stop/SKILL.md @@ -1,7 +1,7 @@ --- name: multi-agent-mux-stop description: "Stop an agent herdr session (claude, antigravity/agy) and update .mam/agent-sessions.yaml. Default stops gracefully and marks status=stopped with conversation preserved for resume. Does NOT delete on-disk conversation artifacts (jsonl/db) — those are preserved unless --purge-conversation is passed. Use when ending a work session, switching to a different one, or cleaning up before a fresh start." -version: 4.4.4 +version: 4.4.5 author: godopu license: MIT platforms: [linux, macos] diff --git a/BOOTSTRAP.ko.md b/BOOTSTRAP.ko.md index f5c26bf..521d34d 100644 --- a/BOOTSTRAP.ko.md +++ b/BOOTSTRAP.ko.md @@ -2,7 +2,7 @@ 본 문서는 `herdr_agent_orchestration` 오케스트레이션 및 메시징 백플레인 워크플로우를 새로운 프로젝트에 도입하여 이식하고, 새로운 개발자/에이전트가 초기 가동을 시작할 때 수행해야 하는 환경 설정 및 구축 절차를 안내합니다. -[![Version: v4.4.4](https://img.shields.io/badge/version-v4.4.4-blue.svg)](VERSIONS.md) +[![Version: v4.4.5](https://img.shields.io/badge/version-v4.4.5-blue.svg)](VERSIONS.md) 새로운 에이전트는 이 안내서에 기술된 절차를 순차적으로 실행하여 초기 환경을 안정적으로 설정할 수 있습니다. diff --git a/BOOTSTRAP.md b/BOOTSTRAP.md index 76cb5fe..acc57b7 100644 --- a/BOOTSTRAP.md +++ b/BOOTSTRAP.md @@ -2,7 +2,7 @@ This document guides you through the setup and initialization procedures required to adopt the `herdr_agent_orchestration` orchestration and messaging backplane workflow in a new project, enabling a new developer or agent to get up and running quickly. -[![Version: v4.4.4](https://img.shields.io/badge/version-v4.4.4-blue.svg)](VERSIONS.md) +[![Version: v4.4.5](https://img.shields.io/badge/version-v4.4.5-blue.svg)](VERSIONS.md) A new agent can follow the steps in this guide sequentially to establish a stable and reliable initial environment. diff --git a/MESSAGING.md b/MESSAGING.md index 5abf578..abb26fd 100644 --- a/MESSAGING.md +++ b/MESSAGING.md @@ -1,6 +1,6 @@ # Messaging System Technical Analysis & Architecture Report -Applies to: MAM v4.4.4 +Applies to: MAM v4.4.5 This report provides a comprehensive, deep-dive analysis of the messaging system implemented in the `multi-agent-mux-delegate-job` skill. It covers the MQTT broker architecture, event protocols, job lifecycles, codebase internals, cross-system integration, and a list of known limitations along with production recommendations. diff --git a/README.ko.md b/README.ko.md index 17d11c0..c271662 100644 --- a/README.ko.md +++ b/README.ko.md @@ -2,7 +2,7 @@ Herdr와 MQTT 브로커를 기반으로 구축된 고신뢰성 **다중 에이전트 오케스트레이션 및 메시징 백플레인** 프레임워크입니다. `claude`, `agy`, `hermes`, `grok`, `opencode`, `codex` 등 다중 LLM 백엔드 에이전트 전반에 걸쳐 장시간 수행되는 작업(코드 생성, 리팩토링, 보안 검토 등)을 조정, 격리 및 감사할 수 있도록 설계되었습니다. -[![Version: v4.4.4](https://img.shields.io/badge/version-v4.4.4-blue.svg)](VERSIONS.md) +[![Version: v4.4.5](https://img.shields.io/badge/version-v4.4.5-blue.svg)](VERSIONS.md) --- diff --git a/README.md b/README.md index 14dd2ee..ae49114 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ An advanced, high-reliability **Multi-Agent Orchestration & Messaging Backplane** framework built on Herdr and MQTT. It is designed to coordinate, isolate, and audit long-running agent tasks (such as code generation, refactoring, and security reviews) across multiple LLM backend clients (`claude`, `agy`, `hermes`, `grok`, `opencode`, `codex`). -[![Version: v4.4.4](https://img.shields.io/badge/version-v4.4.4-blue.svg)](VERSIONS.md) +[![Version: v4.4.5](https://img.shields.io/badge/version-v4.4.5-blue.svg)](VERSIONS.md) --- diff --git a/VERSIONS.md b/VERSIONS.md index 18e1a6e..0d89654 100644 --- a/VERSIONS.md +++ b/VERSIONS.md @@ -6,7 +6,7 @@ ## 📌 현재 버전 개요 (Current Release) -- **프레임워크 버전**: `v4.4.4` +- **프레임워크 버전**: `v4.4.5` - **최신 릴리스 일시**: 2026-09-13 (KST) - **기준 브랜치**: `main` - **핵심 아키텍처**: @@ -20,23 +20,38 @@ ## 🧭 스킬 패키지 버전 매트릭스 (Skills Version Matrix) -모든 8개 스킬은 YAML frontmatter 메타데이터(`author`, `version`, `platforms`, `environments`) 표준화를 통해 `v4.4.4`로 동기화되어 배포됩니다. +모든 8개 스킬은 YAML frontmatter 메타데이터(`author`, `version`, `platforms`, `environments`) 표준화를 통해 `v4.4.5`로 동기화되어 배포됩니다. | 스킬명 | 버전 | 역할 및 주요 책임 | 상태 | | :--- | :---: | :--- | :---: | -| **`multi-agent-mux-create`** | `4.4.4` | 에이전트 세션 신규 생성 및 Herdr 컨테이너 격리 스폰 | ✅ 배포 | -| **`multi-agent-mux-stop`** | `4.4.4` | 대화 UUID 원자적 캡처 및 세션 안전 종료 (Graceful Stop) | ✅ 배포 | -| **`multi-agent-mux-resume`** | `4.4.4` | 온디스크 대화 컨텍스트 기반 Tier-1 초고속 세션 복원 | ✅ 배포 | -| **`multi-agent-mux-status`** | `4.4.4` | 실시간 Herdr 세션 및 레지스트리 드리프트 스냅샷 조회 | ✅ 배포 | -| **`multi-agent-mux-monitor`** | `4.4.4` | YAML ↔ 런타임 상태 간 자율 조정자 (Reconciler Loop) | ✅ 배포 | -| **`multi-agent-mux-delegate-job`** | `4.4.4` | MQTT 이벤트 채널 기반 비동기 단위 작업 위임 | ✅ 배포 | -| **`multi-agent-mux-loop`** | `4.4.4` | Planner-Creator-Reviewer 3자 자율 계획·실행·피어리뷰 루프 | ✅ 배포 | -| **`multi-agent-mux-orc-onboard`** | `4.4.4` | 오케스트레이터 UUID 격리 등록 및 서브 세션 오염 방지 | ✅ 배포 | +| **`multi-agent-mux-create`** | `4.4.5` | 에이전트 세션 신규 생성 및 Herdr 컨테이너 격리 스폰 | ✅ 배포 | +| **`multi-agent-mux-stop`** | `4.4.5` | 대화 UUID 원자적 캡처 및 세션 안전 종료 (Graceful Stop) | ✅ 배포 | +| **`multi-agent-mux-resume`** | `4.4.5` | 온디스크 대화 컨텍스트 기반 Tier-1 초고속 세션 복원 | ✅ 배포 | +| **`multi-agent-mux-status`** | `4.4.5` | 실시간 Herdr 세션 및 레지스트리 드리프트 스냅샷 조회 | ✅ 배포 | +| **`multi-agent-mux-monitor`** | `4.4.5` | YAML ↔ 런타임 상태 간 자율 조정자 (Reconciler Loop) | ✅ 배포 | +| **`multi-agent-mux-delegate-job`** | `4.4.5` | MQTT 이벤트 채널 기반 비동기 단위 작업 위임 | ✅ 배포 | +| **`multi-agent-mux-loop`** | `4.4.5` | Planner-Creator-Reviewer 3자 자율 계획·실행·피어리뷰 루프 | ✅ 배포 | +| **`multi-agent-mux-orc-onboard`** | `4.4.5` | 오케스트레이터 UUID 격리 등록 및 서브 세션 오염 방지 | ✅ 배포 | --- ## 📋 버전별 상세 변경 내역 (Changelog) +### 🚀 `v4.4.5` — Issue #19 구독 데몬 수명 계약: 샌드박스 옵트아웃·회수, 워크스페이스 소실 시 자기 종료, SIGTERM 전달 (2026-09-13) + +> **패치 마일스톤 (PATCH Release)**: `create_session.sh` 가 워크스페이스마다 분리 세션으로 띄우는 구독 데몬(`reconcile.sh --subscribe --idle-timeout 0`)이 테스트 샌드박스에서 회수되지 않아 스위트 1회당 약 30개씩 쌓이던 문제(Issue #19, 실측 고아 프로세스 202개)를 해결했다. 데몬은 이제 자기 워크스페이스가 사라지면 스스로 끝나고 `SIGTERM` 에 즉시 응답하며, 샌드박스는 데몬을 띄우지 않고 띄워진 것은 회수한다. 공개 계약·종료 코드 변경은 없다(옵트아웃 환경변수 1개 추가). + +#### 🌟 주요 개선 사항 +1. **`MAM_NO_SUBSCRIBER_DAEMON=1` 옵트아웃 (`lib_py/delegation/daemon.py`, `lib.sh: _start_watchdog_legacy`)**: 리터럴 `1` 일 때만 데몬을 새로 띄우지 않는다. 이미 도는 데몬의 PID 는 그대로 보고하므로 `start_watchdog`/`ensure_subscriber_daemon` 의 dedup 계약은 유지되고, 미기동 시 stderr 에 한 줄 남긴다(stdout 은 비움). 테스트 샌드박스(`mam_sandbox`)가 기본으로 켜고, 데몬 자체를 검증하는 테스트만 `0`/삭제로 되돌린다. +2. **데몬 자기 종료 (`reconcile.sh --subscribe`)**: MQTT 루프는 5초 주기 wake 마다 워크스페이스 디렉터리 존재와 `.mam/monitor.lock` 의 inode 가 자신이 잡은 것과 같은지 확인하고, 사라졌거나 교체됐으면(`.mam/` 재생성 → 새 데몬이 새 lock 을 잡을 수 있는 상태) 정리 후 rc 0 으로 끝난다. 브로커 불가 폴링 폴백도 매 회 같은 검사를 한다. 삭제된 pytest 임시 디렉터리나 `rm -rf` 된 프로젝트를 바라보며 공개 브로커 구독을 영원히 유지하던 데몬이 사라진다. +3. **`SIGTERM`/`SIGINT` 즉시 전달 (`reconcile.sh`)**: python 구독자를 백그라운드 자식으로 두고 bash 가 `wait` 하도록 바꿔 시그널이 미뤄지지 않는다(종전엔 포그라운드 자식이 끝날 때까지 bash 가 시그널을 보류해 `SIGKILL` 만 통했다). 트랩이 자식에 `SIGTERM` 을 전달하고 rc 143 으로 끝나며, python 쪽은 핸들러로 이벤트를 깨워 `loop_stop`/`disconnect` 를 거친다. 폴백 루프의 `sleep` 도 `& wait` 로 바꿔 즉시 중단된다. +4. **샌드박스 회수·회귀 가드 (`tests/conftest.py`)**: `mam_sandbox` finalizer 가 샌드박스 경로를 cmdline 에 가진 `reconcile.sh --subscribe` 와 그 자식을 `SIGTERM`→`SIGKILL` 로 회수한다. 세션 스코프 `subscriber_daemon_leak_guard` 는 스위트 종료 시 basetemp 아래 데몬이 남아 있으면 회수한 뒤 실패시킨다(#15 레지스트리 가드와 같은 방식). + +#### ✅ 품질 검증 +- **신규 테스트 11개** (`tests/test_issue19_subscriber_daemon_lifecycle.py`): 샌드박스 기본값에서 두 helper 본체 모두 데몬 미기동(2), 옵트아웃 중에도 기존 데몬 PID 보고, 리터럴 `1` 만 인정, `create_session.sh` 샌드박스 실행 후 데몬 없음, 옵트아웃 해제 시 누수 재현 + 회수기 제거, 회수기 단위 검증, 폴링 폴백의 lock 소실 종료·`SIGTERM` 즉시 종료(2, 브로커 불필요), MQTT 루프의 lock 교체 종료·`SIGTERM` 정상 종료(2, 브로커 미도달 시 skip). +- **기존 테스트 조정**: `tests/test_issue5_phase1_delegation.py` 의 watchdog/daemon 특성화 테스트가 `MAM_NO_SUBSCRIBER_DAEMON=0`(또는 delenv)으로 옵트아웃을 해제. +- **실측**: 스위트 실행 후 `pgrep -f 'pytest-of-.*reconcile.sh --subscribe'` 0건. + ### 🚀 `v4.4.4` — Issue #12 레지스트리 비밀번호 제거, Issue #8 업데이트 스탬프 보존, Issue #7 형제 워크스페이스 패킹 (2026-09-13) > **패치 마일스톤 (PATCH Release)**: 모든 서브에이전트가 읽을 수 있는 잡 레코드에 MQTT 브로커 비밀번호가 평문으로 복사되던 문제(Issue #12), `deploy/update.sh` 가 인스톨러가 새로 쓴 `version.txt`·`asset_hashes.txt` 를 업데이트 전 값으로 되돌리던 문제(Issue #8), 같은 cwd 의 첫 워크스페이스만 검사해 다섯 번째 에이전트부터 에이전트마다 워크스페이스를 새로 열던 문제(Issue #7)를 해결했다. 새 `registry.py scrub-secrets` 는 추가 서브커맨드이며 공개 계약·종료 코드 변경은 없다. 브로커 비밀번호는 이제 레코드가 아니라 환경(`MQTT_PASSWORD`, `.mam.env`)에서만 온다. From 4da4a50682bd44e0656625d450bd80a79876753e Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 13 Sep 2026 16:09:28 +0900 Subject: [PATCH 3/7] test(issue-19): run the sandbox reaper through an import-time Popen The reaper executes in fixture teardown, before monkeypatch restores a test's patches; G5 forbids subprocess.run/Popen for the test's duration and subprocess.run resolves Popen through the module at call time, so the teardown tripped the spy. Bind the Popen class at import instead. Full suite at 38d8ccb: 870 passed, 1 error (this teardown), 0 leaked daemons. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- tests/conftest.py | 46 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 6b1798c..51f5d55 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,9 +4,27 @@ import shutil import json import uuid import sqlite3 +import subprocess as _subprocess from pathlib import Path import pytest +# Issue #19: the sandbox reaper runs in fixture teardown, before `monkeypatch` +# undoes a test's patches. Bind the real Popen class once at import so a test +# that spies on or forbids subprocess.run/Popen (G5) cannot break the reaper. +# (subprocess.run resolves `Popen` through the module at call time, so binding +# `run` alone is not enough.) +_REAL_POPEN = _subprocess.Popen + + +def _run_real(cmd): + """stdout of `cmd` via the import-time Popen; '' on any OS error.""" + try: + proc = _REAL_POPEN(cmd, stdout=_subprocess.PIPE, stderr=_subprocess.DEVNULL, text=True) + out, _ = proc.communicate(timeout=10) + return out + except (OSError, _subprocess.SubprocessError): + return "" + SKILLS_DIR = str(Path(__file__).resolve().parent.parent / ".agents" / "skills") if SKILLS_DIR not in sys.path: sys.path.insert(0, SKILLS_DIR) @@ -19,29 +37,14 @@ def _subscriber_daemon_pids(root): detached into its own session with `--idle-timeout 0`. A sandbox that ran it must reap it, or every suite run leaves ~30 daemons holding wildcard subscriptions on the default public broker behind.""" - import subprocess as _sp root = str(root) - try: - out = _sp.run(["pgrep", "-f", "reconcile.sh --subscribe"], - capture_output=True, text=True).stdout.split() - except OSError: - return [] found = [] - for pid in out: - try: - args = _sp.run(["ps", "-o", "args=", "-p", pid], - capture_output=True, text=True).stdout - except OSError: - continue - if root in args: + for pid in _run_real(["pgrep", "-f", "reconcile.sh --subscribe"]).split(): + if root in _run_real(["ps", "-o", "args=", "-p", pid]): found.append(int(pid)) kids = [] for pid in found: - try: - kids += [int(k) for k in _sp.run(["pgrep", "-P", str(pid)], - capture_output=True, text=True).stdout.split()] - except (OSError, ValueError): - pass + kids += [int(k) for k in _run_real(["pgrep", "-P", str(pid)]).split() if k.isdigit()] return found + kids @@ -73,12 +76,7 @@ def _pid_alive(pid): except PermissionError: return True # a zombie still answers kill(0); ps state Z means it is gone for our purposes - import subprocess as _sp - try: - state = _sp.run(["ps", "-o", "stat=", "-p", str(pid)], - capture_output=True, text=True).stdout.strip() - except OSError: - return True + state = _run_real(["ps", "-o", "stat=", "-p", str(pid)]).strip() return bool(state) and not state.startswith("Z") From c096cb455baf300dd682c30375f52f994d539ce7 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 13 Sep 2026 16:11:53 +0900 Subject: [PATCH 4/7] fix(issue-19): close the trap race, select daemons by argv shape, align docs with the polling fallback Review round 1 (claude PASS, agy PASS) recommendations: - reconcile.sh: `_sub_stop` falls back to `$!` for the window before `_sub_pid=$!` (R-1); lock identity compared as (st_dev, st_ino). - lib.sh legacy start_watchdog prints nothing when opted out and no daemon runs, byte-identical to the python body. - conftest/test: `_is_subscriber_daemon_argv` selects only `bash|sh /.../reconcile.sh --subscribe`, never a brief-carrying wrapper that quotes the words or a prefix-colliding sibling root (R-2); one new unit test for the selector. - monitor SKILL.md / VERSIONS.md: the polling fallback checks the lock file's presence only and does not exclude a second daemon; measurement wording uses the argv-shape criterion instead of raw pgrep. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- .agents/skills/lib.sh | 5 +++- .../skills/multi-agent-mux-monitor/SKILL.md | 11 +++++---- .../scripts/reconcile.sh | 18 ++++++++++---- VERSIONS.md | 8 +++---- tests/conftest.py | 19 ++++++++++++++- ...est_issue19_subscriber_daemon_lifecycle.py | 24 ++++++++++++++++++- 6 files changed, 69 insertions(+), 16 deletions(-) diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index 8889f54..fb8f828 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -2021,7 +2021,10 @@ _start_watchdog_legacy() { cd "$orig_pwd" || return 1 fi - echo "$pid" + # Byte-identical to the python body: nothing on stdout when no daemon runs. + if [ -n "$pid" ]; then + echo "$pid" + fi } # --------------------------------------------------------------------------- diff --git a/.agents/skills/multi-agent-mux-monitor/SKILL.md b/.agents/skills/multi-agent-mux-monitor/SKILL.md index 75a197d..dd7e7b1 100644 --- a/.agents/skills/multi-agent-mux-monitor/SKILL.md +++ b/.agents/skills/multi-agent-mux-monitor/SKILL.md @@ -76,10 +76,13 @@ per workspace through `lib.sh::start_watchdog` (`lib_py.delegation.daemon`). Tha - is **one per workspace** — `.mam/monitor.lock` (flock) rejects a second instance, and `start_watchdog` returns the running PID instead of spawning again; -- **ends on its own only when its workspace is gone** — every wake-up (≤ 5 s on the MQTT - loop, every poll on the broker-down fallback) it checks that the workspace directory - still exists and that `.mam/monitor.lock` is still the inode it holds. A deleted - workspace, a purged `.mam/`, or a re-created lock file makes it disconnect and exit 0; +- **ends on its own only when its workspace is gone** — every wake-up (≤ 5 s) the MQTT + loop checks that the workspace directory still exists and that `.mam/monitor.lock` is + still the `(st_dev, st_ino)` it holds; a deleted workspace, a purged `.mam/`, or a + re-created lock file makes it disconnect and exit 0. The broker-down polling fallback + checks the directory and the lock file's *presence* on every pass — it no longer holds + the lock (the python subscriber released it on exit), so while polling it does not + exclude a second daemon, as before; - **stops promptly on `SIGTERM`/`SIGINT`** — the bash wrapper forwards the signal to the python subscriber and exits 143. `SIGKILL` is never required; - is **not started when `MAM_NO_SUBSCRIBER_DAEMON=1`** — the literal `1` only. Use it when diff --git a/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh b/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh index 7a3a288..1d0dffa 100755 --- a/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh +++ b/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh @@ -79,9 +79,12 @@ if [ "$SUBSCRIBE" = "1" ]; then _sub_pid="" _sub_stop() { trap - TERM INT - if [ -n "$_sub_pid" ]; then - kill -TERM "$_sub_pid" 2>/dev/null || true - wait "$_sub_pid" 2>/dev/null || true + # `$!` covers the window between `&` and `_sub_pid=$!` (review R-1); a + # child that already exited makes kill/wait harmless no-ops. + local child="${_sub_pid:-${!:-}}" + if [ -n "$child" ]; then + kill -TERM "$child" 2>/dev/null || true + wait "$child" 2>/dev/null || true fi exit 143 } @@ -115,7 +118,8 @@ except BlockingIOError: except Exception as e: print(f"MQTT Monitor: failed to acquire monitor lock ({e}). Exiting.", flush=True) sys.exit(1) -lock_ino = os.fstat(lock_file.fileno()).st_ino +_lock_st = os.fstat(lock_file.fileno()) +lock_ident = (_lock_st.st_dev, _lock_st.st_ino) def _workspace_gone(): @@ -131,7 +135,8 @@ def _workspace_gone(): if not os.path.isdir(root): return f"workspace {root} no longer exists" try: - if os.stat(lock_file_path).st_ino != lock_ino: + st = os.stat(lock_file_path) + if (st.st_dev, st.st_ino) != lock_ident: return f"{lock_file_path} was replaced" except OSError: return f"{lock_file_path} no longer exists" @@ -367,6 +372,9 @@ PYEOF _start=$(date +%s) # Issue #19: same lifecycle as the MQTT loop - stop when the workspace is # gone, and sleep in a waited child so SIGTERM/SIGINT are not deferred. + # The python child released .mam/monitor.lock when it exited, so this + # fallback checks the lock file's *presence* only and does not exclude a + # second daemon (pre-existing behaviour of the fallback). trap 'trap - TERM INT; exit 143' TERM INT while :; do if [ ! -d "$WORKSPACE_ROOT" ] || [ ! -f "$WORKSPACE_ROOT/.mam/monitor.lock" ]; then diff --git a/VERSIONS.md b/VERSIONS.md index 0d89654..a5a83af 100644 --- a/VERSIONS.md +++ b/VERSIONS.md @@ -43,14 +43,14 @@ #### 🌟 주요 개선 사항 1. **`MAM_NO_SUBSCRIBER_DAEMON=1` 옵트아웃 (`lib_py/delegation/daemon.py`, `lib.sh: _start_watchdog_legacy`)**: 리터럴 `1` 일 때만 데몬을 새로 띄우지 않는다. 이미 도는 데몬의 PID 는 그대로 보고하므로 `start_watchdog`/`ensure_subscriber_daemon` 의 dedup 계약은 유지되고, 미기동 시 stderr 에 한 줄 남긴다(stdout 은 비움). 테스트 샌드박스(`mam_sandbox`)가 기본으로 켜고, 데몬 자체를 검증하는 테스트만 `0`/삭제로 되돌린다. -2. **데몬 자기 종료 (`reconcile.sh --subscribe`)**: MQTT 루프는 5초 주기 wake 마다 워크스페이스 디렉터리 존재와 `.mam/monitor.lock` 의 inode 가 자신이 잡은 것과 같은지 확인하고, 사라졌거나 교체됐으면(`.mam/` 재생성 → 새 데몬이 새 lock 을 잡을 수 있는 상태) 정리 후 rc 0 으로 끝난다. 브로커 불가 폴링 폴백도 매 회 같은 검사를 한다. 삭제된 pytest 임시 디렉터리나 `rm -rf` 된 프로젝트를 바라보며 공개 브로커 구독을 영원히 유지하던 데몬이 사라진다. +2. **데몬 자기 종료 (`reconcile.sh --subscribe`)**: MQTT 루프는 5초 주기 wake 마다 워크스페이스 디렉터리 존재와 `.mam/monitor.lock` 의 `(st_dev, st_ino)` 가 자신이 잡은 것과 같은지 확인하고, 사라졌거나 교체됐으면(`.mam/` 재생성 → 새 데몬이 새 lock 을 잡을 수 있는 상태) 정리 후 rc 0 으로 끝난다. 브로커 불가 폴링 폴백은 매 회 디렉터리와 lock 파일의 **존재만** 확인한다(python 자식이 끝나며 lock 을 놓았으므로 폴백 중 두 번째 데몬을 배제하지는 못한다 — 종전 동작). 삭제된 pytest 임시 디렉터리나 `rm -rf` 된 프로젝트를 바라보며 공개 브로커 구독을 영원히 유지하던 데몬이 사라진다. 3. **`SIGTERM`/`SIGINT` 즉시 전달 (`reconcile.sh`)**: python 구독자를 백그라운드 자식으로 두고 bash 가 `wait` 하도록 바꿔 시그널이 미뤄지지 않는다(종전엔 포그라운드 자식이 끝날 때까지 bash 가 시그널을 보류해 `SIGKILL` 만 통했다). 트랩이 자식에 `SIGTERM` 을 전달하고 rc 143 으로 끝나며, python 쪽은 핸들러로 이벤트를 깨워 `loop_stop`/`disconnect` 를 거친다. 폴백 루프의 `sleep` 도 `& wait` 로 바꿔 즉시 중단된다. -4. **샌드박스 회수·회귀 가드 (`tests/conftest.py`)**: `mam_sandbox` finalizer 가 샌드박스 경로를 cmdline 에 가진 `reconcile.sh --subscribe` 와 그 자식을 `SIGTERM`→`SIGKILL` 로 회수한다. 세션 스코프 `subscriber_daemon_leak_guard` 는 스위트 종료 시 basetemp 아래 데몬이 남아 있으면 회수한 뒤 실패시킨다(#15 레지스트리 가드와 같은 방식). +4. **샌드박스 회수·회귀 가드 (`tests/conftest.py`)**: `mam_sandbox` finalizer 가 argv 형태(`bash|sh <샌드박스>/…/reconcile.sh --subscribe`)로 선정한 데몬과 그 자식을 `SIGTERM`→`SIGKILL` 로 회수한다(브리프를 argv 에 실은 래퍼나 접두 충돌 형제 루트는 선정되지 않음). 세션 스코프 `subscriber_daemon_leak_guard` 는 스위트 종료 시 basetemp 아래 데몬이 남아 있으면 회수한 뒤 실패시킨다(#15 레지스트리 가드와 같은 방식). #### ✅ 품질 검증 -- **신규 테스트 11개** (`tests/test_issue19_subscriber_daemon_lifecycle.py`): 샌드박스 기본값에서 두 helper 본체 모두 데몬 미기동(2), 옵트아웃 중에도 기존 데몬 PID 보고, 리터럴 `1` 만 인정, `create_session.sh` 샌드박스 실행 후 데몬 없음, 옵트아웃 해제 시 누수 재현 + 회수기 제거, 회수기 단위 검증, 폴링 폴백의 lock 소실 종료·`SIGTERM` 즉시 종료(2, 브로커 불필요), MQTT 루프의 lock 교체 종료·`SIGTERM` 정상 종료(2, 브로커 미도달 시 skip). +- **신규 테스트 12개** (`tests/test_issue19_subscriber_daemon_lifecycle.py`): 샌드박스 기본값에서 두 helper 본체 모두 데몬 미기동(2), 옵트아웃 중에도 기존 데몬 PID 보고, 리터럴 `1` 만 인정, `create_session.sh` 샌드박스 실행 후 데몬 없음, 옵트아웃 해제 시 누수 재현 + 회수기 제거, 회수기 단위 검증·argv 선정 기준 검증, 폴링 폴백의 lock 소실 종료·`SIGTERM` 즉시 종료(2, 브로커 불필요), MQTT 루프의 lock 교체 종료·`SIGTERM` 정상 종료(2, 브로커 미도달 시 skip). - **기존 테스트 조정**: `tests/test_issue5_phase1_delegation.py` 의 watchdog/daemon 특성화 테스트가 `MAM_NO_SUBSCRIBER_DAEMON=0`(또는 delenv)으로 옵트아웃을 해제. -- **실측**: 스위트 실행 후 `pgrep -f 'pytest-of-.*reconcile.sh --subscribe'` 0건. +- **실측**: 전체 스위트(870 통과) 후 pytest basetemp 아래 `bash …/reconcile.sh --subscribe` 데몬 0건(argv 형태 기준; raw `pgrep -f` 는 브리프를 실은 루프 드라이버·래퍼도 매치하므로 판정에 쓰지 않음). ### 🚀 `v4.4.4` — Issue #12 레지스트리 비밀번호 제거, Issue #8 업데이트 스탬프 보존, Issue #7 형제 워크스페이스 패킹 (2026-09-13) diff --git a/tests/conftest.py b/tests/conftest.py index 51f5d55..e64f515 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -29,6 +29,23 @@ SKILLS_DIR = str(Path(__file__).resolve().parent.parent / ".agents" / "skills") if SKILLS_DIR not in sys.path: sys.path.insert(0, SKILLS_DIR) +def _is_subscriber_daemon_argv(args, root): + """True only for the daemon's own command line: `bash|sh /.../reconcile.sh + --subscribe ...`. A process that merely *quotes* those words (a loop driver + or delegate wrapper carrying a brief in argv) and a sibling root that is a + string prefix of `root` (`.../pytest-9` vs `.../pytest-99`) are excluded + (review R-2) - the reaper kills what it selects.""" + argv = args.split() + if len(argv) < 3: + return False + if os.path.basename(argv[0]) not in ("bash", "sh"): + return False + script = argv[1] + return (script.startswith(str(root).rstrip(os.sep) + os.sep) + and script.endswith(os.sep + "reconcile.sh") + and argv[2] == "--subscribe") + + def _subscriber_daemon_pids(root): """PIDs of `reconcile.sh --subscribe` daemons whose command line lives under `root`, plus their children (the python subscriber the bash wrapper waits on). @@ -40,7 +57,7 @@ def _subscriber_daemon_pids(root): root = str(root) found = [] for pid in _run_real(["pgrep", "-f", "reconcile.sh --subscribe"]).split(): - if root in _run_real(["ps", "-o", "args=", "-p", pid]): + if _is_subscriber_daemon_argv(_run_real(["ps", "-o", "args=", "-p", pid]), root): found.append(int(pid)) kids = [] for pid in found: diff --git a/tests/test_issue19_subscriber_daemon_lifecycle.py b/tests/test_issue19_subscriber_daemon_lifecycle.py index af7ba4b..cacafd6 100644 --- a/tests/test_issue19_subscriber_daemon_lifecycle.py +++ b/tests/test_issue19_subscriber_daemon_lifecycle.py @@ -58,11 +58,14 @@ def _run_helper(sandbox: Path, snippet: str, env=None) -> subprocess.CompletedPr def _daemons_for(sandbox: Path) -> list: + """Same argv-token selection as the conftest reaper (review R-2).""" + from conftest import _is_subscriber_daemon_argv + out = subprocess.run(["pgrep", "-f", "reconcile.sh --subscribe"], capture_output=True, text=True).stdout.split() hits = [] for pid in out: args = subprocess.run(["ps", "-o", "args=", "-p", pid], capture_output=True, text=True).stdout - if str(sandbox) in args: + if _is_subscriber_daemon_argv(args, str(sandbox)): hits.append(int(pid)) return hits @@ -210,6 +213,25 @@ def test_create_session_without_opt_out_spawns_a_daemon_the_reaper_removes(mam_s # 2. the sandbox reaper # --------------------------------------------------------------------------- +def test_reaper_selects_only_the_daemon_argv_shape(tmp_path): + """Review R-2: a process that quotes the words in a brief, or lives under + a sibling root that is a string prefix, must never be selected.""" + from conftest import _is_subscriber_daemon_argv + + root = str(tmp_path / "sbx1") + daemon = "bash {}/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh --subscribe --idle-timeout 0".format(root) + assert _is_subscriber_daemon_argv(daemon, root) + assert _is_subscriber_daemon_argv("/bin/sh " + daemon.split(" ", 1)[1], root) + # sibling root that is a prefix of this one + assert not _is_subscriber_daemon_argv(daemon, root[:-1]) + assert not _is_subscriber_daemon_argv(daemon, root + "0") + # brief-carrying wrapper that merely quotes the words and the path + quoting = "python3 -m lib_py.delegation.cli submit --prompt 'note: reconcile.sh --subscribe under {}/.agents/skills/x'".format(root) + assert not _is_subscriber_daemon_argv(quoting, root) + # the real reconcile.sh in a different mode + assert not _is_subscriber_daemon_argv(daemon.replace("--subscribe", "--once"), root) + assert not _is_subscriber_daemon_argv("", root) + def test_sandbox_reaper_terminates_daemons_anchored_in_the_sandbox(mam_sandbox, monkeypatch): from conftest import _reap_subscriber_daemons, _subscriber_daemon_pids from lib_py.delegation import daemon From 8f9745b1286f35c1fd6a6d05eba3ff77ddc421cf Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 13 Sep 2026 16:35:23 +0900 Subject: [PATCH 5/7] fix(issue-19): match the daemon's whole script path so spaced sandbox paths are reaped Review round 2 (planner-reviewer-codex-01, NOT PASS P2): - conftest `_is_subscriber_daemon_argv`: `ps -o args=` joins argv unquoted, so the script path is taken from the shell word up to the first `/reconcile.sh --subscribe` instead of as one whitespace token. A sandbox or --basetemp path with a space no longer hides its daemon from the fixture reaper and the session leak guard. A candidate with whitespace counts only when it names an existing file, so a bash wrapper under the root whose later argv carries the words is still excluded. - tests: a whitespace-path selector regression and an owned live-process cleanup check (both fail on c096cb4); the R-2 test also pins the bash wrapper exclusion. - docs: the polling fallback's signal handling is stated precisely in monitor SKILL.md and VERSIONS.md (a running --once pass finishes first, the interval's sleep child ends on its own); create SKILL.md says an already-running daemon's PID is still reported under the opt-out; VERSIONS selector wording and test count (14). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe --- .../skills/multi-agent-mux-create/SKILL.md | 2 +- .../skills/multi-agent-mux-monitor/SKILL.md | 5 +- VERSIONS.md | 6 +- tests/conftest.py | 28 +++++--- ...est_issue19_subscriber_daemon_lifecycle.py | 68 ++++++++++++++++++- 5 files changed, 95 insertions(+), 14 deletions(-) diff --git a/.agents/skills/multi-agent-mux-create/SKILL.md b/.agents/skills/multi-agent-mux-create/SKILL.md index 79df6b2..608d7c9 100644 --- a/.agents/skills/multi-agent-mux-create/SKILL.md +++ b/.agents/skills/multi-agent-mux-create/SKILL.md @@ -68,7 +68,7 @@ If any check fails → abort with a non-zero exit and report the reason (automat When running multiple agent sessions alongside other workflows (e.g., cmux, background workers, manual herdr sessions), sharing the default herdr server can lead to session name conflicts, monitoring clutter, and accidental destruction of user sessions via global commands. To prevent this, you can run this skill inside an **isolated herdr session** using the `HERDR_SESSION_NAME` environment variable or the `--herdr-session ` flag (opt-in; alias: `--herdr-server`; legacy env alias: `HERDR_SERVER_NAME`). -On success the script also starts the per-workspace monitor daemon through `start_watchdog` (`reconcile.sh --subscribe --idle-timeout 0`, one per workspace, see the lifecycle contract in [multi-agent-mux-monitor/SKILL.md](../multi-agent-mux-monitor/SKILL.md)). Set `MAM_NO_SUBSCRIBER_DAEMON=1` to skip that spawn when the monitor is supervised elsewhere or the workspace is ephemeral (Issue #19); the `watchdog PID:` line is then empty. +On success the script also starts the per-workspace monitor daemon through `start_watchdog` (`reconcile.sh --subscribe --idle-timeout 0`, one per workspace, see the lifecycle contract in [multi-agent-mux-monitor/SKILL.md](../multi-agent-mux-monitor/SKILL.md)). Set `MAM_NO_SUBSCRIBER_DAEMON=1` to skip that spawn when the monitor is supervised elsewhere or the workspace is ephemeral (Issue #19); the `watchdog PID:` line is then empty unless a daemon already runs for the workspace, whose PID is still reported. Additionally, you can specify `--herdr-workspace ` (default: workspace slug without `mam-` prefix) to name and group the agent panes inside a dedicated Herdr workspace tab in the Herdr runtime (`herdr workspace create --label` / `rename`) as well as recording it in `.mam/agent-sessions.yaml`. Note that `--herdr-workspace` configures the workspace tab label within the session, whereas `--herdr-session` selects the daemon socket itself. diff --git a/.agents/skills/multi-agent-mux-monitor/SKILL.md b/.agents/skills/multi-agent-mux-monitor/SKILL.md index dd7e7b1..45159e3 100644 --- a/.agents/skills/multi-agent-mux-monitor/SKILL.md +++ b/.agents/skills/multi-agent-mux-monitor/SKILL.md @@ -84,7 +84,10 @@ per workspace through `lib.sh::start_watchdog` (`lib_py.delegation.daemon`). Tha the lock (the python subscriber released it on exit), so while polling it does not exclude a second daemon, as before; - **stops promptly on `SIGTERM`/`SIGINT`** — the bash wrapper forwards the signal to the - python subscriber and exits 143. `SIGKILL` is never required; + python subscriber and exits 143. In the broker-down polling fallback the signal cuts the + interval wait short, but a reconcile pass (`--once`) already running finishes first, and + the interval's `sleep` child is left to end on its own (≤ `RECONCILE_POLL_INTERVAL`). + `SIGKILL` is never required; - is **not started when `MAM_NO_SUBSCRIBER_DAEMON=1`** — the literal `1` only. Use it when the monitor is supervised elsewhere; a daemon that already runs is still reported. Test sandboxes set it by default and additionally reap any daemon anchored in the sandbox. diff --git a/VERSIONS.md b/VERSIONS.md index a5a83af..214d17c 100644 --- a/VERSIONS.md +++ b/VERSIONS.md @@ -44,11 +44,11 @@ #### 🌟 주요 개선 사항 1. **`MAM_NO_SUBSCRIBER_DAEMON=1` 옵트아웃 (`lib_py/delegation/daemon.py`, `lib.sh: _start_watchdog_legacy`)**: 리터럴 `1` 일 때만 데몬을 새로 띄우지 않는다. 이미 도는 데몬의 PID 는 그대로 보고하므로 `start_watchdog`/`ensure_subscriber_daemon` 의 dedup 계약은 유지되고, 미기동 시 stderr 에 한 줄 남긴다(stdout 은 비움). 테스트 샌드박스(`mam_sandbox`)가 기본으로 켜고, 데몬 자체를 검증하는 테스트만 `0`/삭제로 되돌린다. 2. **데몬 자기 종료 (`reconcile.sh --subscribe`)**: MQTT 루프는 5초 주기 wake 마다 워크스페이스 디렉터리 존재와 `.mam/monitor.lock` 의 `(st_dev, st_ino)` 가 자신이 잡은 것과 같은지 확인하고, 사라졌거나 교체됐으면(`.mam/` 재생성 → 새 데몬이 새 lock 을 잡을 수 있는 상태) 정리 후 rc 0 으로 끝난다. 브로커 불가 폴링 폴백은 매 회 디렉터리와 lock 파일의 **존재만** 확인한다(python 자식이 끝나며 lock 을 놓았으므로 폴백 중 두 번째 데몬을 배제하지는 못한다 — 종전 동작). 삭제된 pytest 임시 디렉터리나 `rm -rf` 된 프로젝트를 바라보며 공개 브로커 구독을 영원히 유지하던 데몬이 사라진다. -3. **`SIGTERM`/`SIGINT` 즉시 전달 (`reconcile.sh`)**: python 구독자를 백그라운드 자식으로 두고 bash 가 `wait` 하도록 바꿔 시그널이 미뤄지지 않는다(종전엔 포그라운드 자식이 끝날 때까지 bash 가 시그널을 보류해 `SIGKILL` 만 통했다). 트랩이 자식에 `SIGTERM` 을 전달하고 rc 143 으로 끝나며, python 쪽은 핸들러로 이벤트를 깨워 `loop_stop`/`disconnect` 를 거친다. 폴백 루프의 `sleep` 도 `& wait` 로 바꿔 즉시 중단된다. -4. **샌드박스 회수·회귀 가드 (`tests/conftest.py`)**: `mam_sandbox` finalizer 가 argv 형태(`bash|sh <샌드박스>/…/reconcile.sh --subscribe`)로 선정한 데몬과 그 자식을 `SIGTERM`→`SIGKILL` 로 회수한다(브리프를 argv 에 실은 래퍼나 접두 충돌 형제 루트는 선정되지 않음). 세션 스코프 `subscriber_daemon_leak_guard` 는 스위트 종료 시 basetemp 아래 데몬이 남아 있으면 회수한 뒤 실패시킨다(#15 레지스트리 가드와 같은 방식). +3. **`SIGTERM`/`SIGINT` 즉시 전달 (`reconcile.sh`)**: python 구독자를 백그라운드 자식으로 두고 bash 가 `wait` 하도록 바꿔 시그널이 미뤄지지 않는다(종전엔 포그라운드 자식이 끝날 때까지 bash 가 시그널을 보류해 `SIGKILL` 만 통했다). 트랩이 자식에 `SIGTERM` 을 전달하고 rc 143 으로 끝나며, python 쪽은 핸들러로 이벤트를 깨워 `loop_stop`/`disconnect` 를 거친다. 폴백 루프의 대기도 `sleep … & wait` 로 바꿔 시그널이 곧바로 처리된다(끊긴 대기의 `sleep` 자식은 남은 간격 안에 스스로 끝나고, 이미 돌고 있는 `--once` 조정 1회는 끝난 뒤 처리된다). +4. **샌드박스 회수·회귀 가드 (`tests/conftest.py`)**: `mam_sandbox` finalizer 가 argv 형태(`bash|sh <샌드박스>/…/reconcile.sh --subscribe`)로 선정한 데몬과 그 자식을 `SIGTERM`→`SIGKILL` 로 회수한다(브리프를 argv 에 실은 래퍼나 접두 충돌 형제 루트는 선정되지 않음). `ps -o args=` 는 argv 를 따옴표 없이 공백으로 이어 붙이므로 스크립트 경로는 공백 토큰이 아니라 셸 단어부터 첫 `/reconcile.sh --subscribe` 까지 통째로 잡는다 — 공백이 든 샌드박스·`--basetemp` 경로의 데몬도 선정되고, 공백이 든 후보는 실제 파일일 때만 인정해 뒤쪽 인자에 같은 낱말을 실은 bash 래퍼는 배제한다. 세션 스코프 `subscriber_daemon_leak_guard` 는 스위트 종료 시 basetemp 아래 데몬이 남아 있으면 회수한 뒤 실패시킨다(#15 레지스트리 가드와 같은 방식). #### ✅ 품질 검증 -- **신규 테스트 12개** (`tests/test_issue19_subscriber_daemon_lifecycle.py`): 샌드박스 기본값에서 두 helper 본체 모두 데몬 미기동(2), 옵트아웃 중에도 기존 데몬 PID 보고, 리터럴 `1` 만 인정, `create_session.sh` 샌드박스 실행 후 데몬 없음, 옵트아웃 해제 시 누수 재현 + 회수기 제거, 회수기 단위 검증·argv 선정 기준 검증, 폴링 폴백의 lock 소실 종료·`SIGTERM` 즉시 종료(2, 브로커 불필요), MQTT 루프의 lock 교체 종료·`SIGTERM` 정상 종료(2, 브로커 미도달 시 skip). +- **신규 테스트 14개** (`tests/test_issue19_subscriber_daemon_lifecycle.py`): 샌드박스 기본값에서 두 helper 본체 모두 데몬 미기동(2), 옵트아웃 중에도 기존 데몬 PID 보고, 리터럴 `1` 만 인정, `create_session.sh` 샌드박스 실행 후 데몬 없음, 옵트아웃 해제 시 누수 재현 + 회수기 제거, 회수기 단위 검증·argv 선정 기준 검증, 공백이 든 경로의 argv 선정·실제 데몬 회수(2), 폴링 폴백의 lock 소실 종료·`SIGTERM` 즉시 종료(2, 브로커 불필요), MQTT 루프의 lock 교체 종료·`SIGTERM` 정상 종료(2, 브로커 미도달 시 skip). - **기존 테스트 조정**: `tests/test_issue5_phase1_delegation.py` 의 watchdog/daemon 특성화 테스트가 `MAM_NO_SUBSCRIBER_DAEMON=0`(또는 delenv)으로 옵트아웃을 해제. - **실측**: 전체 스위트(870 통과) 후 pytest basetemp 아래 `bash …/reconcile.sh --subscribe` 데몬 0건(argv 형태 기준; raw `pgrep -f` 는 브리프를 실은 루프 드라이버·래퍼도 매치하므로 판정에 쓰지 않음). diff --git a/tests/conftest.py b/tests/conftest.py index e64f515..32587ad 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -34,16 +34,28 @@ def _is_subscriber_daemon_argv(args, root): --subscribe ...`. A process that merely *quotes* those words (a loop driver or delegate wrapper carrying a brief in argv) and a sibling root that is a string prefix of `root` (`.../pytest-9` vs `.../pytest-99`) are excluded - (review R-2) - the reaper kills what it selects.""" - argv = args.split() - if len(argv) < 3: + (review R-2) - the reaper kills what it selects. + + `ps -o args=` joins argv with single spaces and quotes nothing, so the + script path is taken whole - from the shell word up to the first + `/reconcile.sh --subscribe` - not as one whitespace token: a sandbox or + `--basetemp` path with a space in it must not hide its daemon from the + reaper and the leak guard (codex review P2). In that flat form a path with + whitespace is ambiguous - a bash wrapper under `root` can carry the words + later in its argv - so such a path counts only when it names an existing + file. A daemon whose workspace is already gone exits on its own (Issue #19).""" + import re as _re + shell, _, rest = args.strip().partition(" ") + if os.path.basename(shell) not in ("bash", "sh"): return False - if os.path.basename(argv[0]) not in ("bash", "sh"): + rest = rest.lstrip() + if not rest.startswith(str(root).rstrip(os.sep) + os.sep): return False - script = argv[1] - return (script.startswith(str(root).rstrip(os.sep) + os.sep) - and script.endswith(os.sep + "reconcile.sh") - and argv[2] == "--subscribe") + m = _re.match(r"(.*?" + _re.escape(os.sep) + r"reconcile\.sh) --subscribe(?: |$)", rest) + if not m: + return False + script = m.group(1) + return not any(c.isspace() for c in script) or os.path.isfile(script) def _subscriber_daemon_pids(root): diff --git a/tests/test_issue19_subscriber_daemon_lifecycle.py b/tests/test_issue19_subscriber_daemon_lifecycle.py index cacafd6..23e5d9f 100644 --- a/tests/test_issue19_subscriber_daemon_lifecycle.py +++ b/tests/test_issue19_subscriber_daemon_lifecycle.py @@ -58,7 +58,7 @@ def _run_helper(sandbox: Path, snippet: str, env=None) -> subprocess.CompletedPr def _daemons_for(sandbox: Path) -> list: - """Same argv-token selection as the conftest reaper (review R-2).""" + """Same argv-shape selection as the conftest reaper (review R-2, P2).""" from conftest import _is_subscriber_daemon_argv out = subprocess.run(["pgrep", "-f", "reconcile.sh --subscribe"], capture_output=True, text=True).stdout.split() @@ -228,10 +228,76 @@ def test_reaper_selects_only_the_daemon_argv_shape(tmp_path): # brief-carrying wrapper that merely quotes the words and the path quoting = "python3 -m lib_py.delegation.cli submit --prompt 'note: reconcile.sh --subscribe under {}/.agents/skills/x'".format(root) assert not _is_subscriber_daemon_argv(quoting, root) + # ...and one run by bash from under the root, its prompt ending in the daemon's words + wrapper = "bash {0}/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job submit --prompt restart {1}".format(root, daemon) + assert not _is_subscriber_daemon_argv(wrapper, root) # the real reconcile.sh in a different mode assert not _is_subscriber_daemon_argv(daemon.replace("--subscribe", "--once"), root) assert not _is_subscriber_daemon_argv("", root) + +def _spaced_monitor(tmp_path: Path): + """A monitor script under a sandbox path with a space in it, as a pytest + `--basetemp` or temp dir may have (codex review P2).""" + root = tmp_path / "pytest review" / "case" + script = root / ".agents" / "skills" / "multi-agent-mux-monitor" / "scripts" / "reconcile.sh" + script.parent.mkdir(parents=True) + script.write_text("#!/usr/bin/env bash\nwhile :; do sleep 1; done\n", encoding="utf-8") + script.chmod(0o755) + return root, script + + +def test_reaper_selects_daemons_under_whitespace_paths(tmp_path): + """`ps -o args=` does not quote: the script path must be matched whole, both + at the sandbox root (fixture reaper) and at an ancestor (session guard).""" + from conftest import _is_subscriber_daemon_argv + + root, script = _spaced_monitor(tmp_path) + daemon = "bash {} --subscribe --idle-timeout 0".format(script) + assert _is_subscriber_daemon_argv(daemon + "\n", str(root)) + assert _is_subscriber_daemon_argv(daemon, str(tmp_path)) + assert _is_subscriber_daemon_argv("/bin/sh " + daemon.split(" ", 1)[1], str(root)) + # the text before the space names another root, not an ancestor + assert not _is_subscriber_daemon_argv(daemon, str(tmp_path / "pytest")) + # sibling root that this one is a string prefix of + assert not _is_subscriber_daemon_argv(daemon.replace("/case/", "/case2/"), str(root)) + assert not _is_subscriber_daemon_argv(daemon.replace("--subscribe", "--once"), str(root)) + # wrappers carrying the words: python, and bash from under the spaced root + quoting = "python3 -m lib_py.delegation.cli submit --prompt note: {}".format(daemon) + assert not _is_subscriber_daemon_argv(quoting, str(root)) + wrapper = "bash {0}/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job submit --prompt restart {1}".format(root, daemon) + assert not _is_subscriber_daemon_argv(wrapper, str(root)) + + +def test_reaper_removes_a_live_daemon_under_a_whitespace_path(tmp_path): + """The owned live-process half of P2: the reaper and the leak guard find a + running `bash /reconcile.sh --subscribe` and take it down.""" + from conftest import _reap_subscriber_daemons, _subscriber_daemon_pids + + root, script = _spaced_monitor(tmp_path) + proc = subprocess.Popen( + ["bash", str(script), "--subscribe", "--idle-timeout", "0"], + stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + start_new_session=True, + ) + try: + deadline = time.time() + 5 + while time.time() < deadline and proc.pid not in _subscriber_daemon_pids(root): + time.sleep(0.1) + assert proc.pid in _subscriber_daemon_pids(root), "fixture reaper does not see the daemon" + assert proc.pid in _subscriber_daemon_pids(tmp_path), "session leak guard does not see the daemon" + assert proc.pid in _reap_subscriber_daemons(root) + assert _wait_exit(proc, 5) is not None, "reaper left the daemon running" + assert _subscriber_daemon_pids(root) == [] + finally: + if proc.poll() is None: + try: + os.killpg(proc.pid, signal.SIGKILL) + except OSError: + pass + proc.wait(timeout=5) + + def test_sandbox_reaper_terminates_daemons_anchored_in_the_sandbox(mam_sandbox, monkeypatch): from conftest import _reap_subscriber_daemons, _subscriber_daemon_pids from lib_py.delegation import daemon From 134fe9704c63bd9b1f46b64b01868f904f5a33cc Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 13 Sep 2026 17:22:39 +0900 Subject: [PATCH 6/7] fix(issue-19): arm the subscriber's stop handlers before it connects; read $! with nounset off Review round 3 (reviewer-grok-01, NOT PASS N1; non-blocking B1): - reconcile.sh (N1): the python subscriber installs its SIGTERM/SIGINT handlers before connect/loop_start. A signal during the broker handshake, the CONNACK wait, or right after the paho thread logs "subscribed to" now takes the clean path (the signal line, loop_stop/disconnect, rc 0) instead of Python's default action; the CONNACK wait ends on it, and a signalled subscriber never falls back to polling (rc 3). - reconcile.sh (B1): `_sub_stop` reads `$!` with nounset off. On bash 3.2 `${!:-}` and `${!+x}` are both unbound before the first `&`, so the trap died with rc 1 instead of 143. - tests: SIGTERM during a CONNACK wait against a silent local listener (no broker needed), and the verbatim `_sub_stop` run without and with a child; both fail on 8f9745b. - docs: monitor SKILL.md and VERSIONS.md state the handler order; test count 16. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe --- .../skills/multi-agent-mux-monitor/SKILL.md | 4 +- .../scripts/reconcile.sh | 64 ++++++++++++------- VERSIONS.md | 4 +- ...est_issue19_subscriber_daemon_lifecycle.py | 52 +++++++++++++++ 4 files changed, 97 insertions(+), 27 deletions(-) diff --git a/.agents/skills/multi-agent-mux-monitor/SKILL.md b/.agents/skills/multi-agent-mux-monitor/SKILL.md index 45159e3..6a5e70a 100644 --- a/.agents/skills/multi-agent-mux-monitor/SKILL.md +++ b/.agents/skills/multi-agent-mux-monitor/SKILL.md @@ -84,7 +84,9 @@ per workspace through `lib.sh::start_watchdog` (`lib_py.delegation.daemon`). Tha the lock (the python subscriber released it on exit), so while polling it does not exclude a second daemon, as before; - **stops promptly on `SIGTERM`/`SIGINT`** — the bash wrapper forwards the signal to the - python subscriber and exits 143. In the broker-down polling fallback the signal cuts the + python subscriber and exits 143; the subscriber's handlers are in place before it + connects, so a signal during the broker handshake or right after it subscribes takes + the same clean path. In the broker-down polling fallback the signal cuts the interval wait short, but a reconcile pass (`--once`) already running finishes first, and the interval's `sleep` child is left to end on its own (≤ `RECONCILE_POLL_INTERVAL`). `SIGKILL` is never required; diff --git a/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh b/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh index 1d0dffa..fbf25fc 100755 --- a/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh +++ b/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh @@ -80,8 +80,15 @@ if [ "$SUBSCRIBE" = "1" ]; then _sub_stop() { trap - TERM INT # `$!` covers the window between `&` and `_sub_pid=$!` (review R-1); a - # child that already exited makes kill/wait harmless no-ops. - local child="${_sub_pid:-${!:-}}" + # child that already exited makes kill/wait harmless no-ops. Before the + # first `&` bash 3.2 treats `$!` as unbound under `set -u` - `${!:-}` and + # `${!+x}` included - so it is read with nounset off (review B1). + local child="${_sub_pid:-}" + if [ -z "$child" ]; then + set +u + child="$!" + set -u + fi if [ -n "$child" ]; then kill -TERM "$child" 2>/dev/null || true wait "$child" 2>/dev/null || true @@ -290,28 +297,6 @@ def on_connect(_c, _u, _flags, reason_code, _props): print(f"MQTT Monitor connection failed: rc={rc}", flush=True) -cfg = mqtt_common.broker_config_from_env() -client = mqtt_common.make_client("monitor_sub", cfg) -client.on_message = on_message -client.on_connect = on_connect -print(f"MQTT Monitor: connecting to {cfg.host}:{cfg.port} (TLS={cfg.tls})...", flush=True) - -# Connection failure → fall back to polling (review item 4). -try: - client.connect(cfg.host, cfg.port, cfg.keepalive) -except Exception as e: - print(f"MQTT Monitor: connect failed ({e}); falling back to polling", flush=True) - sys.exit(3) - -client.loop_start() -_wait = time.time() -while time.time() - _wait < 5 and not state['connected'] and not state['failed']: - time.sleep(0.1) -if not state['connected']: - print("MQTT Monitor: broker did not accept connection; falling back to polling", flush=True) - client.loop_stop() - sys.exit(3) - import threading, signal stop_event = threading.Event() state['stop_signal'] = None @@ -322,8 +307,39 @@ def _on_stop_signal(signum, _frame): stop_event.set() +# Issue #19 (review N1): the handlers go in before connect/loop_start. The paho +# thread logs "subscribed to" from on_connect, and a SIGTERM during the broker +# handshake or right after that line must take the same clean path as a later +# one - the "signal N received" line and loop_stop/disconnect - not Python's +# default action. A signalled subscriber exits 0, never 3: rc 3 would send the +# bash wrapper into the polling fallback. signal.signal(signal.SIGTERM, _on_stop_signal) signal.signal(signal.SIGINT, _on_stop_signal) + +cfg = mqtt_common.broker_config_from_env() +client = mqtt_common.make_client("monitor_sub", cfg) +client.on_message = on_message +client.on_connect = on_connect +print(f"MQTT Monitor: connecting to {cfg.host}:{cfg.port} (TLS={cfg.tls})...", flush=True) + +# Connection failure → fall back to polling (review item 4). +try: + client.connect(cfg.host, cfg.port, cfg.keepalive) +except Exception as e: + if state['stop_signal'] is None: + print(f"MQTT Monitor: connect failed ({e}); falling back to polling", flush=True) + sys.exit(3) +else: + client.loop_start() + _wait = time.time() + while (time.time() - _wait < 5 and not state['connected'] and not state['failed'] + and not stop_event.is_set()): + stop_event.wait(0.1) + if not state['connected'] and state['stop_signal'] is None: + print("MQTT Monitor: broker did not accept connection; falling back to polling", flush=True) + client.loop_stop() + sys.exit(3) + start = time.time() try: while True: diff --git a/VERSIONS.md b/VERSIONS.md index 214d17c..403ce79 100644 --- a/VERSIONS.md +++ b/VERSIONS.md @@ -44,11 +44,11 @@ #### 🌟 주요 개선 사항 1. **`MAM_NO_SUBSCRIBER_DAEMON=1` 옵트아웃 (`lib_py/delegation/daemon.py`, `lib.sh: _start_watchdog_legacy`)**: 리터럴 `1` 일 때만 데몬을 새로 띄우지 않는다. 이미 도는 데몬의 PID 는 그대로 보고하므로 `start_watchdog`/`ensure_subscriber_daemon` 의 dedup 계약은 유지되고, 미기동 시 stderr 에 한 줄 남긴다(stdout 은 비움). 테스트 샌드박스(`mam_sandbox`)가 기본으로 켜고, 데몬 자체를 검증하는 테스트만 `0`/삭제로 되돌린다. 2. **데몬 자기 종료 (`reconcile.sh --subscribe`)**: MQTT 루프는 5초 주기 wake 마다 워크스페이스 디렉터리 존재와 `.mam/monitor.lock` 의 `(st_dev, st_ino)` 가 자신이 잡은 것과 같은지 확인하고, 사라졌거나 교체됐으면(`.mam/` 재생성 → 새 데몬이 새 lock 을 잡을 수 있는 상태) 정리 후 rc 0 으로 끝난다. 브로커 불가 폴링 폴백은 매 회 디렉터리와 lock 파일의 **존재만** 확인한다(python 자식이 끝나며 lock 을 놓았으므로 폴백 중 두 번째 데몬을 배제하지는 못한다 — 종전 동작). 삭제된 pytest 임시 디렉터리나 `rm -rf` 된 프로젝트를 바라보며 공개 브로커 구독을 영원히 유지하던 데몬이 사라진다. -3. **`SIGTERM`/`SIGINT` 즉시 전달 (`reconcile.sh`)**: python 구독자를 백그라운드 자식으로 두고 bash 가 `wait` 하도록 바꿔 시그널이 미뤄지지 않는다(종전엔 포그라운드 자식이 끝날 때까지 bash 가 시그널을 보류해 `SIGKILL` 만 통했다). 트랩이 자식에 `SIGTERM` 을 전달하고 rc 143 으로 끝나며, python 쪽은 핸들러로 이벤트를 깨워 `loop_stop`/`disconnect` 를 거친다. 폴백 루프의 대기도 `sleep … & wait` 로 바꿔 시그널이 곧바로 처리된다(끊긴 대기의 `sleep` 자식은 남은 간격 안에 스스로 끝나고, 이미 돌고 있는 `--once` 조정 1회는 끝난 뒤 처리된다). +3. **`SIGTERM`/`SIGINT` 즉시 전달 (`reconcile.sh`)**: python 구독자를 백그라운드 자식으로 두고 bash 가 `wait` 하도록 바꿔 시그널이 미뤄지지 않는다(종전엔 포그라운드 자식이 끝날 때까지 bash 가 시그널을 보류해 `SIGKILL` 만 통했다). 트랩이 자식에 `SIGTERM` 을 전달하고 rc 143 으로 끝나며(첫 백그라운드 자식 전의 시그널 포함), python 쪽 핸들러는 브로커 연결(`connect`/`loop_start`) 전에 설치되어 핸드셰이크·CONNACK 대기 중이나 구독 직후의 시그널도 이벤트를 깨워 `loop_stop`/`disconnect` 를 거친 뒤 rc 0 으로 끝난다(시그널을 받은 뒤에는 폴링 폴백으로 가지 않는다). 폴백 루프의 대기도 `sleep … & wait` 로 바꿔 시그널이 곧바로 처리된다(끊긴 대기의 `sleep` 자식은 남은 간격 안에 스스로 끝나고, 이미 돌고 있는 `--once` 조정 1회는 끝난 뒤 처리된다). 4. **샌드박스 회수·회귀 가드 (`tests/conftest.py`)**: `mam_sandbox` finalizer 가 argv 형태(`bash|sh <샌드박스>/…/reconcile.sh --subscribe`)로 선정한 데몬과 그 자식을 `SIGTERM`→`SIGKILL` 로 회수한다(브리프를 argv 에 실은 래퍼나 접두 충돌 형제 루트는 선정되지 않음). `ps -o args=` 는 argv 를 따옴표 없이 공백으로 이어 붙이므로 스크립트 경로는 공백 토큰이 아니라 셸 단어부터 첫 `/reconcile.sh --subscribe` 까지 통째로 잡는다 — 공백이 든 샌드박스·`--basetemp` 경로의 데몬도 선정되고, 공백이 든 후보는 실제 파일일 때만 인정해 뒤쪽 인자에 같은 낱말을 실은 bash 래퍼는 배제한다. 세션 스코프 `subscriber_daemon_leak_guard` 는 스위트 종료 시 basetemp 아래 데몬이 남아 있으면 회수한 뒤 실패시킨다(#15 레지스트리 가드와 같은 방식). #### ✅ 품질 검증 -- **신규 테스트 14개** (`tests/test_issue19_subscriber_daemon_lifecycle.py`): 샌드박스 기본값에서 두 helper 본체 모두 데몬 미기동(2), 옵트아웃 중에도 기존 데몬 PID 보고, 리터럴 `1` 만 인정, `create_session.sh` 샌드박스 실행 후 데몬 없음, 옵트아웃 해제 시 누수 재현 + 회수기 제거, 회수기 단위 검증·argv 선정 기준 검증, 공백이 든 경로의 argv 선정·실제 데몬 회수(2), 폴링 폴백의 lock 소실 종료·`SIGTERM` 즉시 종료(2, 브로커 불필요), MQTT 루프의 lock 교체 종료·`SIGTERM` 정상 종료(2, 브로커 미도달 시 skip). +- **신규 테스트 16개** (`tests/test_issue19_subscriber_daemon_lifecycle.py`): 샌드박스 기본값에서 두 helper 본체 모두 데몬 미기동(2), 옵트아웃 중에도 기존 데몬 PID 보고, 리터럴 `1` 만 인정, `create_session.sh` 샌드박스 실행 후 데몬 없음, 옵트아웃 해제 시 누수 재현 + 회수기 제거, 회수기 단위 검증·argv 선정 기준 검증, 공백이 든 경로의 argv 선정·실제 데몬 회수(2), 폴링 폴백의 lock 소실 종료·`SIGTERM` 즉시 종료(2, 브로커 불필요), CONNACK 대기 중 `SIGTERM` 정상 종료(응답 없는 로컬 리스너, 브로커 불필요), 첫 백그라운드 자식 전의 시그널에도 `_sub_stop` 트랩 rc 143(bash 3.2 `set -u`), MQTT 루프의 lock 교체 종료·`SIGTERM` 정상 종료(2, 브로커 미도달 시 skip). - **기존 테스트 조정**: `tests/test_issue5_phase1_delegation.py` 의 watchdog/daemon 특성화 테스트가 `MAM_NO_SUBSCRIBER_DAEMON=0`(또는 delenv)으로 옵트아웃을 해제. - **실측**: 전체 스위트(870 통과) 후 pytest basetemp 아래 `bash …/reconcile.sh --subscribe` 데몬 0건(argv 형태 기준; raw `pgrep -f` 는 브리프를 실은 루프 드라이버·래퍼도 매치하므로 판정에 쓰지 않음). diff --git a/tests/test_issue19_subscriber_daemon_lifecycle.py b/tests/test_issue19_subscriber_daemon_lifecycle.py index 23e5d9f..7b37e0e 100644 --- a/tests/test_issue19_subscriber_daemon_lifecycle.py +++ b/tests/test_issue19_subscriber_daemon_lifecycle.py @@ -358,6 +358,58 @@ def test_poll_fallback_sigterm_exits_promptly(mam_sandbox, mock_herdr, monkeypat proc.kill() +def test_sigterm_during_broker_handshake_takes_the_clean_path(mam_sandbox, mock_herdr, monkeypatch): + """Review N1: the python handlers are installed before connect/loop_start, + so a SIGTERM while the subscriber still waits for CONNACK (a local listener + that never answers) exits through the same clean path as a later one - the + "signal 15 received" line, rc 143 from the wrapper - and never drops into + the polling fallback.""" + import socket + + monkeypatch.delenv("MAM_NO_SUBSCRIBER_DAEMON", raising=False) + silent = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + silent.bind(("127.0.0.1", 0)) + silent.listen(1) # the kernel completes the TCP handshake; no CONNACK ever comes + log = mam_sandbox / "sub.log" + proc = _start_subscriber(mam_sandbox, log, { + "MQTT_BROKER": "127.0.0.1", "MQTT_PORT": str(silent.getsockname()[1]), "MQTT_TLS": "0", + }) + try: + assert _wait_for(log, r"MQTT Monitor: connecting to 127\.0\.0\.1", 30), log.read_text() + time.sleep(1.0) # well inside the 5 s CONNACK wait + t0 = time.time() + proc.send_signal(signal.SIGTERM) + rc = _wait_exit(proc, 10) + assert rc is not None, "SIGTERM was deferred: daemon still running after 10s" + assert time.time() - t0 < 5 + assert rc == 143, (rc, log.read_text()) + text = log.read_text() + assert "signal 15 received, exiting" in text, text + assert "falling back to polling" not in text, text + finally: + if proc.poll() is None: + proc.kill() + silent.close() + + +def test_sub_stop_trap_exits_143_before_the_first_background_child(): + """Review B1: the wrapper's `_sub_stop` trap must end with rc 143 even when + the signal lands before the first `&` - bash 3.2 treats `$!` as unbound + under `set -u` there, and the trap died with rc 1. The function is taken + verbatim from reconcile.sh and run without and with a child to stop.""" + src = (Path(__file__).resolve().parent.parent / ".agents" / "skills" / "multi-agent-mux-monitor" + / "scripts" / "reconcile.sh").read_text(encoding="utf-8") + m = re.search(r"^ _sub_stop\(\) \{\n.*?^ \}\n", src, re.S | re.M) + assert m, "_sub_stop not found in reconcile.sh" + # With a child, let it exec first: a TERM forwarded in the instant between + # its fork and its exec is swallowed by the child's copy of the trap - a + # separate window from B1, which is about reading `$!` at all. + for child in ("", "sleep 30 &\nsleep 0.5\n"): + script = "set -euo pipefail\n_sub_pid=''\n{}trap _sub_stop TERM INT\n{}kill -TERM $$\nsleep 5\n".format(m.group(0), child) + res = subprocess.run(["bash", "-c", script], capture_output=True, text=True, timeout=30) + assert res.returncode == 143, (child, res.returncode, res.stderr) + + # --------------------------------------------------------------------------- # 4. self-termination and signals - MQTT loop (needs a reachable broker) # --------------------------------------------------------------------------- From 1bd643e9795ea41070c5ee2199516d12165083e4 Mon Sep 17 00:00:00 2001 From: Godopu Date: Sun, 13 Sep 2026 17:54:52 +0900 Subject: [PATCH 7/7] docs(reports): archive v4.4.5 unanimous PASS review reports (Issue #19) Round 3 at 134fe97: claude, agy, grok and codex all PASS. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- .../report-ff5f9925.md | 184 ++++++++++++++++++ .../report-b30d151d.md | 39 ++++ .../reviewer-agy-01/report-e2420d1b.md | 109 +++++++++++ .../reviewer-grok-01/report-b6915b56.md | 51 +++++ 4 files changed, 383 insertions(+) create mode 100644 .agents/reports/planner-reviewer-claude-01/report-ff5f9925.md create mode 100644 .agents/reports/planner-reviewer-codex-01/report-b30d151d.md create mode 100644 .agents/reports/reviewer-agy-01/report-e2420d1b.md create mode 100644 .agents/reports/reviewer-grok-01/report-b6915b56.md diff --git a/.agents/reports/planner-reviewer-claude-01/report-ff5f9925.md b/.agents/reports/planner-reviewer-claude-01/report-ff5f9925.md new file mode 100644 index 0000000..35c251d --- /dev/null +++ b/.agents/reports/planner-reviewer-claude-01/report-ff5f9925.md @@ -0,0 +1,184 @@ +# 🔍 Review Report — Job ff5f9925 (branch `fix/issue-19`, reviewed HEAD `134fe97`) + +- **Reviewer**: `planner-reviewer-claude-01` (role: planner,reviewer) · 2026-09-13 +- **대상**: `git log 8e7282c..HEAD` 6커밋, `git diff 8e7282c...HEAD` 22파일 +791/−50. + - 커밋: `ce315d8`(fix #19), `38d8ccb`(release v4.4.5), `4da4a50`, `c096cb4`, `8f9745b`, **`134fe97`**. + - 이번 라운드에 새로 들어온 것은 `134fe97` 하나다. grok 의 N1·B1 을 반영했고, 4파일 +97/−27 이다. + - 브리프에는 HEAD 가 `38d8ccb` 로 적혀 있다. 실제로 검토한 HEAD 는 **`134fe97`** 이며, `completed` 발행 명령에서 HEAD 를 다시 확인한다. +- **방법**: + - 브랜치 전체 diff 와 `134fe97` 을 정독했다. + - macOS `/bin/bash` 3.2.57(PATH 의 bash 와 같다)에서 B1 을 재현하고 수정을 확인했다. + - paho 2.1.0 의 연결 중 시그널 지연을 데몬 없이 python 단독으로 실측했다. + - 선택 테스트 5파일을 실행하고, 신규 시그널 테스트를 3회 반복했다. 정적 검사도 했다. + - 리뷰 중 45초 간격으로 `progress` 를 발행했다. 코드는 수정하지 않았다. +- **안전 규칙 준수**: + - 실제 monitor 데몬 35505(이 저장소)와 8152(다른 프로젝트), 그리고 `.mam/monitor.lock` 은 읽기 전용 `ps` 로만 확인했다. + - `scrub-secrets`·purge 는 실행하지 않았다. + - 데몬·트랩 실험은 pytest 샌드박스, `bash -c` 단독 스크립트, python 단독 프로세스에서만 했다. + +## 0. 이전 리뷰와 이번 커밋 + +| 리뷰 | 대상 HEAD | 판정 | 요지 | +|---|---|---|---| +| `planner-reviewer-claude-01`(`9114ac86`) | `8f9745b` | PASS | **B1 을 놓쳤다**(§0-1) | +| `reviewer-agy-01`(`0f0327f6`) | `8f9745b` | PASS | — | +| `reviewer-grok-01`(`0dc6400b`) | `8f9745b` | **NOT PASS** | **N1(차단)**: 구독 직후의 SIGTERM 이 정리 경로를 타지 않아 `test_mqtt_loop_sigterm_exits_cleanly` 가 실패. **B1(비차단)**: 첫 `&` 전의 TERM 에서 트랩이 rc 1 로 끝남 | +| `planner-reviewer-codex-01`(`236cc993`) | `8f9745b` | PASS | — | + +creator(`fdb85679`)는 반론 없이 N1·B1 을 `134fe97` 에 반영했다. N1 은 grok 의 대안 1(핸들러를 연결 전에 설치), B1 은 `set +u` 로 읽는 방식이다. + +### 0-1. 지난 리뷰의 누락 인정 + +- 나는 `9114ac86` §1-1 에서 `${_sub_pid:-${!:-}}` 를 승인했다. 근거는 "`$!` 는 비어 있거나 구독자 자신" 이라는 추론뿐이었고, 대상 셸인 bash 3.2 에서 실제로 실행해 보지 않았다. +- bash 3.2 는 `set -u` 아래에서 첫 `&` 전에 `${!:-}` 를 `$!: unbound variable` 로 처리해 셸을 죽인다. 이것을 grok 이 찾았다(B1). +- 이번에는 트랩과 `set -u` 경로를 `/bin/bash` 3.2.57 에서 직접 실행해 확인했다(§1-1). + +## 1. `134fe97` 검증 + +### 1-1. B1: `_sub_stop` 의 `$!` 읽기 (`reconcile.sh:80-98`) + +수정 내용(`:86-91`): 먼저 `local child="${_sub_pid:-}"` 로 읽고, 비어 있을 때만 `set +u; child="$!"; set -u` 로 `$!` 를 읽는다. + +| 실측 (`/bin/bash` 3.2.57, `set -euo pipefail`) | 결과 | +|---|---| +| `set -u; x="${!:-}"` (수정 전 방식), 첫 `&` 전 | `$!: unbound variable`. B1 재현 | +| 수정 전 트랩 본문 + `kill -TERM $$`, 자식 없음 | 같은 오류로 rc 1 | +| **수정 후 `_sub_stop` 원문**, 자식 없음 | **rc 143** | +| **수정 후 `_sub_stop` 원문**, `sleep 30 &` 자식 있음 | **rc 143**. `sleep` 도 종료됨 | + +- `set +u` 가 풀려 있는 구간은 대입 한 줄뿐이다. 트랩은 곧바로 `exit 143` 으로 끝나므로 nounset 해제가 다른 코드로 새지 않는다. +- 테스트 `test_sub_stop_trap_exits_143_before_the_first_background_child`(`tests/test_issue19_subscriber_daemon_lifecycle.py:395-411`): + - `_sub_stop` 을 정규식으로 원문 그대로 추출한다. 정규식은 두 칸 들여쓴 첫 `}` 까지 잡는데, 함수 본문의 `fi` 는 네 칸 들여쓰기라 경계가 정확하다. + - `set -euo pipefail` 인 `bash -c` 에서 자식 없이 한 번, 자식이 있는 상태로 한 번 실행한다. + - 자식이 있는 경우는 0.5 s 기다린 뒤 TERM 을 보낸다. §1-3 의 fork-exec 창과 섞이지 않게 분리한 것이다. + - 이 테스트는 PATH 의 `bash` 로 실행된다. 따라서 B1 회귀는 bash 3.2 환경(macOS)에서 잡힌다. 이 머신에는 bash 4 이상이 없어 다른 버전의 동작은 확인하지 못했다. + +### 1-2. N1: python 핸들러를 연결 전에 설치 (`reconcile.sh:300-377`) + +**순서**: +1. `stop_event`, `_on_stop_signal`, `signal.signal` ×2 (`:300-317`) +2. 클라이언트 생성 (`:319-323`) +3. `connect` (`:326-331`) +4. `loop_start` + CONNACK 대기 (`:332-341`) +5. 메인 루프 (`:343-370`) +6. `finally` 에서 `loop_stop`/`disconnect` (`:371-376`) +7. `sys.exit(0)` (`:377`) + +| 시그널이 도착한 시점 | 동작 | 결과 | +|---|---|---| +| 인터프리터 기동·import 중 (핸들러 설치 전) | python 기본 동작으로 즉시 종료 | 연결·구독 전이라 정리할 것이 없다. lock 은 프로세스 종료로 풀리고 bash 는 143 | +| `connect` 중 (TCP 연결 대기) | 핸들러는 플래그만 세우고, `connect` 는 PEP 475 재시도로 시간 초과까지 계속된다. 예외가 나면 `stop_signal` 이 있으므로 폴백하지 않는다(`:329`). 메인 루프 첫 검사(`:346`)에서 `signal 15 received` 를 찍고 `finally` 로 간다 | rc 0, 폴백 없음. **최대 약 5 s 지연**(§3-1) | +| `connect` 성공 뒤 CONNACK 대기 중 | `stop_event.wait(0.1)` 이 곧바로 깨어나 대기 루프를 빠져나온다(`:335-337`). `stop_signal` 이 있으므로 폴백하지 않는다(`:338`) | 즉시, rc 0 | +| 구독 직후 (`subscribed to` 로그 뒤) | 메인 루프의 `stop_event.wait` 이 깨어난다 | 즉시, rc 0. grok N1 의 사례 | +| `loop_start` 전 (`connect` 예외 경로) | `finally` 의 `loop_stop()` 은 스레드가 없어 `MQTT_ERR_INVAL` 을 반환할 뿐이다(paho 2.1.0). `disconnect` 는 `try` 안에 있다 | 예외 없음 | + +- **rc 계약**: + - 시그널을 받은 구독자는 rc 3 을 내지 않는다(`:329`, `:338`). 그래서 bash 가 폴링 폴백으로 들어가지 않는다. + - rc 0(정상), 3(폴백), 1(lock 실패)의 의미는 그대로다. + - python PID 에만 직접 TERM 을 보낸 경우 python 은 0, bash 는 `exit 0` 으로 끝난다. 수정 전(python 이 시그널로 죽고 bash 는 0)과 외부 결과가 같다. +- **paho 스레드와의 충돌 없음**: + - python 시그널 핸들러는 항상 메인 스레드에서 실행되고, 플래그와 `threading.Event` 만 건드린다. + - paho 스레드는 시그널을 처리하지 않는다. `loop_stop()` 은 메인 스레드에서 스레드를 join 한다. +- **테스트 `test_sigterm_during_broker_handshake_takes_the_clean_path`** (`:361-392`): + - 브로커 없이, 임의 포트에서 응답하지 않는 로컬 리스너를 쓴다. + - `connecting to` 로그 1 s 뒤에 TERM 을 보내고, rc 143, `signal 15 received` 있음, 폴백 없음을 확인한다. + - CONNACK 대기 5 s 중 1 s 시점이므로 약 4 s 여유가 있다. `connect` 전에 TERM 이 닿아도 로컬 연결은 곧바로 끝나 같은 결과가 된다. + - 부하 때문에 로그 확인에서 TERM 까지 5 s 를 넘길 때만 실패할 수 있어, 결정적이라고 판단한다. +- grok 이 실패를 본 `test_mqtt_loop_sigterm_exits_cleanly` 는 이번 실행에서 모두 통과했다. 이제 `subscribed to` 로그는 "핸들러 설치 이후" 를 뜻하므로, 테스트의 트리거 조건을 바꾸지 않은 판단이 옳다. + +### 1-3. creator 가 새로 보고한 fork-exec 창 (고치지 않음. 후속 과제에 동의) + +- **현상**: TERM 이 `&` 의 fork 와 exec 사이 수 ms 안에 오는 경우다. 자식이 물려받은 트랩이 전달된 TERM 을 삼키고, bash 는 트랩의 `wait` 에 묶인다. +- **비차단으로 보는 이유**: + - 기동 직후의 매우 좁은 창이다. + - 그렇게 살아남은 python 도 #19 의 자기 종료가 그대로 적용된다. 워크스페이스가 삭제되거나 lock 이 교체되면 5 s 안에 종료하므로, 샌드박스에서 무한 누수가 되지는 않는다. +- **후속 수정 때 함께 볼 점**: + - 부모 bash 를 잃은 python 구독자(argv 는 `python -`)는 conftest 선정자(`bash|sh /…/reconcile.sh --subscribe`)에도 세션 가드에도 보이지 않는다. + - creator 가 제안한 방식(자식 생존 확인 → 제한 횟수 TERM 재전송 → 마지막에 KILL)에 동의한다. + +## 2. 브랜치 전체 검증 항목 (1)–(7) + +| # | 항목 | 결과 | +|---|---|---| +| (1) | `reconcile.sh` | ✅ 아래 1번 목록 | +| (2) | `daemon.py`/`lib.sh`/`cli.py` | ✅ `134fe97` 에서 바뀌지 않았다. 아래 2번 목록 | +| (3) | `tests/conftest.py` | ✅ `134fe97` 에서 바뀌지 않았다. 아래 3번 목록 | +| (4) | 신규 테스트 결정성 | ✅ 아래 4번 목록 | +| (5) | 문서 | ✅ 아래 5번 목록 | +| (6) | 버전 락스텝 · 릴리스 커밋 | ✅ 아래 6번 목록 | +| (7) | 테스트 · 정적 검사 | ✅ 아래 7번 목록 | + +1. **`reconcile.sh`**: + - **rc 계약**: `set +e`(`:101`) → `&`(`:105`) → `_sub_pid=$!`/`wait`/`sub_rc`(`:379-381`) → `set -e`(`:383`) 순서다. rc 가 `3` 일 때만 폴백하고(`:385`), 그 밖에는 `exit 0`(`:408`)으로 끝나 종전 계약과 같다. + - **트랩**: 첫 자식 전(B1 수정), `&` 직후(`$!`, R-1), 자식이 이미 끝난 뒤(kill/wait 가 아무 일도 하지 않음) 모두 rc 143 이다. + - **`_workspace_gone`**(`:132-150`): + - `os.getcwd()` 예외를 처리한다(`:138-141`). + - `(st_dev, st_ino)` 로 비교한다(`:128-129`, `:145-147`). 원래 lock 을 열어 둔 채이므로 그 inode 는 재사용될 수 없고, 교체 판정이 안전하다. + - **paho 스레드와 충돌 없음**: §1-2. + - **폴백**: `trap … exit 143`(`:394`) 과 `sleep & wait $! || true`(`:404-405`). 여기의 `$!` 는 바로 앞 `&` 로 이미 값이 있으므로 B1 과 같은 문제가 없다. + - **`set -euo pipefail`**: B1 외에 의도치 않게 종료되는 새 경로는 없다. +2. **`daemon.py`/`lib.sh`/`cli.py`**: + - 옵트아웃은 리터럴 `1` 만 인정한다(`daemon.py:28-31`). + - 옵트아웃 중에도 기존 데몬 PID 를 보고한다. stdout 은 비우고 stderr 에 한 줄 남긴다. + - legacy 경로(`lib.sh:2024-2027`)가 python 본체와 바이트 단위로 같다. +3. **`tests/conftest.py`**: + - reaper 는 import 시점에 잡은 `_REAL_POPEN`(`:16-26`)으로 명령을 실행한다. + - 선정자는 argv 형태와 스크립트 경로 전체로 대상을 고르고, 공백이 든 후보는 실제 파일일 때만 인정한다(`:32-58`). + - 3 s 유예 뒤 SIGKILL 로 넘어간다(`:86-97`). + - `mam_sandbox`(`:113`)는 finalizer 를 먼저 등록해(`:123`) 맨 마지막에 실행되게 하고, 옵트아웃을 기본으로 켠다(`:161`). + - 세션 가드는 basetemp 기준으로만 판정한다(`:1384-1398`). +4. **신규 테스트 결정성**: + - 테스트는 16건이다. 15개 함수에 파라미터화 1개(`:130`, 2케이스)가 더해진 수로, 실제 수집 수와 같다. + - 신규 2건은 임의 포트 리스너와 스크립트 원문 추출만 쓰므로 외부에 의존하지 않는다. + - 시그널 테스트 4건을 3회 반복했고 매번 모두 통과했다. 소요 시간 편차는 ±0.06 s 이하였다(§2 표). + - 포트 9 가정, 브로커 skip 조건, `from conftest import` 에 대한 판단은 지난 리뷰와 같다. +5. **문서**: + - monitor `SKILL.md:86-92` 의 핸들러 순서·폴백 설명이 코드와 일치한다. + - `VERSIONS.md:47`(항목 3)과 `:51`(신규 테스트 16개, 수집 수와 같음)이 정확하다. + - create `SKILL.md:71` 은 바뀌지 않았고 여전히 정확하다. +6. **버전 락스텝 · 릴리스 커밋**: + - `lib.sh:32`, `lib_py/loop/eager.py:16` 이 4.4.5 이고, `SKILL.md` 8개가 모두 `version: 4.4.5` 다. + - `VERSIONS.md:9`(현재 버전)와 `:23-`(매트릭스), `README.md:5`, `BOOTSTRAP.md:5`, `MESSAGING.md:3` 도 4.4.5 다. + - `verify_framework_version` 을 통과했다. + - `134fe97` 은 버전 값을 바꾸지 않았다. `VERSIONS.md` 는 본문만 수정했다. + - 릴리스 커밋 `38d8ccb` 의 형식은 그대로다. +7. **테스트 · 정적 검사**: 아래 표. + +| 테스트 · 정적 검사 | 결과 | +|---|---| +| **선택 실행 5파일** (issue19 16건, `test_issue5_phase1_delegation`, `test_tier2_component`, `test_version_consistency`, `test_herdr_shim_contract`) | **144 passed in 284.44 s, rc 0, skipped 0**. 지난 라운드 142건에 신규 2건이 더해졌다. 브로커에 의존하는 2건도 skip 없이 실제로 실행되었다 | +| **시그널 테스트 반복** (`-k "sigterm or handshake or sub_stop"` ×3) | 매회 4 passed. 핸드셰이크 2.30–2.32 s, 폴백 SIGTERM 1.33 s, MQTT SIGTERM 1.16–1.22 s, `_sub_stop` 0.51–0.52 s | +| **데몬 누수** (정밀 필터: argv0 `bash`/`sh` + `…/reconcile.sh` + `--subscribe`) | 실행 전후 모두 실제 데몬 2개(8152, 35505)만 같은 PID 로 살아 있었다. 샌드박스 데몬 **0개**. B1 테스트가 띄운 `sleep 30` 잔여도 0개 | +| **#15 레지스트리 가드** | 실행 전후 실제 `.mam/jobs` 레코드 목록이 **같다**(추가 0건) | +| **정적 검사** | `bash -n`(`reconcile.sh`, `lib.sh`) 통과. `py_compile`(`tests/conftest.py`, 신규 테스트) 통과. `git diff --check 8e7282c...HEAD` 클린. 작업 트리 클린, HEAD `134fe97` | + +## 3. 비차단 권고 + +1. **연결 중 SIGTERM 이 최대 약 5 s 늦게 처리된다 (N1 의 부작용)**: + - 실측(데몬 없이 python 단독): paho 2.1.0 `connect` 를 응답 없는 주소(`192.0.2.1`, `10.255.255.1`)로 걸고 1 s 뒤 TERM 을 보냈다. + - 핸들러는 1.00 s 에 실행되었다. + - 그러나 `connect` 는 5.00 s 시간 초과까지 계속되었고, 프로세스는 TERM 뒤 **4.0 s** 에 끝났다. + - 수정 전에는 이 구간의 TERM 이 곧바로 프로세스를 죽였다(정리할 것은 없었다). + - 영향: 브로커 주소가 블랙홀일 때만 생긴다. conftest reaper 는 3 s 유예(`conftest.py:92`) 뒤 SIGKILL 로 넘어가므로 회수는 된다. 다만 드물게 monitor `SKILL.md:92` 의 "`SIGKILL` is never required" 문구와 어긋난다. + - 개선안(선택): 연결 단계에서만 핸들러가 예외를 던지게 하거나, 문서에 "연결 중에는 최대 약 5 s" 를 적는다. +2. **fork-exec 창**: §1-3 의 후속 과제. +3. **폴백의 남는 `sleep`**: 지난 권고를 유지한다. 문서화되어 있고, 간격(`RECONCILE_POLL_INTERVAL`) 안에 스스로 끝난다. +4. **`_run_real` 시간 초과**: 지난 권고를 유지한다. 시간 초과 시 자식 `ps`/`pgrep` 을 끝내지 않는다(`conftest.py:19-26`). + +## 4. GM 참고 + +- **HEAD 가 브리프의 `38d8ccb` 에서 `134fe97` 로 옮겨졌다.** + - 2회차에서 agy·codex 의 PASS 와 grok 의 NOT PASS 는 모두 `8f9745b` 까지만 본 것이다. + - `134fe97` 을 본 것은 creator 자체 검증과 이 리포트뿐이다. 만장일치 판정 전에 각 리뷰어가 `134fe97` 을 확인했는지 볼 것. +- 이 저장소의 실제 데몬 PID 35505 는 수정 전 스크립트로 계속 돌고 있다. 재기동 여부는 오케스트레이터가 판단한다. +- 내 지난 리뷰(`9114ac86`)에는 B1 누락이 있었다(§0-1). + +## 5. 종합 + +- grok 의 N1(차단)과 B1 이 정확히 반영되었다. + - python 핸들러는 연결 전에 설치된다. 시그널을 받은 구독자는 폴백하지 않고 정리 경로(rc 0, bash 143)로 끝난다. + - `_sub_stop` 은 bash 3.2 에서 첫 자식 전에도 rc 143 으로 끝난다. +- 두 수정 모두 회귀 테스트로 고정되었다. +- 브리프의 7개 검증 항목을 브랜치 전체 기준으로 충족한다. 남은 사항(연결 중 약 5 s 지연, fork-exec 창)은 좁은 창이며 비차단이다. 설계 재검토는 필요 없다. + +[VERDICT: PASS] diff --git a/.agents/reports/planner-reviewer-codex-01/report-b30d151d.md b/.agents/reports/planner-reviewer-codex-01/report-b30d151d.md new file mode 100644 index 0000000..0acfa2f --- /dev/null +++ b/.agents/reports/planner-reviewer-codex-01/report-b30d151d.md @@ -0,0 +1,39 @@ +# Issue #19 review — Codex + +PASS for clean HEAD `134fe9704c63bd9b1f46b64b01868f904f5a33cc`, base `8e7282c`. No new blocking findings. No planner escalation is needed. + +Reviewed the branch log from the base and cumulative changes, carrying forward the full-file review through `8f9745b` from job `236cc993` and inspecting the entire new `8f9745b..HEAD` diff. Also checked `git diff 8e7282c...HEAD` for the cumulative subscriber implementation and cumulative diff hygiene. The six commits are `ce315d8`, `38d8ccb`, `4da4a50`, `c096cb4`, `8f9745b`, and `134fe97`. Only the subscriber script, its lifecycle tests, monitor documentation, and VERSIONS changed since the last review. HEAD and the tracked worktree remained stable and clean during verification. + +## Latest corrections + +- **Bash nounset:** `_sub_stop` now reads the fallback `$!` with nounset temporarily disabled, avoiding the Bash 3.2 unbound-variable failure before the first background child. It restores nounset, skips an empty PID, tolerates an exited child, and exits 143. The regression passed with no child and with an already-executing child on this host's actual **GNU Bash 3.2.57**. This test does not prove every fork-to-exec scheduling race; its comment accurately limits the exercised window. +- **Connection-phase signals:** Python installs both handlers before configuring/connecting the MQTT client. The CONNACK wait uses the stop event, and connection-error/acceptance-failure branches check the recorded stop signal before choosing rc 3. The new local silent-listener test verifies signal logging, wrapper rc 143, prompt exit, and absence of polling fallback while awaiting CONNACK. Both existing connected-MQTT tests also passed, exercising the subscription-ready path. +- **Scope of prompt shutdown:** The local test completes the TCP handshake and then waits for CONNACK. It does not exercise stalled DNS resolution or a blocked synchronous TCP/TLS connection operation; setting an event does not itself cancel those operations. Treat the documentation as the intended cleanup path, not a measured latency guarantee for every network stall. + +## Cumulative review + +- Python remains a background child of the waiting Bash wrapper. The trap forwards TERM and waits; strict-shell nonzero paths are intentionally tolerated or captured. Workspace checks use `(st_dev, st_ino)`, catch missing paths, and handle `getcwd` failure in the new check. +- Internal Python statuses remain 0 normally, 3 for broker fallback, and 1 for lock setup errors. The shell still masks non-3 child failures with its final `exit 0`, as the base already did. This preserves existing behavior; it does not provide external lock-error rc 1. +- Polling intentionally checks directory/lock presence without retaining the Python lock. An active foreground `--once` pass can defer its trap, and an interrupted sleep can survive until its finite interval expires. These limitations are now documented. +- Literal `MAM_NO_SUBSCRIBER_DAEMON=1` remains the sole opt-out value. Python and legacy paths preserve existing-PID reporting, missing-monitor validation, and empty stdout/single stderr diagnostic when disabled without an existing daemon. Create documentation correctly qualifies when the PID line is empty. +- The prior whitespace-path cleanup blocker remains fixed. The full-path matcher preserves root boundaries and rejects ordinary prompt-bearing wrappers; whitespace candidates require an existing file. The live-process regression verifies discovery at both fixture and basetemp roots and actual termination. Cleanup still uses a direct-child snapshot rather than recursive ownership tracking. Import-time Popen binding protects teardown from subprocess monkeypatches, and zombie handling avoids treating terminated processes as live. +- Fixture cleanup is registered first within that fixture, and the session leak guard uses its own basetemp. Existing subprocess patches need not have been undone when cleanup runs, which is why import-time Popen binding matters. +- There are now 16 lifecycle cases. Port 9 remains an environmental closed-port assumption. The new local listener uses a kernel-selected port. Two MQTT tests can skip when no broker is reachable; neither skipped here. Timing assertions and conftest imports passed on this host. +- Version 4.4.5 remains synchronized across lib.sh, eager.py, all eight skill frontmatters, VERSIONS current/matrix, README/BOOTSTRAP variants, and MESSAGING. The release commit remains `release(v4.4.5)`. Updated documentation reflects early handler installation and correctly counts 16 lifecycle tests. The changelog's full-suite figure is recorded prior evidence, not a full-suite run conducted for this job. + +## Validation + +Fresh run on `134fe97`: + +```text +PYTHONDONTWRITEBYTECODE=1 .venv/bin/python -m pytest -p no:cacheprovider tests/test_issue19_subscriber_daemon_lifecycle.py tests/test_version_consistency.py -q +19 passed in 39.39s +``` + +`bash -n` passed for reconcile.sh and lib.sh. `git diff --check 8e7282c...HEAD` passed. Post-run raw `pgrep -f 'pytest-of-.*reconcile.sh --subscribe'` produced three matches; filtering for actual Bash/sh subscriber command shape found **zero daemons**. The session leak guard passed. + +Earlier independent evidence: all five requested test files passed on `c096cb4` (**140 passed in 314.62s**); the lifecycle/version subset passed on `8f9745b` (**17 passed in 39.74s**). These are explicitly prior-commit results. This review reran the lifecycle and version checks affected by the latest correction, not the entire five-file suite. + +No source edits, commits, production monitor/lock manipulation, real conversation purge, registry scrubbing, push, merge, or issue updates were performed. Workspace writes were limited to this report and authorized lifecycle events. + +[VERDICT: PASS] diff --git a/.agents/reports/reviewer-agy-01/report-e2420d1b.md b/.agents/reports/reviewer-agy-01/report-e2420d1b.md new file mode 100644 index 0000000..db3cf3b --- /dev/null +++ b/.agents/reports/reviewer-agy-01/report-e2420d1b.md @@ -0,0 +1,109 @@ +# Peer Review Report: Branch `fix/issue-19` (Comprehensive Review — Job `e2420d1b`) + +- **Reviewer**: agy (session: `herdr:reviewer-agy-01`) +- **Job ID**: `e2420d1b` +- **Role**: Reviewer +- **Target Branch**: `fix/issue-19` +- **Base Commit**: `8e7282c` (v4.4.4) +- **Evaluated Commits**: + - `ce315d8` — `fix(issue-19): bound the subscriber daemon to its workspace, forward SIGTERM, keep sandboxes from leaking it` + - `38d8ccb` — `release(v4.4.5): bump framework and eight skills to 4.4.5 in lockstep` + - `4da4a50` — `test(issue-19): run the sandbox reaper through an import-time Popen` + - `c096cb4` — `fix(issue-19): close the trap race, select daemons by argv shape, align docs with the polling fallback` + - `8f9745b` — `fix(issue-19): match the daemon's whole script path so spaced sandbox paths are reaped` + +--- + +## 1. Executive Summary + +This peer review evaluated branch `fix/issue-19` through its latest commit `8f9745b`. The implementation addresses Issue #19, where subscriber daemons (`reconcile.sh --subscribe --idle-timeout 0`) spawned per workspace were leaking across test runs (202 orphan processes observed holding broker connections for deleted temp directories). + +The complete branch includes the initial fix (`ce315d8`), version lockstep v4.4.5 (`38d8ccb`), and three peer-review hardening commits: +1. `4da4a50`: Binds `_REAL_POPEN = _subprocess.Popen` at import time in `conftest.py` so test mocks cannot sabotage the sandbox reaper. +2. `c096cb4`: Closes the race window between background spawn and PID assignment via `${!:-}` with `nounset` protection in bash 3.2, installs Python signal handlers before network operations, matches daemon processes strictly by argv shape to avoid false positives on prompt-bearing wrappers, and documents the polling fallback lock-presence contract. +3. `8f9745b`: Extracts the full script path up to `/reconcile.sh --subscribe` so sandboxes or `--basetemp` paths containing whitespace are correctly identified and reaped. + +All review criteria are satisfied with complete fidelity. A total of 144 test cases (19 targeted + 125 regression) executed with a 100% pass rate, zero orphan processes, and zero job registry leaks. + +--- + +## 2. Detailed Technical Audit + +### 2.1 Reconcile Shell Script (`reconcile.sh`) +- **Background Execution & Return Code Contract**: + - Python subscriber runs in background: `"$PYBIN" - <<'PYEOF' &; _sub_pid=$!; wait "$_sub_pid"; sub_rc=$?; _sub_pid=""; set -e`. + - Exit code 0 indicates normal exit; exit code 3 indicates broker unavailability and triggers polling fallback; lock contention returns 1 and exits cleanly. +- **Trap Safety & Bash 3.2 Compatibility**: + - `_sub_stop()` handles `TERM` and `INT`. + - In bash 3.2 (default `/bin/bash` on macOS), expanding `$!` before the first background child aborts with an unbound variable error under `set -u`. Commit `c096cb4` temporarily unsets `nounset` (`set +u; child="$!"; set -u`), ensuring the trap reliably exits 143 even if a signal arrives before child launch. + - Signal forwarding safely suppresses errors (`kill -TERM "$child" 2>/dev/null || true; wait "$child" 2>/dev/null || true`). +- **Identity Comparison `(st_dev, st_ino)`**: + - `lock_ident = (_lock_st.st_dev, _lock_st.st_ino)` checks both device and inode. + - `_workspace_gone()` catches `OSError` on `os.getcwd()` and detects replaced or deleted `.mam/monitor.lock` files, terminating the daemon cleanly when its workspace is removed. +- **Early Signal Handlers**: + - Signal handlers are installed *before* `client.connect(...)` and `client.loop_start()`. + - If `SIGTERM` arrives while awaiting `CONNACK`, `stop_event.wait(0.1)` catches it immediately, cleanly exiting 0 without triggering the broker-down fallback. +- **Polling Fallback**: + - `sleep "$POLL_INTERVAL" & wait $! || true` allows immediate signal handling (exit 143). + - Verifies presence of `$WORKSPACE_ROOT` and `.mam/monitor.lock` on each cycle. + +### 2.2 Opt-Out Mechanism (`daemon.py`, `lib.sh`, `cli.py`) +- **Literal Value Check**: + - `daemon_disabled()` requires literal `"1"`. Loose values ("yes", "true", "0") do not disable the daemon. +- **Deduplication & Output Parity**: + - Active daemons continue to be reported during opt-out. + - When opting out without an existing daemon: + - Python CLI prints nothing to stdout and emits a single note to stderr. + - Legacy shell (`lib.sh`) only echoes when `$pid` is non-empty (`if [ -n "$pid" ]; then echo "$pid"; fi`), preventing extraneous blank lines on stdout and matching Python behavior byte-for-byte. + +### 2.3 Conftest Sandboxing & Reaper Hardening +- **Import-Time Popen (`_REAL_POPEN`)**: + - Capturing `_subprocess.Popen` at import time insulates fixture teardown from test monkeypatching or spying of `subprocess.run` / `subprocess.Popen`. +- **Argv-Shape Matching (`_is_subscriber_daemon_argv`)**: + - Confirms shell executable (`bash` or `sh`), verifies path prefix under `root`, and regex matches `(.*reconcile\.sh) --subscribe`. + - Excludes processes that merely quote the command line in prompt arguments (e.g. `submit --prompt "..."`) or sibling roots (`/sbx1` vs `/sbx10`). + - Supports whitespace-containing paths by verifying `os.path.isfile(script)`. +- **Session Leak Guard**: + - `subscriber_daemon_leak_guard` scopes strictly to pytest `basetemp`, guaranteeing no daemon outlives the test suite. + +--- + +## 3. Version Lockstep v4.4.5 Audit + +All components remain strictly locked at `4.4.5`: +- `.agents/skills/lib.sh`: `MAM_VERSION="4.4.5"` +- `.agents/skills/lib_py/loop/eager.py`: `FRAMEWORK_VERSION = "4.4.5"` +- Frontmatter `version: 4.4.5` across all 8 skills (`create`, `stop`, `resume`, `status`, `monitor`, `delegate-job`, `loop`, `orc-onboard`). +- `VERSIONS.md`: Current release overview, 8-row skills matrix, and detailed `v4.4.5` changelog accounting for all 16 lifecycle tests and architectural refinements. +- Badges and headers in `README.md`, `README.ko.md`, `BOOTSTRAP.md`, `BOOTSTRAP.ko.md`, and `MESSAGING.md`. +- Release commit title: `release(v4.4.5): bump framework and eight skills to 4.4.5 in lockstep`. +- Remaining occurrences of `4.4.4` in markdown files are purely historical. + +--- + +## 4. Test Suite Execution & Audits + +### 4.1 Targeted Suites +```bash +.venv/bin/python -m pytest tests/test_issue19_subscriber_daemon_lifecycle.py tests/test_version_consistency.py +``` +- **Result**: 19 passed in 37.99s (16 lifecycle tests + 3 version consistency tests). + +### 4.2 Regression Suites +```bash +.venv/bin/python -m pytest tests/test_issue5_phase1_delegation.py tests/test_tier2_component.py tests/test_herdr_shim_contract.py +``` +- **Result**: 125 passed in 240.03s. + +### 4.3 Process & Registry Integrity +- Orphan daemons under `/private/var/folders`: **0** +- Real workspace monitor daemon: **Preserved intact** (PID 35505) +- Registry file count in `.mam/jobs`: **52** before and after (0 leaks) + +--- + +## 5. Conclusion + +Branch `fix/issue-19` across commits `ce315d8` through `8f9745b` delivers a complete, robust, and well-verified solution to Issue #19. All criteria are fully satisfied. + +[VERDICT: PASS] diff --git a/.agents/reports/reviewer-grok-01/report-b6915b56.md b/.agents/reports/reviewer-grok-01/report-b6915b56.md new file mode 100644 index 0000000..5a1b4df --- /dev/null +++ b/.agents/reports/reviewer-grok-01/report-b6915b56.md @@ -0,0 +1,51 @@ +# 🔍 Review Report — Job b6915b56 (`fix/issue-19` @ `134fe97`) + +- **Reviewer**: `reviewer-grok-01` (role: reviewer) · 2026-09-13 +- **범위**: `git log`/`git diff` **`8e7282c...HEAD`** +- **HEAD**: `134fe97` `fix(issue-19): arm the subscriber's stop handlers before it connects; read $! with nounset off` +- **선행 NOT PASS**: `0dc6400b` N1 (핸들러가 subscribe 이후), B1 (`set -u` 에서 빈 `$!`) +- **안전**: 실제 `.mam/monitor.lock`·워크스페이스 데몬 미접촉. 소스 무변경. + +커밋: `ce315d8` → `38d8ccb` → `4da4a50` → `c096cb4` → `8f9745b` → **`134fe97`**. `git diff --check` 클린. + +## N1 닫힘 — SIGTERM 핸들러를 connect 앞에 + +`reconcile.sh:300-341`: `signal.signal(SIGTERM/INT)` 을 `client.connect` / `loop_start` **앞**에 둔다. CONNACK 대기는 `stop_event.wait(0.1)`. 시그널이 있으면 `sys.exit(3)` 을 타지 않아 폴링 폴백으로 안 떨어진다. 메인 루프가 `"signal N received, exiting"` 을 찍고 `finally: loop_stop/disconnect`, 파이썬 rc 0, 래퍼 143. + +`test_sigterm_during_broker_handshake_takes_the_clean_path`: 응답 없는 로컬 listen 포트, `connecting to` 후 1s, SIGTERM → 143, 로그에 signal 15, `falling back to polling` 없음. 직전 실패 테스트 `test_mqtt_loop_sigterm_exits_cleanly` 도 통과(핸들러가 subscribe 이전에 이미 있음). + +paho `loop_start` 스레드는 핸들러가 만지지 않고, 메인만 `stop_event` 를 깨운 뒤 `loop_stop` — 충돌 없음. + +## B1 닫힘 — 첫 `&` 전 `set -u` + +`_sub_stop` (`reconcile.sh:80-97`): `_sub_pid` 가 비면 `set +u; child="$!"; set -u`. bash 3.2 에서 `${!:-}` 가 unbound 로 죽던 경로가 143 으로 끝남. `test_sub_stop_trap_exits_143_before_the_first_background_child` 가 자식 없음/있음 둘 다 143. + +`&` 와 `_sub_pid=$!` 사이는 `$!` 로 커버. 이미 끝난 자식은 kill/wait no-op. + +## 유지되는 계약 + +- 래퍼: 자식 0 → `exit 0`; 자식 3 → 폴링 후 0; 자식 1(lock 예외) → 폴링 없이 0; TERM → 143. +- `_workspace_gone`: `(st_dev, st_ino)`, `getcwd` OSError, lock 교체/삭제. +- 폴링: lock **존재**만. sleep `& wait`. 트랩은 sleep 자식을 안 죽임(문서화, ≤ POLL_INTERVAL). +- 옵트아웃 리터럴 `1`, 기존 PID 보고, stdout 비움. argv 형태 회수, 접두/래퍼/공백 경로. 세션 가드 basetemp. + +## 문서·락스텝 + +SKILL.md: 핸들러는 connect 전, 폴링은 presence, sleep 잔여. VERSIONS.md 항목 3·테스트 16개가 `134fe97` 과 맞음. **4.4.5** lockstep 유지 (`38d8ccb` 이후는 패치 커밋). + +## 테스트 + +``` +env -u MAM_VERSION .venv/bin/python -m pytest -q \ + tests/test_issue19_subscriber_daemon_lifecycle.py \ + tests/test_issue5_phase1_delegation.py tests/test_tier2_component.py \ + tests/test_version_consistency.py tests/test_herdr_shim_contract.py +``` + +**144 passed in 284.23s** (직전 141+1 fail → N1/B1 테스트 2 + 실패분 회복). issue19 **16 collected**. `bash -n` 클린. 실제 `pytest-of-*/…/bash …/reconcile.sh --subscribe` 0건(argv 에 브리프를 실은 래퍼만 `pgrep -f` 에 걸림). 워크스페이스 모니터 미접촉. + +## 종합 + +N1·B1 이 코드·테스트로 닫혔고, 1회차 #19 수명 계약은 회귀 없음. 설계 재작업 불필요. + +[VERDICT: PASS]