The record (status, token, attempt, transition_seq) was read before the audit status.json; if A error -> redispatch (attempt 2) -> B completed happened in between, the audit terminal was B's while the token and attempt validated were A's, and A's caller was told "completed". The audit status is now taken only when a fresh read of the record still shows the same token, attempt and transition_seq; otherwise the job stays pending and the next round reads both afresh. The no-record case is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
602 lines
33 KiB
Python
602 lines
33 KiB
Python
#!/usr/bin/env python3
|
|
"""Issue #57 — the audit log follows the registry.
|
|
|
|
`--registry-dir` / `MAM_JOBS_DIR` moved the registry, but the audit log kept
|
|
its own root (`DELEGATE_JOB_LOGS_DIR`, else the workspace's), so an isolated
|
|
experiment left audit-only directories in the real control plane. The rule is
|
|
now one function, `mqtt_common.get_logs_dir(registry_dir)`: an explicit
|
|
`DELEGATE_JOB_LOGS_DIR` wins, otherwise the audit log is the
|
|
`delegate_job_logs` next to the registry in use. Every test ends by asserting
|
|
that the workspace's own `.mam/delegate_job_logs` was never created.
|
|
|
|
Comment 5 of the issue: `job_subscriber._check_disk_fallback` compared the
|
|
dict `read_logged_status()` returns with status strings, so an audit-only
|
|
terminal status was never recognised.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import os
|
|
import subprocess
|
|
import sys
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
import _mam_state_guard # noqa: F401
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
|
SKILLS = REPO_ROOT / ".agents" / "skills"
|
|
WRAPPER = SKILLS / "multi-agent-mux-delegate-job" / "multi-agent-mux-delegate-job"
|
|
REGISTRY_PY = SKILLS / "multi-agent-mux-delegate-job" / "scripts" / "registry.py"
|
|
PUBLISH_PY = SKILLS / "multi-agent-mux-delegate-job" / "scripts" / "publish_event.py"
|
|
|
|
if str(SKILLS) not in sys.path:
|
|
sys.path.insert(0, str(SKILLS))
|
|
|
|
from lib_py import control_plane # noqa: E402
|
|
from lib_py.delegation import engine, job_subscriber, mqtt_common, registry # noqa: E402
|
|
from test_issue21_phase2_loop_a import _mutate, _prepare, _rows, _skill, _wt # noqa: E402
|
|
from test_issue21_phase2_loop_a import ctl # noqa: E402,F401
|
|
from test_issue21_worktree_prereqs import _env_flags, _sent_texts # noqa: E402
|
|
|
|
UNREACHABLE = {"MQTT_BROKER": "127.0.0.1", "MQTT_PORT": "9", "MQTT_TLS": "0"}
|
|
DROP = ("MAM_JOBS_DIR", "DELEGATE_JOB_LOGS_DIR")
|
|
|
|
|
|
@pytest.fixture
|
|
def moved(tmp_path, monkeypatch):
|
|
"""A workspace whose registry lives elsewhere, with neither override set:
|
|
conftest's autouse defaults would otherwise hide the fallback (comment 7)."""
|
|
for k in DROP:
|
|
monkeypatch.delenv(k, raising=False)
|
|
ws = tmp_path / "ws"
|
|
(ws / ".mam").mkdir(parents=True)
|
|
reg = tmp_path / "elsewhere" / "jobs"
|
|
monkeypatch.setenv("WORKSPACE_ROOT", str(ws))
|
|
monkeypatch.chdir(ws)
|
|
yield ws, reg, tmp_path / "elsewhere" / "delegate_job_logs"
|
|
assert not (ws / ".mam" / "delegate_job_logs").exists(), "the workspace's audit-log root was created"
|
|
|
|
|
|
def _env(ws: Path, **extra) -> dict:
|
|
env = {k: v for k, v in os.environ.items() if k not in DROP}
|
|
env.update(WORKSPACE_ROOT=str(ws), PYTHONPATH=str(SKILLS), DELEGATE_JOB_PYTHON=sys.executable,
|
|
AGENT_PYTHON_BIN=sys.executable, MAM_ENV_FILE=str(ws / ".mam.env.absent"), **UNREACHABLE)
|
|
env.update({k: str(v) for k, v in extra.items()})
|
|
return env
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# comment 5: the disk fallback reads status.json's status, not the dict
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_disk_fallback_reads_the_audit_only_terminal_status(moved):
|
|
ws, reg, logs = moved
|
|
reg.mkdir(parents=True)
|
|
# no registry record at all: only the audit log says the job ended
|
|
jid = "a57fb001"
|
|
mqtt_common.update_logged_status(jid, "completed", logs_dir=mqtt_common.get_logs_dir(str(reg)))
|
|
pending, terminal = {jid}, {}
|
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, None, None)
|
|
assert terminal == {jid: "completed"}
|
|
assert pending == set()
|
|
|
|
# a pending record with a dispatch token, an audit-only `error`
|
|
tok = "0123456789abcdef0123456789abcdef"
|
|
jid2 = registry.register_job("fb2", registry_dir=str(reg), agent_session="herdr:t57", dispatch_token=tok)
|
|
mqtt_common.update_logged_status(jid2, "error", logs_dir=mqtt_common.get_logs_dir(str(reg)))
|
|
pending, terminal = {jid2}, {}
|
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid2: tok}, None)
|
|
assert terminal == {jid2: "error"} and pending == set()
|
|
# a different expected token still ignores it (#14/#18 rule, unchanged)
|
|
pending, terminal = {jid2}, {}
|
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid2: "f" * 32}, None)
|
|
assert terminal == {} and pending == {jid2}
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# L1 — the resolver
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_resolver_pairs_the_logs_with_the_registry(moved, monkeypatch):
|
|
ws, reg, logs = moved
|
|
# the usual roots are unchanged
|
|
assert mqtt_common.get_logs_dir() == str(ws / ".mam" / "delegate_job_logs")
|
|
assert mqtt_common.get_logs_dir(str(ws / ".mam" / "jobs")) == str(ws / ".mam" / "delegate_job_logs")
|
|
# a registry the caller names
|
|
assert mqtt_common.get_logs_dir(str(reg)) == str(logs)
|
|
# a registry moved by MAM_JOBS_DIR, and the compat alias
|
|
monkeypatch.setenv("MAM_JOBS_DIR", str(reg))
|
|
assert mqtt_common.get_logs_dir() == str(logs)
|
|
assert mqtt_common.LOGS_DIR == str(logs)
|
|
# an explicit root still wins
|
|
monkeypatch.setenv("DELEGATE_JOB_LOGS_DIR", str(ws / "x"))
|
|
assert mqtt_common.get_logs_dir() == str(ws / "x")
|
|
assert mqtt_common.get_logs_dir(str(reg)) == str(ws / "x")
|
|
monkeypatch.delenv("DELEGATE_JOB_LOGS_DIR")
|
|
# a relative registry resolves against the cwd, as registry.py opens it
|
|
assert mqtt_common.get_logs_dir("r/jobs") == os.path.join(os.getcwd(), "r", "delegate_job_logs")
|
|
# the sibling rule itself reads no environment
|
|
monkeypatch.setenv("DELEGATE_JOB_LOGS_DIR", str(ws / "x"))
|
|
assert mqtt_common.sibling_logs_dir(str(reg)) == str(logs)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# L2 — registry.py writes and reads next to --registry-dir
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def _registry(ws: Path, reg: Path, *args) -> subprocess.CompletedProcess:
|
|
return subprocess.run([sys.executable, str(REGISTRY_PY), "--registry-dir", str(reg), *args],
|
|
capture_output=True, text=True, env=_env(ws), cwd=str(ws), timeout=60)
|
|
|
|
|
|
def test_registry_cli_writes_and_reads_next_to_the_registry(moved):
|
|
ws, reg, logs = moved
|
|
res = _registry(ws, reg, "register", "--prompt", "p57", "--agent", "human", "--agent-session", "herdr:t57")
|
|
assert res.returncode == 0, res.stderr
|
|
jid = res.stdout.strip()
|
|
assert (logs / jid / "meta.json").is_file() and (logs / jid / "status.json").is_file()
|
|
assert _registry(ws, reg, "pick", "--agent-session", "herdr:t57").stdout.strip() == jid
|
|
assert _registry(ws, reg, "update", "--job", jid, "--status", "completed").returncode == 0
|
|
assert json.loads((logs / jid / "status.json").read_text())["status"] == "completed"
|
|
assert _registry(ws, reg, "get-feedback", "--job", jid).returncode == 0
|
|
res = _registry(ws, reg, "logs", jid)
|
|
assert res.returncode == 0, res.stderr
|
|
assert "registered" in res.stdout and "status_changed" in res.stdout
|
|
res = _registry(ws, reg, "logs", "--list")
|
|
assert res.returncode == 0 and jid in res.stdout, res.stdout + res.stderr
|
|
events = [json.loads(l) for l in (logs / jid / "events.ndjson").read_text().splitlines() if l.strip()]
|
|
assert [e["event"] for e in events][:2] == ["registered", "status_changed"]
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# L3 — the wrapper's last line names the directory it wrote
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def _wrapper(ws: Path, *args, cwd: Path = None, **extra) -> subprocess.CompletedProcess:
|
|
return subprocess.run(["bash", str(WRAPPER), *args], capture_output=True, text=True,
|
|
env=_env(ws, **extra), cwd=str(cwd or ws), timeout=120)
|
|
|
|
|
|
def _only_job_id(reg: Path) -> str:
|
|
ids = [p.stem for p in reg.glob("*.json")]
|
|
assert len(ids) == 1, ids
|
|
return ids[0]
|
|
|
|
|
|
def test_wrapper_output_names_the_dir_it_wrote(moved):
|
|
ws, reg, logs = moved
|
|
real_logs = Path(os.path.realpath(logs.parent)) / "delegate_job_logs"
|
|
# (a) dry-run: registered and seeded, both next to the registry
|
|
res = _wrapper(ws, "submit", "--dry-run", "--agent", "human", "--registry-dir", str(reg),
|
|
"--workdir", str(ws), "--prompt", "p57")
|
|
assert res.returncode == 0, res.stdout[-800:] + res.stderr[-800:]
|
|
jid = _only_job_id(reg)
|
|
assert res.stdout.strip().splitlines()[-1] == str(real_logs / jid)
|
|
assert (logs / jid / "meta.json").is_file()
|
|
# (b) a real human job against an unreachable broker: rc 3, same rule
|
|
res = _wrapper(ws, "submit", "--agent", "human", "--registry-dir", str(reg), "--workdir", str(ws),
|
|
"--prompt", "p57b", "--timeout", "3", "--idle-timeout", "2")
|
|
assert res.returncode == 3, res.stdout[-800:] + res.stderr[-800:]
|
|
jid2 = [p.stem for p in reg.glob("*.json") if p.stem != jid]
|
|
assert len(jid2) == 1, jid2
|
|
jid2 = jid2[0]
|
|
assert res.stdout.strip().splitlines()[-1] == str(real_logs / jid2)
|
|
events = [json.loads(l) for l in (logs / jid2 / "events.ndjson").read_text().splitlines() if l.strip()]
|
|
assert "published" in {e.get("event") for e in events}, events
|
|
# (c) wait and (d) logs find the same registry and its audit log
|
|
res = _wrapper(ws, "wait", "--registry-dir", str(reg), "--job", str(jid2), "--timeout", "2")
|
|
assert res.returncode in (0, 1, 2, 3), res.stdout[-400:] + res.stderr[-400:]
|
|
res = _wrapper(ws, "logs", "--registry-dir", str(reg), jid2)
|
|
assert res.returncode == 0 and "registered" in res.stdout, res.stdout + res.stderr
|
|
res = _wrapper(ws, "logs", "--registry-dir", str(reg), "--list")
|
|
assert res.returncode == 0 and jid2 in res.stdout, res.stdout + res.stderr
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# L4 — publish_event.py records next to --registry-dir
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_publish_event_records_next_to_the_registry(moved):
|
|
ws, reg, logs = moved
|
|
jid = registry.register_job("p4", registry_dir=str(reg), agent_session="herdr:t57")
|
|
(logs / jid / "events.ndjson").unlink() # only what publish_event writes counts
|
|
|
|
def publish(*args):
|
|
return subprocess.run([sys.executable, str(PUBLISH_PY), "--registry-dir", str(reg), "--job", jid,
|
|
"--attempts", "1", *args],
|
|
capture_output=True, text=True, env=_env(ws), cwd=str(ws), timeout=60)
|
|
|
|
publish("--event", "progress", "--detail", "x")
|
|
events = [json.loads(l) for l in (logs / jid / "events.ndjson").read_text().splitlines() if l.strip()]
|
|
assert [e for e in events if e.get("event") == "published" and e.get("source_event") == "progress"], events
|
|
publish("--event", "completed", "--detail", "done")
|
|
assert json.loads((logs / jid / "status.json").read_text())["status"] == "completed"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# the session binding derives the audit-log root from the registry
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_from_env_derives_the_logs_dir_from_a_moved_registry(moved):
|
|
ws, reg, logs = moved
|
|
b = control_plane.from_env({"WORKSPACE_ROOT": str(ws), "MAM_JOBS_DIR": str(reg)})
|
|
assert b["mam_jobs_dir"] == os.path.realpath(reg)
|
|
assert b["delegate_job_logs_dir"] == os.path.realpath(logs)
|
|
# an explicit root is kept as it is
|
|
b = control_plane.from_env({"WORKSPACE_ROOT": str(ws), "MAM_JOBS_DIR": str(reg),
|
|
"DELEGATE_JOB_LOGS_DIR": str(ws / "custom-logs")})
|
|
assert b["delegate_job_logs_dir"] == os.path.realpath(ws / "custom-logs")
|
|
# the usual binding is unchanged
|
|
assert control_plane.from_env({"WORKSPACE_ROOT": str(ws)}) == control_plane.defaults(str(ws))
|
|
# and the CLI lib.sh calls agrees
|
|
res = subprocess.run([sys.executable, "-m", "lib_py.control_plane", "json", str(ws / "wt")],
|
|
capture_output=True, text=True, env=_env(ws, MAM_JOBS_DIR=str(reg)), cwd=str(ws), timeout=60)
|
|
assert res.returncode == 0, res.stderr
|
|
assert json.loads(res.stdout)["delegate_job_logs_dir"] == os.path.realpath(logs)
|
|
|
|
|
|
def test_an_explicit_logs_dir_still_wins_over_a_moved_registry(tmp_path, monkeypatch):
|
|
"""Comment 1: DELEGATE_JOB_LOGS_DIR means the audit log was moved to a place
|
|
that cannot be derived; it wins over --registry-dir as before."""
|
|
for k in DROP:
|
|
monkeypatch.delenv(k, raising=False)
|
|
main, custom, exp = tmp_path / "main", tmp_path / "custom-logs", tmp_path / "exp" / "jobs"
|
|
(main / ".mam").mkdir(parents=True)
|
|
monkeypatch.setenv("WORKSPACE_ROOT", str(main))
|
|
monkeypatch.chdir(main)
|
|
env = dict(MAM_JOBS_DIR=str(main / ".mam" / "jobs"), DELEGATE_JOB_LOGS_DIR=str(custom))
|
|
res = _wrapper(main, "submit", "--dry-run", "--agent", "human", "--registry-dir", str(exp),
|
|
"--workdir", str(main), "--prompt", "p57", **env)
|
|
assert res.returncode == 0, res.stdout[-800:] + res.stderr[-800:]
|
|
jid = _only_job_id(exp)
|
|
assert res.stdout.strip().splitlines()[-1] == str(custom / jid)
|
|
assert (custom / jid / "meta.json").is_file()
|
|
assert not (tmp_path / "exp" / "delegate_job_logs").exists()
|
|
res = subprocess.run([sys.executable, str(PUBLISH_PY), "--registry-dir", str(exp), "--job", jid,
|
|
"--attempts", "1", "--event", "progress", "--detail", "x"],
|
|
capture_output=True, text=True, env=_env(main, **env), cwd=str(main), timeout=60)
|
|
events = [json.loads(l) for l in (custom / jid / "events.ndjson").read_text().splitlines() if l.strip()]
|
|
assert [e for e in events if e.get("event") == "published"], events
|
|
assert not (tmp_path / "exp" / "delegate_job_logs").exists()
|
|
assert not (main / ".mam" / "delegate_job_logs").exists()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# worktree_prepare keeps the creator's audit log next to the control root's registry
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_worktree_prepare_keeps_the_logs_next_to_the_control_root_registry(ctl, mock_herdr, mock_agents, monkeypatch):
|
|
main = ctl
|
|
reg = main / "elsewhere" / "jobs"
|
|
monkeypatch.setenv("MAM_JOBS_DIR", str(reg))
|
|
monkeypatch.delenv("DELEGATE_JOB_LOGS_DIR", raising=False)
|
|
wt = _wt(main)
|
|
res = _prepare(main, wt, "--creator-agent", "claude", "--onboard")
|
|
assert res.returncode == 0, res.stdout + res.stderr
|
|
(row,) = _rows(main)
|
|
assert row["control_plane"]["mam_jobs_dir"] == os.path.realpath(reg)
|
|
assert row["control_plane"]["delegate_job_logs_dir"] == os.path.realpath(main / "elsewhere" / "delegate_job_logs")
|
|
jid = row["delegate_job_id"]
|
|
assert (main / "elsewhere" / "delegate_job_logs" / jid / "status.json").is_file()
|
|
assert not (main / ".mam" / "delegate_job_logs").exists()
|
|
# the pane carries the registry, not the audit-log root derived from it
|
|
flags = _env_flags(mock_herdr)
|
|
assert any(f == f"MAM_JOBS_DIR={os.path.realpath(reg)}" for f in flags), flags
|
|
assert not [f for f in flags if f.startswith("DELEGATE_JOB_LOGS_DIR=")], flags
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# status.sh reads the audit log next to a moved registry
|
|
# ---------------------------------------------------------------------------
|
|
|
|
@pytest.mark.parametrize("mode", ["json", "text"])
|
|
def test_status_reads_the_audit_log_next_to_a_moved_registry(mam_sandbox, monkeypatch, mode):
|
|
tmp = mam_sandbox
|
|
reg, logs = tmp / "elsewhere" / "jobs", tmp / "elsewhere" / "delegate_job_logs"
|
|
jid = "s57aa001"
|
|
(logs / jid).mkdir(parents=True)
|
|
(logs / jid / "status.json").write_text('{"job_id": "s57aa001", "status": "completed"}')
|
|
reg.mkdir(parents=True) # the registry exists, but the record was cleaned up
|
|
_mutate(tmp, f"""
|
|
d['herdr_sessions'] = [{{'name': 's57-creator-claude', 'status': 'running', 'delegate_job_id': {jid!r},
|
|
'pane': {{'cwd': {str(tmp)!r}}}}}]
|
|
""")
|
|
monkeypatch.setenv("MAM_JOBS_DIR", str(reg))
|
|
monkeypatch.delenv("DELEGATE_JOB_LOGS_DIR", raising=False)
|
|
args = ["--json"] if mode == "json" else []
|
|
res = subprocess.run(["bash", _skill(tmp, "multi-agent-mux-status", "scripts", "status.sh"), *args],
|
|
capture_output=True, text=True, timeout=120)
|
|
assert res.returncode == 0, res.stderr
|
|
if mode == "json":
|
|
assert json.loads(res.stdout)["sessions_detail"][0]["job_status"] == "completed"
|
|
else:
|
|
(line,) = [l for l in res.stdout.splitlines() if l.startswith("s57-creator-claude")]
|
|
assert jid in line and "completed" in line, line
|
|
assert not (tmp / ".mam" / "delegate_job_logs").exists()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# review (codex P2): conflict refusals and legacy claims follow the registry too
|
|
# ---------------------------------------------------------------------------
|
|
|
|
@pytest.mark.parametrize("path", ["conflict", "legacy-claim"])
|
|
def test_conflict_refusals_and_legacy_claims_follow_the_registry(moved, monkeypatch, path):
|
|
ws, reg, logs = moved
|
|
jid = registry.register_job("p2", registry_dir=str(reg), agent_session="herdr:t57")
|
|
if path == "conflict":
|
|
assert engine.transition(jid, "completed", registry_dir=str(reg), source="operator").committed
|
|
assert not engine.transition(jid, "error", registry_dir=str(reg), source="late").committed
|
|
expected = "conflict_refused"
|
|
else:
|
|
monkeypatch.setenv("MAM_ENGINE_CAS", "0")
|
|
assert registry.pick_pending("herdr:t57", registry_dir=str(reg)) == jid
|
|
expected = "status_changed"
|
|
assert expected in [e.get("event") for e in mqtt_common.iter_logged_events(jid, str(logs))]
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# review (codex P1): an old audit terminal must not close a redispatched attempt
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_old_audit_terminal_does_not_complete_the_redispatched_attempt(moved, monkeypatch):
|
|
ws, reg, logs = moved
|
|
jid = registry.register_job("p1", registry_dir=str(reg), agent_session="herdr:t57")
|
|
assert engine.transition(jid, "completed", registry_dir=str(reg), source="operator").committed
|
|
old_audit = mqtt_common.read_logged_status(jid, str(logs))
|
|
flush, observed = engine._flush, {}
|
|
|
|
def before_the_mirror(notes, registry_dir):
|
|
# the real gap: attempt 2 is committed, the audit still says attempt 1 completed
|
|
rec = mqtt_common.load_job(jid, registry_dir)
|
|
assert rec["status"] == "pending" and rec["attempt_id"] == 2
|
|
assert old_audit["transition_seq"] < rec["transition_seq"]
|
|
pending, terminal = {jid}, {}
|
|
job_subscriber._check_disk_fallback(pending, registry_dir, terminal, {jid: rec["dispatch_token"]}, {jid: 2})
|
|
observed.update(pending=pending, terminal=terminal)
|
|
flush(notes, registry_dir)
|
|
|
|
monkeypatch.setattr(engine, "_flush", before_the_mirror)
|
|
assert engine.transition(jid, "redispatch", registry_dir=str(reg), source="operator",
|
|
fields={"attempt_id": 2}).committed
|
|
assert observed == {"pending": {jid}, "terminal": {}}, "the old completion was attributed to attempt 2"
|
|
rec = mqtt_common.load_job(jid, str(reg))
|
|
# a failed mirror leaves the old status.json behind (update_logged_status
|
|
# itself refuses to go backwards, so the file is written directly): still ignored
|
|
mqtt_common.job_log_path(jid, mqtt_common.STATUS_FILENAME, str(logs)).write_text(json.dumps(old_audit))
|
|
assert mqtt_common.read_logged_status(jid, str(logs))["status"] == "completed"
|
|
pending, terminal = {jid}, {}
|
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid: rec["dispatch_token"]}, {jid: 2})
|
|
assert pending == {jid} and terminal == {}
|
|
# an audit status without a sequence cannot prove it is about this attempt
|
|
mqtt_common.update_logged_status(jid, "completed", logs_dir=str(logs))
|
|
pending, terminal = {jid}, {}
|
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid: rec["dispatch_token"]}, {jid: 2})
|
|
assert pending == {jid} and terminal == {}
|
|
# a damaged sequence is ignored too, and does not take the subscriber down
|
|
mqtt_common.job_log_path(jid, mqtt_common.STATUS_FILENAME, str(logs)).write_text(
|
|
json.dumps({"job_id": jid, "status": "completed", "transition_seq": "n/a"}))
|
|
pending, terminal = {jid}, {}
|
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid: rec["dispatch_token"]}, {jid: 2})
|
|
assert pending == {jid} and terminal == {}
|
|
# a newer audit-only terminal is still taken
|
|
mqtt_common.update_logged_status(jid, "completed", logs_dir=str(logs), transition_seq=rec["transition_seq"] + 1)
|
|
pending, terminal = {jid}, {}
|
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid: rec["dispatch_token"]}, {jid: 2})
|
|
assert pending == set() and terminal == {jid: "completed"}
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# plan §2-B (user decision): the derivable audit-log root is recorded, not forwarded
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def _cp_cli(ws: Path, *args, **extra) -> subprocess.CompletedProcess:
|
|
return subprocess.run([sys.executable, "-m", "lib_py.control_plane", *args], capture_output=True, text=True,
|
|
env=_env(ws, **extra), cwd=str(ws), timeout=60)
|
|
|
|
|
|
def test_prefix_forwards_only_what_cannot_be_derived(moved, monkeypatch, tmp_path):
|
|
ws, reg, logs = moved
|
|
monkeypatch.setenv("MAM_JOBS_DIR", str(reg))
|
|
d = control_plane.describe(str(ws / "wt"))
|
|
assert d["forward"] is True
|
|
assert d["delegate_job_logs_dir"] == os.path.realpath(logs), "the row still records the sibling"
|
|
p = control_plane.prefix(d)
|
|
assert "WORKSPACE_ROOT=" in p and "AGENT_SESSIONS_YAML=" in p and f"MAM_JOBS_DIR={os.path.realpath(reg)} " in p, p
|
|
assert "DELEGATE_JOB_LOGS_DIR=" not in p, p
|
|
# an audit-log root that cannot be derived is still forwarded
|
|
custom = tmp_path / "custom-logs"
|
|
monkeypatch.setenv("DELEGATE_JOB_LOGS_DIR", str(custom))
|
|
d2 = control_plane.describe(str(ws / "wt"))
|
|
assert d2["delegate_job_logs_dir"] == os.path.realpath(custom)
|
|
assert f"DELEGATE_JOB_LOGS_DIR={os.path.realpath(custom)} " in control_plane.prefix(d2)
|
|
# the CLI lib.sh calls agrees on both
|
|
monkeypatch.delenv("DELEGATE_JOB_LOGS_DIR")
|
|
res = _cp_cli(ws, "json", str(ws / "wt"), MAM_JOBS_DIR=str(reg))
|
|
assert res.returncode == 0, res.stderr
|
|
cli = _cp_cli(ws, "prefix", res.stdout.strip(), MAM_JOBS_DIR=str(reg))
|
|
assert cli.returncode == 0 and "MAM_JOBS_DIR=" in cli.stdout and "DELEGATE_JOB_LOGS_DIR=" not in cli.stdout, cli.stdout
|
|
res = _cp_cli(ws, "json", str(ws / "wt"), MAM_JOBS_DIR=str(reg), DELEGATE_JOB_LOGS_DIR=str(custom))
|
|
cli = _cp_cli(ws, "prefix", res.stdout.strip(), MAM_JOBS_DIR=str(reg), DELEGATE_JOB_LOGS_DIR=str(custom))
|
|
assert f"DELEGATE_JOB_LOGS_DIR={os.path.realpath(custom)} " in cli.stdout, cli.stdout
|
|
|
|
|
|
def test_create_binds_the_sibling_and_does_not_forward_it(mam_sandbox, mock_herdr, mock_agents, monkeypatch):
|
|
tmp = mam_sandbox
|
|
reg, sibling = tmp / "elsewhere" / "jobs", tmp / "elsewhere" / "delegate_job_logs"
|
|
monkeypatch.setenv("MAM_JOBS_DIR", str(reg))
|
|
monkeypatch.delenv("DELEGATE_JOB_LOGS_DIR", raising=False)
|
|
wt = tmp / "wt"
|
|
wt.mkdir()
|
|
res = subprocess.run(["bash", _skill(tmp, "multi-agent-mux-create", "scripts", "create_session.sh"),
|
|
"--workspace", str(wt), "--agent", "claude", "--role", "creator", "--onboard"],
|
|
capture_output=True, text=True, cwd=str(tmp), timeout=180)
|
|
assert res.returncode == 0, res.stdout + res.stderr
|
|
(row,) = _rows(tmp)
|
|
assert row["control_plane"]["mam_jobs_dir"] == os.path.realpath(reg)
|
|
assert row["control_plane"]["delegate_job_logs_dir"] == os.path.realpath(sibling)
|
|
flags = _env_flags(mock_herdr)
|
|
assert any(f == f"MAM_JOBS_DIR={os.path.realpath(reg)}" for f in flags), flags
|
|
assert not [f for f in flags if f.startswith("DELEGATE_JOB_LOGS_DIR=")], flags
|
|
# the injected publish commands still pin the root per command
|
|
texts = [t for t in _sent_texts(json.loads(Path(mock_herdr).read_text())) if "publish_event.py" in t]
|
|
assert texts and f"DELEGATE_JOB_LOGS_DIR={os.path.realpath(sibling)} " in "".join(texts), texts
|
|
jid = row["delegate_job_id"]
|
|
assert (sibling / jid / "status.json").is_file()
|
|
assert not (tmp / ".mam" / "delegate_job_logs").exists()
|
|
|
|
|
|
def test_a_forwarded_pane_moves_its_logs_with_the_registry(tmp_path, monkeypatch):
|
|
"""The issue's scene: a worktree creator pane after §2-B carries the control
|
|
root's WORKSPACE_ROOT and MAM_JOBS_DIR but no DELEGATE_JOB_LOGS_DIR."""
|
|
for k in DROP:
|
|
monkeypatch.delenv(k, raising=False)
|
|
main, wt, exp = tmp_path / "main", tmp_path / "wt", tmp_path / "exp" / "jobs"
|
|
real_logs = main / ".mam" / "delegate_job_logs"
|
|
real_logs.mkdir(parents=True)
|
|
wt.mkdir()
|
|
pane = dict(WORKSPACE_ROOT=str(main), MAM_JOBS_DIR=str(main / ".mam" / "jobs"))
|
|
res = _wrapper(main, "submit", "--dry-run", "--agent", "human", "--registry-dir", str(exp),
|
|
"--workdir", str(wt), "--prompt", "t", cwd=wt, **pane)
|
|
assert res.returncode == 0, res.stdout[-800:] + res.stderr[-800:]
|
|
jid = _only_job_id(exp)
|
|
assert res.stdout.strip().splitlines()[-1] == str(Path(os.path.realpath(tmp_path / "exp")) / "delegate_job_logs" / jid)
|
|
assert (tmp_path / "exp" / "delegate_job_logs" / jid / "meta.json").is_file()
|
|
assert os.listdir(real_logs) == [], "the experiment leaked into the control root"
|
|
# the usual run in that pane is unchanged: the default registry's sibling
|
|
res = _wrapper(main, "submit", "--dry-run", "--agent", "human", "--workdir", str(wt), "--prompt", "t",
|
|
cwd=wt, **pane)
|
|
assert res.returncode == 0, res.stdout[-800:] + res.stderr[-800:]
|
|
jid2 = _only_job_id(main / ".mam" / "jobs")
|
|
assert res.stdout.strip().splitlines()[-1] == str(Path(os.path.realpath(main)) / ".mam" / "delegate_job_logs" / jid2)
|
|
assert os.listdir(real_logs) == [jid2]
|
|
assert not (wt / ".mam" / "delegate_job_logs").exists()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Mac review F1: the path helpers must not need paho
|
|
# ---------------------------------------------------------------------------
|
|
|
|
@pytest.fixture
|
|
def nopaho(tmp_path):
|
|
"""A `paho` package that fails to import, first on PYTHONPATH: the real
|
|
import path then behaves like an interpreter without paho-mqtt."""
|
|
d = tmp_path / "nopaho"
|
|
(d / "paho").mkdir(parents=True)
|
|
(d / "paho" / "__init__.py").write_text("raise ModuleNotFoundError(\"No module named 'paho'\")\n")
|
|
probe = subprocess.run([sys.executable, "-c", "import paho.mqtt.client"], capture_output=True, text=True,
|
|
env={**os.environ, "PYTHONPATH": f"{d}:{SKILLS}"}, timeout=60)
|
|
assert probe.returncode != 0 and "No module named 'paho'" in probe.stderr, probe.stderr
|
|
return d
|
|
|
|
|
|
def test_control_plane_binding_works_without_paho(moved, nopaho):
|
|
"""_delegate_py_bin's last fallback is the PATH's python3, which may lack
|
|
paho: create/resume's binding and lib.sh's publish-command prefix must still
|
|
resolve the audit-log root."""
|
|
ws, reg, logs = moved
|
|
env = _env(ws, MAM_JOBS_DIR=str(reg), PYTHONPATH=f"{nopaho}:{SKILLS}", AGENT_PYTHON_BIN=sys.executable)
|
|
res = subprocess.run([sys.executable, "-m", "lib_py.control_plane", "json", str(ws / "wt")],
|
|
capture_output=True, text=True, env=env, cwd=str(ws), timeout=60)
|
|
assert res.returncode == 0, res.stderr
|
|
desc = json.loads(res.stdout)
|
|
assert desc["delegate_job_logs_dir"] == os.path.realpath(logs)
|
|
res = subprocess.run([sys.executable, "-m", "lib_py.control_plane", "prefix", json.dumps(desc)],
|
|
capture_output=True, text=True, env=env, cwd=str(ws), timeout=60)
|
|
assert res.returncode == 0 and "MAM_JOBS_DIR=" in res.stdout and "DELEGATE_JOB_LOGS_DIR=" not in res.stdout, res.stdout
|
|
# lib.sh's injected publish command still pins the root (it resolved it with the same interpreter)
|
|
res = subprocess.run(["bash", "-c", f"source '{SKILLS}/lib.sh' && delegate_publish_cmd j57"],
|
|
capture_output=True, text=True, env=env, cwd=str(ws), timeout=60)
|
|
assert res.returncode == 0, res.stderr
|
|
assert res.stdout.startswith(f"DELEGATE_JOB_LOGS_DIR={os.path.realpath(logs)} "), res.stdout + res.stderr
|
|
|
|
|
|
@pytest.mark.parametrize("mode", ["json", "text"])
|
|
def test_status_works_without_paho(mam_sandbox, nopaho, monkeypatch, mode):
|
|
tmp = mam_sandbox
|
|
reg = tmp / ".mam" / "jobs"
|
|
jid = registry.register_job("s57 nopaho", registry_dir=str(reg), agent_session="herdr:s57-nopaho")
|
|
registry.update_status(jid, str(reg), "completed")
|
|
_mutate(tmp, f"""
|
|
d['herdr_sessions'] = [{{'name': 's57-creator-claude', 'status': 'running', 'delegate_job_id': {jid!r},
|
|
'pane': {{'cwd': {str(tmp)!r}}}}}]
|
|
""")
|
|
monkeypatch.setenv("PYTHONPATH", str(nopaho))
|
|
monkeypatch.setenv("AGENT_PYTHON_BIN", sys.executable)
|
|
args = ["--json"] if mode == "json" else []
|
|
res = subprocess.run(["bash", _skill(tmp, "multi-agent-mux-status", "scripts", "status.sh"), *args],
|
|
capture_output=True, text=True, timeout=120)
|
|
assert res.returncode == 0, res.stdout[-500:] + res.stderr[-800:]
|
|
if mode == "json":
|
|
assert json.loads(res.stdout)["sessions_detail"][0]["job_status"] == "completed"
|
|
else:
|
|
(line,) = [l for l in res.stdout.splitlines() if l.startswith("s57-creator-claude")]
|
|
assert "completed" in line, line
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Mac review F2: `logs` without --registry-dir follows the workspace, not the cwd
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_logs_without_registry_dir_follows_the_workspace_from_any_cwd(tmp_path, monkeypatch):
|
|
for k in DROP:
|
|
monkeypatch.delenv(k, raising=False)
|
|
ws, elsewhere = tmp_path / "ws", tmp_path / "elsewhere"
|
|
(ws / ".mam").mkdir(parents=True)
|
|
elsewhere.mkdir()
|
|
monkeypatch.setenv("WORKSPACE_ROOT", str(ws))
|
|
res = subprocess.run([sys.executable, str(REGISTRY_PY), "register", "--prompt", "f2", "--agent", "human",
|
|
"--agent-session", "herdr:f2"], capture_output=True, text=True, env=_env(ws),
|
|
cwd=str(elsewhere), timeout=60)
|
|
assert res.returncode == 0, res.stderr
|
|
jid = res.stdout.strip()
|
|
assert (ws / ".mam" / "jobs" / f"{jid}.json").is_file() and (ws / ".mam" / "delegate_job_logs" / jid).is_dir()
|
|
res = _wrapper(ws, "logs", "--list", cwd=elsewhere)
|
|
assert res.returncode == 0 and jid in res.stdout, res.stdout + res.stderr
|
|
res = _wrapper(ws, "logs", jid, cwd=elsewhere)
|
|
assert res.returncode == 0 and "registered" in res.stdout, res.stdout + res.stderr
|
|
assert not (elsewhere / ".mam").exists()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Mac review R1: an audit terminal of a later dispatch is not the earlier one's success
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_disk_fallback_rejects_a_terminal_that_belongs_to_a_later_dispatch(moved, monkeypatch):
|
|
"""Between the registry read and the audit read: A error -> redispatch
|
|
(attempt 2) -> B completed. The caller waiting on A must not be told
|
|
"completed"; the caller of B must."""
|
|
ws, reg, logs = moved
|
|
jid = registry.register_job("r1", registry_dir=str(reg), agent_session="herdr:t57")
|
|
assert registry.pick_pending("herdr:t57", registry_dir=str(reg)) == jid # claim: running, token A
|
|
rec = mqtt_common.load_job(jid, str(reg))
|
|
tok_a = rec["dispatch_token"]
|
|
assert rec["status"] == "running" and rec["attempt_id"] == 1
|
|
read, fired = mqtt_common.read_logged_status, []
|
|
|
|
def interleave(job, logs_dir=None):
|
|
if not fired: # once, right between the two reads, with real transitions
|
|
fired.append(1)
|
|
assert engine.transition(jid, "error", registry_dir=str(reg), source="operator").committed
|
|
assert engine.transition(jid, "redispatch", registry_dir=str(reg), source="operator",
|
|
fields={"attempt_id": 2}).committed
|
|
assert engine.transition(jid, "completed", registry_dir=str(reg), source="operator").committed
|
|
return read(job, logs_dir)
|
|
|
|
monkeypatch.setattr(mqtt_common, "read_logged_status", interleave)
|
|
pending, terminal = {jid}, {}
|
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid: tok_a}, {jid: 1})
|
|
assert fired, "the interleaving did not run"
|
|
assert pending == {jid} and terminal == {}, "B's completion was attributed to A"
|
|
rec = mqtt_common.load_job(jid, str(reg))
|
|
tok_b = rec["dispatch_token"]
|
|
assert tok_b != tok_a and rec["attempt_id"] == 2 and rec["status"] == "completed"
|
|
# the next round, and the caller of B, see the truth
|
|
pending, terminal = {jid}, {}
|
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid: tok_b}, {jid: 2})
|
|
assert pending == set() and terminal == {jid: "completed"}
|
|
pending, terminal = {jid}, {}
|
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid: tok_a}, {jid: 1})
|
|
assert pending == {jid} and terminal == {}
|