diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index 63ced3d..c694aa8 100644 --- a/.agents/skills/lib.sh +++ b/.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 -# Populate MAM_STARTUP_DIALOGS for , 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 , 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 } diff --git a/tests/test_issue23_24_startup_path.py b/tests/test_issue23_24_startup_path.py index 9321743..9b74104 100644 --- a/tests/test_issue23_24_startup_path.py +++ b/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): diff --git a/tests/test_issue24_resume_dialogs.py b/tests/test_issue24_resume_dialogs.py index 544006e..19e1dd3 100644 --- a/tests/test_issue24_resume_dialogs.py +++ b/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 `