fix(issue-9,14): bound the R1 grace by the wall clock and keep other ls failures unknown

Round-2 review of d5a191c (planner-reviewer-claude-01, planner-reviewer-codex-01).

- delegate wrapper: the R1 idle grace and the broker_unavailable re-check
  counted only their sleeps against TIMEOUT, so python/herdr probe time plus
  fresh disk heartbeats let a job run far past --timeout (55 s for a 20 s
  budget in the new test). Both loops now end at START_TIME + TIMEOUT read
  off the clock on every pass; heartbeats extend only the idle window, and
  the herdr probe is cut off at the deadline (_within_deadline)
- document the window policy d5a191c actually implements: an agent with no
  evidence is closed ~2 x IDLE_TIMEOUT after its last wire event (Issue #6
  promised IDLE_TIMEOUT + 5 s) and heartbeats may be ~2 x IDLE_TIMEOUT apart;
  correct the claim that publish_event records a heartbeat on disk before
  the wire publish - it records right after the publish attempt
- shim ls: every failed `agent list` other than a structured
  server_not_running now exits nonzero with a "state unknown" diagnostic
  instead of reaching the list parser, where an error document on stdout
  read as an empty list and let drift A terminate live rows
- tests: stdout variants for down and unrelated errors, reconcile keeping
  herdr_confirmed False on an unrelated failure, and a wall-clock upper bound
  under continuous heartbeats and slow probes; the three new checks fail on
  d5a191c
- VERSIONS.md v4.4.2: items 3 and 6 and the test count (19)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe

tests/test_issue14_delivery_ack.py

@@ -366,3 +366,77 @@ def test_wrapper_idle_timeout_grace_accepts_disk_heartbeat(tmp_path):
     job_data = json.loads((reg_dir / f"{jid}.json").read_text())
     assert job_data.get("status") == "completed", f"status={job_data.get('status')}\n{stdout}\n{stderr}"
     assert proc.returncode == 0, f"expected rc=0 while heartbeats kept landing, got {proc.returncode}\n{stdout}\n{stderr}"
+
+
+def test_wrapper_idle_grace_never_outlives_the_wall_clock(tmp_path):
+    """Heartbeats may extend the idle window, never the overall --timeout.
+    Fresh heartbeats land in events.log every second for the whole run, and
+    every python probe the wrapper makes is slowed down (herdr-RPC-like
+    latency). Counting sleeps instead of reading the clock let the grace loop
+    run about twice as long as TIMEOUT; the job must instead close as
+    deadline_timeout within TIMEOUT plus a small close/publish margin."""
+    import threading
+
+    timeout_s, idle_s = 20, 4
+    reg_dir = tmp_path / "jobs"
+    reg_dir.mkdir(parents=True, exist_ok=True)
+    slow_py = tmp_path / "slow-python"
+    slow_py.write_text(f'#!/bin/sh\nsleep 0.7\nexec "{sys.executable}" "$@"\n')
+    slow_py.chmod(0o755)
+    env = _base_env(tmp_path, {"DELEGATE_JOB_PYTHON": slow_py})
+
+    started = time.time()
+    proc = subprocess.Popen(
+        [
+            "bash", str(WRAPPER), "submit",
+            "--agent", "human",
+            "--prompt", "issue 14 wall clock",
+            "--registry-dir", str(reg_dir),
+            "--workdir", str(tmp_path),
+            "--timeout", str(timeout_s),
+            "--idle-timeout", str(idle_s),
+        ],
+        cwd=str(tmp_path), env=env, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True,
+    )
+
+    jid = ""
+    for _ in range(200):
+        job_files = list(reg_dir.glob("*.json"))
+        if job_files:
+            jid = job_files[0].stem
+            break
+        time.sleep(0.1)
+    assert jid, "job was not registered"
+    events_log = reg_dir / f"{jid}.events.log"
+
+    stop = threading.Event()
+
+    def keep_heartbeating():
+        # The same line shape publish_event appends; only its timestamp matters
+        # to _recent_disk_heartbeat.
+        seq = 10_000
+        while not stop.is_set():
+            seq += 1
+            line = {"schema_version": 1, "seq": seq, "job_id": jid, "event": "progress",
+                    "timestamp": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()),
+                    "detail": "still working", "data": {}}
+            with open(events_log, "a") as fh:
+                fh.write(json.dumps(line) + "\n")
+            stop.wait(1)
+
+    beat = threading.Thread(target=keep_heartbeating, daemon=True)
+    beat.start()
+    try:
+        stdout, stderr = proc.communicate(timeout=120)
+    finally:
+        stop.set()
+    elapsed = time.time() - started
+
+    job_data = json.loads((reg_dir / f"{jid}.json").read_text())
+    assert proc.returncode == 2, f"expected the deadline_timeout path (rc=2), got {proc.returncode}\n{stderr}"
+    assert job_data.get("status") == "error", f"status={job_data.get('status')}\n{stderr}"
+    # The heartbeats were honoured: the job was not closed after the first idle
+    # window, it stayed open up to the wall clock ...
+    assert elapsed >= timeout_s - 1, f"closed before the wall clock: {elapsed:.1f}s\n{stderr}"
+    # ... and not a moment past it beyond closing and publishing the error.
+    assert elapsed <= timeout_s + 12, f"idle grace outlived --timeout {timeout_s}s: {elapsed:.1f}s\n{stderr}"

tests/test_issue9_server_env.py

@@ -195,18 +195,32 @@ def test_reconcile_and_status_opt_out_of_autostart_statically():
             f"{script.name} must opt out of the shim's server bootstrap"
 
 
-@pytest.mark.parametrize("envelope", [
-    {"error": {"code": "server_not_running"}},
-    REAL_DOWN_ENVELOPE,
-], ids=["minimal", "herdr-0.9"])
-def test_ls_reports_down_server_as_no_server_running(mam_sandbox, envelope):
+def _fail_with(text: str, stream: str) -> str:
+    """Fake `agent list` body: write `text` to `stream` and exit 1."""
+    return f'sys.{stream}.write({json.dumps(text)} + "\\n"); sys.exit(1)'
+
+
+# reconcile.sh reads a nonzero `ls` whose stderr contains any of these as a
+# confirmed-EMPTY server (`is_empty`); anything else as unknown.
+RECONCILE_EMPTY_PHRASES = ("no server running", "no sessions", "failed to connect")
+
+# An unrelated structured failure whose message happens to mention the code.
+TRANSPORT_ERROR = json.dumps({"error": {"code": "transport_error",
+                                        "message": "socket reset while probing server_not_running"}})
+
+
+@pytest.mark.parametrize("envelope,stream", [
+    ({"error": {"code": "server_not_running"}}, "stderr"),
+    (REAL_DOWN_ENVELOPE, "stderr"),
+    ({"error": {"code": "server_not_running"}}, "stdout"),
+], ids=["minimal", "herdr-0.9", "minimal-on-stdout"])
+def test_ls_reports_down_server_as_no_server_running(mam_sandbox, envelope, stream):
     """reconcile.sh treats a nonzero `ls` as 'empty' only when stderr says
     'no server running'; anything else flips herdr_confirmed=False for every
     server. The shim must therefore translate server_not_running explicitly."""
     env = _install_fake_herdr(mam_sandbox)
     env["MAM_NO_AUTOSTART"] = "1"
-    _fake_agent_list(mam_sandbox,
-                     f'sys.stderr.write({json.dumps(json.dumps(envelope))} + "\\n"); sys.exit(1)')
+    _fake_agent_list(mam_sandbox, _fail_with(json.dumps(envelope), stream))
 
     res = _shim_ls(mam_sandbox, env, "down-srv")
     assert res.returncode != 0
@@ -229,51 +243,75 @@ def test_ls_lists_agents_whose_names_contain_server_not_running(mam_sandbox):
     assert names == ["server_not_running-codex", "creator-claude-01"], res.stdout
 
 
-@pytest.mark.parametrize("stderr_text", [
-    json.dumps({"error": {"code": "transport_error",
-                          "message": "socket reset while probing server_not_running"}}),
-    "warning: server_not_running cache entry is stale",
-    "",
-], ids=["other-code", "plain-text", "empty"])
-def test_ls_leaves_other_failures_unknown(mam_sandbox, stderr_text):
-    """Only the structured code means "down". Any other failure keeps the
-    unknown path (nonzero exit without 'no server running'), so reconcile
-    drops herdr_confirmed instead of reading a confirmed-empty server."""
+@pytest.mark.parametrize("text,stream", [
+    (TRANSPORT_ERROR, "stderr"),
+    (TRANSPORT_ERROR, "stdout"),
+    ("warning: server_not_running cache entry is stale", "stderr"),
+    ("", "stderr"),
+], ids=["other-code", "other-code-on-stdout", "plain-text", "empty"])
+def test_ls_leaves_other_failures_unknown(mam_sandbox, text, stream):
+    """Only the structured code means "down". Any other failure must stay
+    unknown - nonzero exit, no empty-server phrase - wherever its error
+    document lands. On stdout it used to reach the list parser, which read a
+    JSON error without result.agents as a successful EMPTY list (rc 0)."""
     env = _install_fake_herdr(mam_sandbox)
     env["MAM_NO_AUTOSTART"] = "1"
