Skip to content

fix(review): keep gate evaluation for ignored authors - #3698

Merged
JSONbored merged 2 commits into
mainfrom
codex/propose-fix-for-ignored-author-gate-bypass
Jul 6, 2026
Merged

fix(review): keep gate evaluation for ignored authors#3698
JSONbored merged 2 commits into
mainfrom
codex/propose-fix-for-ignored-author-gate-bypass

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The review.auto_review.ignore_authors path previously created a completed skipped gate check and returned early, which could suppress deterministic gate evaluation and allow required gate blockers (like linked-issue) to be bypassed.
  • The change scopes ignored-author behavior to suppress public review output and auditing only, while preserving the deterministic gate evaluation when a gate/autonomy conclusion is required.

Description

  • In maybePublishPrPublicSurface (src/queue/processors.ts) stop publishing a skipped gate check for ignored authors and avoid the early return; instead set publicSurfaceSkipped = true and only early-return when shouldEvaluateGate is false. This preserves downstream deterministic gate computation such as evaluateGateCheck.
  • Prevent type-label publication when the public surface has been suppressed by adding a !publicSurfaceSkipped guard to the type-label branch in maybePublishPrPublicSurface (src/queue/processors.ts).
  • Update the regression unit test (test/unit/queue.test.ts) to assert that an ignored author still triggers the gate evaluation (POST+PATCH check-run sequence with a failure conclusion for a linked-issue block) and to adjust mocked call counts accordingly.

Testing

  • Ran git diff --check which produced no whitespace/conflict errors and passed.
  • Ran npx vitest run test/unit/queue.test.ts -t "ignored authors" --reporter=verbose --no-color which passed.
  • Ran npx vitest run test/unit/queue.test.ts -t "evaluates the gate while suppressing public review output for ignored authors|audits ignored authors without a skipped check" --reporter=verbose --no-color which passed.
  • Ran npm run typecheck (tsc --noEmit) which completed without type errors.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 6, 2026
@loopover-orb

loopover-orb Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-06 08:27:59 UTC

2 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI failing · blocked

🛑 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 diff fixes a real gate-bypass bug: previously, ignored authors got an early `return undefined` after publishing a `skipped` gate check, which suppressed deterministic gate evaluation (e.g. linked-issue blocking) whenever `gateEnabled`. Now the function sets `publicSurfaceSkipped = true` and only returns early when `!shouldEvaluateGate`, letting downstream `evaluateGateCheck` logic run for ignored authors, and a new `!publicSurfaceSkipped` guard stops type-label publication from leaking through when the public surface is suppressed. The test rewrite matches the new behavior (POST in_progress + PATCH failure/linked-issue conclusion instead of a single skipped check) and is internally consistent with the code change. The change is narrowly scoped and the described intent (preserve gate evaluation, suppress only public output/audit) matches what the diff actually does.

Nits — 5 non-blocking
  • The old ignored-author path had an explicit `permission_missing` audit call for the skipped-check creation; confirm the downstream `evaluateGateCheck` path (not shown in this diff) still performs the equivalent permission-missing audit for ignored authors, since that handling was deleted here.
  • Only the type-label branch got the new `!publicSurfaceSkipped` guard — worth double-checking there's no other publish site between the ignored-author block and the type-label branch in `maybePublishPrPublicSurface` that also needs this guard (not visible in the diff).
  • No test appears added for the `gateEnabled === false && shouldEvaluateGate === true` combination to confirm the fallthrough behaves correctly with no gate check calls.
  • Add a unit test for ignored author + `gateEnabled=false` + `shouldEvaluateGate=true` to lock in the fallthrough path.
  • Add a short comment near `publicSurfaceSkipped = true` in src/queue/processors.ts explaining that it now only suppresses public surface, not gate evaluation, to prevent future regressions of the bug this PR fixes.

Why this is blocked

  • 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.

CI checks failing

  • validate
  • validate-code
  • validate
  • validate-code
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: 54 registered-repo PR(s), 46 merged, 413 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 54 PR(s), 413 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 is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 54 PR(s), 413 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.
  • 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

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.29%. Comparing base (5172dc4) to head (eb3810b).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3698      +/-   ##
==========================================
- Coverage   93.29%   93.29%   -0.01%     
==========================================
  Files         317      317              
  Lines       32368    32367       -1     
  Branches    11872    11872              
==========================================
- Hits        30199    30198       -1     
  Misses       1537     1537              
  Partials      632      632              
Files with missing lines Coverage Δ
src/queue/processors.ts 94.10% <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.

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 6, 2026
@JSONbored
JSONbored force-pushed the codex/propose-fix-for-ignored-author-gate-bypass branch 3 times, most recently from e382cd8 to 066958c Compare July 6, 2026 05:57
@JSONbored JSONbored closed this Jul 6, 2026
@JSONbored JSONbored reopened this Jul 6, 2026
@JSONbored
JSONbored force-pushed the codex/propose-fix-for-ignored-author-gate-bypass branch from 066958c to 5b1d74c Compare July 6, 2026 09:11
JSONbored added 2 commits July 6, 2026 02:15
#3694 (merged independently, unaware of this fix) made the gate check-run
publish a quiet "skipped (reason)" status instead of the evaluated gate
conclusion whenever auto-review is skipped -- including for an ignored
author, since evaluateAutoReviewSkipReason checks the same
review.auto_review.ignore_authors config this PR's own eligibility check
does. That silently reintroduced the exact bypass this PR exists to close:
an ignored author's PR would show "skipped" on its gate check-run instead of
a real hard-rule failure (e.g. no linked issue), even though the gate
evaluation itself was computed correctly.

Only take the "skipped status" shortcut when publicSurfaceSkipped is false.
publicSurfaceSkipped is already the flag this PR's ignored-author path (and
decidePublicSurface's own skip conditions) sets specifically so the
deterministic gate keeps reporting truthfully even when public review output
is suppressed -- reusing it here keeps the two mechanisms consistent instead
of introducing new reason-string matching. A benign content-based skip
(draft, WIP title, too-large, docs-only, base-branch, auto-pause) never sets
publicSurfaceSkipped, so #3694's quiet status is unaffected for those cases.
@JSONbored
JSONbored force-pushed the codex/propose-fix-for-ignored-author-gate-bypass branch from 5b1d74c to eb3810b Compare July 6, 2026 09:17
@JSONbored

Copy link
Copy Markdown
Owner Author

Rebased onto current `main`. Found and fixed an additional issue surfaced by the rebase: #3694 (merged independently, unaware of this PR) made the gate check-run publish a quiet "skipped (reason)" status whenever auto-review is skipped — including for an ignored author, since it checks the same `review.auto_review.ignore_authors` config this PR's eligibility check does. That silently reintroduced the exact bypass this PR closes: an ignored author's gate check-run would show "skipped" instead of the real evaluated conclusion (e.g. a linked-issue hard-rule failure).

Fix: the "skipped status" shortcut now only applies when `publicSurfaceSkipped` is false — reusing the existing flag this PR's own ignored-author path already sets, rather than introducing new reason-string matching. A benign content-based skip (draft, WIP title, too-large, docs-only, base-branch, auto-pause) never sets `publicSurfaceSkipped`, so #3694's quiet status is unaffected for those cases.

Full `test/unit/queue.test.ts` (588 tests) + typecheck + actionlint + audit all green locally.

@JSONbored
JSONbored merged commit 8398615 into main Jul 6, 2026
10 checks passed
@JSONbored
JSONbored deleted the codex/propose-fix-for-ignored-author-gate-bypass branch July 6, 2026 09:26
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. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

1 participant