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

Add .state.loop File Lock

Close the tick/approve TOCTOU races flagged in add-status-brakes/ADVERSARIAL_BUG_REPORT.md (A6) and add-loop-runner/ADVERSARIAL_BUG_REPORT.md (A2, A7). Both reports name the same v1.1 fix: a file lock on .state.loop that serializes read-modify-write cycles across processes.

This is a v1.1 hardening task. No new features; no user-visible CLI change. Pure robustness.

Goal

Add a cross-platform file-lock helper that wraps every _read_state_loop → mutate → _write_state_loop cycle in status.py and loop-runner.py. Concurrent ticks (two schedulers firing the same loop) and concurrent approve-vs-tick writes will serialize instead of overwriting each other.

Background — the race

_write_state_loop already does atomic tmp-then-replace (status.py:1662). The write itself is atomic. The race is read-modify-write:

1. Tick A reads .state.loop (count=9).
2. Tick B reads .state.loop (count=9).
3. Tick A passes --check-gate (count=9 < max=10).
4. Tick B passes --check-gate (count=9 < max=10).
5. Tick A runs harness, writes count=10.
6. Tick B runs harness, writes count=10. (Still bounded, but two ticks ran for one increment.)

The --approve --loop write vs a concurrent tick's iteration increment is the same shape (status.py A6): approve wins, tick's increment is lost.

The lock closes both by serializing the full read-modify-write critical section.

Requirements

R1. New helper: _loop_lock(loop_path, exclusive=True)

A context manager (contextlib.contextmanager or __enter__/__exit__ class) that:

