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