fix(selfhost): backlog-vs-fresh fairness, convergence-cap shadowing, and maintenance admission feedback loop#9233
Merged
Conversation
…riority gate The lane-scoped foreground claim requires beating the best due unclassified priority, but githubWebhookPriority returns 10 (equal to fresh's own priority, above backlog's 9) for nearly every webhook other than a fresh PR open/reopen/ synchronize/ready-for-review event. On any repo with CI at least one such row is essentially always due, so neither lane's priority can ever satisfy the strict `>` gate and both lane claims permanently fall back to plain priority ordering -- the exact starvation the fairness mechanism exists to prevent. Add shouldEscapeLanePriorityGate: once the oldest due row in the lane being claimed has waited at least DEFAULT_FOREGROUND_LANE_MAX_STARVE_AGE_MS (10 minutes, mirroring maintenance-admission's trickle_max_defer_age), the gate is bypassed so an old lane row can win on its own merits. Closes #9153
Wires shouldEscapeLanePriorityGate into claimNextForegroundLane for both the Postgres and sqlite queue backends: each computes the oldest due row's age for the lane being claimed (fresh via a dedicated query, backlog by reusing the row set already fetched for repo selection) and falls back to the plain `>=` floor once that age crosses the bounded threshold, instead of staying permanently gated behind an unclassified priority-10 webhook. Includes regression tests for both backends covering the escape arm (an aged backlog row wins preferentially) and the control arm (a fresh backlog row stays gated this cycle), plus a fix to a pre-existing pg-queue test whose placeholder created_at (an epoch-adjacent 1000ms) incidentally tripped the new age escape. Closes #9153
…ence cap selectBacklogConvergenceCandidates sorted oldest-open-first, sliced to `max`, and only then did the caller filter out repair-exhausted PRs -- so five old, permanently-unpublishable PRs could occupy every slot forever, shadowing the 6th+ PR needing convergence behind them indefinitely. Split the pure ordering out into sortedBacklogConvergenceCandidates (unsliced) and have sweepRepoBacklogConvergence walk that full order, skipping exhausted candidates as it goes and stopping once `max` actionable ones are found -- selectBacklogConvergenceCandidates itself becomes a thin slice-after-sort wrapper over the new function for callers that don't need the exhaustion filter. Also logs + records an audit event when every examined candidate is repair-exhausted, so a permanently wedged head of the backlog is visible instead of the sweep silently returning early every cycle. Closes #9154
…e pressure signals Two compounding defects in maintenance admission: - oldestLiveRunnableAgeMs was computed over rows that are either 'processing' or due-and-pending, so a normal in-flight job (a routine multi-minute AI review) alone pushed the age past maxLiveJobAgeMs for its whole duration, tripping live_job_age_high with no drain escape and collapsing the entire maintenance lane -- including the watchdog/alerter that would report an actual overload -- to the 4-hour trickle backstop. Narrow oldest_runnable's own FILTER to 'pending AND due' rows only, excluding 'processing' entirely. - Every claimed maintenance job recomputes maintenancePressureSignals' four aggregate scans, and a denial returns `true` from processOne(), so the pump loop immediately claims the next due maintenance row and repeats -- a burst of N due maintenance rows means 4N sequential scans in one tight loop, a positive feedback loop (more denials -> more scans -> higher load -> more denials). Add a short-TTL (default 1.5s, configurable via MAINTENANCE_ADMISSION_PRESSURE_CACHE_TTL_MS) memoized wrapper shared by the admission check and the pressureSignals() observability method, in both the Postgres and sqlite backends. Also add a partial index on (is_maintenance, status) for the maintenance-lane aggregate, which previously had no supporting index. Closes #9155
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 45536f7 | Commit Preview URL Branch Preview URL |
Jul 27 2026, 08:17 AM |
Bundle ReportChanges will increase total bundle size by 92 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9233 +/- ##
==========================================
- Coverage 93.94% 92.74% -1.21%
==========================================
Files 825 825
Lines 81835 81866 +31
Branches 24849 24856 +7
==========================================
- Hits 76882 75923 -959
- Misses 3558 4838 +1280
+ Partials 1395 1105 -290
Flags with carried forward coverage won't be shown. Click here to find out more.
|
loopover-orb
Bot
added
the
gittensor:bug
Contributor
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-27 09:06:26 UTC
Review summary Nits — 6 non-blocking
CI checks failing
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk 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.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
🟩 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.
|
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
1 participant
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.
Summary
Three self-hosted queue fairness/admission bugs, bundled per the ORB backlog convention:
#9153 — backlog-vs-fresh fairness lane inert whenever any unclassified priority-10 webhook is due
claimNextForegroundLane's lane-scoped claim requires beating the best due unclassified foregroundpriority with a strict
>.githubWebhookPriorityreturns 10 (equal to fresh's own priority, abovebacklog's 9) for nearly every webhook other than a fresh PR open/reopen/synchronize/ready-for-review
event, so on any repo with CI at least one such row is essentially always due — neither lane's priority
can ever satisfy
> 10, and both lane claims permanently fall back to plain priority ordering, exactlythe starvation the mechanism exists to prevent.
Added
shouldEscapeLanePriorityGate(inqueue-fairness.ts): once the oldest due row in the lane beingclaimed has waited at least
DEFAULT_FOREGROUND_LANE_MAX_STARVE_AGE_MS(10 minutes, mirroringmaintenance-admission's
trickle_max_defer_age), the gate is bypassed and the lane's own priority orderingdecides instead of being blocked outright. Wired into both the Postgres and sqlite backends (each computes
the oldest due row's age for the lane being claimed — fresh via a dedicated query, backlog by reusing the
row set already fetched for repo selection).
Judgment call: the issue offered two options (a bounded age escape, or comparing against
claim_sort_key/age directly). Went with the age escape since it's a strictly smaller, more surgicalchange that preserves the existing priority-gate semantics in the common case and only changes behavior
once genuine starvation is detected.
#9154 — 5-PR backlog-convergence cap applied before the repair-exhausted filter
selectBacklogConvergenceCandidatessorted oldest-open-first, sliced to 5, and only then did the callerfilter out repair-exhausted PRs — so five old, permanently-unpublishable PRs could occupy every slot
forever, shadowing every other PR needing convergence behind them.
Split the pure ordering into
sortedBacklogConvergenceCandidates(unsliced) and hadsweepRepoBacklogConvergencewalk that full order, skipping exhausted candidates as it goes and stoppingonce
maxactionable ones are found.selectBacklogConvergenceCandidatesbecomes a thinslice-after-sort wrapper for callers that don't need the exhaustion filter (kept for API compatibility).
Also logs + records an audit event (
agent.sweep.backlog_convergence, outcomedenied) when everyexamined candidate is repair-exhausted, so a permanently wedged head of the backlog is visible instead of
the sweep silently returning early every cycle.
Judgment call: didn't add a secondary sort/rotation (the issue's third, optional suggestion) — the
exhaustion filter plus the new audit-visible signal already resolves the "silently stuck forever" failure
mode; a stuck PR now shows up in the audit log every cycle rather than needing a rotation scheme.
#9155 —
live_job_age_highcollapses the maintenance lane on normal operation, plus a feedback-loop perf bugTwo compounding defects:
oldestLiveRunnableAgeMscounted a row as active whenstatus='processing'OR due-pending, so anormal in-flight job (a routine multi-minute AI review) alone pushed the age past
maxLiveJobAgeMsfor its whole duration, tripping
live_job_age_highwith no drain escape and collapsing the wholemaintenance lane — including the watchdog/alerter that would report an actual overload — to the 4-hour
trickle backstop. Narrowed
oldest_runnable's own filter to'pending' AND duerows only, excluding'processing'entirely.maintenancePressureSignals' four aggregate scans, and adenial returns
truefromprocessOne(), so the pump loop immediately claims the next due maintenancerow and repeats — a burst of N due maintenance rows means 4N sequential scans in one tight loop (a
positive feedback loop: more denials → more scans → higher load → more denials). Added a short-TTL
(default 1.5s, configurable via
MAINTENANCE_ADMISSION_PRESSURE_CACHE_TTL_MS) memoized wrapper sharedby the admission check and the
pressureSignals()observability method, in both backends. Also added apartial index on
(is_maintenance, status)for the maintenance-lane aggregate, which previously had nosupporting index at all.
Judgment call: chose a fixed drain-escape-free fix (exclude
processingfrom the age signal) ratherthan adding a second drain escape to
live_job_age_high— the issue offered both options, and excludingprocessingis the more precise fix since the underlying problem is that the signal conflates "running"with "starved," not that the escape mechanism itself is missing.
Test plan
npx tsc --noEmit -p tsconfig.json --incremental false— cleannpx vitest run test/unit/selfhost-sqlite-queue.test.ts test/unit/selfhost-pg-queue.test.ts test/unit/selfhost-queue-fairness.test.ts test/unit/selfhost-backlog-convergence.test.ts test/unit/selfhost-maintenance-admission.test.ts test/unit/queue-2.test.ts— 552 passednpm run selfhost:env-referenceregenerated (newMAINTENANCE_ADMISSION_PRESSURE_CACHE_TTL_MSenv var)npm run db:schema-drift:check/npm run db:migrations:check— clean (the new partial indexes areself-managed idempotent DDL inside
pg-queue.ts/sqlite-queue.ts, not versioned D1 migrations, so nonew migration file is needed)
main; re-typechecked and re-ran the full relevant test set post-rebaseCloses #9153
Closes #9154
Closes #9155