fix(agent-approval-queue): recheck legacy heuristic closes with unknown mergeable-state justification#2987
Conversation
…wn mergeable-state justification closeRequiresMergeableState was compared with a strict === true check, so any pending close row staged before this field existed (closeRequiresMergeableState undefined) silently skipped the accept-time live conflict recheck and executed unchecked, even if it was originally staged for a base conflict that has since cleared. Scope the recheck to !== false instead, so only an explicit false (a known non-conflict duplicate/slop/blocker close) is excluded and an unknown legacy justification fails toward revalidating rather than skipping.
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Gittensory review result - approve/merge recommendedReview updated: 2026-07-04 08:40:47 UTC
✅ Suggested Action - Approve/Merge
Review summary Nits — 4 non-blocking
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.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2987 +/- ##
=======================================
Coverage 96.06% 96.06%
=======================================
Files 260 260
Lines 28687 28687
Branches 10436 10436
=======================================
Hits 27558 27558
Misses 493 493
Partials 636 636
🚀 New features to boost your workflow:
|
Summary
closeRequiresMergeableState === truecheck.auto_with_approvalclose row staged beforecloseRequiresMergeableStateexisted (field isundefined, notfalse) silently skipped the live recheck and executed unchecked, even if originally staged for a base conflict that has since cleared.closeRequiresMergeableState !== falseinstead: a known non-conflict close (false, always explicit for freshly planned closes) is still excluded, but an unknown/legacy justification (undefined) now fails toward revalidating rather than silently skipping.Scope
src/services/agent-approval-queue.ts: broadenshouldRecheckLiveDisposition's close-side predicate and update the two comments describing it.test/unit/agent-approval-queue.test.ts: two new regression tests covering both branches of the legacy (undefined) case, plus an update to a pre-existing unrelated test fixture (accept downgrades a staged heuristic close to a needs-human-review label when the close breaker engaged (#2127)) to explicitly setcloseRequiresCiState/closeRequiresMergeableStateso it isn't accidentally treated as an unknown-justification legacy row by this change.Validation
npm run typechecknpx vitest run test/unit/agent-approval-queue.test.ts test/unit/agent-actions.test.ts test/unit/agent-action-executor.test.ts— 346 passednpx vitest run test/unit/agent-approval-queue.test.ts --coverage --coverage.include='src/services/agent-approval-queue.ts'— 100% statements/branches/functions/lines on the changed fileSafety
closeRequiresMergeableState: false, duplicate/slop/blocker-only) is still never touched by the recheck.