From a93c9bf29279f65ce0b9c9fb2c1aeea90a659420 Mon Sep 17 00:00:00 2001 From: Godopu Date: Fri, 9 Oct 2026 09:01:34 +0900 Subject: [PATCH] test(loop-lock): the b13 runs of the checkout's run_loop.sh take a loop lock of their own test_b13_reexec_preserves_original_argv and test_b13_no_freeze_switch_ disables_reexec ran the checkout's run_loop.sh with --creator, which gets past argument parsing to mam_acquire_loop_lock on the default marker, $MAM_REAL_ROOT/.mam/loop-guard-active with MAM_REAL_ROOT = the script's own checkout. In the main checkout that is the live loop's lock (taken and released for a moment: a real loop refused, or the test refused); in a review tree it left an empty .mam. Both now set MAM_LOOP_MARKER under their tmp_path. A sweep of the 16 modules that use run_loop.sh (a per-test watcher for a new .mam in the checkout, an empty .mam removed before each test) found only these two; the others pin MAM_LOOP_MARKER already, end before the lock (--help, argument errors), or run a copy of the tree. Tests: test_loop_marker_isolation.py L1 runs every test that reaches the lock through the checkout's run_loop.sh in a copy of the checkout and requires no .mam there; W1 of the Issue #17 isolation test now also requires that no .mam exists (it only looked for files, so the empty lock directory passed it). At 97c2946 both fail. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe --- .../test_issue17_workspace_root_isolation.py | 1 + tests/test_loop_marker_isolation.py | 50 +++++++++++++++++++ tests/test_o3_scoped_guard.py | 4 ++ 3 files changed, 55 insertions(+) create mode 100644 tests/test_loop_marker_isolation.py diff --git a/tests/test_issue17_workspace_root_isolation.py b/tests/test_issue17_workspace_root_isolation.py index 50c7a9b..fa2913f 100644 --- a/tests/test_issue17_workspace_root_isolation.py +++ b/tests/test_issue17_workspace_root_isolation.py @@ -68,6 +68,7 @@ def test_w1_no_test_writes_into_the_checkout_it_runs_in(tmp_path): assert int(re.search(r"(\d+) passed", summary).group(1)) >= len(NODES) and "skipped" not in summary, summary written = sorted(str(p.relative_to(copy)) for p in (copy / ".mam").rglob("*")) if (copy / ".mam").exists() else [] assert written == [], f"tests wrote into the checkout they ran in: {written}" + assert not (copy / ".mam").exists(), "tests created the checkout's .mam (e.g. the loop lock's directory)" _ONBOARD_PROBE = r""" diff --git a/tests/test_loop_marker_isolation.py b/tests/test_loop_marker_isolation.py new file mode 100644 index 0000000..4b535e9 --- /dev/null +++ b/tests/test_loop_marker_isolation.py @@ -0,0 +1,50 @@ +"""Tests never take the loop lock of the checkout they run in. + +run_loop.sh puts its control root at its own checkout (MAM_REAL_ROOT = +REPO_ROOT) and takes `${MAM_LOOP_MARKER:-$MAM_REAL_ROOT/.mam/loop-guard-active}` +once the arguments parse (mam_acquire_loop_lock creates the directory). In the +main checkout that is the live loop's lock: a test run would take it for a +moment - refusing a real loop, or being refused by one - and leave `.mam/` +behind in a review tree. Every test that reaches the lock through the +checkout's own run_loop.sh therefore names a marker under its tmp_path. + +L1 copies this checkout and runs those tests there: the copy gets no `.mam`. +""" +from __future__ import annotations + +import os +import re +import subprocess +import sys + +from test_issue17_workspace_root_isolation import _copy_checkout + +# every test that runs the checkout's run_loop.sh far enough to take the lock +# (a sweep of the run_loop.sh users on 97c2946; --help and argument errors +# end before the lock, copies of the tree and mam_sandbox trees are not the +# checkout). The first two took the checkout's lock at 97c2946. +NODES = [ + "tests/test_o3_scoped_guard.py::test_b13_reexec_preserves_original_argv", + "tests/test_o3_scoped_guard.py::test_b13_no_freeze_switch_disables_reexec", + "tests/test_issue5_phase3_loop.py::test_p3_02_run_loop_wrapper_delegates_to_python_default", + "tests/test_issue5_phase3_loop.py::test_p3_02_run_loop_wrapper_rollback_mode", + "tests/test_loop_cli.py::test_creator_parses_and_reaches_session_check", + "tests/test_o2_race_free_lock.py::test_o2_12_run_loop_exits_on_lock_failure", + "tests/test_issue25_plan_reuse.py::test_p7b_the_legacy_shell_loop_refuses_the_option", +] + + +def test_l1_no_test_takes_the_loop_lock_of_its_checkout(tmp_path): + copy = tmp_path / "checkout" + _copy_checkout(copy) + assert not (copy / ".mam").exists() + env = {k: v for k, v in os.environ.items() if not k.startswith(("HERDR_", "MAM_", "MQTT_", "SKS_"))} + env.update(WORKSPACE_ROOT=str(copy), AGENT_PYTHON_BIN=sys.executable, PYTHONDONTWRITEBYTECODE="1") + env.pop("PYTHONPATH", None) + res = subprocess.run([sys.executable, "-m", "pytest", "-p", "no:cacheprovider", "-q", + f"--basetemp={tmp_path / 'bt'}", *NODES], + capture_output=True, text=True, cwd=str(copy), env=env, timeout=900) + assert res.returncode == 0, res.stdout[-3000:] + res.stderr[-2000:] + summary = res.stdout.strip().splitlines()[-1] + assert int(re.search(r"(\d+) passed", summary).group(1)) >= len(NODES) and "skipped" not in summary, summary + assert not (copy / ".mam").exists(), "a test took the checkout's loop lock (its .mam was created)" diff --git a/tests/test_o3_scoped_guard.py b/tests/test_o3_scoped_guard.py index 8618351..c91c176 100644 --- a/tests/test_o3_scoped_guard.py +++ b/tests/test_o3_scoped_guard.py @@ -382,10 +382,13 @@ def test_b13_reexec_preserves_original_argv(tmp_path): The arg parser consumes "$@" (shift), so capturing argv beforehand is mandatory. """ run_loop_sh = REPO_ROOT / ".agents" / "skills" / "multi-agent-mux-loop" / "scripts" / "run_loop.sh" + # The run reaches the loop lock: a marker of this test's own, never the + # checkout's .mam/loop-guard-active (the live loop's, in the main checkout). res = subprocess.run( ["bash", str(run_loop_sh), "--creator", "nonexistent-agent", "--task", "test goal with spaces"], capture_output=True, text=True, + env=dict(os.environ, MAM_LOOP_MARKER=str(tmp_path / "loop-guard-active")), cwd=str(REPO_ROOT), ) combined = res.stdout + res.stderr @@ -464,6 +467,7 @@ def test_b13_no_freeze_switch_disables_reexec(tmp_path): run_loop_sh = REPO_ROOT / ".agents" / "skills" / "multi-agent-mux-loop" / "scripts" / "run_loop.sh" env = os.environ.copy() env["MAM_LOOP_NO_FREEZE"] = "1" + env["MAM_LOOP_MARKER"] = str(tmp_path / "loop-guard-active") # not the checkout's loop lock res = subprocess.run( ["bash", str(run_loop_sh), "--creator", "nonexistent-agent", "--task", "test goal"], capture_output=True,