fix(selfhost): heal restart-orphaned active reviews at boot instead of 15-25 minutes later - #9802
Merged
Merged
Conversation
…5 minutes later A container recreation kills any in-flight review pass -- the process, its provider subprocess, its locks -- but the pass's active_review_tracking row stays 'active'. While it exists, every new pass for that head skips with "AI review already in progress" (aiReviewLockContendedResult), INCLUDING a maintainer's forced re-runs from the panel checkbox. The existing reconciliation sweep only touches rows older than 15 minutes and runs on a 10-minute cron, so each deploy wedged every mid-review PR head for 15-25 minutes. Observed live on the ORB (2026-07-29, five deploys in one day): a recreate landed 3 seconds after a review pass started, and the maintainer's re-ticks bounced off the orphan until it was terminalized by hand. At boot the age heuristic is unnecessary: started_at < process boot time is EXACT -- no review survives a restart -- so the sweep can heal immediately with zero risk to a live pass. Runs right after env construction, before the server listens; fail-safe (a failure logs and boot proceeds; the cron sweep remains the backstop).
Contributor
|
Important 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏳ LoopOver is waiting…LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9802 +/- ##
==========================================
- Coverage 90.33% 89.52% -0.82%
==========================================
Files 918 918
Lines 113936 113937 +1
Branches 26975 26975
==========================================
- Hits 102926 101997 -929
- Misses 9681 10850 +1169
+ Partials 1329 1090 -239
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This was referenced Jul 29, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…ock (#9868) * fix(selfhost): re-drive the review a restart orphaned, not just its lock #9802 heals the active_review_tracking row at boot so the next pass cannot bounce off a stale lock. That is only half the failure: the interrupted pass had ALREADY published the "LoopOver is reviewing..." placeholder comment, and terminalizing the row does not replace it. Nothing else re-drives the PR either -- the head has not moved so no webhook fires, and the review cache is empty because the pass never finished. The PR sits claiming a review is in progress, forever. Observed on JSONbored/metagraphed#8693: three container recreates in one afternoon -- two of them routine config reloads -- each killed a mid-flight review. That PR showed "reviewing..." for 25+ minutes with zero published reviews and zero queued work until a human noticed. Every deploy during a review does this, which is why it has recurred. So the boot sweep now enqueues an agent-regate-pr for each row it heals. `force` bypasses the AI-review cache and reuse cooldown, which is right here: the interrupted pass produced nothing to reuse, and the one-shot cadence would otherwise replay a stale review or none at all. prCreatedAt is carried deliberately (#9499): omitting it INVERTS the queue order -- jobClaimSortKey would fall back to a ~9.5e11 base that sorts ahead of every real 2026 PR, so an orphan re-queue would jump the whole contributor backlog. The repo's regate-sort-key check caught exactly that on the first attempt. Fail-safe throughout: a lookup or enqueue failure logs and continues, because a healed row with no re-queue is still strictly better than a wedged one, and the remaining rows still need healing. An unregistered repo logs a named skip rather than enqueueing a job that cannot act. * docs: cite the real issue number — #9866 is an unrelated open issue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
A container recreation kills any in-flight review pass — the process, its provider subprocess, its locks — but the pass's
active_review_trackingrow staysactive. While that orphan exists, every new pass for that head skips with "AI review already in progress" (aiReviewLockContendedResult) — including a maintainer's forced re-runs from the panel checkbox.The existing self-heal (
runActiveReviewReconciliation) only considers rows older thanSTALE_ACTIVE_REVIEW_MIN_AGE_MS(15 min) and runs on a 10-minute cron. So each deploy wedges every mid-review PR head for 15–25 minutes, and the redeploy-companion deploys automatically.Observed live on the ORB (2026-07-29, five recreations in one day — two beta deploys, a secret rotation, a config change): one recreate landed 3 seconds after a review pass started for
JSONbored/loopover#9793. The row satactive; the maintainer's re-ticks bounced off it; a background monitor caught it still wedged 10+ minutes later; it was terminalized by hand. Every "AI review already in progress" / "manual review recommended" flurry the maintainer saw today traces to this plus the deploys themselves.The fix
Boot-time sweep: terminalize every
activerow withstarted_atbefore the process boot time, immediately afterenvconstruction and before the server listens.At boot the 15-minute age heuristic is unnecessary —
started_at < bootis exact, not a guess: no review survives a restart, and there are no live passes yet to protect. The cron sweep stays as the backstop for non-restart orphan shapes, unchanged. Fail-safe: a sweep failure logsactive_review_boot_sweep_failedand boot proceeds.Each healed row logs
active_review_boot_orphan_terminalizedatwarn(same level discipline as the cron sweep's LOOPOVER-2K lesson: healing working is not an anomaly).Tests
3 cases: the live regression (row written before boot → terminalized and reported), the safety invariant (a review started after boot is never touched), and idempotency (second sweep heals nothing, reports nothing). 37 pass in the file;
tscclean.