automaton/tasks/add-state-loop-lock/IMPLEMENTATION.md

Implementation: add-state-loop-lock

SCOPE

Closed the read-modify-write TOCTOU race flagged in add-status-brakes/ADVERSARIAL_BUG_REPORT.md A6 and add-loop-runner/ADVERSARIAL_BUG_REPORT.md A2/A7 by wrapping the critical section in a cross-process _loop_lock (POSIX fcntl.flock, Windows msvcrt.locking).

FILES TOUCHED

• scripts/status.py

  • Added import contextlib.
  • Added _LOOP_LOCK_ENV_BYPASS = "AUTOMATON_NO_LOOP_LOCK" constant.
  • Added _loop_lock(loop_path, exclusive=True) context manager (with docstring + per-loop granularity + env-bypass for the runner-spawns-check-gate subprocess case).
  • Wrapped cmd_pause_loop's read-modify-write block in with _loop_lock(loop_path): (re-read state inside the lock before the pause branch decision and write).
  • Wrapped cmd_resume_loop's read-modify-write block the same way.
  • Wrapped cmd_approve_loop's read-modify-write block the same way.
  • Wrapped cmd_check_gate's evaluate-gates-then-maybe-halt-write block in with _loop_lock(loop_path):. Re-read state inside the lock. _halt_loop itself is left unwrapped (the lock is held at the caller; re-acquiring would deadlock).
  • --create-loop path is intentionally unwrapped (D-L3): no prior state to race against; create is name-unique-refused.

• scripts/loop-runner.py

  • Added import contextlib.
  • Added _LOOP_LOCK_ENV_BYPASS = "AUTOMATON_NO_LOOP_LOCK" constant.
  • Added _loop_lock(loop_path, exclusive=True) context manager (without env bypass — the runner is the lock holder, not the bypass consumer).
  • Modified _run_json to accept an optional env dict passed through to subprocess.run.
  • Modified _gate to pass env={**os.environ, _LOOP_LOCK_ENV_BYPASS: "1"} so the spawned status.py --check-gate subprocess's _loop_lock becomes a no-op (avoiding a self-deadlock on the same flock). Env var is scoped to _gate's subprocess only — the Harness subprocesses (Implement/Verify/Orchestrate) do NOT inherit it, so any status.py --transition the harness transitively invokes will lock normally.
  • Wrapped cmd_tick's body in with _loop_lock(loop_path):. The fast untracked early-return still happens OUTSIDE the lock (no .state.loop to race against). Inside the lock, state is re-read fresh; if it transitioned to untracked between the unlocked read and the lock acquire, we return SKIP untracked.

D-ITEMS Locked

• D-L1: blocking acquire, no timeout in v1.1 (ticks short; operator notices via --loop-list stale last_tick_at).
• D-L2: .state.lock is per-loop, lives in the loop's own dir, not garbage-collected.
• D-L3: --create-loop path is unwrapped.
• D-L4: stdlib only (fcntl POSIX, msvcrt Windows). No filelock.
• D-L5: existing atomic write semantics retained (defense-in-depth).
• D-L6 (new, this task): subprocess-deadlock avoidance via env-var bypass. _loop_lock in status.py checks $AUTOMATON_NO_LOOP_LOCK. If set, it yields without flocking (trusting the caller's outer lock). The runner sets this env var ONLY in the --check-gate subprocess's env; harness subprocesses inherit a clean env. Not user-settable.

NOT RE-ENTRANT

_loop_lock is not re-entrant across processes. POSIX flock is per-fd-per-process; a second runner process blocks cleanly until the first releases. Nested _loop_lock within the same with block is forbidden (would deadlock). Audited all callsites — none nest.

NESTED-LOCK AUDIT

Status.py callsites:

• cmd_pause_loop: acquires once, no nested acquires inside.
• cmd_resume_loop: acquires once, calls _enable_schedule AFTER the with block (outside the lock — keeps critical section tight).
• cmd_approve_loop: acquires once, calls _enable_schedule AFTER the with block.
• cmd_check_gate: acquires once; inside calls _halt_loop (which does _write_state_loop + _append_tick_log + _disable_schedule). None of those re-acquire the lock. Safe.

Loop-runner.py callsites:

• cmd_tick: acquires once. Inside, calls _gate (subprocess: status.py acquires its own lock, but env-var bypass makes it a no-op — safe). Calls _halt_loop (does not re-acquire). Calls _ensure_worktree→_git_run (subprocess git, doesn't touch .state.lock). Calls _invoke_harness (subprocess: harness calls unknown code, but loop-runner does NOT pass AUTOMATON_NO_LOOP_LOCK to the harness env, so any status.py the harness transitively invokes will lock normally — and the parent runner holds the loop's outer lock, so those transitions block until the tick releases. This is the intended serialization). Calls _append_tick_log and _write_state_loop inside the lock — safe (neither re-acquires).

LATE BUG FIXED INLINE

While running the broken test file from the first py_compile pass, I hit NameError: name 'loop_file' is not defined in status.py._loop_lock. I'd typed lock_file = loop_path / ".state.lock" then os.open(str(loop_file), ...) — wrong variable name. Fixed to os.open(str(lock_file), ...). Caught by manual status.py --check-gate invocation before pytest; never reached CI.

TESTS

New file tests/test_state_loop_lock.py — 7 tests:

1. TestSerializeConcurrent::test_lock_serializes_concurrent_writes — two threads, read→sleep(0.05)→write under the lock. Asserts one thread's enter time is >= the other's exit time (serialization).
2. TestReleasesClean::test_lock_releases_on_clean_exit — acquire, release, re-acquire succeeds immediately.
3. TestReleasesOnException::test_lock_releases_on_exception — with _loop_lock: raise ValueError then re-acquire succeeds.
4. TestPerLoop::test_lock_is_per_loop — two threads holding locks on different loop dirs concurrently; B's acquire completes within 1s while A holds a different lock.
5. TestNoLockOnCreate::test_no_lock_on_create_loop — --create-loop does NOT leave a .state.lock (D-L3); first --check-gate does.
6. TestPauseSerializedWithConcurrentHolder::test_pause_loop_serialized_with_concurrent_read — a thread holds _loop_lock for 0.1s; main thread invokes status.py --pause-loop. Asserts pause completed after the holder released (i.e. --pause-loop blocked on flock).
7. TestRunnerHoldsLockAcrossStateWrite::test_runner_tick_holds_lock_across_state_write — monkeypatches _invoke_harness, _gate, _context_floor_ok, _ensure_worktree, _find_work, _read_task_brief, etc. A thread runs runner_mod.cmd_tick; the implement-stub blocks on a threading.Event until tick_can_finish is set. Meanwhile main thread starts status.py --approve --loop. Asserts --approve hadn't completed BEFORE tick_can_finish was set (proves the lock is held across the harness subprocess), then sets tick_can_finish and asserts approve completed after.

TEST RESULTS

• python3 -m py_compile scripts/status.py scripts/loop-runner.py ✓
• python3 -m pytest tests/test_state_loop_lock.py -v — 7 passed
• python3 -m pytest tests/ -q — 447 passed (was 440; +7 new; 0 regressions).
• Manual: python3 scripts/status.py --create-loop t --from-template ci-triage && python3 scripts/status.py --check-gate t — succeeds and leaves .state.lock behind on first acquire.
• Manual env-bypass: AUTOMATON_NO_LOOP_LOCK=1 python3 scripts/status.py --check-gate t — succeeds (bypass path exercised).

PIPELINE TO COMPLETION

Driven through research -> research:awaiting_approval -> research:approved -> implement -> code_review. Next: code_review awaited approval -> bug_find -> adversarial_bug_find -> doc_review -> referee -> complete.