fix(review): harden backlog-convergence-sweep against duplicate work#4563
Merged
Merged
Conversation
…4502) backlog-convergence-sweep had none of the three anti-duplication layers its sibling agent-regate-sweep already has: no trigger-backlog check, no atomic fan-out-slot claim, and no per-repo draining guard, plus its settings resolution loop was still sequential (the #3899 bug already fixed for the sibling). A crashed/restarted worker's stuck "processing" trigger row (reclaimed only after queueProcessingTimeoutMs's 30-minute default, which coincides with this sweep's own cadence) could go unnoticed by the next tick and re-enqueue duplicate per-repo and per-PR jobs underneath an unfinished fan-out. Ports all three protections from fanOutAgentRegateSweepJobs: a BACKLOG_CONVERGENCE_SWEEP_TRIGGER_TYPES backlog check in index.ts, claimBacklogConvergenceFanoutSlot for atomic fan-out dedup, and a dedicated last_backlog_convergence_regated_at marker (stamped at dispatch by sweepRepoBacklogConvergence) feeding a draining check in the now concurrency-bounded fan-out resolution loop.
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 #4563 +/- ##
=======================================
Coverage 94.08% 94.08%
=======================================
Files 427 427
Lines 37961 37999 +38
Branches 13864 13871 +7
=======================================
+ Hits 35715 35753 +38
Misses 1586 1586
Partials 660 660
🚀 New features to boost your workflow:
|
5 tasks
JSONbored
added a commit
that referenced
this pull request
Jul 10, 2026
…ay collision on main) main currently has three DIFFERENT already-merged files at 0134 (#4549/#4558/#4563) -- tracked separately as its own fix (PR #4577, not yet merged). Since that fix already claims through 0138, take 0139 here to avoid colliding with it once it lands; this branch's own db:migrations:check will stay red until #4577 merges (unrelated to this branch's own changes), and will need one more rebase afterward.
24 tasks
JSONbored
added a commit
that referenced
this pull request
Jul 10, 2026
…ay collision on main) main currently has three DIFFERENT already-merged files at 0134 (#4549/#4558/#4563) -- tracked separately as its own fix (PR #4577, not yet merged). Since that fix already claims through 0138, take 0139 here to avoid colliding with it once it lands; this branch's own db:migrations:check will stay red until #4577 merges (unrelated to this branch's own changes), and will need one more rebase afterward.
JSONbored
added a commit
that referenced
this pull request
Jul 10, 2026
…tion ledger (#4577) * feat(review): persist a login-keyed predict-gate-vs-live-gate calibration ledger Neither the MCP predict_gate tool nor contributor_gate_history persists whether a contributor's self-reported predict-gate verdict matched the REAL gate decision their PR eventually received -- the one calibration signal the review stack itself uniquely owns, since it terminates both the self-review call and the live gate for the same login/repo. predicted_gate_calibration_ledger pairs the most recent predict_gate_calls row (#4516) for a (login, project) against the real decision at the same call sites as recordContributorGateDecision, writing one immutable row per (login, project, pr, commit) -- ON CONFLICT DO NOTHING, never DO UPDATE, so a webhook replay can never overwrite an already-recorded pairing. Write-only and server-side only: nothing reads it yet (mirrors contributor_gate_history's own precedent), and no MCP tool or other contributor-reachable surface can write to or read from it, preserving its value as anti-farming-resistant ground truth for a future #2349 consumer. Depends on #4516 (predicted_gate_calls) -- built stacked on that branch per the issue's own explicit note that the two may be built together; will be rebased onto main once that PR merges. Fixes #4517 * fix(db): renumber ledger migration 0135 -> 0138 (0134/0135 collide on main) origin/main independently landed three files at 0134 and two at 0135 (from already-merged PRs #4549, #4558, #4563) since this branch's last rebase. Rather than touch already-merged migration files here, take the next genuinely free number for this branch's own new migration; the 0134/0135 collision among already-merged files is a separate main-red issue handled in its own PR. * fix(db): resolve migration collision at 0134 on main (already-merged PRs) Three already-merged PRs independently claimed 0134 (#4549's review_targets_cadence_idx, #4558's predicted_gate_calls, and #4563's pr_last_backlog_convergence_regated_at). CI on this branch inherits main's full migrations/ directory regardless of which PR fixes it, so this can't be deferred to a separate PR without also blocking this one. Renumber the two newer files (keeping #4563's oldest 0134 file in place); this branch's own new migration was already moved to 0138 to stay clear of both this collision and #4563's separate 0135 collision.
JSONbored
added a commit
that referenced
this pull request
Jul 10, 2026
…PRs) (#4581) Three already-merged PRs independently claimed 0134 (#4549's review_targets_cadence_idx, #4558's predicted_gate_calls, and #4563's pr_last_backlog_convergence_regated_at). CI on this branch inherits main's full migrations/ directory regardless of which PR fixes it, so this can't be deferred to a separate PR without also blocking this one. Renumber the two newer files (keeping #4563's oldest 0134 file in place); this branch's own new migration was already moved to 0138 to stay clear of both this collision and #4563's separate 0135 collision.
JSONbored
added a commit
that referenced
this pull request
Jul 10, 2026
…-wide for confirmed miners (#4546) * fix(review): make submitter-reputation burst/AI-spend defense install-wide for confirmed miners getSubmitterReputation's burst/low-sample thresholds are scoped WHERE project = ? AND submitter = ? -- a single repo. A fleet identity spreading a handful of gate-passing-but-low-value PRs across dozens of repos in one self-hosted install never accumulates enough same-repo sample density to read as "burst" or "low" anywhere, so the reputation defense never fires for it and every one of its submissions burns full paid AI-review spend indefinitely. - New getSubmitterReputationAcrossInstall in submitter-reputation.ts: the identical quality-weighted signal derivation, scoped by review_targets.installation_id instead of project (that column already existed, migrations/0050; this adds the supporting index). - New getEffectiveSubmitterReputation in reputation-wire.ts: the per-repo signal, additionally widened to the install-wide view for a CONFIRMED official Gittensor miner -- but only when the per-repo signal alone doesn't already justify caution, so an ordinary contributor or an already-flagged submitter pays no extra lookup. - Extracted the miner-identity check (shared with #4512) into src/gittensor/miner-detection-cache.ts so both this module and unlinked-issue-guardrail.ts can use it without a circular import through processors.ts. - Wired into both call sites: shouldSkipAiForReputation (the main AI-spend gate) and the vision-review reputation check. Fixes #4513 * fix: renumber migration 0130 -> 0131 (0130 was claimed by #4538, already merged to main) * fix(test): account for getEffectiveSubmitterReputation's miner-identity check in visual-vision tests runVisualVisionForAdvisory now resolves reputation via getEffectiveSubmitterReputation (#4513), which checks confirmed-official-miner identity (a fetch to api.gittensor.io/miners) whenever the submitter's per-repo signal is neutral -- a real, intentional behavior change this test file's existing "no network calls at all" assertions didn't account for. Stub that one identity check to resolve cleanly and assert precisely that no OTHER (BYOK/vision-spend) network call happens, rather than asserting zero fetch calls outright. * fix(db): renumber migration 0131 -> 0133 (0131/0132 claimed by merged PRs) origin/main has since merged #4545 (0131_screenshot_table_gate_matrix.sql) and #4554 (0132_impact_map_query_cache.sql, itself a collision fix); rebase onto current main and take the next free number. * fix(db): renumber migration 0133 -> 0134 (0133 claimed by merged PR #4556) Rebase onto current main and take the next free number now that migrations/0133_screenshot_table_gate_skill_link.sql (from #4556) occupies the number this branch previously took. * fix(db): renumber migration 0134 -> 0139 (0134 has a pre-existing 3-way collision on main) main currently has three DIFFERENT already-merged files at 0134 (#4549/#4558/#4563) -- tracked separately as its own fix (PR #4577, not yet merged). Since that fix already claims through 0138, take 0139 here to avoid colliding with it once it lands; this branch's own db:migrations:check will stay red until #4577 merges (unrelated to this branch's own changes), and will need one more rebase afterward. * test(review): close 2 coverage gaps surfaced by the #4514 rebase-merge getEffectiveSubmitterReputation's getRepository().catch() needed a real read-failure test (added); its isConfirmedOfficialMiner().catch() is unreachable (that function already catches every internal failure point itself) and getSubmitterReputationAcrossInstall's results ?? [] fallback is the same "D1 always populates results" case already v8-ignored elsewhere in this codebase -- both marked accordingly. * fix(review): isolate #4507's reputation-single-read invariant from cross-test miner-cache pollution Several earlier tests in this file cache "contributor" as a confirmed official Gittensor miner (5-min TTL) in the shared per-file D1 instance. That collided with #4513's new install-wide reputation widening, which correctly detects the stale cache and adds a 4th reputation-scan D1 read for a submitter this test never intended to be a miner. Use a submitter login unique to this test instead.
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
backlog-convergence-sweephad none of the three anti-duplication layers its siblingagent-regate-sweepalready has, and its settings-resolution loop was still sequential (the perf(queue): parallelize regate-sweep per-repo settings/drain-state resolution #3899 bug already fixed for the sibling). Ports all three protections fromfanOutAgentRegateSweepJobs+ the concurrency fix:BACKLOG_CONVERGENCE_SWEEP_TRIGGER_TYPESbacklog check inindex.ts— defers a second trigger while one is pending/processing.claimBacklogConvergenceFanoutSlot— atomic fan-out-slot claim on a dedicatedglobal_agent_controls.last_backlog_convergence_fanout_atcolumn, collapsing a burst of fan-out triggers to one.sweepRepoBacklogConvergencenow stamps a dedicatedpull_requests.last_backlog_convergence_regated_atmarker at dispatch (mirroring#audit-sweep-dispatch-stamp), andfanOutBacklogConvergenceSweepJobs's resolution loop skips any repo whose marker is still fresh (isRegateSweepDraining, a newBACKLOG_CONVERGENCE_SWEEP_FRESHNESS_MS= 30 min window sized to this sweep's own cadence).mapWithConcurrencyLimit-bounded (was a sequentialfor...await), porting perf(queue): parallelize regate-sweep per-repo settings/drain-state resolution #3899's fix.last_regated_at/last_regate_fanout_at) so the two differently-cadenced sweeps' in-flight signals never conflate.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlint(vianpm run test:ci)npm run typechecknpm run test:coveragelocally (unsharded) — every new invariant/regression test confirmed to FAIL without its corresponding fix and pass with it, verified by temporarily reverting each mechanism and re-runningnpm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateSafety
UI Evidence— N/A, no visible/UI change.