Skip to content

fix(review): auto-pause count excluded the PR's own current head, so it never paused - #4319

Merged
JSONbored merged 1 commit into
mainfrom
fix/auto-pause-current-head-count
Jul 8, 2026
Merged

fix(review): auto-pause count excluded the PR's own current head, so it never paused#4319
JSONbored merged 1 commit into
mainfrom
fix/auto-pause-current-head-count

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • countPublishedAiReviewHeads excluded the PR's own current head SHA from its count (fix(review): exclude current head from auto-pause count #3719), so a PR swept repeatedly with NO new commits could never reach review.auto_review.auto_pause_after_reviewed_commits's threshold — the one head it had ever been reviewed on was always "the current one," always subtracted back out. Confirmed live: one open PR (gittensory#4122) took 63 fresh AI review calls across a 12-hour window of scheduled sweeps, with zero new commits — the exact same cycle (miner-check → slop-advisory → AI-review → publish) repeating every 15-40 minutes, all night.
  • fix(review): exclude current head from auto-pause count #3719 excluded the current head to avoid a real, previously-hit problem: once the pause engaged, the cached review's findings were never reapplied, so an already-published blocker would silently vanish from later gate evaluations. That PR's fix avoided the symptom by making the pause effectively never engage for the (overwhelmingly common) "same PR, no new commits" case — which is exactly this bug.
  • This PR fixes both properly instead of trading one for the other:
    • countPublishedAiReviewHeads now counts the current head too, matching its own always-documented "published AI review count" contract (and its signature drops the now-unused currentHeadSha parameter).
    • A new reuse branch in processors.ts (autoReviewSkipReason === "review paused (commit threshold)") reapplies the last published review's findings whenever the pause fires — mirroring the existing isFrozenForManualReview reuse pattern exactly, under its own distinct github_app.ai_review_paused_reuse audit event / gittensory_ai_review_paused_reuse_total counter.

Scope

Validation

  • git diff --check
  • npm run typecheck
  • npx vitest run test/unit/ai-review-cache.test.ts test/unit/auto-review-wiring.test.ts — 45/45 passing, including the repurposed fix(review): exclude current head from auto-pause count #3719 regression test (now asserting the count correctly INCLUDES the current head) and the pre-existing threshold/draft/label/docs-only auto-review tests.
  • New end-to-end regression test in test/unit/queue.test.ts: simulates 3 consecutive scheduled sweeps of the SAME unchanged PR (matching the real-world pattern exactly) — asserts zero fresh AI calls across all 3 passes, the new reuse audit event fires on every pass, and the cached review's blocker text is still present in the public comment on every pass (proving fix(review): exclude current head from auto-pause count #3719's original concern — a vanishing blocker — is actually fixed, not just avoided).
  • Live-verified the root cause empirically before writing any code: queried the self-hosted instance's audit_events table directly, confirmed github_app.autoreview_paused/_resumed (a DIFFERENT, unrelated manual-command marker) has zero rows ever, then traced the real mechanism to countPublishedAiReviewHeads's SQL and confirmed the exclusion via git log -S back to fix(review): exclude current head from auto-pause count #3719's own issue text.

Safety

  • No secrets, wallet/hotkey/trust-score/reward data anywhere.
  • The one existing caller of the changed function (resolveAutoReviewSkipForPullRequest) is updated in the same commit; no other call sites exist.
  • Directly addresses the safety concern fix(review): exclude current head from auto-pause count #3719 was protecting against (a blocker silently vanishing once paused) — the new reuse branch is the actual fix for that, not a regression risk.

…it never paused

countPublishedAiReviewHeads excluded the current head SHA from its count
(#3719), meaning a PR swept repeatedly with no new commits could never
reach auto_pause_after_reviewed_commits' threshold -- the one head it had
ever been reviewed on was always the "current" one, always subtracted
back out. Confirmed live: one PR took 63 fresh AI calls across 12 hours
of scheduled sweeps with zero new commits.

#3719 excluded the current head specifically to avoid a real problem:
once paused, the cached review's findings were never reapplied, so an
already-published blocker would silently vanish from later gate
evaluations. Fixes that properly instead of avoiding it: the count now
includes the current head (matching its own "published review count"
contract), and a new reuse branch in processors.ts reapplies the last
published review's findings whenever the pause reason fires, mirroring
the existing frozen-for-manual-review reuse pattern.
@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 8, 2026
@JSONbored
JSONbored merged commit 0df7946 into main Jul 8, 2026
7 checks passed
@JSONbored
JSONbored deleted the fix/auto-pause-current-head-count branch July 8, 2026 20:28
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 8, 2026
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.81%. Comparing base (0777152) to head (2cefba4).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/queue/processors.ts 72.72% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4319      +/-   ##
==========================================
- Coverage   93.81%   93.81%   -0.01%     
==========================================
  Files         398      398              
  Lines       36780    36788       +8     
  Branches    13443    13444       +1     
==========================================
+ Hits        34506    34511       +5     
- Misses       1620     1621       +1     
- Partials      654      656       +2     
Files with missing lines Coverage Δ
src/db/repositories.ts 96.71% <ø> (-0.01%) ⬇️
src/queue/processors.ts 95.37% <72.72%> (-0.08%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant