Lap Tran
bc7daf8590
- **Restore 82 completed tasks** from tasks/complete/ back to tasks/ top level (all <7 days old per the cleanup policy; premature bulk archive was fixed). - **Dashboard: fix scroll-reset on auto-refresh** — renderBoard rebuilds the board via innerHTML every 2s, destroying each column-body's scrollTop. Now snapshots column-body scrollTop + board.scrollLeft + view.scrollTop before rebuild and restores after (matched by PHASE_GROUPS index). - **Dashboard UI additions** (pre-existing unstaged work): approval section cards, transition buttons, inline artifact editor (textarea for writing missing SPEC/VERDICT/etc from the detail modal). - **Bind ornith as Implement model** — config.md: Model explicit to omlx/Ornith-1.0-35B-4bit-mlx, context window 32768. Interactive autopilot already used ornith via opencode default; now explicit. - **Fix cleanup stub** — automaton-cleanup.sh had a stale --project arg pointing at a pytest temp dir (test isolation leak). Rewired to point at ~/.automaton. - **Fix plist-isolation test** — test asserted host plist doesn't exist, but a real install creates it. Now snapshots mtime before run, asserts unchanged after (only a write during the test counts as bleed). - **New Playwright smoke test** (tests/test_dashboard_ui.py) — 2 tests: board renders tasks, column scroll survives auto-refresh tick. Verified the test fails without the scroll fix (scrollTop resets to 0). Skipped via importorskip when playwright is absent (main CI stays green). - **Clarify SI loop scope in README** — new-project onboarding section documents the framework-scoped self-improvement loop and options (leave/pause/create project loop). - **CHANGELOG** documents all changes including the known model-divergence gap (mde tasks marked complete but per-role model binding was never implemented).
82 lines
4.0 KiB
Markdown
82 lines
4.0 KiB
Markdown
# Verdict: add-state-loop-lock
|
|
|
|
## Status: PASS
|
|
|
|
## Summary
|
|
|
|
Closed the TOCTOU read-modify-write race flagged in
|
|
`add-status-brakes/ADVERSARIAL_BUG_REPORT.md` (A6) and
|
|
`add-loop-runner/ADVERSARIAL_BUG_REPORT.md` (A2, A7) by wrapping the
|
|
read-modify-write cycles on `.state.loop` in a cross-process file lock
|
|
(`_loop_lock`). POSIX `fcntl.flock(LOCK_EX)`, Windows
|
|
`msvcrt.locking(LK_LOCK, 1)`, per-loop granularity, blocking acquire,
|
|
no timeout in v1.1. Stdlib only.
|
|
|
|
## SPEC compliance
|
|
|
|
| Requirement | Status |
|
|
|-------------|--------|
|
|
| R1 — `_loop_lock` context manager (POSIX/Windows, blocking, finally-safe FD lifecycle) | ✓ |
|
|
| R2 — Wrap status.py `cmd_pause_loop`, `cmd_resume_loop`, `cmd_approve_loop`, `cmd_check_gate` (NOT `--create-loop`) | ✓ |
|
|
| R3 — Wrap `cmd_tick`'s step-10 state write; lock covers `_gate` subprocess + state write | ✓ |
|
|
| R4 — Idempotence + early-return inside `with` releases cleanly (try/finally in the context manager, not caller) | ✓ |
|
|
| R5 — Lock file `<loop_path>/.state.lock` (per-loop granularity) | ✓ |
|
|
| R6 — Stdlib only (`fcntl` POSIX, `msvcrt` Windows, `contextlib`, `sys`, `os`) | ✓ |
|
|
| R7 — Existing atomic write (`_write_state_loop` tmp-then-replace) retained | ✓ |
|
|
| D-L1 — Blocking acquire, no timeout | ✓ |
|
|
| D-L2 — `.state.lock` per-loop, not GC'd | ✓ |
|
|
| D-L3 — `--create-loop` unwrapped | ✓ |
|
|
| D-L4 — Stdlib only, no `filelock` package | ✓ |
|
|
| D-L5 — Existing atomic write retained (defense-in-depth) | ✓ |
|
|
| D-L6 (new, necessary for SPEC R2+R3 consistency) — Env-var bypass ($AUTOMATON_NO_LOOP_LOCK=1) avoids self-deadlock when the runner spawns the `--check-gate` subprocess inside its held lock | ✓ |
|
|
|
|
## Bug reports
|
|
|
|
- BUG_REPORT: 5 non-blocking observations (O1-O5), all documented behaviors.
|
|
- ADVERSARIAL_BUG_REPORT: 8 attack vectors probed (A1-A8). One LOW finding
|
|
(A4: harness calling loop-control command inside a tick would deadlock;
|
|
deferred to harness-integration contract docs follow-up). All others
|
|
verified safe.
|
|
|
|
## 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: `--create-loop` does NOT leave `.state.lock` (D-L3 ✓); first
|
|
`--check-gate` does; `AUTOMATON_NO_LOOP_LOCK=1 status.py --check-gate`
|
|
works (env var bypass exercised).
|
|
- Manual adversarial: 5 concurrent `--approve --loop` on a halted loop —
|
|
only one wins (code 0, resumed_count=1); 4 re-read inside the lock and
|
|
see status=running, exit 1. Race closed.
|
|
- Manual adversarial: 5 concurrent `--pause-loop` — all succeed
|
|
(idempotent; pause is well-defined on already-paused); final state
|
|
consistent.
|
|
- Manual adversarial: monkeypatch `_gate` to raise → exit exception →
|
|
follow-up `_loop_lock` acquires immediately (lock released in finally).
|
|
|
|
## D-items applied
|
|
|
|
- D-L1 to D-L6 all locked (see SPEC compliance table).
|
|
|
|
## Subprocess-deadlock avoidance
|
|
|
|
The original SPEC's R2 and R3 contradict each other (both list `--check-gate`
|
|
to acquire `_loop_lock` AND the runner to acquire the same lock across the
|
|
`--check-gate` subprocess — would deadlock). Resolved via D-L6 (env-var
|
|
bypass). The runner sets `$AUTOMATON_NO_LOOP_LOCK=1` in the `--check-gate`
|
|
subprocess's env ONLY (scoped via `_run_json`'s `env` kwarg, propagated to
|
|
`_gate`'s `subprocess.run`). Harness subprocesses inherit `os.environ`
|
|
unchanged (no env var) so their nested `status.py` calls lock normally and
|
|
serialize against the runner's outer lock (intended for tasks; harness
|
|
typically does `status.py --transition` only which doesn't touch
|
|
`.state.lock`). Documented in `_loop_lock`'s docstring + design doc.
|
|
|
|
## Pipeline
|
|
|
|
research → research:awaiting_approval → research:approved → implement →
|
|
code_review → code_review:awaiting_approval → code_review:approved →
|
|
bug_find → adversarial_bug_find → doc_review → referee → complete
|
|
|
|
Pipeline driven end-to-end. Ready for `--transition complete` (relocates
|
|
to `tasks/complete/`). |