- admission: a `submit --dry-run` record is marked dry_run and is not an open job of its session (review 1e25cf2f F1) - create: the monitor-row adoption requires the row's server to be the one this create spawned on (review 1e25cf2f F2) - pane lock: retry only on contention (BlockingIOError); the docs say the flock lasts while any fd-9 holder (the claim shell and the shim) lives (review 66680d9d) - tests on macOS: real-herdr box HOMEs under /tmp (sun_path is 104 bytes), resolved paths, the fake claude reads non-canonical input (MAX_CANON 1024) and the pane is read unwrapped; lock holders exec their sleep so killing them frees the lock; the adoption tests name the server Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
193 lines
9.7 KiB
Python
193 lines
9.7 KiB
Python
"""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; exec 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
|
|
|
|
|
|
# review e8e471ff R1: the ambient target (no -L, HERDR_SESSION_NAME empty or
|
|
# `default`) is answered by the server of the pane the caller runs in - the
|
|
# pane's HERDR_SESSION / HERDR_SOCKET_PATH say which - so that is the lock;
|
|
# only outside a pane is it the default server's
|
|
|
|
def test_p1b_the_ambient_target_locks_the_callers_server(tmp_path):
|
|
home = {"HOME": str(tmp_path)}
|
|
sock = {"HERDR_SOCKET_PATH": str(tmp_path / ".config/herdr/sessions/t50-x/herdr.sock")}
|
|
named = pane_lock.lock_path("t50-x", home)
|
|
for ambient in ("", "default", None):
|
|
assert pane_lock.lock_path(ambient, {**home, **sock}) == named, ambient
|
|
assert pane_lock.lock_path(ambient, {**home, "HERDR_SESSION": "t50-x"}) == named, ambient
|
|
assert pane_lock.lock_path(ambient, {**home, "HERDR_SESSION": "t50-x", # HERDR_SESSION first
|
|
"HERDR_SOCKET_PATH": str(tmp_path / ".config/herdr/sessions/t50-y/herdr.sock")}) == named
|
|
assert pane_lock.lock_path(ambient, {**home, "HERDR_SOCKET_PATH": str(tmp_path / ".config/herdr/herdr.sock")}) \
|
|
== pane_lock.lock_path("default", home) == str(tmp_path / ".local/state/mam/pane-lock/default.lock")
|
|
assert pane_lock.lock_path(ambient, {**home, "HERDR_SOCKET_PATH": "/elsewhere/herdr.sock"}).endswith("/default.lock")
|
|
assert pane_lock.lock_path(ambient, home).endswith("/default.lock")
|
|
assert pane_lock.lock_path("t50-y", {**home, **sock}) != named, "a named target is that server's, wherever it is called from"
|
|
outside = {k: v for k, v in os.environ.items() if not k.startswith("HERDR_")} # this test may run in a pane
|
|
res = subprocess.run([sys.executable, "-m", "lib_py.pane_lock", "path", "--session", ""], capture_output=True, text=True,
|
|
env=dict(outside, PYTHONPATH=str(SKILLS), HOME=str(tmp_path), **sock), timeout=60)
|
|
assert res.stdout.strip() == named, 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.
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
|
|
SERVER = "t50-srv" # the server _create names; a monitor row carries the server it found the pane on
|
|
|
|
|
|
def _seed_row(tmp_path, name, epoch, note="auto-registered by monitor", job=None, server=SERVER):
|
|
row = {"name": name, "status": "running", "role": "creator", "herdr_session_created_at": "2026-10-11T00:00:00Z",
|
|
"herdr_session_epoch": epoch, "herdr_session": server, "herdr_server": server,
|
|
"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,
|
|
"--herdr-session", SERVER, "--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()) + 60) # 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", "other-server"])
|
|
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
|
|
elif kind == "other-server": # review 1e25cf2f F2: names are unique per server
|
|
yaml_path = _seed_row(mam_sandbox, name, now + 60, server="different-server")
|
|
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"
|