fix(queue): re-verify live PR state before draft-dodge and reopen-reclose closes

src/github/pr-freshness.ts

@@ -11,7 +11,7 @@ export type PullRequestFreshness =
     }
   | {
       status: "stale";
-      reason: "unavailable" | "closed" | "head_unresolved" | "head_changed";
+      reason: "unavailable" | "closed" | "head_unresolved" | "head_changed" | "no_longer_draft";
       expectedHeadSha: string | null;
       liveHeadSha: string | null;
       liveState: string | null;
@@ -30,8 +30,9 @@ export function reviewedPullRequestHeadSha(
 }
 
 export function classifyPullRequestFreshness(
-  live: Pick<GitHubPullRequestPayload, "state" | "head"> | null | undefined,
+  live: Pick<GitHubPullRequestPayload, "state" | "head" | "draft"> | null | undefined,
   expectedHeadSha: string | null | undefined,
+  options?: { requireDraft?: boolean },
 ): PullRequestFreshness {
   const expected = normalizedHead(expectedHeadSha);
   if (!live) {
@@ -51,6 +52,12 @@ export function classifyPullRequestFreshness(
   if (expected && liveHeadSha !== expected) {
     return { status: "stale", reason: "head_changed", expectedHeadSha: expected, liveHeadSha, liveState };
   }
+  // The draft-dodge close is only justified while the PR is STILL a draft -- a same-head, still-open PR
+  // that was converted back to ready_for_review before the close fires has cleared its own justification
+  // (#2130 follow-up: head/state alone can't see this transition).
+  if (options?.requireDraft && live.draft !== true) {
+    return { status: "stale", reason: "no_longer_draft", expectedHeadSha: expected, liveHeadSha, liveState };
+  }
   return { status: "current", liveHeadSha, liveState };
 }
 
@@ -61,21 +68,26 @@ export async function fetchPullRequestFreshness(
     repoFullName: string;
     pullNumber: number;
     expectedHeadSha?: string | null | undefined;
+    // Require the LIVE PR to still be a draft (the draft-dodge close's own justification). Absent/false
+    // preserves every other caller's existing head/state-only behavior exactly.
+    requireDraft?: boolean;
   },
 ): Promise<PullRequestFreshness> {
+  const options = args.requireDraft !== undefined ? { requireDraft: args.requireDraft } : {};
   const token =
     (await createInstallationToken(env, args.installationId).catch(() => undefined)) ??
     env.GITHUB_PUBLIC_TOKEN;
-  if (!token) return classifyPullRequestFreshness(undefined, args.expectedHeadSha);
+  if (!token) return classifyPullRequestFreshness(undefined, args.expectedHeadSha, options);
   const admissionKey = githubRateLimitAdmissionKeyForToken(env, token, args.installationId);
   const live = await fetchLivePullRequest(env, args.repoFullName, args.pullNumber, token, admissionKey);
-  return classifyPullRequestFreshness(live, args.expectedHeadSha);
+  return classifyPullRequestFreshness(live, args.expectedHeadSha, options);
 }
 
 export function pullRequestFreshnessDetail(result: PullRequestFreshness): string {
   if (result.status === "current") return "PR is current";
   if (result.reason === "unavailable") return "live PR state could not be verified";
   if (result.reason === "closed") return `PR is no longer open (live state: ${result.liveState ?? "unknown"})`;
   if (result.reason === "head_unresolved") return "live PR head SHA could not be verified";
+  if (result.reason === "no_longer_draft") return "PR is no longer a draft";
   return `PR head changed from ${result.expectedHeadSha ?? "unknown"} to ${result.liveHeadSha ?? "unknown"}`;
 }

src/queue/processors.ts

@@ -3502,33 +3502,64 @@ async function processGitHubWebhook(
             agentDryRun: settings.agentDryRun,
           });
           if (draftMode === "live") {
-            const codes = block.blockerCodes.join(", ");
-            await createIssueComment(
-              env,
+            // Live re-check (#2130): the two async DB reads above (getGateBlockOutcome, resolveAgentActionMode's
+            // isGlobalAgentFrozen) leave a window where a maintainer could merge/close the PR, or a fresh push
+            // could clear the gate failure, before this fires. Unlike the main gate-close path — which routes
+            // every close through executeAgentMaintenanceActions's freshness guard — this handler acted purely
+            // off the stale webhook-ingestion payload. Re-verify live state immediately before the mutation.
+            // requireDraft: head/state alone would still read "current" if the author converted the PR BACK
+            // to ready_for_review in that window -- the draft-dodge close's own justification no longer
+            // holds, since there is no longer a draft to be "dodging" the gate through.
+            const freshness = await fetchPullRequestFreshness(env, {
               installationId,
               repoFullName,
-              pr.number,
-              `Gate verdict stands for this commit — converting to draft does not reset the review. Re-submit a new PR with the issues addressed${codes ? ` (${codes})` : ""}.`,
-            ).catch(() => undefined);
-            await closePullRequest(
-              env,
-              installationId,
-              repoFullName,
-              pr.number,
-            ).catch(() => undefined);
-            await recordAuditEvent(env, {
-              eventType: "github_app.draft_dodge_closed",
-              actor: "gittensory",
-              targetKey: `${repoFullName}#${pr.number}`,
-              outcome: "completed",
-              detail: `closed draft-dodge attempt by ${pr.authorLogin ?? "unknown"} — prior gate failure on headSha ${pr.headSha} stands`,
-              metadata: {
-                deliveryId,
+              pullNumber: pr.number,
+              expectedHeadSha: pr.headSha,
+              requireDraft: true,
+            });
+            if (freshness.status !== "current") {
+              await recordAuditEvent(env, {
+                eventType: "github_app.draft_dodge_closed",
+                actor: "gittensory",
+                targetKey: `${repoFullName}#${pr.number}`,
+                outcome: "denied",
+                detail: `${pullRequestFreshnessDetail(freshness)} — draft-dodge close not executed`,
+                metadata: {
+                  deliveryId,
+                  repoFullName,
+                  headSha: pr.headSha,
+                  blockerCodes: block.blockerCodes,
+                },
+              }).catch(() => undefined);
+            } else {
+              const codes = block.blockerCodes.join(", ");
+              await createIssueComment(
+                env,
+                installationId,
                 repoFullName,
-                headSha: pr.headSha,
-                blockerCodes: block.blockerCodes,
-              },
-            }).catch(() => undefined);
+                pr.number,
+                `Gate verdict stands for this commit — converting to draft does not reset the review. Re-submit a new PR with the issues addressed${codes ? ` (${codes})` : ""}.`,
+              ).catch(() => undefined);
+              await closePullRequest(
+                env,
+                installationId,
+                repoFullName,
+                pr.number,
+              ).catch(() => undefined);
+              await recordAuditEvent(env, {
+                eventType: "github_app.draft_dodge_closed",
+                actor: "gittensory",
+                targetKey: `${repoFullName}#${pr.number}`,
+                outcome: "completed",
+                detail: `closed draft-dodge attempt by ${pr.authorLogin ?? "unknown"} — prior gate failure on headSha ${pr.headSha} stands`,
+                metadata: {
+                  deliveryId,
+                  repoFullName,
+                  headSha: pr.headSha,
+                  blockerCodes: block.blockerCodes,
+                },
+              }).catch(() => undefined);
+            }
           } else if (draftMode === "dry_run") {
             /* v8 ignore next -- a deleted-account PR yields a null author login; the fallback is defensive */
             const draftAuthor = pr.authorLogin ?? "unknown";
@@ -7491,6 +7522,42 @@ async function maybeRecloseDisallowedReopen(
     );
     return true; // handled (decision made); never falls through to act on a stood-down repo
   }
+  // Live re-check (#2130): the maintainer-permission lookup, getLastCloserLogin's timeline read, and
+  // resolveRepositorySettings/isGlobalAgentFrozen above leave a window where the PR's live state could have
+  // moved — e.g. a maintainer re-closes it themselves, or reopens it a second time with real authorization —
+  // before this fires. Mirrors the draft-dodge sibling's identical fix; re-verify immediately before the mutation.
+  const reopenFreshness = await fetchPullRequestFreshness(env, {
+    installationId,
+    repoFullName,
+    pullNumber: pr.number,
+    expectedHeadSha: pr.headSha,
+  });
+  if (reopenFreshness.status !== "current") {
+    await recordAuditEvent(env, {
+      eventType: "github_app.reopen_reclosed",
+      actor: "gittensory",
+      targetKey: `${repoFullName}#${pr.number}`,
+      outcome: "denied",
+      detail: `${pullRequestFreshnessDetail(reopenFreshness)} — reopen re-close not executed`,
+      metadata: { deliveryId, repoFullName },
+    }).catch(() => undefined);
+    return true; // handled (decision made); a stale re-check still counts as handled, not a fallthrough
+  }
+  // Head/state freshness alone can't see a permission grant: the SAME reopener could be promoted to a
+  // maintainer/admin/write collaborator (or added as one) in the window since the check above ran, which
+  // would authorize exactly the reopen this handler is about to undo. Re-verify immediately before the
+  // mutation, not just once at ingestion time.
+  if (await hasMaintainerPermission(reopener)) {
+    await recordAuditEvent(env, {
+      eventType: "github_app.reopen_reclosed",
+      actor: "gittensory",
+      targetKey: `${repoFullName}#${pr.number}`,
+      outcome: "denied",
+      detail: `${reopener} now holds maintainer permission — reopen re-close not executed`,
+      metadata: { deliveryId, repoFullName },
+    }).catch(() => undefined);
+    return true; // handled (decision made); a newly-authorized reopener still counts as handled
+  }
   await createIssueComment(
     env,
     installationId,

test/unit/pr-freshness.test.ts

@@ -76,6 +76,41 @@ describe("PR freshness guards", () => {
     ).toBe("PR head changed from unknown to unknown");
   });
 
+  it("does not require draft state by default, even when the PR is no longer a draft", () => {
+    const result = classifyPullRequestFreshness({ state: "open", head: { sha: "sha1" }, draft: false }, "sha1");
+    expect(result).toEqual({ status: "current", liveHeadSha: "sha1", liveState: "open" });
+  });
+
+  it("REGRESSION (#2130 follow-up): treats a same-head PR converted back to ready_for_review as stale when the caller requires draft", () => {
+    const result = classifyPullRequestFreshness({ state: "open", head: { sha: "sha1" }, draft: false }, "sha1", { requireDraft: true });
+    expect(result).toMatchObject({ status: "stale", reason: "no_longer_draft", liveState: "open", liveHeadSha: "sha1" });
+    expect(pullRequestFreshnessDetail(result)).toBe("PR is no longer a draft");
+  });
+
+  it("treats a still-draft PR as current when the caller requires draft", () => {
+    const result = classifyPullRequestFreshness({ state: "open", head: { sha: "sha1" }, draft: true }, "sha1", { requireDraft: true });
+    expect(result).toEqual({ status: "current", liveHeadSha: "sha1", liveState: "open" });
+  });
+
+  it("treats a missing draft field as stale when the caller requires draft (fail-safe: only an explicit true counts)", () => {
+    const result = classifyPullRequestFreshness({ state: "open", head: { sha: "sha1" } }, "sha1", { requireDraft: true });
+    expect(result).toMatchObject({ status: "stale", reason: "no_longer_draft" });
+  });
+
+  it("fetches live PR state including draft, and requires draft when requested", async () => {
+    const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
+    vi.stubGlobal("fetch", async () => Response.json({ state: "open", head: { sha: "sha7" }, draft: false }));
+    await expect(
+      fetchPullRequestFreshness(env, {
+        installationId: 123,
+        repoFullName: "owner/repo",
+        pullNumber: 7,
+        expectedHeadSha: "sha7",
+        requireDraft: true,
+      }),
+    ).resolves.toMatchObject({ status: "stale", reason: "no_longer_draft" });
+  });
+
   it("uses the stored PR head before falling back to advisory metadata", () => {
     expect(reviewedPullRequestHeadSha(" pr-sha ", "advisory-sha")).toBe("pr-sha");
     expect(reviewedPullRequestHeadSha(null, " advisory-sha ")).toBe("advisory-sha");