-    _fake_agent_list(mam_sandbox,
-                     f'sys.stderr.write({json.dumps(stderr_text)} + "\\n"); sys.exit(1)')
+    _fake_agent_list(mam_sandbox, _fail_with(text, stream))
 
     res = _shim_ls(mam_sandbox, env, "flaky-srv")
-    assert res.returncode != 0
-    assert "no server running" not in res.stderr, res.stderr
+    assert res.returncode != 0, f"a failed list must not exit 0: {res.stdout!r} {res.stderr!r}"
+    for phrase in RECONCILE_EMPTY_PHRASES:
+        assert phrase not in res.stderr.lower(), res.stderr
     assert res.stdout.strip() == ""
 
 
+def _write_running_rows(sandbox: Path, names, server: str = "live-srv") -> None:
+    rows = "".join(
+        f"  - name: {name}\n"
+        "    status: running\n"
+        f"    herdr_session: {server}\n"
+        "    pane:\n"
+        "      cwd: /tmp/nonexistent-workspace\n"
+        for name in names
+    )
+    (sandbox / ".mam" / "agent-sessions.yaml").write_text("herdr_sessions:\n" + rows)
+
+
+def _reconcile_dry_run(sandbox: Path, env: dict) -> dict:
+    reconcile = sandbox / "skills" / "multi-agent-mux-monitor" / "scripts" / "reconcile.sh"
+    res = subprocess.run(["bash", str(reconcile), "--once", "--emit-diff", "--dry-run"],
+                         capture_output=True, text=True, cwd=str(sandbox), env=env, timeout=120)
+    assert res.returncode == 0, res.stderr
+    return json.loads(res.stdout)
+
+
 def test_reconcile_keeps_live_rows_when_a_name_contains_server_not_running(mam_sandbox):
     """End to end through reconcile.sh: the server answers a successful list in
     which the down-server code appears as data. Both registered rows must stay
     alive with herdr_confirmed intact, so drift A cannot mark them terminated."""
     env = _install_fake_herdr(mam_sandbox)
     _fake_agent_list(mam_sandbox, f"print({json.dumps(json.dumps(LISTING_WITH_SUBSTRING))})")
-    rows = "".join(
-        f"  - name: {name}\n"
-        "    status: running\n"
-        "    herdr_session: live-srv\n"
-        "    pane:\n"
-        "      cwd: /tmp/nonexistent-workspace\n"
-        for name in ("server_not_running-codex", "creator-claude-01")
-    )
-    (mam_sandbox / ".mam" / "agent-sessions.yaml").write_text("herdr_sessions:\n" + rows)
-    reconcile = mam_sandbox / "skills" / "multi-agent-mux-monitor" / "scripts" / "reconcile.sh"
+    _write_running_rows(mam_sandbox, ("server_not_running-codex", "creator-claude-01"))
 
-    res = subprocess.run(["bash", str(reconcile), "--once", "--emit-diff", "--dry-run"],
-                         capture_output=True, text=True, cwd=str(mam_sandbox), env=env, timeout=120)
-    assert res.returncode == 0, res.stderr
-
-    report = json.loads(res.stdout)
+    report = _reconcile_dry_run(mam_sandbox, env)
     assert report["herdr_confirmed"] is True, report
     alive = set(report["herdr_sessions_alive"])
     assert {"server_not_running-codex|live-srv", "creator-claude-01|live-srv"} <= alive, alive
     drift_a = [d for d in report["drifts"] if d.get("class") == "A"]
     assert not drift_a, drift_a
+
+
+def test_reconcile_treats_other_list_failures_as_unknown(mam_sandbox):
+    """End to end through reconcile.sh: a server whose `agent list` fails with
+    an unrelated structured error on stdout must drop herdr_confirmed, so
+    drift A never marks its live row terminated."""
+    env = _install_fake_herdr(mam_sandbox)
+    _fake_agent_list(mam_sandbox, _fail_with(TRANSPORT_ERROR, "stdout"))
+    _write_running_rows(mam_sandbox, ("creator-claude-01",))
+
+    report = _reconcile_dry_run(mam_sandbox, env)
+    assert report["herdr_confirmed"] is False, report
+    drift_a = [d for d in report["drifts"] if d.get("class") == "A"]
+    assert not drift_a, drift_a
+    terminated = [a for a in report.get("actions", []) if str(a).startswith("terminated")]
+    assert not terminated, terminated