Skip to content

orb(review): forceAiReview does not bypass the AI-review lock — the re-run button is silently inert behind an orphaned lock (root cause of #9000) #9008

Description

@JSONbored

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

  1. 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.
  2. 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.
  3. Fix the masquerade: the lock placeholder must not satisfy hasPublicReviewAssessment, so the missing-summary alarm fires as designed.
  4. 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.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.orbGittensory Orb related - maintainer self-hosting analytics.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions