Skip to content

fix(queue): verify the latest reopen actor before re-closing a disallowed reopen - #2421

Merged
JSONbored merged 3 commits into
mainfrom
fix/reopen-reclose-latest-actor
Jul 1, 2026
Merged

fix(queue): verify the latest reopen actor before re-closing a disallowed reopen#2421
JSONbored merged 3 commits into
mainfrom
fix/reopen-reclose-latest-actor

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Follow-up to fix(queue): re-verify live PR state before draft-dodge and reopen-reclose closes #2369 (merged): the reopen-reclose handler's live re-checks (head/state freshness, and re-checking the original reopener's own permission) both miss a real race — a different person, e.g. an actual maintainer, can reopen the same PR again after the original disallowed reopen. Since the PR was already open, that second reopen changes neither head SHA nor open/closed state, so neither existing check can see it, and the handler would still wrongly undo the maintainer's authorized reopen.
  • Generalizes getLastCloserLogin's timeline-scanning pagination engine (src/github/pr-actions.ts) into a shared, event-type-parameterized helper, and adds getLastReopenerLogin alongside it (same signature shape, same fail-conservative error handling).
  • Wires a third live re-check into maybeRecloseDisallowedReopen: deny the close unless the original disallowed reopener is still the most recent reopened actor on the PR's live timeline. Fails closed (denies) when the timeline scan can't prove that — mirroring the existing fail-closed precedent for an unresolvable freshness read — since wrongly re-closing a maintainer-authorized PR is worse than leaving a disallowed reopen open for one more tick.

Test plan

  • npx vitest run test/unit/queue.test.ts test/unit/github-pr-actions.test.ts — all passing, including regressions for: a different (maintainer) reopener superseding the original disallowed one (deny), the happy path unaffected (same reopener still latest → still closes), a timeline-read error (fail-closed deny), the "genuinely no reopen event found" case (deny with an "unknown" detail), and an audit-write failure on the new denial path (swallowed, handler still completes)
  • npm run test:coverage — full suite green, 100% patch coverage on changed src/** lines/branches
  • npm run typecheck — clean
  • git diff --check — clean
  • npm run test:ci + npm audit --audit-level=moderate — clean

JSONbored added 3 commits July 1, 2026 14:55
…owed reopen

Head/state freshness and a fresh re-check of the original reopener's own
permission both miss a real race: a DIFFERENT person (e.g. an actual
maintainer) can reopen the same PR again after the original disallowed
reopen — a legitimate, authorized action. Since the PR was already open,
that second reopen changes neither head SHA nor open/closed state, so
neither existing live re-check can see it, and the handler would still
wrongly undo the maintainer's reopen.

Generalizes getLastCloserLogin's timeline-scanning pagination engine
(src/github/pr-actions.ts) into a shared, event-type-parameterized helper,
adds getLastReopenerLogin alongside it, and wires a third live re-check
into maybeRecloseDisallowedReopen: deny the close unless the ORIGINAL
disallowed reopener is still the most recent "reopened" actor on the PR's
live timeline. Fails closed (denies) when the timeline scan can't prove
that, mirroring the existing fail-closed precedent for an unresolvable
freshness read.
…nches

The audit-write catch on the new superseded-reopener denial path, the
null-latest-reopener 'unknown' fallback in its detail message, and the
lastPage<=1 single-page-with-Link-header branch in the generalized
timeline scanner were all reachable but untested.
@dosubot dosubot Bot added the size:M label Jul 1, 2026
@loopover-orb

loopover-orb Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Important

🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪

🔍 Gittensory is reviewing…

AI analysis is in progress. This comment will update when the review is complete.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟪 Reviewing

@JSONbored JSONbored self-assigned this Jul 1, 2026
@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 1, 2026
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.84%. Comparing base (4389690) to head (909b42f).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2421   +/-   ##
=======================================
  Coverage   95.84%   95.84%           
=======================================
  Files         224      224           
  Lines       25048    25060   +12     
  Branches     9106     9112    +6     
=======================================
+ Hits        24007    24020   +13     
  Misses        427      427           
+ Partials      614      613    -1     
Files with missing lines Coverage Δ
src/github/pr-actions.ts 100.00% <100.00%> (+1.07%) ⬆️
src/queue/processors.ts 91.36% <100.00%> (+0.04%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit c279576 into main Jul 1, 2026
12 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 1, 2026
@JSONbored
JSONbored deleted the fix/reopen-reclose-latest-actor branch July 1, 2026 22:07
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

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant