Skip to content

fix(queue): draft-dodge close has no live PR-state re-check before closing #2130

Description

@JSONbored

Parent: #1936

Problem

The draft-dodge close path (payload.action === "converted_to_draft", src/queue/processors.ts:3354) gates on pr.state === "open" captured from the webhook payload, then performs two more async reads — getGateBlockOutcome (a D1 read) and resolveAgentActionMode (which includes the D1-backed isGlobalAgentFrozen check) — before calling closePullRequest directly. Unlike the main gate-close path, which routes every close through executeAgentMaintenanceActions's freshness guard (a live GitHub GET immediately before the mutation), this path never re-fetches the PR's live state before firing the PATCH.

Failure scenario: a contributor converts a gate-rejected PR to draft. Within the same processing window — while the two async DB reads above are in flight — a maintainer manually merges/closes the PR, or pushes a new commit that clears the gate failure. The stale in-memory pr.state/pr.headSha no longer reflects reality, and closePullRequest fires a blind state: closed PATCH regardless. GitHub's own semantics bound the worst case for an already-merged PR (state is already closed), but a PR whose gate failure was just resolved by a fresh push gets closed anyway, since the handler never observes the new head. Note: this is the same class of bug already fixed in the sibling reReviewStoredPullRequest sweep path (a recent PR added a live fetch + early-exit there) — this hardening was never backported to the draft-dodge path.

Requirements

  • A close triggered by this path must reflect the PR's live state at the moment of the mutation, not a snapshot from webhook ingestion.

Deliverables

  • Add a fetchPullRequestFreshness call (mirroring executeAgentMaintenanceActions step 5) immediately before the closePullRequest call at processors.ts:3389, denying the close (with a "denied — stale" audit outcome) when freshness.status !== "current".
  • Apply the identical fix to the structurally-matching reopen-reclose path (maybeRecloseDisallowedReopen, processors.ts:7077), which has the same bypass pattern.
  • Add a regression test exercising: PR converted to draft, then a new commit lands (or the PR is closed by a human) before the async chain completes — assert the close is denied rather than firing blind.

Acceptance criteria

  • A draft-dodge close re-verifies live PR state immediately before the mutation and is denied if state/head has changed since the webhook was received.
  • The existing draft-dodge test suite (pause/freeze/dry-run/override/owner-exemption) continues to pass unchanged.

Expected outcome

The draft-dodge close path gets the same live-freshness guarantee every other actuation path already has, closing the narrow race between webhook ingestion and mutation.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

Status
Done

Relationships

None yet

Development

No branches or pull requests

Issue actions