Skip to content

fix(selfhost): re-drive the review a restart orphaned, not just its lock - #9868

Open
JSONbored wants to merge 2 commits into
mainfrom
fix/reenqueue-orphaned-reviews
Open

fix(selfhost): re-drive the review a restart orphaned, not just its lock#9868
JSONbored wants to merge 2 commits into
mainfrom
fix/reenqueue-orphaned-reviews

Conversation

@JSONbored

@JSONbored JSONbored commented Jul 29, 2026

Copy link
Copy Markdown
Owner

The half-fix

#9802 heals the active_review_tracking row at boot, so a restart no longer leaves a PR wedged behind a stale lock. That solved the lock. It did not solve the comment.

An interrupted pass has already published "🔍 LoopOver is reviewing…". Terminalizing its tracking row does not replace that comment, and nothing else re-drives the PR:

  • the head hasn't moved, so no webhook fires
  • the published-review cache is empty, because the pass never finished
  • reconcile-open-prs / reconcile-active-review-tracking are both disabled on the production ORB

So the PR sits claiming a review is in progress. Forever.

Observed, not theorised

JSONbored/metagraphed#8693. Three container recreates in one afternoon — two of them routine config reloads — each killed a mid-flight review. State 25 minutes later:

active_review_tracking : terminal          (healed — the lock was fine)
ai_review_cache        : 0 rows            (no review was ever published)
audit_events since     : none
queued work            : none

The maintainer saw "reviewing…" indefinitely. Every deploy during a review does this, which is why it has recurred rather than being a one-off.

The fix

The boot sweep now enqueues an agent-regate-pr for each row it heals. force: true bypasses the AI-review cache and the reuse cooldown — correct here, because 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. Omitting it doesn't merely lose ordering, it inverts it: jobClaimSortKey falls back to a ~9.5e11 base that sorts ahead of every real 2026 PR (~1.78e12), so an orphan re-queue would jump the entire contributor backlog. A restart-orphaned review deserves to be re-driven, not prioritised over work that has waited longer. The repo's own regate-sort-key:check caught exactly this on my first attempt — the guard did its job.

Fail-safe throughout: a lookup or enqueue failure logs and continues (a healed row with no re-queue still beats a wedged one, and the remaining rows still need healing), and an unregistered repo logs a named skip rather than enqueueing a job that cannot act.

Tests

4 new cases on the lookup: both fields returned, the #9499 ordering invariant, null for an unregistered repo (rather than a bogus id), and a registered repo with no stored PR row still resolving so the pass runs.

Full suite 25,453 passed; regate-sort-key, dead-source-files, dead-exports, dispatch-gate-reasons green.

Note

This lands after orb-v3.6.0-beta.8, so the live ORB still has the gap until the next image. #9863's ledger content-waiver is in the same position — both want the same cut.

Closes #9870.

#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.
@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-29 18:39:59 UTC

3 files · 1 AI reviewer · 1 blocker · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This PR completes the fix started in #9802 by not just healing the terminalized active_review_tracking row at boot but also re-driving the interrupted review via a `force: true` agent-regate-pr job, so the wedged "reviewing…" placeholder actually gets replaced rather than sitting forever. The new `loadOrphanRequeueContext` helper correctly resolves both the installation id (returning null to skip unregistered repos) and the PR's real creation time, and deliberately threads `prCreatedAt` through to avoid inverting the queue's oldest-first sort — a subtlety the PR explains clearly and backs with a dedicated invariant test. The per-row try/catch around the re-queue keeps a single re-queue failure from aborting the rest of the healing sweep, which is the right fail-safe shape for boot-time code.

