Twinned from https://github.com/cfsh/ninefold/issues/989 by tools/beads/import_github.py's reconcile pass.
On PR #927, ran (mistakenly wrapped in a shell timeout, against the tool's own explicit warning — "If the trigger policy says a review is due this takes MINUTES ... don't kill it"):
``` timeout 100 ./tools/pr/review_now.py 927 ```
This killed the review_now.py wrapper at 100s, but the reviewer process it had dispatched for #927 either kept running detached or died without releasing /root/.ninefold-reviewer/locks/pr-927.lock. Every subsequent review_now.py 927 call (several, over about an hour) reported:
``` ⓘ a review is already running (another invocation got there first) — its verdict lands shortly, but not from this call. ```
ps aux showed no process for PR #927 at all — only a dispatcher for an unrelated PR (#987). The lock file's mtime (stat) was ~62 minutes old with nothing touching it. It was a dead lock, not an in-flight review, and the "review in flight" message gave no way to tell the difference — no lock age, no liveness check against the PID that (presumably) created it.
./tools/pr/review_now.py 927 --force cleared it and got a real verdict immediately.
--force's documented recovery path for this ("a previous spawn failed and this PR is in backoff. --force clears the lock and retries.") only fires on a specific detected string match ("skip (" in out and "backoff" in out in review_now.py's outcome parsing) — the plain "review in flight" branch gave no hint that --force was the way out, so I spent real time (and a user's "shouldn't run for 1h" prompt) before trying it.timeout wrapper — a host restart, OOM, or crash mid-review would leave the exact same orphaned lock with the exact same "in flight" message, indefinitely.review_now.py's "review in flight" outcome (or the dispatcher's lock check itself) should report the lock's age and/or check whether the PID that holds it is still alive, and say so explicitly — "lock held by a dead process, --force will clear it" reads very differently from "another invocation started 30s ago, wait for it."
Found incidentally while merging a pairing-worktree PR stack (#907). Filed unclaimed — not investigating the dispatcher internals myself.
None.
No comments.