reviewer: a 45-minute lock silently suppresses review of a PR whose spawn crashed nf-g7uz ← Beads

Twinned from https://github.com/cfsh/ninefold/issues/467 by tools/beads/import_github.py's reconcile pass.

Split out of #461 so that issue can close cleanly. Unclaimed and unlabelled — inert backlog, per CLAUDE.md.

The observation

From a live dispatch.sh run:

``` #447: skip (review in flight, lock < 45m old) ```

The dispatcher's per-PR lockfile is three-state (tools/reviewer/dispatch.py):

• absent — nothing in flight
• present and EMPTY — a review is running right now
• present and NON-EMPTY — the previous spawn *failed*; the content is a marker, and the 45-minute TTL is a deliberate backoff

The backoff case is fine and is documented: a failed spawn posts no verdict, so without it a moved-head PR would re-spawn every tick with no rate limit.

The in-flight case is the one worth a second look. Reviews are *synchronous* — dispatch.sh runs claude -p inline because Git Bash on Windows kills nohup'd children when the invoking shell exits. So a lock is created, the review runs, and the lock is removed on success. If the process dies in a way that skips the cleanup — box sleep, a reboot mid-tick, the scheduled task being killed, a hung review child — the empty lock survives, and for up to 45 minutes that PR reports review in flight when nothing is running.

Nobody is lied to loudly; the line is printed into dispatch.log, which nothing reads. The PR just sits at rev:pending for a cycle.

Why it is not obviously a bug

45 minutes is a plausible upper bound for a genuine xhigh review, and shortening the TTL risks double-spawning a live review — which costs real tokens and can produce two verdicts racing on one sha. So the current value is defensible; what is missing is a way to tell *a review that is running* from *a lock that outlived its process*.

Options, none costed

• Write the PID into the lock and check liveness. ⚠️ This directly conflicts with the three-state format — a non-empty lock currently *means* "failure backoff", so a PID would turn every in-flight lock into a permanent backoff. It would need the state encoding redesigned, not extended. Design 001 §6 lists this file format as something a port must preserve verbatim, for exactly that reason.
• Separate the two states into two files (pr-N.running / pr-N.failed), which removes the encoding conflict at the cost of a migration.
• Leave it and make it visible — have poll.sh surface a lock older than N minutes, so a stuck lock is noticed rather than waited out. Cheapest, and consistent with how the heartbeat problem was solved (#437, #461, #462): detection where someone is already looking, rather than a new mechanism.

The third is probably right, and it is the one that composes with the liveness work already landed.

Provenance

Raised by aedanpope on #461 as an aside while diagnosing the STALE-DISPATCH false alarm, and deliberately not folded into that PR (it is lock semantics, not liveness). Design 001 §6 preserves the lockfile *mechanism* for the dispatcher port but says nothing about this suppression window, and the port (#466) reproduces the current behaviour verbatim — correctly, since a port is not the place to change it.

Filing it here so Closes #461 doesn't take an untracked item down with it.