refactor(queue): extract review-evasion close-enforcement guards into their own module - #4957
Conversation
… their own module Part of #4013's module-split sequence (step 5, after transient-locks.ts, signal-snapshot.ts, duplicate-detection.ts, and slop-detection.ts): the 5 review-evasion / one-shot-close guards (draft-dodge, reopen-reclose, review-evasion self-close, review-evasion draft-conversion, repeated draft-cycling) plus their shared scaffolding (withPrActuationLock, evaluateCloseEnforcementGate, hasMaintainerOrOwnerPermission) move to src/queue/review-evasion.ts. Pure mechanical move, no behavior change. Only the 5 top-level entry points are exported (each called from exactly one webhook-handler call site still in processors.ts); every other function/type/constant here was already private with no caller outside this cluster, so no re-export shim was needed -- nothing external ever imported them from processors.ts. Isolating this code surfaced two real, previously-invisible coverage gaps: - 8 best-effort `.catch(() => undefined)` handlers (label-application, review-tracking cleanup, courtesy comments, audit writes) had no v8 ignore marker, unlike their identically-shaped siblings elsewhere in this same cluster -- added the same marker, matching this codebase's established convention for this exact class of fail-safe catch. - The bot's-own-reopen exemption in the one-shot-reopen guard had no test at all -- added one exercising the real behavioral branch rather than suppressing it.
|
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 #4957 +/- ##
=======================================
Coverage 94.24% 94.25%
=======================================
Files 445 446 +1
Lines 38869 38855 -14
Branches 14157 14153 -4
=======================================
- Hits 36633 36623 -10
+ Misses 1577 1575 -2
+ Partials 659 657 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-11 03:07:15 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 5 non-blocking
Concerns raised — review before merging
Review context
Contributor next steps
Signal definitions
🟩 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.
|
Summary
src/queue/processors.ts. Step 5, aftertransient-locks.ts(refactor(queue): extract transient-lock primitives into their own module #4157),signal-snapshot.ts(refactor(queue): extract signal-snapshot generation into its own module #4820),duplicate-detection.ts(refactor(queue): extract duplicate-cluster adjudication into its own module #4823), andslop-detection.ts(refactor(queue): extract AI-slop-advisory gating/orchestration into its own module #4946).withPrActuationLock,evaluateCloseEnforcementGate,hasMaintainerOrOwnerPermission) intosrc/queue/review-evasion.ts. Pure mechanical move, no behavior change. This is the largest and most self-contained cluster in the split so far: none of these 11 functions had a caller outside this exact block in the original file, and none wereexported, so no re-export shim was needed for backward compatibility -- only the 5 top-level entry points are exported now, forprocessors.ts's own 5 webhook-handler call sites.processors.ts's overall coverage before):.catch(() => undefined)handlers (label-application, review-tracking cleanup, courtesy comments, audit writes) had nov8 ignoremarker, unlike their identically-shaped siblings elsewhere in the same functions. Added the same marker these siblings already use, matching the established convention for this class of fail-safe catch.reopener === botLogin) had no test at all. Added one that exercises the real behavior (agittensory[bot]reopen is allowed, no reclose) rather than suppressing the gap.Part of #4013 (more extraction steps remain in the sequence — not closing the tracking issue).
Test plan
npx tsc --noEmit -p .— zero errorsnpx vitest run test/unit test/integration— 696/697 files passed (1 skipped, pre-existing)npm run test:coverage(unsharded) — 94.46% statements / 93.42% branches / 93.69% functions / 95.03% lines, no threshold failures;review-evasion.tsitself is 100% line + branch + function coverednpm run docs:drift-check,npm run manifest:drift-check,npm run engine-parity:drift-check— all oknpm audit --audit-level=moderate— 0 vulnerabilities