diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index 10adeed..744559f 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -1495,8 +1495,10 @@ _herdr_scope_is_pane_inherited() { # 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 +# MAM_PANE_CLAIM_WAIT_S). It ends with the block, or when every process that +# holds fd 9 - this shell and the shim it runs - has ended, however that ends, +# so a dead holder leaves nothing behind. Nothing long-lived may inherit fd 9 +# (the herdr server is spawned with close_fds; review 66680d9d); 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). # diff --git a/.agents/skills/lib_py/delegation/registry.py b/.agents/skills/lib_py/delegation/registry.py index 29fe970..18241cf 100755 --- a/.agents/skills/lib_py/delegation/registry.py +++ b/.agents/skills/lib_py/delegation/registry.py @@ -90,6 +90,7 @@ def register_job( transport: str = "herdr", supersede_older_attempt: bool = False, parent_job: Optional[str] = None, + dry_run: bool = False, ) -> str: """Create a new ``pending`` job record and return its id. @@ -106,6 +107,10 @@ def register_job( Issue #34: ``parent_job`` names the job this one was delegated from, so a loop waiting on the parent can find and wait for it. None is a root job. + + Issue #50 (review 1e25cf2f F1): ``dry_run`` marks the record of a + ``submit --dry-run``, which is never dispatched, so the registry admission + does not count it as an open job of its session. """ job_id = job_id or generate_job_id(bits) heartbeat_sec = max(1, int(heartbeat_sec)) @@ -152,6 +157,8 @@ def register_job( # Issue #34: the job this one was delegated from (None = a root job) "parent_job": parent_job, } + if dry_run: + record["dry_run"] = True with registry_lock(registry_dir): if mqtt_common._job_path(job_id, registry_dir).exists(): if not supersede_older_attempt: @@ -393,6 +400,8 @@ def _build_parser() -> argparse.ArgumentParser: p_reg.add_argument("--reviewer", default=None) p_reg.add_argument("--reviewer-session", default=None) p_reg.add_argument("--max-iterations", type=int, default=5) + p_reg.add_argument("--dry-run", action="store_true", + help="the record of a submit --dry-run: never dispatched, not an open job for admission") p_reg.add_argument("--parent-job", default=None, help="the job this one is delegated from (Issue #34); must exist in this registry") @@ -569,6 +578,7 @@ def main(argv: Optional[List[str]] = None) -> int: attempt_id=args.attempt_id, job_id=args.job_id, parent_job=args.parent_job, + dry_run=args.dry_run, ) print(job_id) return 0 diff --git a/.agents/skills/lib_py/delegation/wrapper.py b/.agents/skills/lib_py/delegation/wrapper.py index a4ea77d..a6aa42f 100644 --- a/.agents/skills/lib_py/delegation/wrapper.py +++ b/.agents/skills/lib_py/delegation/wrapper.py @@ -297,6 +297,8 @@ def open_jobs(registry_dir: str, session: str, job_id: str, now: Optional[int] = rec = _load_record(registry_dir, other) if rec is None or _bare(str(rec.get("agent_session") or "")) != sess: continue + if rec.get("dry_run"): # a submit --dry-run is never dispatched (review 1e25cf2f F1) + continue status = rec.get("status") if status == "running": idle = _int_field(rec, "idle_timeout_sec", DEFAULT_IDLE_TIMEOUT_SEC) diff --git a/.agents/skills/lib_py/pane_lock.py b/.agents/skills/lib_py/pane_lock.py index ef18d04..bfc2cc9 100644 --- a/.agents/skills/lib_py/pane_lock.py +++ b/.agents/skills/lib_py/pane_lock.py @@ -19,9 +19,12 @@ one lock per server from the choice to the claim. `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 +The lock is a BSD flock, which belongs to the open file on fd 9: it outlives +this helper and ends when the last process holding that fd ends, however it +ends (exit, kill -9), so a dead holder leaves nothing to steal. The holders are +the shell running the claim block and the command it runs (the shim); a child +that outlived them would keep it too, so nothing long-lived may inherit fd 9 - +the herdr server is spawned with close_fds (review 66680d9d). flock(1) is not on macOS; fcntl.flock is. Stdlib only, Python 3.9. """ from __future__ import annotations @@ -67,13 +70,14 @@ def wait_s(env: Mapping[str, str]) -> int: 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.""" + stretch or cut it. Only contention is waited out: any other error (a bad + fd) raises, which the shim reports as an undecided lock.""" deadline = time.monotonic() + wait while True: try: fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) return "locked" - except OSError: + except BlockingIOError: if time.monotonic() >= deadline: return "timeout" time.sleep(0.2) 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 530bbb7..6f5c960 100755 --- a/.agents/skills/multi-agent-mux-create/scripts/create_session.sh +++ b/.agents/skills/multi-agent-mux-create/scripts/create_session.sh @@ -533,9 +533,13 @@ running_same = [s for s in sessions if s.get('name') == name and s.get('status') # 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. +# refused as a second live session. Names are unique per server only, so the +# row must be this server's (review 1e25cf2f F2: another server's row of the +# name was replaced). The pane pid is no identity: it is the pane's foreground +# process at the time it was read. def _adopted_own(s): return (s.get('last_visible_note') == 'auto-registered by monitor' and not s.get('delegate_job_id') + and (s.get('herdr_server') or s.get('herdr_session') or 'default') == server_name 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) diff --git a/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job b/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job index 4370f1d..83531b0 100755 --- a/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job +++ b/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job @@ -292,6 +292,10 @@ cmd_submit() { fi fi + # Issue #50: a dry-run's record is never dispatched, so admission skips it + local reg_dry=() + if [[ "$DRY_RUN" == "1" ]]; then reg_dry=(--dry-run); fi + # 1) register job (prints the new job id) JOB_ID="$("$PY" "$SCRIPT_DIR/scripts/registry.py" --registry-dir "$REGISTRY_DIR" register \ "${reg_prompt[@]}" --agent "$AGENT" --agent-session "$AGENT_SESSION" --role "$DELEGATE_ROLE" \ @@ -299,7 +303,7 @@ cmd_submit() { --heartbeat-sec "$HEARTBEAT_SEC" --heartbeat-misses "$HEARTBEAT_MISSES" --ack-timeout "$ACK_TIMEOUT" \ --transport "$(_is_remote "$AGENT_SESSION" && echo remote || echo herdr)" \ --job-type "$TYPE" --reviewer "$REVIEWER" --reviewer-session "$REVIEWER_SESSION" \ - --max-iterations "$MAX_ITERATIONS" ${reg_parent[@]+"${reg_parent[@]}"})" + --max-iterations "$MAX_ITERATIONS" ${reg_parent[@]+"${reg_parent[@]}"} ${reg_dry[@]+"${reg_dry[@]}"})" echo "registered job: $JOB_ID" # 1-1) Provision job directory and write direct brief.md (MAM Job Restructuring) diff --git a/tests/test_herdr_shim_contract.py b/tests/test_herdr_shim_contract.py index bfc562f..9023258 100644 --- a/tests/test_herdr_shim_contract.py +++ b/tests/test_herdr_shim_contract.py @@ -620,7 +620,7 @@ def test_h26_new_session_claims_its_pane_under_the_servers_lock(mam_sandbox, moc 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 = subprocess.Popen(["bash", "-c", 'exec 9>>"$1"; python3 -m lib_py.pane_lock acquire --fd 9; exec sleep 600', "holder", locks[0]], stdout=subprocess.PIPE, text=True, env=dict(os.environ, PYTHONPATH=str(tmp_path / ".agents" / "skills"))) try: diff --git a/tests/test_issue50_dispatch.py b/tests/test_issue50_dispatch.py index 298d677..d1a5403 100644 --- a/tests/test_issue50_dispatch.py +++ b/tests/test_issue50_dispatch.py @@ -159,6 +159,26 @@ def test_u1c_the_job_itself_and_unreadable_records_are_skipped(tmp_path): assert decide.open_jobs(str(tmp_path), "herdr:s", me) == ([], []) +def test_u1e_a_dry_run_record_is_not_an_open_job(tmp_path): + """review 1e25cf2f F1: `submit --dry-run` registers a record it never + dispatches; it stays pending (Issue #34 shows it as an open job), but the + admission must not wait for it.""" + reg = tmp_path / "jobs" + env = dict(os.environ, HOME=str(tmp_path), MAM_ENV_FILE=str(tmp_path / ".mam.env.absent")) + res = subprocess.run(["bash", str(WRAPPER), "submit", "--agent", "human", "--agent-session", "herdr:s", + "--dry-run", "--registry-dir", str(reg), "--prompt", "preview only"], + capture_output=True, text=True, env=env, cwd=str(tmp_path), timeout=120) + assert res.returncode == 0, res.stdout + res.stderr + (dry,) = [p.stem for p in reg.glob("*.json")] + assert _rec(reg, dry)["status"] == "pending" and _rec(reg, dry)["dry_run"] is True + me = _register(reg) + assert decide.open_jobs(str(reg), "herdr:s", me) == ([], []) + assert decide.wait_open_jobs(_ns(reg, me)) == "clear" + # control: the same record without the mark is an `injected` job that holds the session + _edit(reg, dry, dry_run=None) + assert decide.wait_open_jobs(_ns(reg, me)) == f"busy {dry}" + + # --------------------------------------------------------------------------- # U2 - FIFO release and the bound (wait_s=0: an immediate decision) # --------------------------------------------------------------------------- diff --git a/tests/test_issue50_dispatch_herdr.py b/tests/test_issue50_dispatch_herdr.py index cf36bee..70b606d 100644 --- a/tests/test_issue50_dispatch_herdr.py +++ b/tests/test_issue50_dispatch_herdr.py @@ -55,6 +55,9 @@ FAKE_CLAUDE = r'''#!/usr/bin/env bash if [ "${1:-}" = "auth" ]; then echo '{"loggedIn": true}'; exit 0; fi printf '\n ▐▛███▛█ Claude Code v2.1.0 (fake)\n▝▜██████▀ Sonnet · Claude Max\n\n' printf '───────────────────────────────────────────────\n❯ Try "how does work?"\n───────────────────────────────────────────────\n ⏵⏵ bypass permissions on (shift+tab to cycle)\n' +# non-canonical input, as a real TUI reads it: a canonical tty line is capped at +# MAX_CANON (1024 bytes on macOS), shorter than one injected prompt +stty -icanon 2>/dev/null while IFS= read -r line; do printf '\n● got: %s\n❯ \n' "$line"; done ''' @@ -99,7 +102,7 @@ class Box: return "" def pane_text(self, name=SESSION) -> str: - res = self.herdr("agent", "read", name, "--source", "recent") + res = self.herdr("agent", "read", name, "--source", "recent-unwrapped", "--lines", "1000") try: d = json.loads(res.stdout)["result"] return (d.get("read") or d).get("text", "") @@ -192,7 +195,9 @@ def _row(sess: str, name=SESSION, ws="") -> str: def _make_box(sess: str) -> Box: - home = Path(tempfile.mkdtemp(prefix="mam50-")) # short: a unix socket path lives under it + # short: a unix socket path lives under it (macOS sun_path is 104 bytes and + # its TMPDIR is long); resolved, as the scripts print /private/tmp paths + home = Path(tempfile.mkdtemp(prefix="mam50-", dir="/tmp")).resolve() box = Box(home, sess) for d in (box.bin, box.ws, box.ctl / ".agents", box.jobs, box.ctl / ".mam" / "delegate_job_logs"): d.mkdir(parents=True) @@ -382,7 +387,7 @@ def test_c4_the_loops_first_job_waits_for_the_onboarding_to_end(box, monkeypatch def test_c5_a_session_without_a_row_is_judged_by_the_records_alone(local_mqtt_broker): # resolve_herdr_session falls back to the workspace slug when the yaml has no # row, so the server is named exactly that: mam-- of the control root - home = Path(tempfile.mkdtemp(prefix="mam50-")) + home = Path(tempfile.mkdtemp(prefix="mam50-", dir="/tmp")).resolve() sess = f"mam-{home.name}-ctl".lower().replace("_", "-") shutil.rmtree(home) _TIMERS.clear() diff --git a/tests/test_issue50_pane_lock.py b/tests/test_issue50_pane_lock.py index 809bb66..83b8f3a 100644 --- a/tests/test_issue50_pane_lock.py +++ b/tests/test_issue50_pane_lock.py @@ -28,7 +28,7 @@ if str(SKILLS) not in sys.path: 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}"' +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: @@ -133,9 +133,12 @@ def test_p5_the_wait_knob_is_a_decimal_with_a_floor_of_one(monkeypatch): -def _seed_row(tmp_path, name, epoch, note="auto-registered by monitor", job=None): +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": "default", "herdr_server": "default", + "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: @@ -151,7 +154,7 @@ def _seed_row(tmp_path, name, epoch, note="auto-registered by monitor", job=None 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) + "--herdr-session", SERVER, "--no-onboard"], capture_output=True, text=True, cwd=str(tmp_path), timeout=300) def _rows(yaml_path): @@ -161,7 +164,7 @@ def _rows(yaml_path): 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 + 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 @@ -170,7 +173,7 @@ def test_a1_create_replaces_the_monitors_registration_of_its_own_pane(mam_sandbo 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"]) +@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()) @@ -178,6 +181,8 @@ def test_a2_another_live_session_of_that_name_is_still_refused(mam_sandbox, mock 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)