• Opens <loop_path>/.state.lock (creating it if absent) and holds an OS-level exclusive lock for the duration of the with block.
• On exit: releases the lock. The .state.lock file may be left on disk (it's tiny and idempotent across runs); not garbage-collected.
• Blocking acquire: a second acquirer waits until the first releases. No timeout in v1.1 (loop ticks are short; if a tick wedges, the operator notices via --loop-list showing stale last_tick_at and intervenes manually).
• Cross-platform:
  • POSIX (sys.platform != "win32"): fcntl.flock(fd, LOCK_EX) for acquire, fcntl.flock(fd, LOCK_UN) for release.
  • Windows (sys.platform == "win32"): msvcrt.locking(fd, LK_LOCK, 1) blocking acquire on a 1-byte region; release via msvcrt.locking(fd, LK_UNLCK, 1). msvcrt is stdlib on Windows.
• On BrokenPipeError/IOError from a vanished loop dir mid-hold: surface a clear error "loop dir vanished mid-lock" and exit nonzero. Don't mask it.
• File handle is kept open for the life of the with; closed in finally.

R2. Wrap every read-modify-write cycle in status.py

Locate each _write_state_loop(...) callsite in scripts/status.py (lines 1721, 1948, 1971, 1993) and confirm each is preceded by a _read_state_loop(...) that seeds it. Wrap the read+mutate+write block in with _loop_lock(loop_path):. Do NOT wrap the --create-loop path (status.py:1819) — there is no prior state to race against; duplicate create is already refused by name (R-of-create-task).

Affected commands in status.py:

• cmd_check_gate (halt write) — status.py:1721
• cmd_pause_loop (paused) — status.py:1948
• cmd_resume_loop (running) — status.py:1971
• cmd_approve_loop (halt clear + resumed_count++) — status.py:1993

The lock must cover the read that precedes each of these writes, not just the write. (Wrapping only the write wouldn't close the race — that just makes writes atomic, which they already are.)

R3. Wrap every read-modify-write cycle in loop-runner.py

In scripts/loop-runner.py, wrap the read+mutate+write in cmd_tick's step 10 ("atomic state write" per design/loops/technical.md §7) and any other _write_state_loop callsite (lines 125, 244, 684). Mirror the same with _loop_lock(loop_path): pattern.

The lock must be held across:

• The --check-gate subprocess call's effective decision (i.e. the read of iteration_count/status it makes), AND
• The subsequent state mutation write.

Since --check-gate runs as a subprocess and reads .state.loop itself, the lock acquired by the runner blocks the runner's own subsequent subprocess read from racing a concurrent approve write, but it cannot lock the subprocess itself. This is acceptable: the lock scope we control is the parent runner's read-modify-write; a concurrent tick would block on .state.lock at the parent-runner level and the gate call inside it would still see consistent state.

R4. Idempotence and no-op fast path

If a command reads .state.loop, discovers no mutation is needed (e.g. --pause-loop on an already-paused loop), it still releases the lock cleanly. The lock MUST always be released, even on early-return code paths inside the with block. Use try/finally inside the context manager, not inside callers.

R5. Lock file location

.state.lock lives in the loop's own dir (<loop_path>/.state.lock), NOT the framework root. Rationale: per-loop granularity; a lock on loop A's tick must not block loop B's approve. Untracked loops (no .state.loop) still get a .state.lock file on first acquire — that's fine; the file is empty.

R6. No new pip deps

Use stdlib only: fcntl (POSIX), msvcrt (Windows), contextlib, sys, os. Both are already conditionally imported elsewhere in the framework (platform.system() dispatch in task 5).

R7. Compatibility with existing atomic write

The existing _write_state_loop tmp-then-replace stays. The lock adds a coarse mutex around the read-modify-write cycle; the atomic write provides last-write-wins safety even if some future code path forgets the lock. Defense-in-depth; no regression to the existing atomic semantics.

Non-goals

• No --claim-loop-task (that's task 6).
• No timeout / deadlock detection — out of scope; ticks are short. If a future tick grows long, address then.
• No advisory locking visible to harnesses — internal only; no CLI surface.
• No outputs.retention GC (task 3).
• No blast_radius.base_branch parameterization (task 4).

Test plan (tests/test_state_loop_lock.py)

New tests, all stdlib, all using tmp_path:

1. test_lock_serializes_concurrent_writes: two threads kicked off simultaneously, each does read→sleep(0.05)→write under the lock. Assert timestamps don't interleave (one finishes before the other starts its write). Use a shared "interleave detector" (a list append of enter/exit times compared after).
2. test_lock_releases_on_clean_exit: acquire+release; the next acquire on the same loop succeeds immediately.
3. test_lock_releases_on_exception: with _loop_lock(p): raise ValueError; next acquire succeeds.
4. test_lock_is_per_loop: two lock acquisitions on two different loop dirs run concurrently without blocking each other (assert both complete within a tightly bounded wall-clock window).
5. test_no_lock_on_create_loop: --create-loop of a new loop does NOT create a .state.lock file (create-path is unwrapped per R2). Then --check-gate on it acquires/releases the lock, leaving .state.lock behind.
6. test_pause_loop_serialized_with_concurrent_read: spawn a thread that holds _loop_lock for 0.1s; main thread calls --pause-loop and assert it completes after 0.1s (not before). Confirms commands actually acquire the lock.
7. test_runner_tick_holds_lock_across_state_write: integration-style — invoke loop-runner.py --mode tick against a loop whose tick is artificially delayed, while a parallel --approve --loop is held; assert approve completes after the tick. (Skip if it grows flaky — turns into a smoke test asserting the lock file appears.)

Reuse the _make_loop helper pattern from tests/test_status_brakes.py for loop dir scaffolding.

Concrete code shape

@contextlib.contextmanager
def _loop_lock(loop_path: Path, exclusive: bool = True):
    lock_file = loop_path / ".state.lock"
    fd = os.open(str(lock_file), os.O_RDWR | os.O_CREAT, 0o644)
    acquired = False
    try:
        if sys.platform == "win32":
            import msvcrt
            msvcrt.locking(fd, msvcrt.LK_LOCK if exclusive else msvcrt.LK_NBLCK, 1)
        else:
            import fcntl
            fcntl.flock(fd, fcntl.LOCK_EX if exclusive else fcntl.LOCK_SH)
        acquired = True
        yield
    finally:
        if acquired:
            if sys.platform == "win32":
                import msvcrt
                try:
                    msvcrt.locking(fd, msvcrt.LK_UNLCK, 1)
                except OSError:
                    pass
            else:
                import fcntl
                fcntl.flock(fd, fcntl.LOCK_UN)
        os.close(fd)

Callers:

with _loop_lock(loop_path):
    state = _read_state_loop(loop_path) or _initial_state_loop(name)
    state["status"] = "paused"
    _write_state_loop(loop_path, state)

D-items (decisions locked for this task)

• 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 dir, not garbage-collected.
• D-L3: --create-loop path is unwrapped (no prior state to race against; create is name-unique-refused).
• D-L4: stdlib only (fcntl POSIX, msvcrt Windows). No filelock package.
• D-L5: existing atomic write semantics retained (defense-in-depth).

Risks

• Deadlock if a path holds the lock and re-enters a function that tries to re-acquire. Mitigation: _loop_lock is not re-entrant — audit every callsite to ensure no nested _loop_lock within the same with block. POSIX flock is re-entrant on the same fd; Windows msvcrt.locking is not. Safer to forbid nesting and document it.
• Linux flock on NFS has known caveats. Out of scope: the loop dir is always local (project root or ~/.automaton). Document in the helper's docstring.

Verification

• python3 -m py_compile scripts/status.py scripts/loop-runner.py
• python3 -m pytest tests/test_state_loop_lock.py -v
• python3 -m pytest tests/ -q (full suite must remain green; 433 baseline + new)
• Manual: python3 scripts/status.py --create-loop t --from-template ci-triage && python3 scripts/status.py --check-gate t — should succeed and leave .state.lock behind on first acquire.