diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index cd6704e..c74849e 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -1486,6 +1486,54 @@ _herdr_scope_is_pane_inherited() { [ -n "${HERDR_WORKSPACE_ID:-}" ] && [ -n "$pane" ] && [ "${pane%%:*}" = "$HERDR_WORKSPACE_ID" ] } +# Issue #50 follow-up: one pane claim at a time per herdr server. +# +# The shim's new-session chooses a pane - an idle one in a workspace on the +# same cwd (BUG-4), a split of one, or the root pane of a fresh workspace - and +# then claims it with `agent start`. Two shims on one server (several creates +# at once into one folder) could choose the same idle pane in between, and the +# later `agent start` failed with agent_pane_busy. mam_herdr runs the whole +# new-session call holding one lock per server: a flock on fd 9, opened for +# the block below and taken by lib_py.pane_lock (which also bounds the wait, +# MAM_PANE_CLAIM_WAIT_S). It ends with the block or with this process, however +# that ends, so a dead holder leaves nothing behind; the file is the target +# server's, so other servers' creates are not held. The lock lives here, not +# in the shim heredoc, which stays byte-identical (test_issue23_24 S11). +# +# _mam_pane_claim_target : prints the server a new-session call +# targets ("" for the default one); rc 1 for any other call. +_mam_pane_claim_target() { + local sess="${HERDR_SESSION_NAME:-}" + while [ "${1:-}" = "-L" ]; do sess="${2:-}"; shift 2 || return 1; done + [ "${1:-}" = "new-session" ] || return 1 + [ "$sess" = "default" ] && sess="" + printf '%s\n' "$sess" +} + +# _mam_with_pane_claim : run holding 's pane lock. +_mam_with_pane_claim() { + local sess="$1" f="" + shift + f=$(PYTHONPATH="$SKILL_DIR:${PYTHONPATH:-}" python3 -m lib_py.pane_lock path --session "$sess") || f="" + if [ -z "$f" ]; then + echo "ERROR: no pane lock path for herdr session ${sess:-default} (lib_py.pane_lock); not placing this session" >&2 + return 1 + fi + mkdir -p "$(dirname "$f")" + { + case "$(PYTHONPATH="$SKILL_DIR:${PYTHONPATH:-}" python3 -m lib_py.pane_lock acquire --fd 9)" in + locked) ;; + timeout) + echo "ERROR: another pane claim on herdr session ${sess:-default} did not end within MAM_PANE_CLAIM_WAIT_S; not placing this session" >&2 + return 1 ;; + *) + echo "ERROR: the pane lock of herdr session ${sess:-default} could not be decided (lib_py.pane_lock); not placing this session" >&2 + return 1 ;; + esac + "$@" + } 9>>"$f" +} + mam_herdr() { _init_herdr_isolation # Built as an argv array and decided inside `if`: this runs under @@ -1503,6 +1551,11 @@ mam_herdr() { echo "mam_herdr: '$1' looks up by name; the caller pane's own workspace ${HERDR_WORKSPACE_ID} is not a scope" >&2 fi fi + local _claim_sess="" + if _claim_sess=$(_mam_pane_claim_target "$@"); then + _mam_with_pane_claim "$_claim_sess" ${_scope_strip[@]+"${_scope_strip[@]}"} "$WORKSPACE_ROOT/.mam/shim/herdr" "$@" + return + fi ${_scope_strip[@]+"${_scope_strip[@]}"} "$WORKSPACE_ROOT/.mam/shim/herdr" "$@" } diff --git a/.agents/skills/lib_py/atomic_yaml.py b/.agents/skills/lib_py/atomic_yaml.py index 3e9e7e0..32227bf 100644 --- a/.agents/skills/lib_py/atomic_yaml.py +++ b/.agents/skills/lib_py/atomic_yaml.py @@ -115,6 +115,8 @@ def atomic_dump_yaml_main(): d['herdr_sessions'] = [] old_sessions = get_all_sessions_status(d) + # a mutation that replaces a row the monitor registered for a pane it + # created itself (create_session.sh) drops that name from old_roles old_roles = {s.get('name'): s.get('role') for s in db_sessions if s.get('role')} # --- caller mutation (module scope: sees d, yaml, os, glob, subprocess) --- diff --git a/.agents/skills/lib_py/pane_lock.py b/.agents/skills/lib_py/pane_lock.py new file mode 100644 index 0000000..42815e3 --- /dev/null +++ b/.agents/skills/lib_py/pane_lock.py @@ -0,0 +1,75 @@ +"""Issue #50 follow-up: one pane claim at a time per herdr server. + +The herdr shim's `new-session` chooses a pane - an idle one in a workspace on +the same cwd (BUG-4), a split of one, or the root pane of a fresh workspace - +and then claims it with `agent start`. Two shims on one server (several +`create` at once into one folder) could choose the same idle pane in between, +and the later `agent start` failed with agent_pane_busy. The shim now holds +one lock per server from the choice to the claim. + + pane_lock.py path --session S the lock file of herdr session S, under the + HOME whose socket the session uses + pane_lock.py acquire --fd N flock(LOCK_EX) on the inherited fd N; prints + `locked`, or `timeout` once MAM_PANE_CLAIM_WAIT_S + (default 120, minimum 1) has passed + +The lock is a BSD flock, which belongs to the open file the shim holds on fd 9: +it outlives this helper and ends with the shim's process, however that ends +(exit, kill -9), so a dead holder leaves nothing to steal. flock(1) is not on +macOS; fcntl.flock is. Stdlib only, Python 3.9. +""" +from __future__ import annotations + +import argparse +import fcntl +import os +import sys +import time +from typing import List, Mapping, Optional + +from lib_py import ints + +DEFAULT_WAIT_S = 120 + + +def lock_path(session: Optional[str], env: Mapping[str, str]) -> str: + home = env.get("HOME") or env.get("HOME_DIR") or os.path.expanduser("~") + return os.path.join(home, ".local", "state", "mam", "pane-lock", (session or "default") + ".lock") + + +def wait_s(env: Mapping[str, str]) -> int: + return ints.positive_int(env.get("MAM_PANE_CLAIM_WAIT_S"), 1) or DEFAULT_WAIT_S + + +def acquire(fd: int, wait: float) -> str: + """`locked`, or `timeout` when the lock was not free within `wait` s. The + deadline is on the monotonic clock; a step of the wall clock does not + stretch or cut it.""" + deadline = time.monotonic() + wait + while True: + try: + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + return "locked" + except OSError: + if time.monotonic() >= deadline: + return "timeout" + time.sleep(0.2) + + +def main(argv: Optional[List[str]] = None) -> int: + p = argparse.ArgumentParser(prog="pane_lock.py") + sub = p.add_subparsers(dest="cmd", required=True) + s = sub.add_parser("path") + s.add_argument("--session", default="") + s = sub.add_parser("acquire") + s.add_argument("--fd", type=int, required=True) + a = p.parse_args(argv) + if a.cmd == "path": + print(lock_path(a.session, os.environ)) + else: + print(acquire(a.fd, wait_s(os.environ))) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.agents/skills/multi-agent-mux-create/SKILL.md b/.agents/skills/multi-agent-mux-create/SKILL.md index 11c6416..996e0af 100644 --- a/.agents/skills/multi-agent-mux-create/SKILL.md +++ b/.agents/skills/multi-agent-mux-create/SKILL.md @@ -178,13 +178,15 @@ bash .agents/skills/multi-agent-mux-create/scripts/create_session.sh --workspace `MAM_ONBOARD_WAIT_S` (default 600): how long `--onboard` waits for the onboarding job to end after the `started` handshake. +`MAM_PANE_CLAIM_WAIT_S` (default 120): creates (and resumes) on one herdr server choose and claim their pane one at a time; how long one waits for another's claim to end before giving up (exit 1, nothing spawned). + | Exit | Meaning | |---|---| | 0 | Created; or `--dry-run` | | 1 | Pre-flight failure, nothing was spawned: workspace not a directory, herdr not installed, agent CLI not in `PATH`, agent not authenticated. After spawn: the task brief could not be written or the instruction injection failed (the session is rolled back) | | 2 | Bad arguments; control-plane binding refused for a worktree; whitespace in a control-plane path that must be forwarded | | 3 | A herdr session of that name already exists (`multi-agent-mux-resume`, or `multi-agent-mux-stop` first) | -| 4 | The YAML already has a `running` row of that name. The YAML write raises it after spawn, and the herdr session is rolled back | +| 4 | The YAML already has a `running` row of that name. The YAML write raises it after spawn, and the herdr session is rolled back. A row the monitor registered for this very pane while it was being created is replaced instead | | 42 | The TUI never became ready and the process is gone, or it is alive and `MAM_KILL_ON_READY_TIMEOUT=1` (rolled back) | | 43 | The rollback could not kill the herdr session; it is still running | | 44 | The onboarding job ended with `error` (or was cancelled); the session is kept (Issue #50) | diff --git a/.agents/skills/multi-agent-mux-create/scripts/create_session.sh b/.agents/skills/multi-agent-mux-create/scripts/create_session.sh index b8ad613..530bbb7 100755 --- a/.agents/skills/multi-agent-mux-create/scripts/create_session.sh +++ b/.agents/skills/multi-agent-mux-create/scripts/create_session.sh @@ -527,6 +527,20 @@ sessions = d.setdefault('herdr_sessions', []) # P0-D: 같은 이름 엔트리가 status=running 이면만 거부. terminated/archived 는 # 재사용 가능 — 낡은 엔트리를 제거하고 새로 append (create -> delete -> create). running_same = [s for s in sessions if s.get('name') == name and s.get('status') == 'running'] +# Issue #50 follow-up: a reconcile pass (another create's end-of-run `--once`, +# the subscriber's cycle) can adopt the pane this create started moments ago, +# before this row lands. A running row of this name that the monitor +# registered, with no job, for a pane created after this create began, is +# this create's own pane - herdr keeps live names unique per server and the +# preflight refused an older session of this name - so it is replaced, not +# refused as a second live session. +def _adopted_own(s): + return (s.get('last_visible_note') == 'auto-registered by monitor' and not s.get('delegate_job_id') + and epoch.isdigit() and int(s.get('herdr_session_epoch') or 0) >= int(epoch) - 1) +if running_same and all(_adopted_own(s) for s in running_same): + print(f"NOTE: the monitor registered {name} while it was being created; that row is replaced by this one", flush=True) + running_same = [] + old_roles.pop(name, None) # its role was read off the name; --role is the fact (atomic_yaml's role guard) if running_same: print(f"ERROR: {name} already running in agent-sessions.yaml", flush=True) raise SystemExit(4) diff --git a/tests/data/skill_code_blocks.json b/tests/data/skill_code_blocks.json index 541a1a8..2f116f0 100644 --- a/tests/data/skill_code_blocks.json +++ b/tests/data/skill_code_blocks.json @@ -88,8 +88,8 @@ }, { "skill": "multi-agent-mux-create", - "start": 202, - "end": 206, + "start": 204, + "end": 208, "lang": "bash", "sha": "38836d31bf38", "class": "c", @@ -103,8 +103,8 @@ }, { "skill": "multi-agent-mux-create", - "start": 228, - "end": 250, + "start": 230, + "end": 252, "lang": "bash", "sha": "d90cfd2555fd", "class": "b", @@ -617,7 +617,7 @@ "other" ], "passthrough": { - "4": "multi-agent-mux-create/scripts/create_session.sh:532" + "4": "multi-agent-mux-create/scripts/create_session.sh:546" } } }, diff --git a/tests/test_herdr_shim_contract.py b/tests/test_herdr_shim_contract.py index cb4d050..bfc562f 100644 --- a/tests/test_herdr_shim_contract.py +++ b/tests/test_herdr_shim_contract.py @@ -604,3 +604,37 @@ kill "$_mam_server_pid" 2>/dev/null || true f"daemon child stayed in the caller's process group (not detached): {fields}" ) + + +def test_h26_new_session_claims_its_pane_under_the_servers_lock(mam_sandbox, mock_herdr, mock_agents): + """Issue #50 follow-up: from the pane choice to `agent start` the shim holds + one lock per herdr server (lib_py.pane_lock, a flock on fd 9). While + another shim holds it, this one waits MAM_PANE_CLAIM_WAIT_S and then gives + up without creating, splitting or starting anything; once it is free the + same call goes through as before.""" + import glob + import os + tmp_path = mam_sandbox + res = _run_lib(tmp_path, f'herdr new-session -d -s "test-creator-claude" -c "{tmp_path}" "claude --dangerously-skip-permissions"') + assert res.returncode == 0, res.stderr + locks = glob.glob(str(tmp_path / ".local" / "state" / "mam" / "pane-lock" / "*.lock")) + assert len(locks) == 1, locks # one file per server, under the sandbox HOME + n_calls = len(_load_mock_state(mock_herdr).get("calls", [])) + holder = subprocess.Popen(["bash", "-c", 'exec 9>>"$1"; python3 -m lib_py.pane_lock acquire --fd 9; sleep 600', + "holder", locks[0]], stdout=subprocess.PIPE, text=True, + env=dict(os.environ, PYTHONPATH=str(tmp_path / ".agents" / "skills"))) + try: + assert holder.stdout.readline().strip() == "locked" + os.environ["MAM_PANE_CLAIM_WAIT_S"] = "1" + res = _run_lib(tmp_path, f'herdr new-session -d -s "test-reviewer-claude" -c "{tmp_path}" "claude --dangerously-skip-permissions"') + assert res.returncode == 1 and "did not end within MAM_PANE_CLAIM_WAIT_S" in res.stderr, (res.returncode, res.stderr) + new_calls = [c[2:] if c[:1] == ["--session"] else c for c in _load_mock_state(mock_herdr).get("calls", [])[n_calls:]] + assert not [c for c in new_calls if c[:2] in (["workspace", "create"], ["pane", "split"], ["agent", "start"])], new_calls + assert not [n for n in _load_mock_state(mock_herdr).get("agents", {}) if "test-reviewer-claude" in n] + finally: + os.environ.pop("MAM_PANE_CLAIM_WAIT_S", None) + holder.kill() + holder.wait() + res = _run_lib(tmp_path, f'herdr new-session -d -s "test-reviewer-claude" -c "{tmp_path}" "claude --dangerously-skip-permissions"') + assert res.returncode == 0, res.stderr + assert [n for n in _load_mock_state(mock_herdr).get("agents", {}) if "test-reviewer-claude" in n] diff --git a/tests/test_issue50_dispatch_herdr.py b/tests/test_issue50_dispatch_herdr.py index 3934d66..c56de70 100644 --- a/tests/test_issue50_dispatch_herdr.py +++ b/tests/test_issue50_dispatch_herdr.py @@ -196,6 +196,9 @@ def _make_box(sess: str) -> Box: for d in (box.bin, box.ws, box.ctl / ".agents", box.jobs, box.ctl / ".mam" / "delegate_job_logs"): d.mkdir(parents=True) (box.ctl / ".agents" / "skills").symlink_to(SKILLS) + # a fresh HOME without ~/.zshrc puts zsh's first-run menu in every new + # pane: the first `agent start` would type into it and time out (30 s) + (home / ".zshrc").touch() (box.bin / "claude").write_text(FAKE_CLAUDE) (box.bin / "claude").chmod(0o755) box.yaml.write_text("herdr_sessions:\n" + _row(sess, ws=str(box.ws))) @@ -608,32 +611,71 @@ def _drive_onboardings(box: Box, names: list, delays: list, result: dict): _after(delay, box.publish, jid, "completed", "Onboarding complete; aligned with role creator") +def _hold_pane_creation(b: Box, seconds: int) -> None: + """The real herdr on the box's PATH, with the reply of a pane-creating call + (workspace create, pane split) held back `seconds`. The shim's window - + a pane exists idle, `agent start` has not claimed it yet - is the one the + orchestrator hit; this makes it long enough for a shim started a moment + later to walk into it on every run, not one run in three.""" + (b.bin / "herdr").write_text(f'''#!/usr/bin/env bash +case " $* " in + *" workspace create "*|*" pane split "*) + out=$("{HERDR}" "$@"); rc=$? + sleep {seconds} + printf '%s\\n' "$out"; exit $rc ;; +esac +exec "{HERDR}" "$@" +''') + (b.bin / "herdr").chmod(0o755) + + +def _three_creates(b: Box, names: list, workspaces: list, stagger: float = 0.0): + """create --onboard x3 at once in one server; (outcome by name, driver result, elapsed).""" + procs = [] + for name, ws in zip(names, workspaces): + if procs and stagger: + time.sleep(stagger) + out = tempfile.TemporaryFile("w+", dir=str(b.home)) + err = tempfile.TemporaryFile("w+", dir=str(b.home)) + procs.append((name, subprocess.Popen( + ["bash", str(CREATE_SH), "--workspace", str(ws), "--agent", "claude", "--role", "creator", + "--session", name, "--onboard", "--herdr-session", b.sess], + stdout=out, stderr=err, text=True, cwd=str(b.ctl), env=b.env(MAM_ONBOARD_WAIT_S="120")), out, err)) + t0 = time.time() + result = {} + drv = threading.Thread(target=_drive_onboardings, args=(b, names, [10, 15, 20], result), daemon=True) + drv.start() + outcome = {} + for name, proc, out, err in procs: + rc = proc.wait(timeout=240) + out.seek(0); err.seek(0) + outcome[name] = (rc, out.read(), err.read()) + elapsed = time.time() - t0 + drv.join(timeout=5) + return outcome, result, elapsed + + +def _agent_panes(b: Box) -> dict: + """agent name -> (pane_id, pane cwd) from the server.""" + panes = {p["pane_id"]: p for p in json.loads(b.herdr("pane", "list").stdout)["result"]["panes"]} + out = {} + for a in json.loads(b.herdr("agent", "list").stdout)["result"]["agents"]: + pid = a.get("pane_id") or (a.get("pane") or {}).get("pane_id") + out[a["name"]] = (pid, (panes.get(pid) or {}).get("cwd")) + return out + + def test_a_three_concurrent_creates_with_onboarding(local_mqtt_broker): b = _make_box(f"t50-{uuid.uuid4().hex[:8]}") b.yaml.write_text("herdr_sessions: []\n") names = [f"t50-{i}-creator-claude" for i in (1, 2, 3)] - procs = [] try: - for i, name in enumerate(names, 1): - ws = b.home / f"ws{i}" - ws.mkdir() - out = tempfile.TemporaryFile("w+", dir=str(b.home)) - err = tempfile.TemporaryFile("w+", dir=str(b.home)) - procs.append((name, subprocess.Popen( - ["bash", str(CREATE_SH), "--workspace", str(ws), "--agent", "claude", "--role", "creator", - "--session", name, "--onboard", "--herdr-session", b.sess], - stdout=out, stderr=err, text=True, cwd=str(b.ctl), env=b.env(MAM_ONBOARD_WAIT_S="120")), out, err)) - t0 = time.time() - result = {} - drv = threading.Thread(target=_drive_onboardings, args=(b, names, [10, 15, 20], result), daemon=True) - drv.start() - outcome = {} - for name, proc, out, err in procs: - rc = proc.wait(timeout=240) - out.seek(0); err.seek(0) - outcome[name] = (rc, out.read(), err.read()) - elapsed = time.time() - t0 - drv.join(timeout=5) + workspaces = [] + for i in (1, 2, 3): + (b.home / f"ws{i}").mkdir() + workspaces.append(b.home / f"ws{i}") + outcome, result, elapsed = _three_creates(b, names, workspaces) + t0 = time.time() - elapsed assert all(o[0] == 0 for o in outcome.values()), {n: (o[0], o[2][-1500:]) for n, o in outcome.items()} # The three start-ups overlap, and the onboarding waits do too: once the # last prompt was delivered (its `started`), the whole run ends with the @@ -655,6 +697,75 @@ def test_a_three_concurrent_creates_with_onboarding(local_mqtt_broker): _teardown(b) +def test_a3_three_concurrent_creates_in_one_folder_take_distinct_panes(local_mqtt_broker): + """The same --workspace for all three - the control root itself, where + several reviewers are made at once: no control-plane binding is forwarded + to the pane, so the shim reuses an idle pane when it sees one (BUG-4). The + pane choice and its `agent start` are serialised per server, so no two + creates claim one pane (agent_pane_busy).""" + b = _make_box(f"t50-{uuid.uuid4().hex[:8]}") + b.yaml.write_text("herdr_sessions: []\n") + _hold_pane_creation(b, 8) + names = [f"t50-{i}-creator-claude" for i in (1, 2, 3)] + try: + assert b.sh("herdr agent list").returncode == 0 # the server is up: the creates race on panes only + outcome, result, elapsed = _three_creates(b, names, [b.ctl] * 3, stagger=2.0) + assert all(o[0] == 0 for o in outcome.values()), {n: (o[0], o[2][-1500:]) for n, o in outcome.items()} + assert "agent_pane_busy" not in "".join(o[1] + o[2] for o in outcome.values()) + for name in names: + jid = result["jobs"][name] + assert f"onboarding completed: job {jid} (" in outcome[name][1], outcome[name][1] + assert b.record(jid)["status"] == "completed" + rows = re.findall(r"^- name: (\S+)", b.yaml.read_text(), re.M) + assert sorted(rows) == sorted(names), rows + panes = _agent_panes(b) + assert sorted(panes) == sorted(names), panes + assert len({p for p, _c in panes.values()}) == 3, ("two agents on one pane", panes) + assert all(Path(c).resolve() == b.ctl.resolve() for _p, c in panes.values()), panes + assert len(b.server_pids()) == 1 + finally: + _teardown(b) + + +def test_s_three_concurrent_new_sessions_in_one_folder_take_distinct_panes(): + """The shim alone, no create around it: three `new-session` into one cwd on + one running server at once. Before the pane claim was serialised, two of + them chose the same idle pane and the later `agent start` failed with + agent_pane_busy.""" + b = _make_box(f"t50-{uuid.uuid4().hex[:8]}") + _hold_pane_creation(b, 2) + names = [f"t50-{i}-creator-claude" for i in (1, 2, 3)] + try: + assert b.sh("herdr agent list").returncode == 0 # the server is up before the race + procs = [] + for name in names: + if procs: + time.sleep(0.7) + out = tempfile.TemporaryFile("w+", dir=str(b.home)) + err = tempfile.TemporaryFile("w+", dir=str(b.home)) + procs.append((name, subprocess.Popen( + ["bash", "-c", f'source "{SKILLS}/lib.sh"\nherdr new-session -d -s {name} -c "{b.ws}" ' + '"claude --dangerously-skip-permissions"'], + stdout=out, stderr=err, text=True, cwd=str(b.ctl), env=b.env()), out, err)) + outcome = {} + for name, proc, out, err in procs: + rc = proc.wait(timeout=120) + out.seek(0); err.seek(0) + outcome[name] = (rc, out.read(), err.read()) + assert all(o[0] == 0 for o in outcome.values()), {n: (o[0], o[2][-800:]) for n, o in outcome.items()} + panes = _agent_panes(b) + assert sorted(panes) == sorted(names), panes + assert len({p for p, _c in panes.values()}) == 3, ("two agents on one pane", panes) + assert all(Path(c).resolve() == b.ws.resolve() for _p, c in panes.values()), panes + for name in names: + deadline = time.time() + 30 + while time.time() < deadline and b.agent_status(name) != "idle": + time.sleep(0.5) + assert b.agent_status(name) == "idle", (name, b.herdr("agent", "list").stdout) + finally: + _teardown(b) + + def _git(repo: Path, *args): subprocess.run(["git", "-c", "user.email=t@example.com", "-c", "user.name=t", *args], cwd=str(repo), check=True, capture_output=True, text=True) diff --git a/tests/test_issue50_pane_lock.py b/tests/test_issue50_pane_lock.py new file mode 100644 index 0000000..4237b47 --- /dev/null +++ b/tests/test_issue50_pane_lock.py @@ -0,0 +1,162 @@ +"""Issue #50 follow-up: the herdr shim claims one pane at a time per server. + +- P: lib_py.pane_lock - the lock file per server, the acquire decision and its + bound, a lock that ends with its holder's process (no stale lock after + kill -9), the wait knob. +- H: the race itself, on a real herdr server: test_issue50_dispatch_herdr.py + (test_s_three_concurrent_new_sessions_in_one_folder_take_distinct_panes, + test_a3_three_concurrent_creates_in_one_folder_take_distinct_panes). +- the shim's use of the lock against the mock herdr: test_herdr_shim_contract.py h26. +""" +from __future__ import annotations + +import json +import os +import signal +import subprocess +import sys +import time +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +SKILLS = REPO_ROOT / ".agents" / "skills" +if str(SKILLS) not in sys.path: + sys.path.insert(0, str(SKILLS)) + +from lib_py import pane_lock # noqa: E402 + +# a bash holder, as the shim is: fd 9 on the file, the lock taken by the helper, then it sleeps +HOLDER = 'exec 9>>"$1"; python3 -m lib_py.pane_lock acquire --fd 9; echo held; sleep "${2:-600}"' + + +def _holder(path: Path, seconds: str = "600", **env) -> subprocess.Popen: + p = subprocess.Popen(["bash", "-c", HOLDER, "holder", str(path), seconds], stdout=subprocess.PIPE, text=True, + env=dict(os.environ, PYTHONPATH=str(SKILLS), **env)) + assert p.stdout.readline().strip() == "locked" + assert p.stdout.readline().strip() == "held" + return p + + +def _acquire(path: Path, **env) -> subprocess.CompletedProcess: + return subprocess.run(["bash", "-c", 'exec 9>>"$1"; python3 -m lib_py.pane_lock acquire --fd 9', "x", str(path)], + capture_output=True, text=True, env=dict(os.environ, PYTHONPATH=str(SKILLS), **env), timeout=60) + + +def test_p1_the_lock_file_is_per_server_under_the_sessions_home(tmp_path): + env = {"HOME": str(tmp_path)} + assert pane_lock.lock_path("t50-x", env) == str(tmp_path / ".local/state/mam/pane-lock/t50-x.lock") + assert pane_lock.lock_path("", env) == str(tmp_path / ".local/state/mam/pane-lock/default.lock") + assert pane_lock.lock_path("t50-x", {"HOME_DIR": str(tmp_path / "h")}).startswith(str(tmp_path / "h")) + assert pane_lock.lock_path("t50-x", env) != pane_lock.lock_path("t50-y", env) + res = subprocess.run([sys.executable, "-m", "lib_py.pane_lock", "path", "--session", "t50-x"], capture_output=True, + text=True, env=dict(os.environ, PYTHONPATH=str(SKILLS), HOME=str(tmp_path)), timeout=60) + assert res.stdout.strip() == pane_lock.lock_path("t50-x", env), res.stderr + + +def test_p2_a_free_lock_is_taken_at_once_and_a_held_one_times_out(tmp_path): + lock = tmp_path / "s.lock" + t0 = time.time() + assert _acquire(lock).stdout.strip() == "locked" and time.time() - t0 < 5 + holder = _holder(lock) + try: + t0 = time.time() + res = _acquire(lock, MAM_PANE_CLAIM_WAIT_S="2") + assert res.stdout.strip() == "timeout", res.stderr + assert 1.5 <= time.time() - t0 < 10 + assert _acquire(tmp_path / "other-server.lock", MAM_PANE_CLAIM_WAIT_S="2").stdout.strip() == "locked" + finally: + holder.kill() + holder.wait() + + +def test_p3_the_lock_ends_with_its_holders_process(tmp_path): + lock = tmp_path / "s.lock" + holder = _holder(lock) + os.kill(holder.pid, signal.SIGKILL) + holder.wait() + t0 = time.time() + assert _acquire(lock, MAM_PANE_CLAIM_WAIT_S="30").stdout.strip() == "locked" + assert time.time() - t0 < 5, "nothing stale to wait out" + + +def test_p4_a_waiter_gets_the_lock_when_the_holder_ends(tmp_path): + lock = tmp_path / "s.lock" + holder = _holder(lock, "2") + t0 = time.time() + res = _acquire(lock, MAM_PANE_CLAIM_WAIT_S="30") + assert res.stdout.strip() == "locked" + assert 1 <= time.time() - t0 < 10 + holder.wait() + + +def test_p5_the_wait_knob_is_a_decimal_with_a_floor_of_one(monkeypatch): + assert pane_lock.wait_s({}) == 120 + assert pane_lock.wait_s({"MAM_PANE_CLAIM_WAIT_S": "7"}) == 7 + assert pane_lock.wait_s({"MAM_PANE_CLAIM_WAIT_S": "0700"}) == 700 + for bad in ("0", "abc", "", "-1", " "): + assert pane_lock.wait_s({"MAM_PANE_CLAIM_WAIT_S": bad}) == 120, bad + + +# --------------------------------------------------------------------------- +# A - create keeps its own pane when the monitor registered it first. With +# pane claims serialised, one create can reach its end-of-run reconcile while +# another is between `agent start` and its row write; the adopted row is that +# create's own pane, not a second live session of its name. +# --------------------------------------------------------------------------- + + + +def _seed_row(tmp_path, name, epoch, note="auto-registered by monitor", job=None): + row = {"name": name, "status": "running", "role": "creator", "herdr_session_created_at": "2026-10-11T00:00:00Z", + "herdr_session_epoch": epoch, "herdr_session": "default", "herdr_server": "default", + "pane": {"index": 0, "pid": 9999, "cmd": "claude", "cmd_full": "claude", "cwd": str(tmp_path)}, + "last_visible_status": "running"} + if note: + row["last_visible_note"] = note + if job: + row["delegate_job_id"] = job + yaml_path = tmp_path / ".mam" / "agent-sessions.yaml" + yaml_path.parent.mkdir(parents=True, exist_ok=True) + yaml_path.write_text("herdr_sessions:\n- " + json.dumps(row) + "\n") + return yaml_path + + +def _create(tmp_path, name, role="creator"): + return subprocess.run(["bash", str(tmp_path / "skills" / "multi-agent-mux-create" / "scripts" / "create_session.sh"), + "--workspace", str(tmp_path), "--agent", "claude", "--role", role, "--session", name, + "--no-onboard"], capture_output=True, text=True, cwd=str(tmp_path), timeout=300) + + +def _rows(yaml_path): + import yaml + return yaml.safe_load(yaml_path.read_text())["herdr_sessions"] + + +def test_a1_create_replaces_the_monitors_registration_of_its_own_pane(mam_sandbox, mock_herdr, mock_agents): + name = "t50-adopted-creator-claude" + yaml_path = _seed_row(mam_sandbox, name, int(time.time())) # registered after this create began + res = _create(mam_sandbox, name, role="Creator") # the monitor guessed "creator" off the name + assert res.returncode == 0, res.stdout + res.stderr + assert f"NOTE: the monitor registered {name} while it was being created" in res.stdout + rows = [r for r in _rows(yaml_path) if r["name"] == name] + assert len(rows) == 1 and rows[0]["status"] == "running" and rows[0]["role"] == "Creator" + assert rows[0].get("last_visible_note") != "auto-registered by monitor" and rows[0].get("start_command") + + +@pytest.mark.parametrize("kind", ["real", "old-adoption", "adoption-with-job"]) +def test_a2_another_live_session_of_that_name_is_still_refused(mam_sandbox, mock_herdr, mock_agents, kind): + name = "t50-taken-creator-claude" + now = int(time.time()) + if kind == "real": + yaml_path = _seed_row(mam_sandbox, name, now, note=None) + elif kind == "old-adoption": + yaml_path = _seed_row(mam_sandbox, name, now - 3600) # a pane from before this create + else: + yaml_path = _seed_row(mam_sandbox, name, now, job="deadbeef") + res = _create(mam_sandbox, name) + assert res.returncode == 4, res.stdout + res.stderr + assert f"ERROR: {name} already running in agent-sessions.yaml" in res.stdout + rows = [r for r in _rows(yaml_path) if r["name"] == name] + assert len(rows) == 1 and rows[0]["status"] == "running"