fix(agent): three absorbing states get an escape — merge blocks, approval rows, and confidence parking - #9119
Conversation
…oval rows, and confidence parking Three separate paths where LoopOver stopped making progress on a PR and nothing anywhere said so. Same shape each time: a state with no exit and no signal. #9012 — one token blip permanently stranded green, approved PRs. merge_blocked_sha was written for every terminal failure class and cleared by nothing; its only escape was the contributor pushing a new commit. But a 401 (App suspended / key rotated) and an exhausted secondary-rate-limit window are properties of the INSTALLATION, not of the commit — they fail every in-flight merge in the fleet at once, and no contributor has any reason to push, because the PR looks green, approved, and (with review_state_label on) ready-to-merge. classifyMergeFailure now reports a scope; infra-scoped blocks carry an expiry and are re-probed, commit-scoped ones (real conflict, repo merge policy) keep the until-a-new-commit semantics exactly. Two compounding parts fixed alongside: mergeAttemptCount now actually resets when the head advances, which its own schema and function docs have promised from the start but nothing implemented — so once one head exhausted MERGE_RETRY_CAP every later head was one-strike-terminal — and a blocked PR now takes the manual-review label naming the reason instead of keeping a ready-to-merge promise it cannot deliver. mergeBlockedReason previously reached no human-visible surface at all; the planner, audit and PostHog were its only readers. #9032 — the approval queue notified once, ever. stageForApproval returns early on !created and its badge dedup key is per (PR, actionClass) with no time component, so a maintainer who missed the single badge got no further prompt and the row waited indefinitely. Pending rows now get a bucketed reminder badge per interval and expire after a week. Expiry is deliberately NOT a rejection: a rejection is a maintainer's judgment and feeds the trust loop as such, while an expiry only records that consent was never given — neither executes anything, and a later pass that re-plans the action stages a fresh row. The pass rides the re-gate sweep's existing fan-out tick rather than adding a job type and cron entry for a bounded DB scan, and runs before the fan-out so failing it cannot cost the tick its actual work. #9034 — confidence parking never escalated. A sub-floor blocker still blocks, but under the default hold_for_review disposition it converts a one-shot close into an OPEN hold, and nothing counted how many times the same PR re-entered that hold. A PR shaped to keep drawing low-confidence blockers therefore survived indefinitely, cost a maintainer on every roll, and could be walked toward a merge from there. Holds are now counted per distinct head (so the several re-gate passes one commit attracts spend one unit, not several) and never reset by a push — repeated holds are the pattern being capped, so a push must not buy another life. Past the cap the finding has been reproduced by independent passes, which is the corroboration a single pass's confidence number lacked, and the close fires. The low-confidence cap lives in its own module rather than in src/rules/advisory.ts on purpose: advisory.ts is one half of the hand-maintained gate-decision twin pair, and this cap has no engine counterpart to mirror — the engine's gate-advisory.ts carries no low-confidence hold resolver at all — so putting it there would have forced a no-op engine release just to satisfy the parity guard. Two helpers (activeMergeBlockedSha, applyLowConfidenceHoldCap) are extracted into the modules that own their semantics rather than left inline in the re-gate pipeline. Both were unreachable for testing in place: reaching them needs a live gate evaluation, settings, GitHub state and a planner run, which is far too much machinery to stand up to observe one boolean. It also keeps the planner clock-free — resolving the infra expiry happens on the way in, not inside a pure function. Migrations 0185 (merge_blocked_until) and 0186 (low_confidence_hold_count, low_confidence_hold_head_sha). Local gate green end to end (npm run test:ci, exit 0). 100% line and branch coverage on all 413 added src lines. Closes #9012 Closes #9032 Closes #9034
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Logic backtestReplayed 0 historical case(s) for Backtest comparison:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9119 +/- ##
=======================================
Coverage 93.89% 93.90%
=======================================
Files 813 815 +2
Lines 80809 80873 +64
Branches 24538 24564 +26
=======================================
+ Hits 75876 75940 +64
Misses 3564 3564
Partials 1369 1369
Flags with carried forward coverage won't be shown. Click here to find out more.
|
loopover-orb
Bot
added
the
gittensor:bug
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-27 02:45:40 UTC
Review summary Nits — 6 non-blocking
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. 🟩 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.
|
![loopover-orb[bot]](https://avatars.githubusercontent.com/in/4139483?s=60&v=4){kind=small}
1 participant
Three separate paths where LoopOver stopped making progress on a PR and nothing anywhere said so. Same shape each time: a state with no exit and no signal.
#9012 — one token blip permanently stranded green, approved PRs
merge_blocked_shawas written for every terminal failure class and cleared by nothing; its only escape was the contributor pushing a new commit. But a 401 (App suspended / key rotated) and an exhausted secondary-rate-limit window are properties of the installation, not of the commit — they fail every in-flight merge in the fleet at once, and no contributor has any reason to push, because the PR looks green, approved, and (withreview_state_labelon) ready-to-merge.classifyMergeFailurenow reports ascope. Infra-scoped blocks carry an expiry (merge_blocked_until) and are re-probed; commit-scoped ones — a real conflict, a repo merge policy that forbids an App merge — keep the until-a-new-commit semantics byte-for-byte. A 401 stays terminal for the pass, so failing fast against a known-bad credential is unchanged.Two compounding parts fixed alongside:
mergeAttemptCountnever reset. Its own schema and function docs have promised "a new commit's attempts start fresh once the row's head advances" from the start, butbumpPullRequestMergeAttemptonly scoped the increment to the head — the value survived every push. So once one head exhaustedMERGE_RETRY_CAP, every later head was one-strike-terminal on the first transient failure it met.mergeBlockedReasonreached no human-visible surface at all — the planner, audit and PostHog were its only readers — while the PR kept a ready-to-merge label. A blocked PR now takes the manual-review label with the reason in it.Acceptance: a simulated 401 during merge → after the window passes, the PR merges autonomously with no new commit. A genuine merge conflict still blocks terminally until the contributor rebases. Both covered.
#9032 — the approval queue notified once, ever
stageForApprovalreturns early on!createdand its badge dedup key is per (PR, actionClass) with no time component, so a maintainer who missed the single badge got no further prompt and the row waited indefinitely.Pending rows now get a bucketed reminder badge per interval (the bucket index goes into the dedup key, so the ~2-minute sweep collapses to one badge per interval with no extra persisted state) and expire after a week.
Expiry is deliberately not a rejection: a rejection is a maintainer's judgment that the action was wrong and feeds the trust loop as such, while an expiry only records that consent was never given. Neither executes anything, and a later pass that re-plans the same action stages a fresh row with a fresh notification — a still-correct action should not be silenced because a human was on vacation once.
The pass rides the re-gate sweep's existing fan-out tick rather than adding a job type and a cron entry for a bounded DB scan, and runs before the fan-out so failing it cannot cost the tick its actual re-gate work.
#9034 — confidence parking never escalated
A sub-floor blocker still blocks, but under the default
hold_for_reviewdisposition it converts a one-shot close into an open hold — and nothing counted how many times the same PR re-entered it. A PR shaped to keep drawing low-confidence blockers survived indefinitely, cost a maintainer on every roll, and could be walked toward a merge from there.Holds are now counted per distinct head, so the several re-gate passes one commit attracts spend one unit rather than several, and never reset by a push — repeated holds are the pattern being capped, so a push must not buy another life (the same reasoning
bumpPullRequestDraftConversionCountalready applies for the same reason). Past the cap the finding has been reproduced by independent passes, which is the corroboration a single pass's confidence number lacked, so the close is no longer the uncertain call the hold protects against.Notes for review
Where the cap constant lives. It is in its own module, not in
src/rules/advisory.ts.advisory.tsis one half of the hand-maintained gate-decision twin pair enforced byscripts/check-engine-parity.ts, and this cap has no engine counterpart to mirror — the engine'sgate-advisory.tscarries no low-confidence hold resolver at all. Putting it there would have forced a no-op@loopover/enginerelease purely to satisfy the parity guard.advisory.tsis untouched by this PR.Two extracted helpers.
activeMergeBlockedShaandapplyLowConfidenceHoldCapmoved into the modules that own their semantics rather than staying inline in the re-gate pipeline. Both were unreachable for testing in place — reaching them needs a live gate evaluation, settings, GitHub state and a planner run, far too much machinery to stand up to observe one boolean. The merge one also keepsplanAgentMaintenanceActionsclock-free: the infra expiry resolves on the way in, not inside a pure function.Migrations.
0185_merge_block_expiry.sql,0186_low_confidence_hold_counter.sql. Both additive.Verification
npm run test:ci— exit 0, full gate green.src/lines, verified by intersectinggit diff -U0againstlcov.infoDA:/BRDA:records.Closes #9012
Closes #9032
Closes #9034