You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A contributor PR that is auto-closed solely because of an unresolved GitHub review thread (REVIEW_THREAD_BLOCKER_CODE, see src/review/review-thread-findings.ts) has no live re-check of that fact before the close actually executes, at either of the two actuation checkpoints in this codebase.
This is the same staleness pattern #3863 already fixed for base-conflict closes (re-checking mergeable_state live right before acting, since a slow review pass can let the underlying fact change between planning and actuation) -- but #3863's fix only covers action.closeRequiresMergeableState === true (conflict-justified closes). A review-thread-only close is planned with closeRequiresMergeableState: false / closeRequiresCiState: "not_required" (see src/settings/agent-actions.ts), the same bucket the code explicitly and intentionally exempts from CI recheck ("Non-CI closes are exempt"). But a review thread being RESOLVED (a contributor clicking "Resolve conversation" on GitHub) is exactly the kind of live-changeable fact that #3863's own reasoning says needs a recheck -- it's just never been extended to this case.
Where
decidePendingAgentAction in src/services/agent-approval-queue.ts: its shouldRecheckLiveDisposition condition only covers a staged merge or a conflict-justified heuristic close. A review-thread-only close matches neither, so it falls through with no live recheck.
The live-fetch function needed already exists and is already used elsewhere: fetchLiveReviewThreadBlockers (src/github/backfill.ts), currently only called from the live webhook planning pass (src/queue/processors.ts).
Fix
Mirror the #3863 shape for review-thread closes instead of conflict closes: tag a review-thread-justified close with a new closeRequiresThreadResolved marker at planning time, then re-check fetchLiveReviewThreadBlockers live at both actuation checkpoints before honoring the close.
Summary
A contributor PR that is auto-closed solely because of an unresolved GitHub review thread (
REVIEW_THREAD_BLOCKER_CODE, seesrc/review/review-thread-findings.ts) has no live re-check of that fact before the close actually executes, at either of the two actuation checkpoints in this codebase.This is the same staleness pattern #3863 already fixed for base-conflict closes (re-checking
mergeable_statelive right before acting, since a slow review pass can let the underlying fact change between planning and actuation) -- but #3863's fix only coversaction.closeRequiresMergeableState === true(conflict-justified closes). A review-thread-only close is planned withcloseRequiresMergeableState: false/closeRequiresCiState: "not_required"(seesrc/settings/agent-actions.ts), the same bucket the code explicitly and intentionally exempts from CI recheck ("Non-CI closes are exempt"). But a review thread being RESOLVED (a contributor clicking "Resolve conversation" on GitHub) is exactly the kind of live-changeable fact that #3863's own reasoning says needs a recheck -- it's just never been extended to this case.Where
decidePendingAgentActioninsrc/services/agent-approval-queue.ts: itsshouldRecheckLiveDispositioncondition only covers a staged merge or a conflict-justified heuristic close. A review-thread-only close matches neither, so it falls through with no live recheck.src/services/agent-action-executor.ts(added for Review latency can create a merge-conflict close window an author didn't cause #3863):requiresLiveCiRecheckandrequiresLiveMergeableRecheckare the only two live-recheck triggers; neither matches a review-thread-only close.The live-fetch function needed already exists and is already used elsewhere:
fetchLiveReviewThreadBlockers(src/github/backfill.ts), currently only called from the live webhook planning pass (src/queue/processors.ts).Fix
Mirror the #3863 shape for review-thread closes instead of conflict closes: tag a review-thread-justified close with a new
closeRequiresThreadResolvedmarker at planning time, then re-checkfetchLiveReviewThreadBlockerslive at both actuation checkpoints before honoring the close.