Skip to content

fix(review): a manual re-run request loses its forceAiReview intent when deferred for pending CI #7626

Description

@JSONbored

Problem

maybeProcessPrPanelRetrigger (src/queue/processors.ts) correctly threads forceAiReview: true into
maybePublishPrPublicSurface when the "Re-run LoopOver review" checkbox is checked — comment right there:
"The user explicitly asked for a re-run: bypass both the AI-review cache and the manual-review freeze so
this pass always spends a fresh opinion instead of silently replaying a stale/cached one (#3725)."

But before reaching that call, the handler calls prReadyForReview (~line 3477), which can return false
(defer) when CI is still pending for that head SHA. When that happens, maybeProcessPrPanelRetrigger
records github_app.pr_panel_retrigger_deferred and returns — the forceAiReview: true intent is not
persisted anywhere.
The comment on this defer path says "the check_run/check_suite completed webhook
re-triggers once CI settles," but that later event goes through the ordinary handlePullRequestWebhook /
reReviewStoredPullRequest path, which has no knowledge that this specific PR had a pending explicit
retrigger request. If the PR is also frozen for manual review (isFrozenForManualReview) or under
one-shot review cadence, and the cached prior AI review is degraded (e.g. the missing-assessment defect
just fixed in the companion PR), the eventual natural re-evaluation will NOT force a fresh AI call — the
user's click is silently lost, and the PR stays stuck showing the same stale/degraded content.

Why this isn't fixed here

prReadyForReview's CI-wait block (~line 3535-3650) is extremely carefully tuned, with staleness caps,
dirty-base exceptions, missing-required-context handling, and a per-head-SHA finalize-once guard —
explicitly built in response to real production incidents (#3947, #7537, #7556; one incident note cites
"3 PRs whose CI never settled each burned 200-300+ full reviews over 20+ hours"). Simply bypassing this
block for the manual-retrigger caller would remove those cost/incident protections for that caller,
risking reintroducing the same burn pattern via repeated checkbox clicks instead of the sweep. This needs
its own careful, dedicated pass — most likely persisting "this exact (repo, PR, headSha) has a pending
forceAiReview request" somewhere durable (a DB flag or audit-event lookup keyed the same way the existing
per-head-SHA finalize guard already is) and having whatever code path eventually re-evaluates this PR
(natural CI-completion OR the existing staleness-cap finalize path) consume and honor it — NOT a shortcut
around the existing CI-wait/cost-protection logic itself.

Scope

  • Reproduce: a PR held for manual review (or under one-shot cadence) with a degraded/stale cached AI
    review, where checking "Re-run LoopOver review" happens to land while CI is still pending for the
    current head SHA.
  • Fix: persist the pending-retrigger intent, keyed the same way CI_STUCK_FINALIZE_GUARD_EVENT_TYPE
    already scopes by (repoFullName, pr.number, pr.headSha), and consume it wherever this PR's review
    next actually runs — without touching or bypassing any of prReadyForReview's existing staleness-cap /
    dirty-base / missing-required-context logic.
  • Add a regression test reproducing the exact "retrigger clicked while CI pending, PR frozen with a
    degraded cached review, next natural CI-completion event still forces a fresh AI call" scenario.

Links & Resources

  • src/queue/processors.ts: maybeProcessPrPanelRetrigger (~line 11992), prReadyForReview (~line 3477)
  • Related, now-fixed companion issue: the missing-assessment retry gap in src/services/ai-review.ts
    degraded cached reviews (empty assessment) are what make this defer-loses-intent gap actually visible
    to a user, since a healthy cached review reused via the frozen/one-shot paths at least shows real
    content even without a fresh call.

Public-safety check

  • No secrets, tokens, wallet details, private keys, local paths, or private scoring output are
    included above.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions