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

# Adversarial Bug Report: add-state-loop-lock

Adversarial probing of the `_loop_lock` implementation. Each attack vector is
hypothesized, then tested (or static-analyzed for non-testable cases). Verdict
shown against each.

## A1 — Concurrent `--approve --loop` race

**Hypothesis**: With 5 concurrent `--approve --loop` invocations on a halted
loop, more than one might pass the `status != "halted"` check before any of
them writes the cleared state, double-incrementing `resumed_count`.

**Test**: `/tmp/loop-lock-adv` — set status=halted, spawn 5 concurrent
`status.py --approve --loop` subprocesses simultaneously.

**Result**:
```
codes: [1, 1, 1, 1, 0]
outputs: 4× "ERROR: loop 'adv1' is in status 'running', not 'halted'..."
         1× "Approved loop 'adv1'. Halt cleared. Resumed count: 1"
final state: status=running, resumed_count=1
```

**Verdict**: PASS — exactly one approve won; 4 others re-read inside the lock
and saw `status=running`, returning 1 with the "not halted" error. resumed_count
incremented exactly once. The lock serializes approves correctly.

## A2 — Concurrent `--pause-loop` race

**Hypothesis**: With 5 concurrent `--pause-loop` invocations on a running
loop, all 5 succeed (since pause is idempotent — `state["status"] != "paused"`
fails open). resumed_count shouldn't be touched by pause anyway.

**Test**: Spawn 5 concurrent `status.py --pause-loop adv1`.

**Result**: all 5 returned code 0 with "Paused loop..." message; final
state: status=paused (consistent). No `resumed_count` touched (pause
doesn't increment it).

**Verdict**: PASS (no race) — but note pause is idempotent and re-writes
paused state even when already paused. Each writer holds the lock
sequentially and re-writes the same value. Wasteful but consistent. Not a
bug.

## A3 — Lock release on mid-tick exception

**Hypothesis**: If the runner's `_gate` subprocess or any code inside the
`with _loop_lock` block raises, the OS-level flock is held forever, stalling
all future ticks and pause/approve commands.

**Test**: Monkeypatch `_gate` to raise `RuntimeError`, invoke
`cmd_tick(args)`, catch the exception. Verify a follow-up `_loop_lock`
acquire succeeds immediately (<1s elapsed).

**Result**:
```
caught: simulate gate crash
re-acquire elapsed: 2.5e-05 s
PASS — lock released on exception
```

**Verdict**: PASS — `finally` block in `_loop_lock` runs on exception exit
of the `with` body, releases the flock and closes the FD. No resource leak.

## A4 — Harness calling loop-control commands from inside a tick (theoretical deadlock)

**Hypothesis**: The runner holds `_loop_lock` across the harness subprocess
(Implement/Verify/Orchestrate). If the harness transitively invokes
`status.py --pause-loop` / `--resume-loop` / `--approve --loop` / `--check-gate`
(without `AUTOMATON_NO_LOOP_LOCK=1` env var — which is only set in the
runner's own `_gate` call, not in harness subprocess env), that nested
status.py would acquire `_loop_lock` → block waiting for the runner's parent
lock → runner waits for harness to return → harness waits for its
subprocess → subprocess waits for parent lock → DEADLOCK.

**Test**: Not run live (would hang the entire test session). Static analysis
of harness-integration contract:
- Harnesses invoked via `harness.command` are described in
  `design/loops/technical.md` §8 as LLM-driven agents (opencode, aider, Pi
  Dev, generic). They invoke `status.py` for task-level transitions
  (`--transition`, `--can-edit`, `--task`, `--scope-check`) per the
  `contracts/harness-integration.md` requirement. Task commands do NOT touch
  `.state.lock` (only loop commands do).
- No known harness in scope (opencode/aider/Pi Dev) calls `--pause-loop` /
  `--approve --loop` inside a tick. The orchestrator might inspect loop
  state but doesn't write to it.
- The orchestrator prompt (`prompts/orchestrate.md` etc.) is invoked by the
  runner AFTER the verify verdict is parsed; it's expected to call
  `--transition <task>` based on the verdict, not loop commands.

**Severity**: LOW. Hypothetical; no known harness hits this. The
harness-integration contract should explicitly forbid harness invocations of
loop-control commands during a tick.

**Mitigation documented**: Per `_loop_lock`'s docstring and per SPEC D-L1
("ticks short; operator notices via `--loop-list` stale `last_tick_at`"),
ticks are expected to complete in seconds; an operator noticing a wedged tick
would `kill` the runner process, releasing the OS flock. The deadlock
surface area is small and mitigated by operator-wedge-detection.

**Recommendation**: Add a note to `contracts/harness-integration.md`
explicitly listing loop-control commands (`--pause-loop`, `--resume-loop`,
`--approve --loop`, `--check-gate`) as FORBIDDEN inside a tick's harness
subprocess. Not a blocker for this task — defer to a small docs-only follow-up.

## A5 — Manual `AUTOMATON_NO_LOOP_LOCK=1` disables all `status.py` locking

**Hypothesis**: An operator who sets `$AUTOMATON_NO_LOOP_LOCK=1` in their
shell and runs `--pause-loop` etc. bypasses the lock entirely, re-opening the
TOCTOU race that A1/A2 verified is closed.

**Test**: Not run live (requires manual env var setup; covered by code-level
audit). The env-var bypass is unconditional inside `_loop_lock` for the
status.py helper; there's no check that the bypass is actually being
invoked by a trusted caller.

**Severity**: LOW. Documented as an escape hatch in `_loop_lock`'s
docstring; only the runner sets it, and only in the `_gate` subprocess env
(scoped, not global). A malicious or careless shell user could
circumvent, but they're effectively "running alternative middleware" at
that point — no different from killing the runner.

**Verdict**: PASS — escape hatch is documented; same trust boundary as the
"shell user can override anything" assumption.

## A6 — `.state.lock` left on disk after crash

**Hypothesis**: If the runner is killed mid-tick (SIGKILL or power loss),
the `.state.lock` file is left on disk. A subsequent tick's `os.open`
re-uses the orphaned file (with `O_RDWR | O_CREAT`). The
`fcntl.flock` on the new FD succeeds (the previous flock was associated
with a now-closed FD; the kernel auto-releases flocks on FD close /
process exit). No wedged lock.

**Test**: Not run live (would require killing the runner mid-tick). Static
analysis: POSIX `flock` is per-FD-per-process; the OS auto-releases the
flock when the holding process exits. So orphaned `.state.lock` files are
dead bytes, not live locks.

**Verdict**: PASS — the orphan-file situation is benign. Documented in
`_loop_lock`'s docstring ("not garbage-collected").

## A7 — NFS loop dir causes different flock semantics

**Hypothesis**: If the project dir (and therefore `.automaton/loops/<n>/`)
is on an NFS mount, `fcntl.flock` semantics differ — flock may be
advisory-only or behave unpredictably.

**Test**: Not run live (no NFS available). Acknowledged in `_loop_lock`'s
docstring: "NFS caveat: `flock` semantics differ on NFS-mounted loop dirs.
The loop dir is documented to be local (project root or `~/.automaton`)."

**Verdict**: Documented assumption per SPEC "Risks" section; not a bug.

## A8 — Cyclomatic complexity of cmd_tick jumped with the indent

**Hypothesis**: Wrapping cmd_tick's body in `with _loop_lock(loop_path):`
plus re-read state inside increases cyclomatic complexity and re-indent
churn, making future maintenance error-prone.

**Test**: Not run live. Static analysis: the wrap is a single
context-manager level; the body retains its original structure inside.
Re-indent added 4 columns to all lines inside the with block (visible in
git diff), but no control-flow change beyond the re-read.

**Verdict**: PASS — function shape is preserved; the only new control flow
is the early-return on `state is None` retry inside the with block. The
.SMALL cost is offset by the correctness gain.

## Verdict

**No BLOCKERS found.** All hypotheses either verified-safe (A1, A2, A3, A6,
A8, A7), or theoretical-low-severity (A4, A5) with documented mitigations.

Recommend proceeding to doc_review. A4's recommendation (harness-contract
docs note about loop-control commands inside a tick) is a follow-up
improvement, not a blocker for v1.1.