Nits — 6 non-blocking
  • src/server.ts: the job payload is cast `as never` before `backend.queue.binding.send(...)`, which suppresses TypeScript's structural check on the `agent-regate-pr` payload shape — worth double-checking that job type actually declares `force`/`prCreatedAt`/`installationId` rather than relying on the cast to paper over a mismatch.
  • src/db/repositories.ts: `loadOrphanRequeueContext` does two sequential round-trips (repositories then pullRequests) per healed row; fine at boot-sweep scale but consider whether it's worth a single join if the healed-row count ever grows meaningfully.
  • test/unit/db-persistence.test.ts: the new describe block only covers `loadOrphanRequeueContext` in isolation — there's no test exercising the actual boot-sweep wiring in server.ts (the enqueue call, the skip-log on null context, or the catch-and-continue on a re-queue failure), though that's consistent with this file's existing `v8 ignore` treatment of the entrypoint.
  • Consider a lightweight integration-style test (or at least a comment pointing to one) that stubs `backend.queue.binding.send` and asserts the boot sweep calls it with `force: true` and the resolved `prCreatedAt`, so the src/server.ts wiring itself isn't only implicitly covered by the repository-layer tests.
  • If `agent-regate-pr` job validation exists elsewhere (e.g. a zod schema), point to it in a comment near the `as never` cast so a future reader knows the object shape is checked somewhere, even though TS itself is bypassed here.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

Concerns raised — review before merging

  • Linked issue does not appear to be satisfied: AI assessment: this PR does not appear to satisfy its linked issue's scope. The linked issue's scope is strictly the boot-time sweep that terminalizes stale active_review_tracking rows to unblock new review passes; this PR is a follow-up (test(labels): cover the priority-label enforcement path's degraded branches #9866) that instead re-drives the interrupted review via a requeue, addressing a different problem (the stale 'reviewing...' comment) not requested by this issue. — Confirm this PR actually addresses the linked issue's scope, or link the correct issue.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. Linked issue does not appear to be satisfied: AI assessment: this PR does not appear to satisfy its linked issue's scope. The linked issue's scope is strictly the boot-time sweep that terminalizes stale active\_review\_tracking rows to unblock new review passes; this PR is a follow-up \(\#9866\) that instead re-drives the interrupted review via a requeue, addressing a different problem \(the stale 'reviewing...' comment\) not requested by this issue. — Confirm this PR actually addresses the linked issue's scope, or link the correct issue.

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9802, #9870
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ❌ 8/20 High review scope from cached public metadata (2 linked issues).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 14 registered-repo PR(s), 13 merged, 334 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 14 PR(s), 334 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Not yet addressed
The linked issue's scope is strictly the boot-time sweep that terminalizes stale active_review_tracking rows to unblock new review passes; this PR is a follow-up (#9866) that instead re-drives the interrupted review via a requeue, addressing a different problem (the stale 'reviewing...' comment) not requested by this issue.

Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is registered but has no active allocation in the current snapshot.
  • Public profile languages: not available
  • Official Gittensor activity: 14 PR(s), 334 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 3 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Decision record
  • action: hold · clause: linked_issue_scope_mismatch
  • config: d5fe5e6a56611d793b4255744a28f90f6763d5aea51b19d28f0f786faa82e66f · pack: oss-anti-slop · ci: passed
  • record: 6b91e8384463ab6eb1f81d45d376ff285f4ed032f8532c9b9cf2f4eed55f101e (schema v5, head b791888)

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored self-assigned this Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.87%. Comparing base (889defe) to head (b791888).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9868      +/-   ##
==========================================
- Coverage   91.74%   90.87%   -0.88%     
==========================================
  Files         918      918              
  Lines      112965   112972       +7     
  Branches    27184    27187       +3     
==========================================
- Hits       103642   102658     -984     
- Misses       8037     9226    +1189     
+ Partials     1286     1088     -198     
Flag Coverage Δ
backend 94.12% <100.00%> (-1.57%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/db/repositories.ts 96.87% <100.00%> (+0.01%) ⬆️

... and 3 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

selfhost: a restart during a review leaves 'LoopOver is reviewing…' published forever

1 participant