Skip to content

fix(review): exempt the owner/admin/automation-bot from the manual-review freeze#3490

Merged
JSONbored merged 1 commit into
mainfrom
fix/freeze-owner-exemption
Jul 5, 2026
Merged

fix(review): exempt the owner/admin/automation-bot from the manual-review freeze#3490
JSONbored merged 1 commit into
mainfrom
fix/freeze-owner-exemption

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Fixes a real incident on PR #3476, root-caused via the live audit_events trail on the self-host VPS.

The maintainer-gated freeze (#3461) reuses the last published AI review for any PR carrying the manual-review label, so a repeat push doesn't buy a fresh, expensive AI call while a PR is held. It had no author exemption. On PR #3476 (owner-authored, held only on this repo's "must link an issue" policy), pushing genuine follow-up commits kept replaying the original, now-stale AI verdict pass after pass — github_app.ai_review_frozen_reuse fired on every one of that PR's own follow-up commits — hiding the owner's own fix from the review meant to evaluate it. The review comment kept quoting code that no longer existed in the file, which is what surfaced this.

The anti-gaming concern the freeze exists for is specific to a contributor iterating pushes against the bot; it was never meant to apply to the repo owner's own PRs. This adds the same owner/ADMIN_GITHUB_LOGINS/automation-bot exemption this codebase already grants those authors everywhere else (auto-close, review-nag, contributor caps) to the freeze check.

Investigated by SSHing into the self-host VPS and querying the live Postgres audit_events table directly for target_key = 'JSONbored/gittensory#3476', which showed github_app.ai_review_frozen_reuse completing on every push, confirming the freeze — not a review-engine caching bug — was the cause.

No linked issue — direct fix for a live incident found via VPS audit-log investigation, not a pre-filed issue.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run typecheck
  • npx vitest run test/unit/queue.test.ts — full file, 560 passed, including 3 new regression tests reproducing the exact incident (owner, admin, and automation-bot exemptions) plus the 9 pre-existing freeze tests confirming the contributor-facing freeze behavior is unchanged
  • Targeted branch coverage on the changed lines — 0 uncovered statements/branches
  • npm audit --audit-level=moderate — 0 vulnerabilities
  • npm run actionlint / test:workers / build:mcp / test:mcp-pack / ui:* (not re-run — no workflow, workers, MCP, or UI files touched)
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • Single-file backend change (src/queue/processors.ts) plus its test file; workers/MCP/UI suites are unaffected by this diff.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — no public API/schema changes.)
  • UI changes use live API data or real empty/error/loading states. (N/A)
  • Visible UI changes include a UI Evidence section. (N/A — no UI changes.)
  • Public docs/changelogs are updated where needed. (N/A)

…view freeze

The maintainer-gated freeze (#3461) reused the last published AI
review for any PR carrying the manual-review label, with no author
exemption. Confirmed live on PR #3476: pushing genuine follow-up
commits to the owner's own held PR kept replaying the ORIGINAL,
now-stale AI verdict instead of evaluating the new commits, because
github_app.ai_review_frozen_reuse fired on every push. The anti-gaming
concern the freeze exists for is specific to a contributor iterating
pushes against the bot; it never applies to the repo owner, an
ADMIN_GITHUB_LOGINS fleet operator, or a protected automation bot,
matching the exemption this codebase already grants those authors
everywhere else (auto-close, review-nag, contributor caps).

Root-caused via the live audit_events trail on the self-host VPS,
which showed ai_review_frozen_reuse firing on every one of #3476's
own follow-up commits despite the PR being owner-authored.
@superagent-security

Copy link
Copy Markdown

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

@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.04%. Comparing base (29319d4) to head (d8f9542).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3490   +/-   ##
=======================================
  Coverage   93.04%   93.04%           
=======================================
  Files         299      299           
  Lines       31210    31211    +1     
  Branches    11388    11389    +1     
=======================================
+ Hits        29038    29039    +1     
  Misses       1517     1517           
  Partials      655      655           
Files with missing lines Coverage Δ
src/queue/processors.ts 93.46% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.5x multiplier. label Jul 5, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-05 10:18:09 UTC

2 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
The change correctly narrows the manual-review freeze so it still protects contributor repeat pushes while allowing repo owner, configured admin, and protected automation authors to receive a fresh AI review on a held PR. The new guard is placed directly in the freeze predicate, so CI/label reconciliation remains unchanged and the added tests exercise the three exempt author classes against the frozen-reuse audit path. I do not see a reachable correctness defect in the visible diff.

Nits — 4 non-blocking
  • test/unit/queue.test.ts:4148 repeats the same held-PR setup and fetch stub three times; table-driving the owner/admin/bot cases would make future changes to the freeze scenario less error-prone.
  • src/queue/processors.ts:7848 carries a long incident narrative inside the hot-path comment; consider trimming this to the durable rule and pointing at PR test(review): document the last unreachable review-evasion coverage branch #3476/fix(review): stop same-head AI re-review churn and add a maintainer-gated freeze #3461 for the history.
  • test/unit/queue.test.ts:4148 can use a small case table for `{ login, envOverride, prNumber, shaPrefix, response }` while keeping the existing assertions for `aiCalls` and `github_app.ai_review_frozen_reuse`.
  • src/queue/processors.ts:7858 can keep the operational invariant concise: owner/admin/protected automation authors bypass only the AI freeze, while contributor held PRs remain frozen unless `webhook.forceAiReview` is true.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 56 registered-repo PR(s), 46 merged, 586 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 56 PR(s), 586 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 56 PR(s), 586 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 5, 2026
@JSONbored JSONbored merged commit 6452202 into main Jul 5, 2026
10 checks passed
@JSONbored JSONbored deleted the fix/freeze-owner-exemption branch July 5, 2026 10:22
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.5x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant