Root cause of the #9000 mystery — solved
#9000 reported that a forceAiReview: true pass on PR #8972 completed in ~10s, published the stale "AI review already in progress" placeholder, and emitted none of the AI-dispatch audit events. Traced to exact lines:
src/queue/processors.ts (~10119-10127) — the caller-side lock claim inside maybePublishPrPublicSurface:
const aiReviewLock = await claimAiReviewLock(env, repoFullName, pr.number, aiReviewHeadSha, settings.aiReviewMode);
if (!aiReviewLock.acquired) {
aiReview = aiReviewLockContendedResult(advisory); // <-- silent skip
} else {
try { await aiReviewCacheReadDecideAndRun(aiReviewLock); } ...
}
claimAiReviewLock is not passed forceAiReview, and neither it nor claimTransientLock has any steal/force path. So the AI-review lock is a third gate that the retrigger's force intent was never wired to bypass — despite the retrigger call site explicitly documenting that it "bypasses both the AI-review cache and the manual-review freeze so this pass always spends a fresh opinion". Force bypasses the freeze and the cache, then dies at the lock.
Every ai_review_* audit event (force_bypass, cache_miss, cache_hit, non_cacheable) lives inside the lock-acquired branch, so the contended path emits nothing — that's the silence.
And the safety net that should have caught it is defeated too: the ai_review_public_summary_missing alarm at ~10476 is aiReviewExpected && !hasPublicReviewAssessment(aiReview?.notes). The placeholder's notes ("AI review is already running for this PR head…") is plain prose with no Blockers/Nits section, so extractPublicAssessment returns it whole and hasPublicReviewAssessment returns true — the placeholder masquerades as a legitimate review summary and suppresses the alarm.
Net effect: an orphaned lock (see the SIGTERM issue) makes the maintainer's re-run button permanently non-functional for up to 30 minutes, silently. Confirmed live: the lock for #8972@16ccc5ce…:block was still held ~15 minutes after the process that claimed it had been killed.
Fix
- Let force bypass/steal the lock. A human clicking re-run wants a fresh opinion; a duplicate LLM call is the explicitly acceptable cost. Either short-circuit the claim when
forceAiReview === true, or (cleaner) add a steal option to claimTransientLock so the forced pass takes real ownership and its finally releases it.
- Never let the contended path be silent — emit
github_app.ai_review_lock_contended (with a forced flag and the lock's remaining TTL) before assigning the placeholder. Mirror at the second contended site in ai-review-orchestration.ts.
- Fix the masquerade: the lock placeholder must not satisfy
hasPublicReviewAssessment, so the missing-summary alarm fires as designed.
- Note
shouldStartAiReviewForAdvisory also ignores forceAiReview entirely — a low-reputation author's forced re-run would silently skip for a different reason. Wire force through there too.
Acceptance
- A forced re-run against a held lock spends a fresh review (or, if deliberately deferred, emits a named audit event saying so with the TTL).
- Killing a process mid-review no longer disables the re-run button for that PR.
Refs #9000, #8998.
Root cause of the #9000 mystery — solved
#9000 reported that a
forceAiReview: truepass on PR #8972 completed in ~10s, published the stale "AI review already in progress" placeholder, and emitted none of the AI-dispatch audit events. Traced to exact lines:src/queue/processors.ts(~10119-10127) — the caller-side lock claim insidemaybePublishPrPublicSurface:claimAiReviewLockis not passedforceAiReview, and neither it norclaimTransientLockhas any steal/force path. So the AI-review lock is a third gate that the retrigger's force intent was never wired to bypass — despite the retrigger call site explicitly documenting that it "bypasses both the AI-review cache and the manual-review freeze so this pass always spends a fresh opinion". Force bypasses the freeze and the cache, then dies at the lock.Every
ai_review_*audit event (force_bypass,cache_miss,cache_hit,non_cacheable) lives inside the lock-acquired branch, so the contended path emits nothing — that's the silence.And the safety net that should have caught it is defeated too: the
ai_review_public_summary_missingalarm at ~10476 isaiReviewExpected && !hasPublicReviewAssessment(aiReview?.notes). The placeholder's notes ("AI review is already running for this PR head…") is plain prose with no Blockers/Nits section, soextractPublicAssessmentreturns it whole andhasPublicReviewAssessmentreturns true — the placeholder masquerades as a legitimate review summary and suppresses the alarm.Net effect: an orphaned lock (see the SIGTERM issue) makes the maintainer's re-run button permanently non-functional for up to 30 minutes, silently. Confirmed live: the lock for
#8972@16ccc5ce…:blockwas still held ~15 minutes after the process that claimed it had been killed.Fix
forceAiReview === true, or (cleaner) add astealoption toclaimTransientLockso the forced pass takes real ownership and itsfinallyreleases it.github_app.ai_review_lock_contended(with aforcedflag and the lock's remaining TTL) before assigning the placeholder. Mirror at the second contended site inai-review-orchestration.ts.hasPublicReviewAssessment, so the missing-summary alarm fires as designed.shouldStartAiReviewForAdvisoryalso ignoresforceAiReviewentirely — a low-reputation author's forced re-run would silently skip for a different reason. Wire force through there too.Acceptance
Refs #9000, #8998.