Problem
The per-(repo, PR, head, mode) AI-review advisory lock (ai-review-lock:*, AI_REVIEW_LOCK_TTL_SECONDS = 1800) lives in the selfhost transient cache (Redis), which survives container restarts. When a deploy/crash kills a pass mid-LLM-call, the lock is orphaned for the full 30 minutes, and every subsequent pass — scheduled sweeps and explicit maintainer re-runs via the panel checkbox (forceAiReview) — loses the claim and republishes the aiReviewLockContendedResult placeholder ("AI review already in progress for this PR head… holding for manual review").
The TTL was designed as a "crash-safety backstop only", but on a single-instance deployment it is the only recovery, and 30 minutes of a PR that answers every re-run click with "already in progress" reads as the product being broken.
Verified live 2026-07-26 (#8972): fresh review started 14:55:12Z (ai_review_cache_miss → lock claimed), container replaced 14:55:30Z, lock observed in Redis at 15:10Z with 864s TTL remaining (expiry 15:25:12Z — exactly claim+1800s). Maintainer re-run clicks in that window all returned the placeholder. persistable: false worked as designed (nothing cached durably) — the lock alone caused the starvation.
Fix
Layered, all cheap on a single-instance deployment:
- Boot-time lock flush: on startup, delete
ai-review-lock:* (and any other single-holder advisory locks in the transient cache). Any lock present at boot was claimed by the process that just died — there is no legitimate holder. (Multi-instance hosted deployments use the SubmissionLock DO instead, so this flush can be selfhost-scoped.)
- SIGTERM release: best-effort release of held locks during shutdown (the
finally blocks never run on a hard kill; a shutdown hook can drain the known-held set).
- Force lock-steal: an explicit
forceAiReview retrigger should break a lock older than a floor (e.g. > 5 min) instead of deferring to it — a human clicked precisely because nothing is visibly happening. At minimum the contended placeholder should say when the lock expires instead of implying imminent completion.
Acceptance
- Kill -9 mid-review, restart → next pass runs a fresh review immediately (no 30-min placeholder window).
- A checkbox re-run against a >5-min-old lock spends a fresh review instead of publishing the placeholder.
Related: #8972 (evidence), the deploy-drain sibling issue and the sticky manual-review-label sibling issue (filed alongside).
Problem
The per-(repo, PR, head, mode) AI-review advisory lock (
ai-review-lock:*,AI_REVIEW_LOCK_TTL_SECONDS = 1800) lives in the selfhost transient cache (Redis), which survives container restarts. When a deploy/crash kills a pass mid-LLM-call, the lock is orphaned for the full 30 minutes, and every subsequent pass — scheduled sweeps and explicit maintainer re-runs via the panel checkbox (forceAiReview) — loses the claim and republishes theaiReviewLockContendedResultplaceholder ("AI review already in progress for this PR head… holding for manual review").The TTL was designed as a "crash-safety backstop only", but on a single-instance deployment it is the only recovery, and 30 minutes of a PR that answers every re-run click with "already in progress" reads as the product being broken.
Verified live 2026-07-26 (#8972): fresh review started 14:55:12Z (
ai_review_cache_miss→ lock claimed), container replaced 14:55:30Z, lock observed in Redis at 15:10Z with 864s TTL remaining (expiry 15:25:12Z — exactly claim+1800s). Maintainer re-run clicks in that window all returned the placeholder.persistable: falseworked as designed (nothing cached durably) — the lock alone caused the starvation.Fix
Layered, all cheap on a single-instance deployment:
ai-review-lock:*(and any other single-holder advisory locks in the transient cache). Any lock present at boot was claimed by the process that just died — there is no legitimate holder. (Multi-instance hosted deployments use the SubmissionLock DO instead, so this flush can be selfhost-scoped.)finallyblocks never run on a hard kill; a shutdown hook can drain the known-held set).forceAiReviewretrigger should break a lock older than a floor (e.g. > 5 min) instead of deferring to it — a human clicked precisely because nothing is visibly happening. At minimum the contended placeholder should say when the lock expires instead of implying imminent completion.Acceptance
Related: #8972 (evidence), the deploy-drain sibling issue and the sticky manual-review-label sibling issue (filed alongside).