From 8f9745b1286f35c1fd6a6d05eba3ff77ddc421cf Mon Sep 17 00:00:00 2001
From: Godopu <godopu16@gmail.com>
Date: Sun, 13 Sep 2026 16:35:23 +0900
Subject: [PATCH] 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 <noreply@anthropic.com>
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 <name>` 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 <name>` (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 <spaced path>/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