fix(issue-23-24): the dialog handler sees the ready banner on its own; only this dispatch's started is delivery evidence

Review round 1 (planner-reviewer-claude-01, reviewer-agy-01,
planner-reviewer-codex-01): one blocking defect and two P2s, all confirmed.

B-1. handle_startup_dialogs took its ready tokens only from the environment,
which wait_for_tui_ready fills when it evaluates the adapter facts. Called on
its own - as resume_session.sh calls it, with no readiness wait before it -
it had no tokens, its early-return branch could never fire, and every resume
of every agent idled through the full 20 s timeout (base: 0.03 s). rc was 0
throughout, so nothing failed; the resume tests just took ~30 s each.
_mam_load_dialog_facts now also takes MAM_STRONG_READY_TOKENS / MAM_READY_TOKENS
from the facts, but only when the caller has not set them, so a caller-set
value (wait_for_tui_ready's own, or a test steering readiness) is never
clobbered - the concern that made it take a single fact in the first place.
S5d pins one-iteration return on a ready first frame for five adapters with
no environment; S5e pins that caller tokens survive; R4 gets a wall-clock cap
so a timeout-idle can no longer pass as success. The old
_MAM_READY_TOKENS_CLAUDE fallback was left defined but unread - removed.

R1 / N-2. delegate_wait_started treated any status past `pending` as the
agent's acknowledgement. codex reproduced three false positives: `error` and
`cancelled` records with no started line, and a `running` record from another
dispatch. The status branch is gone; the evidence is a started line carrying
this dispatch's token, which the local publisher writes before it syncs the
status, so nothing that really started is missed (S7c re-pinned on the line
with running/completed/error statuses; S7f/S7g pin the three negatives).

N-4, taken with it: the events.log line is parsed as JSON rather than pattern-
matched. publish_event nests dispatch_token under `data`, and the old regex
only matched because of key order (S7h, real line shape and a reordered one).

R2 / N-1. Two tests hardcoded <repo>/.venv/bin/python and failed with
FileNotFoundError in every review worktree. sys.executable.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

.agents/skills/lib.sh

@@ -71,7 +71,6 @@ done
 _MAM_MODAL_TOKENS='Do you trust the files|Yes, proceed|No, exit|Allow this|browser to authenticate|Resuming the full session|Resume from summary|Try the new fullscreen renderer\?'
 _MAM_HINT_TOKENS='Use arrow keys|Esc to cancel|Press Enter to continue'
 _MAM_DIALOG_TOKENS="${_MAM_MODAL_TOKENS}|${_MAM_HINT_TOKENS}"
-_MAM_READY_TOKENS_CLAUDE='Anthropic|Assistant|Chat|Welcome|Claude Code|Opus|Sonnet|Haiku'
 
 # Workspace-relative defaults with environment overrides (Phase Z)
 HOME_DIR="${HOME_DIR:-$HOME}"
@@ -2142,35 +2141,42 @@ delegate_publish_event() {
 # line lands in the job's events.log, which publish_event writes before its
 # wire publish so a slow broker cannot delay the verdict.
 #
-# Returns 0 on that evidence, 1 if neither appeared within T. Never re-injects:
-# the single-injection contract is fail-closed.
+# Returns 0 on that evidence, 1 if it did not appear within T. Never
+# re-injects: the single-injection contract is fail-closed.
+#
+# The evidence is a `started` line in the job's events.log carrying THIS
+# dispatch's token (nested under `data`, as publish_event writes it; the line
+# is parsed as JSON, not pattern-matched, so key order is irrelevant). The
+# registry's status is deliberately NOT evidence: a record can leave `pending`
+# because another writer closed it (`error`, `cancelled`) or because a
+# different dispatch is `running`, none of which says this agent saw this
+# prompt (review R1). The local publisher writes the log line before it syncs
+# the status, so nothing that really started is missed.
 delegate_wait_started() {
   local job_id="$1" ack_s="${2:-180}" tok="${3:-}"
   [ -n "$job_id" ] || return 1
   local reg_dir="${MAM_JOBS_DIR:-${WORKSPACE_ROOT:-$PWD}/.mam/jobs}"
-  local rec="$reg_dir/$job_id.json"
   local ev_log="$reg_dir/$job_id.events.log"
-  local started_re='"event": *"started"'
-  if [ -n "$tok" ]; then
-    started_re='"event": *"started".*"dispatch_token": *"'"$tok"'"'
-  fi
-  local deadline st
+  local deadline
   deadline=$(( $(date +%s) + ack_s ))
   while :; do
-    st=""
-    if [ -f "$rec" ]; then
-      st=$(REC="$rec" python3 -c "
-import json, os
-try:
-    print(json.load(open(os.environ['REC'])).get('status', '') or '')
-except Exception:
-    print('')
-" 2>/dev/null || true)
-    fi
-    if [ -n "$st" ] && [ "$st" != "pending" ]; then
-      return 0
-    fi
-    if [ -f "$ev_log" ] && grep -Eq "$started_re" "$ev_log" 2>/dev/null; then
+    if [ -f "$ev_log" ] && EV_LOG="$ev_log" EV_TOK="$tok" python3 -c '
+import json, os, sys
+tok = os.environ.get("EV_TOK", "")
+with open(os.environ["EV_LOG"], encoding="utf-8", errors="replace") as f:
+    for line in f:
+        try:
+            e = json.loads(line)
+        except Exception:
+            continue
+        if not isinstance(e, dict) or e.get("event") != "started":
+            continue
+        data = e.get("data") if isinstance(e.get("data"), dict) else {}
+        seen = data.get("dispatch_token") or e.get("dispatch_token") or ""
+        if not tok or seen == tok:
+            sys.exit(0)
+sys.exit(1)
+' 2>/dev/null; then
       return 0
     fi
     if [ "$(date +%s)" -ge "$deadline" ]; then
@@ -2752,21 +2758,35 @@ send_keys_safe() {
 }
 
 # _mam_load_dialog_facts <agent>
-# Populate MAM_STARTUP_DIALOGS for <agent>, once per agent per shell. Only that
-# one fact is taken: `facts` is an eval contract (F8), and evaluating all of it
-# here would clobber ready/placeholder tokens a caller deliberately set.
-# An agent the adapters do not know keeps an empty list.
+# Populate MAM_STARTUP_DIALOGS for <agent>, once per agent per shell, and the
+# ready tokens (MAM_STRONG_READY_TOKENS / MAM_READY_TOKENS) only when the caller
+# has not set them. `facts` is an eval contract (F8); evaluating all of it here
+# would clobber tokens a caller deliberately set, so only these are taken, and
+# the tokens only fill a gap. Without them the handler could never recognise a
+# ready screen when called on its own - resume_session.sh does exactly that,
+# with no wait_for_tui_ready before it - and idled through its whole timeout on
+# every resume (review B-1). An agent the adapters do not know keeps an empty
+# list and no tokens.
 _mam_load_dialog_facts() {
-  local agent="$1" _f=""
+  local agent="$1" _f="" _d="" _s="" _r=""
   if [ "${_MAM_DIALOGS_AGENT:-}" = "$agent" ]; then
     return 0
   fi
-  _f="$("$(_delegate_py_bin)" -m lib_py.agents facts "$agent" 2>/dev/null | sed -n 's/^MAM_STARTUP_DIALOGS=//p')" || _f=""
-  if [ -n "$_f" ]; then
-    eval "MAM_STARTUP_DIALOGS=$_f"
+  _f="$("$(_delegate_py_bin)" -m lib_py.agents facts "$agent" 2>/dev/null)" || _f=""
+  _d="$(printf '%s\n' "$_f" | sed -n 's/^MAM_STARTUP_DIALOGS=//p')"
+  _s="$(printf '%s\n' "$_f" | sed -n 's/^MAM_STRONG_READY_TOKENS=//p')"
+  _r="$(printf '%s\n' "$_f" | sed -n 's/^MAM_READY_TOKENS=//p')"
+  if [ -n "$_d" ]; then
+    eval "MAM_STARTUP_DIALOGS=$_d"
   else
     MAM_STARTUP_DIALOGS='[]'
   fi
+  if [ -z "${MAM_STRONG_READY_TOKENS:-}" ] && [ -n "$_s" ]; then
+    eval "MAM_STRONG_READY_TOKENS=$_s"
+  fi
+  if [ -z "${MAM_READY_TOKENS:-}" ] && [ -n "$_r" ]; then
+    eval "MAM_READY_TOKENS=$_r"
+  fi
   _MAM_DIALOGS_AGENT="$agent"
   return 0
 }

tests/test_issue23_24_startup_path.py

@@ -17,6 +17,7 @@ agy/grok/codex/opencode TUI; no such TUI can be started here.
 import json
 import os
 import subprocess
+import sys
 import textwrap
 
 import pytest
@@ -194,7 +195,7 @@ def test_s3_shim_contract_regressions_still_pass(mam_sandbox):
     Run them explicitly as this change's regression guard."""
     repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
     res = subprocess.run(
-        [os.path.join(repo, ".venv", "bin", "python"), "-m", "pytest", "-q",
+        [sys.executable, "-m", "pytest", "-q",
          "-p", "no:cacheprovider",
          "tests/test_herdr_shim_contract.py", "-k",
          "h18 or h21 or h22 or h23"],
@@ -270,7 +271,6 @@ def test_s12_mam_herdr_never_aborts_its_caller_under_set_e(
 def test_s13_the_loop_watcher_applies_the_same_rule(mam_sandbox):
     """S13 (pure unit): watcher.py builds its own herdr argv and never goes
     through mam_herdr, so it needs the same rule."""
-    import sys
     repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
     sys.path.insert(0, os.path.join(repo, ".agents", "skills"))
     from lib_py.loop import watcher
@@ -331,7 +331,11 @@ echo "rc=$?"
 ''' % (lib, pane_file, keys_log, prelude, timeout, agent)
     run_env = dict(os.environ)
     run_env.pop("MAM_AUTO_TRUST", None)
-    run_env.update(env or {})
+    for k, v in (env or {}).items():
+        if v == "":
+            run_env.pop(k, None)     # "" means: not in the environment at all
+        else:
+            run_env[k] = v
     res = subprocess.run(["bash", "-c", script], capture_output=True, text=True,
                          cwd=str(tmp_path), env=run_env)
     sent = keys_log.read_text().splitlines() if keys_log.exists() else []
@@ -372,7 +376,7 @@ def test_s4b_every_adapter_publishes_the_fact(mam_sandbox):
     seen = {}
     for agent in ("claude", "agy", "grok", "codex", "opencode", "hermes"):
         out = subprocess.run(
-            [os.path.join(repo, ".venv", "bin", "python"), "-m", "lib_py.agents",
+            [sys.executable, "-m", "lib_py.agents",
              "facts", agent],
             capture_output=True, text=True, cwd=repo,
             env=dict(os.environ, PYTHONPATH=skills)).stdout
@@ -417,6 +421,47 @@ def test_s5c_readiness_is_judged_by_the_ready_tokens_not_by_the_dialog_leaving(
     assert "rc=0" in res.stdout, res.stdout + res.stderr
 
 
+READY_FRAMES = [
+    ("claude", "Welcome to Claude Code\n❯ "),
+    ("agy", "Antigravity Ready in /tmp\n> "),
+    ("grok", "Grok Ready\n>>> "),
+    ("codex", "Codex\nAsk Codex to do anything"),
+    ("opencode", "OpenCode\nAsk anything"),
+]
+
+
+@pytest.mark.parametrize("agent,frame", READY_FRAMES)
+def test_s5d_a_ready_first_frame_returns_within_one_iteration_with_no_env(
+        mam_sandbox, agent, frame):
+    """S5d (review B-1): called on its own - as resume_session.sh calls it,
+    with no wait_for_tui_ready before it and no MAM_*READY* in the environment
+    - the handler must recognise the adapter's ready banner on the first frame
+    and return at once, not idle through its timeout. `sleep` is stubbed so
+    the iteration count is observable and the run is fast; base returned in
+    0.03s, the version under review took the full timeout."""
+    res, sent = _run_dialogs(
+        mam_sandbox, agent, frame, timeout=20,
+        env={"MAM_STRONG_READY_TOKENS": "", "MAM_READY_TOKENS": "", "MAM_WEAK_READY_TOKENS": ""},
+        prelude='_iters=0\nsleep() { _iters=$((_iters + 1)); }\n'
+                'trap \'echo "iterations=$_iters"\' EXIT')
+    assert "rc=0" in res.stdout, res.stdout + res.stderr
+    assert sent == [], sent
+    iters = int(res.stdout.split("iterations=")[1].split()[0])
+    assert iters <= 1, "the handler idled %d iterations on a ready screen" % iters
+
+
+def test_s5e_caller_set_ready_tokens_are_not_overwritten(mam_sandbox):
+    """S5e: the gap-fill must not clobber tokens a caller set on purpose -
+    wait_for_tui_ready's own evaluation, or a test steering readiness."""
+    res, sent = _run_dialogs(
+        mam_sandbox, "grok", "Grok Ready\n>>> ", timeout=2,
+        env={"MAM_STRONG_READY_TOKENS": "NonExistentStrong", "MAM_READY_TOKENS": "NonExistentStrong"},
+        prelude='_iters=0\nsleep() { _iters=$((_iters + 1)); }\n'
+                'trap \'echo "iterations=$_iters"\' EXIT')
+    iters = int(res.stdout.split("iterations=")[1].split()[0])
+    assert iters >= 2, "the caller's tokens were replaced by the adapter's: " + res.stdout
+
+
 def test_s6_no_auto_trust_leaves_trust_dialogs_alone_but_answers_notices(
         mam_sandbox):
     """S6 (stubbed): --no-auto-trust. codex trust is left to a human with a
@@ -487,12 +532,13 @@ def _seed_job(tmp_path, job_id, status="pending", started_token=None):
     return jobs
 
 
-def _run_delivery(tmp_path, inject_rc, job_id="job1", **seed):
+def _run_delivery(tmp_path, inject_rc, job_id="job1", seed_events=True, **seed):
     """Run create_session.sh's own delivery block with a stubbed injection."""
     lib = tmp_path / ".agents" / "skills" / "lib.sh"
     block = _extract(_create_sh(tmp_path), "# Inject instructions into the herdr pane",
                      "delegate_wait_started", close="fi")
-    _seed_job(tmp_path, job_id, **seed)
+    if seed_events:
+        _seed_job(tmp_path, job_id, **seed)
     pub_log = tmp_path / "published.txt"
     script = '''
 set -euo pipefail
@@ -532,11 +578,46 @@ def test_s7b_delivery_succeeds_on_the_agents_own_started(mam_sandbox):
     assert published == [], published
 
 
-def test_s7c_delivery_succeeds_when_the_registry_left_pending(mam_sandbox):
-    """S7c: publish_event syncs the record on this host, so a status past
-    `pending` is the same evidence."""
-    res, _ = _run_delivery(mam_sandbox, 0, status="running")
-    assert "DELIVERED" in res.stdout, res.stdout + res.stderr
+def test_s7c_a_started_that_was_followed_by_a_terminal_status_still_counts(mam_sandbox):
+    """S7c: a job that started and then finished (or failed) fast still shows
+    its started line - the evidence is the line, whatever the status became."""
+    for status in ("running", "completed", "error"):
+        res, _ = _run_delivery(mam_sandbox, 0, status=status, started_token="tok-1")
+        assert "DELIVERED" in res.stdout, (status, res.stdout + res.stderr)
+
+
+@pytest.mark.parametrize("status", ["error", "cancelled"])
+def test_s7f_a_job_closed_by_someone_else_is_not_an_acknowledgement(mam_sandbox, status):
+    """S7f (review R1): a record can leave `pending` because another writer
+    closed it. No started line from this agent means no delivery."""
+    res, published = _run_delivery(mam_sandbox, 0, status=status)
+    assert published == ["error"], (status, published, res.stderr)
+    assert "did not acknowledge the onboarding prompt" in res.stderr
+
+
+def test_s7g_another_dispatch_running_is_not_this_dispatchs_started(mam_sandbox):
+    """S7g (review R1): `running` set by a different dispatch's claim says
+    nothing about this prompt."""
+    res, published = _run_delivery(mam_sandbox, 0, status="running")
+    assert published == ["error"], (published, res.stderr)
+
+
+def test_s7h_the_real_events_log_shape_is_read(mam_sandbox):
+    """S7h (review N-4): publish_event nests dispatch_token under `data` and
+    the line is JSON, so it is parsed, not pattern-matched. A line with the
+    keys in another order, and a token in either place, both count."""
+    tmp_path = mam_sandbox
+    jobs = _seed_job(tmp_path, "job1")
+    real = {"schema_version": 1, "seq": 1, "job_id": "job1", "event": "started",
+            "timestamp": "2026-09-22T08:04:26Z", "detail": "",
+            "data": {"dispatch_token": "tok-1", "attempt_id": 1}}
+    reordered = {"data": {"attempt_id": 1, "dispatch_token": "tok-1"}, "event": "started",
+                 "job_id": "job1"}
+    for line in (real, reordered):
+        (jobs / "job1.events.log").write_text(json.dumps(line) + "\n")
+        res, published = _run_delivery(mam_sandbox, 0, seed_events=False)
+        assert "DELIVERED" in res.stdout, (line, res.stdout + res.stderr)
+        assert published == [], published
 
 
 def test_s7d_no_started_marks_the_job_but_keeps_the_session(mam_sandbox):

tests/test_issue24_resume_dialogs.py

@@ -15,6 +15,7 @@ gets, and R4 shows the same shipped function is now reached from resume.
 import json
 import os
 import subprocess
+import time
 
 import pytest
 
@@ -108,8 +109,14 @@ def test_r4_the_shipped_handler_really_runs_for_a_non_claude_resume(
     tmp_path = mam_sandbox
     ws = _create_and_stop(tmp_path, "codex", "test-r4-codex")
     before = len(json.load(open(mock_herdr)).get("calls", []))
+    t0 = time.monotonic()
     res = _resume(tmp_path, "codex", "test-r4-codex", ws)
+    elapsed = time.monotonic() - t0
     assert res.returncode == 0, res.stdout + res.stderr
+    # Review B-1: the handler used to idle its whole 20 s timeout here because
+    # it had no ready tokens on the resume path. rc 0 alone cannot tell early
+    # return from timeout; the clock can.
+    assert elapsed < 12, "resume took %.1fs: the dialog handler idled to its timeout" % elapsed
     calls = json.load(open(mock_herdr)).get("calls", [])[before:]
 
     def _has(c, *verbs):   # the mock records argv with a leading `--session <name>`