Skip to content

review-evasion: reopen-reclose guard skips the write-permission-readiness check its 4 sibling guards enforce #6603

Description

@JSONbored

Context

src/queue/review-evasion.ts has a shared helper, evaluateCloseEnforcementGate (lines 82-207), used by all 5 close-enforcement guards in the file. It takes an optional permissionReadiness: { detail, metadata } | null argument (declared at line 98): when non-null, the gate re-verifies the GitHub App actually holds pull_requests: write (via resolveAgentPermissionReadiness, lines 165-183) before the caller is allowed to mutate the PR, and records a denied audit event if the permission is missing. When null, this whole check is skipped.

4 of the 5 guards pass a real permissionReadiness object:

  • closeDraftDodgeAttemptIfBlocked — line 282
  • closeReviewEvasionSelfCloseIfReviewed — line 630
  • closeReviewEvasionDraftConversionIfReviewed — line 835
  • closeRepeatedDraftCyclingIfDetected — line 1002

The 5th, recloseDisallowedReopenIfNeeded (the implementation behind the exported maybeRecloseDisallowedReopen), passes permissionReadiness: null at line 434 — skipping the check entirely. This is explicitly documented as a known, unfixed gap in two places in the same file:

Practical effect: if the GitHub App's pull_requests: write permission is missing or revoked on an installation, the other 4 guards correctly detect that up front and record a denied audit event instead of attempting a doomed GitHub mutation. The reopen-reclose guard has no such check and will still attempt closePullRequest (line 498) relying only on that API call's own failure path.

Requirements

  • In recloseDisallowedReopenIfNeeded (src/queue/review-evasion.ts, around lines 415-439), change the evaluateCloseEnforcementGate call's permissionReadiness argument from null to a real object, following the same shape as the 4 sibling call sites (e.g. line 282-285: `denied draft-dodge close for ${draftDodgeAuthor} — pull_requests: write not granted`).
  • The new detail string must name the reopener variable and the "reopen re-close" action, e.g. `denied reopen re-close for ${reopener} — pull_requests: write not granted`. Reuse the existing gateMetadata object already built at line 406 for metadata, matching how every sibling call site reuses its own local gateMetadata.
  • Update the doc comment at lines 77-79 (on evaluateCloseEnforcementGate) and the doc comment at lines 413-414 (at the call site) so neither still claims this gap exists once it is fixed.
  • Do not change the behavior of any of the other 4 guards, and do not change evaluateCloseEnforcementGate's own signature or the shape of its permissionReadiness parameter.

Deliverables

  • recloseDisallowedReopenIfNeeded passes a real permissionReadiness object to evaluateCloseEnforcementGate, matching its 4 siblings
  • The two now-stale "pre-existing gap" doc comments (lines ~77-79 and ~413-414) are updated to reflect the fix
  • A test asserting a reopen-reclose attempt is denied (with a permission_not_ready-style audit outcome) when write-permission readiness fails, mirroring the existing coverage pattern for the other 4 guards

Test Coverage Requirements

Touches src/queue/review-evasion.ts, which is under src/** — this repo's Codecov patch gate (99%+ on changed lines) applies to every changed line, including the new test. Existing guard tests exercising maybeRecloseDisallowedReopen (via src/queue/processors.ts's re-export) live in test/unit/queue-2.test.ts and test/unit/queue-lifecycle-guards.test.ts — extend one of those with a permission-readiness-denied case for the reopen-reclose guard, mirroring the equivalent test already present for one of the other 4 guards.

Expected Outcome

All 5 close-enforcement guards in src/queue/review-evasion.ts behave identically with respect to write-permission readiness: a missing pull_requests: write grant is detected and denied (with an audit event) before any of the 5 attempts a GitHub mutation, not just 4 of them.

Links & Resources

  • src/queue/review-evasion.tsevaluateCloseEnforcementGate (L82-207), recloseDisallowedReopenIfNeeded (L346-515)
  • Sibling call sites passing a real permissionReadiness: L282, L630, L835, L1002
  • test/unit/queue-2.test.ts, test/unit/queue-lifecycle-guards.test.ts

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions