From c685499bc2da2566f0e8d1fc1eda57c1ed0afcfb Mon Sep 17 00:00:00 2001
From: JSONbored <49853598+JSONbored@users.noreply.github.com>
Date: Wed, 1 Jul 2026 18:01:26 -0700
Subject: [PATCH 1/7] fix(queue): add a per-PR actuation mutex for the
 draft-dodge and reopen-reclose paths
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two different webhook deliveries for the same PR (e.g. a reopened
event and a concurrent check_suite completed event) could be dequeued
by separate workers at nearly the same time. Both would read the same
stale-but-still-"current" state, both pass their own freshness checks,
and both independently fire a mutating call — a TOCTOU window with no
per-PR mutex anywhere in the actuation path.

Add a lightweight interim mutex (short-TTL transient-cache claim,
best-effort release) and wrap the draft-dodge close and reopen-reclose
handlers with it — the two mutating webhook-triggered paths that
weren't already covered by an existing per-PR lock. A lock-contended
caller fails open (skips this pass); the delivery holding the lock is
evaluating the same PR, and the periodic sweep is the backstop if this
specific trigger is dropped.

Deliberately NOT the queue-level "widen the coalesce lookup to match
status='processing'" interim step the issue also floats: that would
have enqueue() silently UPDATE a claimed row's payload, which never
gets re-read before the claiming worker deletes the row on completion
— a coalesce that reports success while permanently discarding the
new event's trigger. The per-PR mutex avoids that failure mode
entirely. A full per-PR Durable Object (SubmissionLock) remains a
separate, larger follow-up per the existing TODO in env.d.ts.

# Conflicts:
#	test/unit/queue.test.ts
---
 src/queue/processors.ts | 425 +++++++++++++++++++++++++---------------
 test/unit/queue.test.ts |  58 ++++++
 2 files changed, 324 insertions(+), 159 deletions(-)

diff --git a/src/queue/processors.ts b/src/queue/processors.ts
index 9829d34c61..367afcdfb0 100644
--- a/src/queue/processors.ts
+++ b/src/queue/processors.ts
@@ -2305,6 +2305,40 @@ async function putTransientKey(
   }
 }
 
+// Per-PR actuation mutex (#2135). Two DIFFERENT webhook deliveries for the same PR (e.g. a `reopened` event and
+// a concurrent `check_suite completed` event) can be dequeued by separate workers at nearly the same time; both
+// would read the same stale-but-still-"current" state, both pass their own freshness checks, and both
+// independently fire a mutating call. This is a lightweight interim mutex (a full per-PR Durable Object /
+// SubmissionLock is a separate, more-involved follow-up — see the TODO in env.d.ts) built on the SAME transient
+// cache used for CI-completion coalescing above: a short-TTL claim, best-effort release. A lock-contended caller
+// fails OPEN (returns false / skips this pass) rather than blocking — the delivery holding the lock is
+// evaluating the SAME PR, and the periodic sweep is the backstop if this specific trigger is dropped.
+const PR_ACTUATION_LOCK_TTL_SECONDS = 60;
+function prActuationLockKey(repoFullName: string, prNumber: number): string {
+  return `pr-actuation-lock:${repoFullName.toLowerCase()}#${prNumber}`;
+}
+async function claimPrActuationLock(
+  env: Env,
+  repoFullName: string,
+  prNumber: number,
+): Promise<boolean> {
+  const key = prActuationLockKey(repoFullName, prNumber);
+  if (await getTransientKey(env, key)) return false;
+  await putTransientKey(env, key, "1", PR_ACTUATION_LOCK_TTL_SECONDS);
+  return true;
+}
+async function releasePrActuationLock(
+  env: Env,
+  repoFullName: string,
+  prNumber: number,
+): Promise<void> {
+  try {
+    await env.SELFHOST_TRANSIENT_CACHE?.del?.(prActuationLockKey(repoFullName, prNumber));
+  } catch {
+    // best-effort
+  }
+}
+
 /**
  * True when CI for this PR+headSha has been pending past STUCK_CI_DEFER_MS. Stamps the first-seen time in a
  * transient cache keyed by repo#pr:headSha — a new push is a new SHA, so the window resets per commit. A missing
@@ -3806,167 +3840,14 @@ async function processGitHubWebhook(
         !settings.agentPaused &&
         !isProtectedAutomationAuthor(pr.authorLogin)
       ) {
-        const block = await getGateBlockOutcome(
+        await maybeCloseDraftDodgeAttempt(
           env,
+          deliveryId,
+          installationId,
           repoFullName,
-          pr.number,
+          pr,
+          settings,
         ).catch(() => undefined);
-        const repoOwner = repoFullName.includes("/")
-          ? repoFullName.slice(0, repoFullName.indexOf("/")).toLowerCase()
-          : "";
-        const draftDodgeAuthorLogin = (pr.authorLogin ?? "").toLowerCase();
-        const authorIsOwner =
-          draftDodgeAuthorLogin === repoOwner && repoOwner.length > 0;
-        // Fleet-operator identity (#2133): same ADMIN_GITHUB_LOGINS exemption as the primary close-eligibility
-        // computation above and hasMaintainerPermission below — an admin login must never be auto-closed here
-        // either, matching every other actuation path's trusted-operator definition.
-        const authorIsAdmin =
-          draftDodgeAuthorLogin.length > 0 &&
-          parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(draftDodgeAuthorLogin);
-        if (
-          block &&
-          block.headSha === pr.headSha &&
-          !block.overridden &&
-          !authorIsOwner &&
-          !authorIsAdmin
-        ) {
-          // Respect the agent action mode (#killswitch-gap): the outer guard already excludes a per-repo pause,
-          // but this close path must also honor the global freeze and dry-run — so a freeze is a COMPLETE stop
-          // and a dry-run records the would-be close without touching GitHub.
-          const draftMode = resolveAgentActionMode({
-            globalPaused:
-              isGlobalAgentPause(env) || (await isGlobalAgentFrozen(env)),
-            agentPaused: settings.agentPaused,
-            agentDryRun: settings.agentDryRun,
-          });
-          if (draftMode === "live") {
-            // Write-permission readiness (#2134): this close bypasses executeAgentMaintenanceActions entirely
-            // (the whole point is to enforce the gate verdict against the CURRENT headSha even though the PR
-            // was converted to draft), so it never got the standard pipeline's step-6 PR_WRITE_CLASSES guard.
-            // Without this, a revoked/never-consented pull_requests:write grant would still attempt the close,
-            // get a 403 from GitHub, and have it silently swallowed by the .catch() below — with the audit
-            // event still recorded as "completed" as if the close actually happened. Checked BEFORE the live
-            // freshness re-check below so a permission-denied installation never pays for a live GitHub fetch.
-            // Deliberately UNCAUGHT: getInstallation itself never swallows a genuine D1 read failure (it only
-            // resolves null on a legitimate "row not found" query result), so let a transient storage hiccup
-            // propagate and fail this whole webhook job -- the queue's own retry re-runs it, and a later attempt
-            // with a working DB read correctly evaluates readiness. Catching it into `null` here would instead
-            // permanently misrecord the outcome as "pull_requests: write not granted" (a real GitHub-permission
-            // problem) when the actual cause was an infra blip, misleading an operator investigating the audit
-            // trail and burying the fact that no retry ever happens for a caught, definitively-denied outcome.
-            const draftDodgeInstallation = await getInstallation(
-              env,
-              installationId,
-            );
-            /* v8 ignore next -- upsertInstallation already ran unconditionally earlier in this same handler for
-             * every webhook, so a genuinely-missing row is not reachable through the normal webhook path
-             * exercised by tests; a synced installation always has a permissions object. */
-            const draftDodgeInstallationPermissions = draftDodgeInstallation?.permissions ?? null;
-            const draftDodgePermissionReadiness = resolveAgentPermissionReadiness({
-              autonomy: settings.autonomy,
-              installationPermissions: draftDodgeInstallationPermissions,
-            });
-            if (draftDodgePermissionReadiness !== "ready") {
-              /* v8 ignore next -- a deleted-account PR yields a null author login; the fallback is defensive */
-              const draftDodgeAuthor = pr.authorLogin ?? "unknown";
-              await recordAuditEvent(env, {
-                eventType: "github_app.draft_dodge_closed",
-                actor: "gittensory",
-                targetKey: `${repoFullName}#${pr.number}`,
-                outcome: "denied",
-                detail: `denied draft-dodge close for ${draftDodgeAuthor} — pull_requests: write not granted`,
-                metadata: {
-                  deliveryId,
-                  repoFullName,
-                  headSha: pr.headSha,
-                  blockerCodes: block.blockerCodes,
-                },
-              }).catch(
-                /* v8 ignore next -- fail-safe: an audit write failure never blocks the handler */
-                () => undefined,
-              );
-            } else {
-              // 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,
-                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,
-                  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";
-            await recordAuditEvent(env, {
-              eventType: "github_app.draft_dodge_closed",
-              actor: "gittensory",
-              targetKey: `${repoFullName}#${pr.number}`,
-              outcome: "completed",
-              detail: `dry-run: would close draft-dodge attempt by ${draftAuthor} — prior gate failure on headSha ${pr.headSha} stands`,
-              metadata: {
-                deliveryId,
-                repoFullName,
-                headSha: pr.headSha,
-                blockerCodes: block.blockerCodes,
-                mode: "dry_run",
-              },
-            }).catch(
-              /* v8 ignore next -- fail-safe: an audit write failure never blocks the handler */
-              () => undefined,
-            );
-          }
-        }
       }
       if (
         installationId &&
@@ -7865,9 +7746,212 @@ async function recordPrPanelRetriggerSkip(
   });
 }
 
+/** Draft-dodge guard (#converted-to-draft): a contributor converting an OPEN PR to draft cannot use draft state
+ *  to keep a gate-rejected PR alive. When a prior gate failure exists for the PR's current headSha (and the
+ *  block has not been maintainer-overridden), close the PR immediately — the gate verdict stands and does not
+ *  reset on draft conversion. Per-PR actuation-locked (#2135): a concurrent delivery for the same PR must not
+ *  evaluate + potentially mutate it at the same time. Lock-contended is a silent no-op for this pass — the
+ *  delivery holding the lock is handling this PR. */
+async function maybeCloseDraftDodgeAttempt(
+  env: Env,
+  deliveryId: string,
+  installationId: number,
+  repoFullName: string,
+  pr: PullRequestRecord,
+  settings: RepositorySettings,
+): Promise<void> {
+  if (!(await claimPrActuationLock(env, repoFullName, pr.number))) return;
+  try {
+    await closeDraftDodgeAttemptIfBlocked(
+      env,
+      deliveryId,
+      installationId,
+      repoFullName,
+      pr,
+      settings,
+    );
+  } finally {
+    await releasePrActuationLock(env, repoFullName, pr.number);
+  }
+}
+
+async function closeDraftDodgeAttemptIfBlocked(
+  env: Env,
+  deliveryId: string,
+  installationId: number,
+  repoFullName: string,
+  pr: PullRequestRecord,
+  settings: RepositorySettings,
+): Promise<void> {
+  const block = await getGateBlockOutcome(
+    env,
+    repoFullName,
+    pr.number,
+  ).catch(() => undefined);
+  const repoOwner = repoFullName.includes("/")
+    ? repoFullName.slice(0, repoFullName.indexOf("/")).toLowerCase()
+    : "";
+  const draftDodgeAuthorLogin = (pr.authorLogin ?? "").toLowerCase();
+  const authorIsOwner =
+    draftDodgeAuthorLogin === repoOwner && repoOwner.length > 0;
+  // Fleet-operator identity (#2133): same ADMIN_GITHUB_LOGINS exemption as the primary close-eligibility
+  // computation elsewhere — an admin login must never be auto-closed here either, matching every other
+  // actuation path's trusted-operator definition.
+  const authorIsAdmin =
+    draftDodgeAuthorLogin.length > 0 &&
+    parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(draftDodgeAuthorLogin);
+  if (
+    block &&
+    block.headSha === pr.headSha &&
+    !block.overridden &&
+    !authorIsOwner &&
+    !authorIsAdmin
+  ) {
+    // Respect the agent action mode (#killswitch-gap): the outer guard already excludes a per-repo pause,
+    // but this close path must also honor the global freeze and dry-run — so a freeze is a COMPLETE stop
+    // and a dry-run records the would-be close without touching GitHub.
+    const draftMode = resolveAgentActionMode({
+      globalPaused:
+        isGlobalAgentPause(env) || (await isGlobalAgentFrozen(env)),
+      agentPaused: settings.agentPaused,
+      agentDryRun: settings.agentDryRun,
+    });
+    if (draftMode === "live") {
+      // Write-permission readiness (#2134): this close bypasses executeAgentMaintenanceActions entirely
+      // (the whole point is to enforce the gate verdict against the CURRENT headSha even though the PR
+      // was converted to draft), so it never got the standard pipeline's step-6 PR_WRITE_CLASSES guard.
+      // Without this, a revoked/never-consented pull_requests:write grant would still attempt the close,
+      // get a 403 from GitHub, and have it silently swallowed by the .catch() below — with the audit
+      // event still recorded as "completed" as if the close actually happened. Checked BEFORE the live
+      // freshness re-check below so a permission-denied installation never pays for a live GitHub fetch.
+      // Deliberately UNCAUGHT: getInstallation itself never swallows a genuine D1 read failure (it only
+      // resolves null on a legitimate "row not found" query result), so let a transient storage hiccup
+      // propagate and fail this whole webhook job -- the queue's own retry re-runs it, and a later attempt
+      // with a working DB read correctly evaluates readiness. Catching it into `null` here would instead
+      // permanently misrecord the outcome as "pull_requests: write not granted" (a real GitHub-permission
+      // problem) when the actual cause was an infra blip, misleading an operator investigating the audit
+      // trail and burying the fact that no retry ever happens for a caught, definitively-denied outcome.
+      const draftDodgeInstallation = await getInstallation(
+        env,
+        installationId,
+      );
+      /* v8 ignore next -- upsertInstallation already ran unconditionally earlier in this same handler for
+       * every webhook, so a genuinely-missing row is not reachable through the normal webhook path
+       * exercised by tests; a synced installation always has a permissions object. */
+      const draftDodgeInstallationPermissions = draftDodgeInstallation?.permissions ?? null;
+      const draftDodgePermissionReadiness = resolveAgentPermissionReadiness({
+        autonomy: settings.autonomy,
+        installationPermissions: draftDodgeInstallationPermissions,
+      });
+      if (draftDodgePermissionReadiness !== "ready") {
+        /* v8 ignore next -- a deleted-account PR yields a null author login; the fallback is defensive */
+        const draftDodgeAuthor = pr.authorLogin ?? "unknown";
+        await recordAuditEvent(env, {
+          eventType: "github_app.draft_dodge_closed",
+          actor: "gittensory",
+          targetKey: `${repoFullName}#${pr.number}`,
+          outcome: "denied",
+          detail: `denied draft-dodge close for ${draftDodgeAuthor} — pull_requests: write not granted`,
+          metadata: {
+            deliveryId,
+            repoFullName,
+            headSha: pr.headSha,
+            blockerCodes: block.blockerCodes,
+          },
+        }).catch(
+          /* v8 ignore next -- fail-safe: an audit write failure never blocks the handler */
+          () => undefined,
+        );
+        return;
+      }
+      // 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,
+        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,
+          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";
+      await recordAuditEvent(env, {
+        eventType: "github_app.draft_dodge_closed",
+        actor: "gittensory",
+        targetKey: `${repoFullName}#${pr.number}`,
+        outcome: "completed",
+        detail: `dry-run: would close draft-dodge attempt by ${draftAuthor} — prior gate failure on headSha ${pr.headSha} stands`,
+        metadata: {
+          deliveryId,
+          repoFullName,
+          headSha: pr.headSha,
+          blockerCodes: block.blockerCodes,
+          mode: "dry_run",
+        },
+      }).catch(
+        /* v8 ignore next -- fail-safe: an audit write failure never blocks the handler */
+        () => undefined,
+      );
+    }
+  }
+}
+
 /** Reopen-prevention (#one-shot-reopen): re-close a contributor's reopen of a PR that gittensory / a maintainer
  *  closed (closes are one-shot). Returns true when it re-closed (caller skips the re-review). Exempt: the bot's
- *  own re-review reopens, owner/admin reopens, and a contributor reopening a PR they CLOSED THEMSELVES. */
+ *  own re-review reopens, owner/admin reopens, and a contributor reopening a PR they CLOSED THEMSELVES.
+ *  Per-PR actuation-locked (#2135): a concurrent delivery for the same PR (e.g. a check_suite completion racing
+ *  this reopen) must not evaluate + potentially mutate this PR at the same time. Lock-contended fails open
+ *  (returns false, falls through to normal re-review) — the delivery holding the lock is handling this PR. */
 async function maybeRecloseDisallowedReopen(
   env: Env,
   deliveryId: string,
@@ -7875,6 +7959,29 @@ async function maybeRecloseDisallowedReopen(
   repoFullName: string,
   pr: PullRequestRecord,
   payload: GitHubWebhookPayload,
+): Promise<boolean> {
+  if (!(await claimPrActuationLock(env, repoFullName, pr.number))) return false;
+  try {
+    return await recloseDisallowedReopenIfNeeded(
+      env,
+      deliveryId,
+      installationId,
+      repoFullName,
+      pr,
+      payload,
+    );
+  } finally {
+    await releasePrActuationLock(env, repoFullName, pr.number);
+  }
+}
+
+async function recloseDisallowedReopenIfNeeded(
+  env: Env,
+  deliveryId: string,
+  installationId: number,
+  repoFullName: string,
+  pr: PullRequestRecord,
+  payload: GitHubWebhookPayload,
 ): Promise<boolean> {
   const reopener = (payload.sender?.login ?? "").toLowerCase();
   if (!reopener) return false;
diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts
index 61d59a5ef9..cb24abeaab 100644
--- a/test/unit/queue.test.ts
+++ b/test/unit/queue.test.ts
@@ -11716,6 +11716,39 @@ describe("one-shot reopen prevention", () => {
     expect(webhookRow?.status).toBe("processed");
   });
 
+  it("skips the reopen-reclose when a concurrent delivery already holds the per-PR actuation lock (#2135)", async () => {
+    const calls: Array<{ url: string; method: string }> = [];
+    vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+      const url = input.toString();
+      const method = init?.method ?? "GET";
+      calls.push({ url, method });
+      if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
+      if (url.endsWith("/collaborators/contributor/permission")) return Response.json({ permission: "read" });
+      if (url.endsWith("/collaborators/maintainer/permission")) return Response.json({ permission: "write" });
+      if (url.includes("/issues/42/events")) return Response.json([{ event: "closed", actor: { login: "maintainer" } }]);
+      if (url.endsWith("/issues/42/comments")) return Response.json({ id: 99 }, { status: 201 });
+      if (url.endsWith("/pulls/42") && method === "PATCH") return Response.json({ state: "closed" });
+      return new Response("not found", { status: 404 });
+    });
+
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" });
+    await repositoriesModule.upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", autonomy: { merge: "auto", request_changes: "auto" } });
+    // Simulates a DIFFERENT concurrent delivery for the same PR already in flight (e.g. the draft-dodge sibling
+    // racing this reopen) — the lock key it would hold is pre-claimed here.
+    await env.SELFHOST_TRANSIENT_CACHE?.set("pr-actuation-lock:jsonbored/gittensory#42", "1", 60);
+
+    await processJob(env, {
+      type: "github-webhook",
+      deliveryId: "reopen-lock-contended",
+      eventName: "pull_request",
+      payload: reopenedPayload("contributor"),
+    });
+
+    expect(calls.some((call) => call.method === "PATCH" && call.url.endsWith("/pulls/42"))).toBe(false);
+    const audit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?").bind("github_app.reopen_reclosed").first<{ n: number }>();
+    expect(audit?.n).toBe(0); // no decision recorded either way — the in-flight delivery owns this pass
+  });
+
   it("does NOT re-close a disallowed reopen on an OBSERVE-only / un-opted-in repo (autonomy floor, #review-audit)", async () => {
     const calls: Array<{ url: string; method: string }> = [];
     vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
@@ -12220,6 +12253,31 @@ describe("converted_to_draft gate-close (draft-dodge prevention)", () => {
     expect(audit?.detail).toContain("dry-run: would close");
   });
 
+  it("skips the draft-dodge close when a concurrent delivery already holds the per-PR actuation lock (#2135)", async () => {
+    const calls: string[] = [];
+    vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+      const url = input.toString();
+      calls.push(`${init?.method ?? "GET"} ${url}`);
+      if (url.includes("/access_tokens")) return Response.json({ token: "t" });
+      if (url.endsWith("/issues/42/comments")) return Response.json({ id: 1 }, { status: 201 });
+      if (url.endsWith("/pulls/42")) return Response.json({ state: "closed" });
+      return new Response("not found", { status: 404 });
+    });
+
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" });
+    await setupRepo(env);
+    await recordGateBlockOutcome(env, { repoFullName: "JSONbored/gittensory", pullNumber: 42, headSha: "abc123", blockerCodes: ["missing_linked_issue"] });
+    // Simulates a DIFFERENT concurrent delivery for the same PR already in flight (e.g. a check_suite completion
+    // racing this converted_to_draft event) — the lock key it would hold is pre-claimed here.
+    await env.SELFHOST_TRANSIENT_CACHE?.set("pr-actuation-lock:jsonbored/gittensory#42", "1", 60);
+
+    await processJob(env, { type: "github-webhook", deliveryId: "draft-dodge-lock-contended", eventName: "pull_request", payload: draftPayload("contributor") });
+
+    expect(calls.some((c) => c.includes("PATCH") && c.includes("/pulls/42"))).toBe(false);
+    const audit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?").bind("github_app.draft_dodge_closed").first<{ n: number }>();
+    expect(audit?.n).toBe(0); // no decision recorded either way — the in-flight delivery owns this pass
+  });
+
   it("no-ops when no prior gate failure exists for the PR", async () => {
     const calls: string[] = [];
     vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {

From fffd245fc171a68d6ef9821f773a5b9bcec22b71 Mon Sep 17 00:00:00 2001
From: JSONbored <49853598+JSONbored@users.noreply.github.com>
Date: Wed, 1 Jul 2026 16:59:57 -0700
Subject: [PATCH 2/7] fix(queue): make claimPrActuationLock atomic, add a real
 concurrency test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review findings on this PR:
- claimPrActuationLock was a non-atomic getTransientKey-then-putTransientKey
  pair, so two genuinely concurrent deliveries for the same PR could both
  observe an absent key and both proceed — defeating the exact race this
  mutex exists to close. The sibling claimAgentMaintenanceLock (#2129,
  #2368) already solved this: env.SELFHOST_TRANSIENT_CACHE.claim performs
  the check-and-set as one atomic operation (Redis SET NX server-side),
  with a documented fallback to the old get/set pair for a cache adapter
  that hasn't implemented claim yet. Mirrored that exact pattern here.
- The existing lock tests only pre-seeded the key before the call started,
  proving the contended branch but not the actual race. Added a Promise.all
  test that fires two draft-dodge deliveries for the SAME PR with neither
  pre-claiming anything, asserting exactly one PATCH and one completed
  audit row. Verified this test is meaningful by temporarily reverting to
  the non-atomic implementation and confirming it fails (2 PATCH calls),
  then restoring the fix and confirming it passes.
- Exported claimPrActuationLock/releasePrActuationLock (matching the
  already-exported claimAgentMaintenanceLock/releaseAgentMaintenanceLock)
  and mirrored that sibling's full direct-unit-test suite — fail-open on a
  broken cache, fail-open when claim() itself throws, atomic-claim-used
  verification, and the no-claim-method fallback — closing the branch
  coverage gap the new code left in the fallback/catch paths.

# Conflicts:
#	test/unit/queue.test.ts
---
 src/queue/processors.ts |  27 +++++++++--
 test/unit/queue.test.ts | 101 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 122 insertions(+), 6 deletions(-)

diff --git a/src/queue/processors.ts b/src/queue/processors.ts
index 367afcdfb0..48471f149b 100644
--- a/src/queue/processors.ts
+++ b/src/queue/processors.ts
@@ -2310,24 +2310,41 @@ async function putTransientKey(
 // would read the same stale-but-still-"current" state, both pass their own freshness checks, and both
 // independently fire a mutating call. This is a lightweight interim mutex (a full per-PR Durable Object /
 // SubmissionLock is a separate, more-involved follow-up — see the TODO in env.d.ts) built on the SAME transient
-// cache used for CI-completion coalescing above: a short-TTL claim, best-effort release. A lock-contended caller
-// fails OPEN (returns false / skips this pass) rather than blocking — the delivery holding the lock is
-// evaluating the SAME PR, and the periodic sweep is the backstop if this specific trigger is dropped.
+// cache used for CI-completion coalescing above, claimed ATOMICALLY (see claimPrActuationLock) so two racing
+// deliveries can never both win the claim — a short TTL, best-effort release. A lock-contended caller fails
+// OPEN (returns false / skips this pass) rather than blocking — the delivery holding the lock is evaluating
+// the SAME PR, and the periodic sweep is the backstop if this specific trigger is dropped.
 const PR_ACTUATION_LOCK_TTL_SECONDS = 60;
 function prActuationLockKey(repoFullName: string, prNumber: number): string {
   return `pr-actuation-lock:${repoFullName.toLowerCase()}#${prNumber}`;
 }
-async function claimPrActuationLock(
+export async function claimPrActuationLock(
   env: Env,
   repoFullName: string,
   prNumber: number,
 ): Promise<boolean> {
   const key = prActuationLockKey(repoFullName, prNumber);
+  // Atomic claim (#2129, mirroring claimAgentMaintenanceLock): a get-then-set pair has a window between the
+  // read and the write where two concurrent deliveries for the SAME PR can both observe an absent key and both
+  // claim it, defeating this mutex entirely. env.SELFHOST_TRANSIENT_CACHE.claim performs the check-and-set as
+  // one operation (Redis SET NX server-side), closing that window. Falls back to the non-atomic get/set pair
+  // only for a cache adapter that hasn't implemented claim yet — strictly no worse than this function's prior
+  // behavior.
+  if (env.SELFHOST_TRANSIENT_CACHE?.claim) {
+    try {
+      return await env.SELFHOST_TRANSIENT_CACHE.claim(key, "1", PR_ACTUATION_LOCK_TTL_SECONDS);
+    } catch {
+      return true; // fail open — see the doc comment above.
+    }
+  }
+  // getTransientKey/putTransientKey already fail open internally (a missing cache or a thrown read/write error
+  // both resolve rather than throw), so this never needs its own try/catch — a cache fault surfaces here as
+  // "no lock held", which correctly falls through to claiming it.
   if (await getTransientKey(env, key)) return false;
   await putTransientKey(env, key, "1", PR_ACTUATION_LOCK_TTL_SECONDS);
   return true;
 }
-async function releasePrActuationLock(
+export async function releasePrActuationLock(
   env: Env,
   repoFullName: string,
   prNumber: number,
diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts
index cb24abeaab..44bbb4655e 100644
--- a/test/unit/queue.test.ts
+++ b/test/unit/queue.test.ts
@@ -44,7 +44,7 @@ import {
   upsertRepositoryFromGitHub,
   putCachedAiReview,
 } from "../../src/db/repositories";
-import { agentMaintenanceHeadMatchesGate, changedPathsForGuardrail, claimAgentMaintenanceLock, claimAiReviewLock, contributorEvidenceBatchSize, processJob, releaseAgentMaintenanceLock, releaseAiReviewLock } from "../../src/queue/processors";
+import { agentMaintenanceHeadMatchesGate, changedPathsForGuardrail, claimAgentMaintenanceLock, claimAiReviewLock, claimPrActuationLock, contributorEvidenceBatchSize, processJob, releaseAgentMaintenanceLock, releaseAiReviewLock, releasePrActuationLock } from "../../src/queue/processors";
 import { aiReviewCacheInputFingerprint } from "../../src/review/ai-review-cache-input";
 import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader";
 import { normalizeRegistryPayload } from "../../src/registry/normalize";
@@ -3676,6 +3676,74 @@ describe("queue processors", () => {
     expect([first, second]).toEqual([true, true]);
   });
 
+  // claimPrActuationLock (#2135) mirrors claimAgentMaintenanceLock's atomic-claim design exactly — same test
+  // shapes, same reasoning, a different lock namespace.
+  it("claimPrActuationLock claims when free, denies when held (per-PR), and release frees it again (#2135)", async () => {
+    const env = createTestEnv({});
+    expect(await claimPrActuationLock(env, "owner/act-repo", 7)).toBe(true);
+    expect(await claimPrActuationLock(env, "owner/act-repo", 7)).toBe(false);
+    expect(await claimPrActuationLock(env, "owner/act-repo", 8)).toBe(true);
+    await releasePrActuationLock(env, "owner/act-repo", 7);
+    expect(await claimPrActuationLock(env, "owner/act-repo", 7)).toBe(true);
+  });
+
+  it("claimPrActuationLock fails OPEN on a broken transient cache — never itself blocks actuation (#2135)", async () => {
+    const env = createTestEnv({
+      SELFHOST_TRANSIENT_CACHE: {
+        get: async () => { throw new Error("cache read error"); },
+        set: async () => { throw new Error("cache write error"); },
+        del: async () => { throw new Error("cache delete error"); },
+      },
+    });
+    expect(await claimPrActuationLock(env, "owner/act-repo", 7)).toBe(true);
+    await expect(releasePrActuationLock(env, "owner/act-repo", 7)).resolves.toBeUndefined();
+  });
+
+  it("claimPrActuationLock fails OPEN when the atomic claim primitive itself throws (#2135)", async () => {
+    const env = createTestEnv({
+      SELFHOST_TRANSIENT_CACHE: {
+        get: async () => null,
+        set: async () => undefined,
+        claim: async () => { throw new Error("redis unavailable"); },
+      },
+    });
+    expect(await claimPrActuationLock(env, "owner/act-repo", 7)).toBe(true);
+  });
+
+  it("REGRESSION (#2135): claimPrActuationLock uses an atomic check-and-set, so two genuinely concurrent claims for the SAME PR can never both succeed", async () => {
+    const env = createTestEnv({});
+    const [first, second] = await Promise.all([
+      claimPrActuationLock(env, "owner/act-repo", 7),
+      claimPrActuationLock(env, "owner/act-repo", 7),
+    ]);
+    expect([first, second].filter(Boolean)).toHaveLength(1);
+  });
+
+  it("REGRESSION (#2135): claimPrActuationLock calls the atomic claim primitive, not a separate get+set pair, when the cache supports it", async () => {
+    const calls: string[] = [];
+    const env = createTestEnv({
+      SELFHOST_TRANSIENT_CACHE: {
+        get: async () => { calls.push("get"); return null; },
+        set: async () => { calls.push("set"); },
+        claim: async () => { calls.push("claim"); return true; },
+      },
+    });
+    expect(await claimPrActuationLock(env, "owner/act-repo", 7)).toBe(true);
+    expect(calls).toEqual(["claim"]); // never falls through to the racy get/set pair when claim is available
+  });
+
+  it("claimPrActuationLock falls back to the get/set pair and still denies a held key when the cache has no claim() (#2135)", async () => {
+    const values = new Map<string, string>();
+    const env = createTestEnv({
+      SELFHOST_TRANSIENT_CACHE: {
+        get: async (key: string) => values.get(key) ?? null,
+        set: async (key: string, value: string) => { values.set(key, value); },
+      },
+    });
+    expect(await claimPrActuationLock(env, "owner/act-repo", 7)).toBe(true);
+    expect(await claimPrActuationLock(env, "owner/act-repo", 7)).toBe(false);
+  });
+
   it("INVARIANT (#2129 per-PR lock): a maintenance pass defers when another pass already holds the PR's lock", async () => {
     const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
     await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } });
@@ -12278,6 +12346,37 @@ describe("converted_to_draft gate-close (draft-dodge prevention)", () => {
     expect(audit?.n).toBe(0); // no decision recorded either way — the in-flight delivery owns this pass
   });
 
+  it("REGRESSION: exactly ONE of two genuinely concurrent draft-dodge deliveries for the SAME PR wins the actuation lock (#2135)", async () => {
+    // Unlike the lock-contended test above (which pre-seeds the key before the call even starts), this fires
+    // two deliveries together via Promise.all with NEITHER pre-claiming anything — exercising the actual
+    // check-and-set race claimPrActuationLock must arbitrate, not just "the key was already there". A
+    // get-then-set (non-atomic) implementation lets both deliveries observe an absent key and both proceed,
+    // which this test would catch as more than one PATCH / more than one completed audit row.
+    const calls: string[] = [];
+    vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+      const url = input.toString();
+      calls.push(`${init?.method ?? "GET"} ${url}`);
+      if (url.includes("/access_tokens")) return Response.json({ token: "t" });
+      if (url.endsWith("/issues/42/comments")) return Response.json({ id: 1 }, { status: 201 });
+      if (url.endsWith("/pulls/42")) return Response.json({ state: "closed" });
+      return new Response("not found", { status: 404 });
+    });
+
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" });
+    await setupRepo(env);
+    await recordGateBlockOutcome(env, { repoFullName: "JSONbored/gittensory", pullNumber: 42, headSha: "abc123", blockerCodes: ["missing_linked_issue"] });
+
+    await Promise.all([
+      processJob(env, { type: "github-webhook", deliveryId: "draft-dodge-race-a", eventName: "pull_request", payload: draftPayload("contributor") }),
+      processJob(env, { type: "github-webhook", deliveryId: "draft-dodge-race-b", eventName: "pull_request", payload: draftPayload("contributor") }),
+    ]);
+
+    const patchCalls = calls.filter((c) => c.includes("PATCH") && c.includes("/pulls/42"));
+    expect(patchCalls).toHaveLength(1); // exactly one delivery won the race and closed the PR
+    const audit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and outcome = 'completed'").bind("github_app.draft_dodge_closed").first<{ n: number }>();
+    expect(audit?.n).toBe(1); // exactly one completed close recorded — not two (the race), not zero
+  });
+
   it("no-ops when no prior gate failure exists for the PR", async () => {
     const calls: string[] = [];
     vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {

From 1f0476ae1fc07dd4304b9b48ca6ea2b32456e106 Mon Sep 17 00:00:00 2001
From: JSONbored <49853598+JSONbored@users.noreply.github.com>
Date: Wed, 1 Jul 2026 16:49:01 -0700
Subject: [PATCH 3/7] fix(queue): raise the actuation lock TTL to make the
 ownership gap unreachable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review finding: claimPrActuationLock stores a constant lock value with no
per-holder ownership token, so a holder running past the TTL could have
its lock claimed by a new holder, then have that new holder's live lock
deleted by the first holder's stale finally-block release — reopening
the concurrent-mutation race this mutex exists to close.

The proper fix (a per-holder token + atomic compare-and-delete) needs a
new cache-adapter primitive and should apply to the sibling
claimAgentMaintenanceLock too for consistency — tracked alongside the
existing Durable Object follow-up rather than done here. As an interim
mitigation, raised the TTL from 60s to 600s: the guarded operations are a
handful of sequential GitHub API calls that should never legitimately run
anywhere near that long, so the window this finding describes is now
practically unreachable rather than architecturally closed.
---
 src/queue/processors.ts | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/queue/processors.ts b/src/queue/processors.ts
index 48471f149b..81a8ae8d6e 100644
--- a/src/queue/processors.ts
+++ b/src/queue/processors.ts
@@ -2314,7 +2314,15 @@ async function putTransientKey(
 // deliveries can never both win the claim — a short TTL, best-effort release. A lock-contended caller fails
 // OPEN (returns false / skips this pass) rather than blocking — the delivery holding the lock is evaluating
 // the SAME PR, and the periodic sweep is the backstop if this specific trigger is dropped.
-const PR_ACTUATION_LOCK_TTL_SECONDS = 60;
+//
+// KNOWN LIMITATION: the lock value is a constant, not a per-holder ownership token, so release does not verify
+// it still owns the key — if a holder ran past the TTL, a later claimer's live lock could be deleted by the
+// first holder's stale `finally` release, reopening the exact race this mutex exists to close. A per-holder
+// token + a conditional (check-then-delete) release would close this properly, but needs a new atomic
+// compare-and-delete primitive on the cache adapter — tracked alongside the Durable Object follow-up above. The
+// TTL is set generously long specifically so this window is practically unreachable: the guarded operations
+// (a handful of sequential GitHub API calls) should never legitimately run anywhere near this long.
+const PR_ACTUATION_LOCK_TTL_SECONDS = 600;
 function prActuationLockKey(repoFullName: string, prNumber: number): string {
   return `pr-actuation-lock:${repoFullName.toLowerCase()}#${prNumber}`;
 }

From ad9386148a1551438d2a84644fefdd1d5ab6ae4d Mon Sep 17 00:00:00 2001
From: JSONbored <49853598+JSONbored@users.noreply.github.com>
Date: Wed, 1 Jul 2026 17:06:17 -0700
Subject: [PATCH 4/7] fix(queue): let a transient getInstallation read
 propagate through the draft-dodge mutex wrapper

The actuation-lock wrapper's call site was swallowing every error from
maybeCloseDraftDodgeAttempt, including the write-permission-readiness
getInstallation read that is deliberately left uncaught so a transient
D1 failure retries instead of misrecording a permission denial.
---
 src/queue/processors.ts | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/queue/processors.ts b/src/queue/processors.ts
index 81a8ae8d6e..63a93aa22e 100644
--- a/src/queue/processors.ts
+++ b/src/queue/processors.ts
@@ -3865,6 +3865,10 @@ async function processGitHubWebhook(
         !settings.agentPaused &&
         !isProtectedAutomationAuthor(pr.authorLogin)
       ) {
+        // Deliberately UNCAUGHT here: closeDraftDodgeAttemptIfBlocked catches every operation that should
+        // fail safely, but leaves the write-permission-readiness getInstallation read (#2134) uncaught on
+        // purpose so a transient D1 failure propagates and the queue retries instead of misrecording a
+        // permission denial.
         await maybeCloseDraftDodgeAttempt(
           env,
           deliveryId,
@@ -3872,7 +3876,7 @@ async function processGitHubWebhook(
           repoFullName,
           pr,
           settings,
-        ).catch(() => undefined);
+        );
       }
       if (
         installationId &&

From adf1f820accae0f169c548446b3490a98f08f5d6 Mon Sep 17 00:00:00 2001
From: JSONbored <49853598+JSONbored@users.noreply.github.com>
Date: Wed, 1 Jul 2026 17:22:47 -0700
Subject: [PATCH 5/7] fix(queue): remove claimPrActuationLock's non-atomic
 get/set fallback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A cache adapter without claim() previously fell back to a get-then-set
pair, which is not a real exclusivity guarantee — two concurrent
callers can both observe an absent key before either writes. Reuse
claimTransientLock's fail-open behavior instead, matching the fix
already applied to the sibling claimAgentMaintenanceLock.
---
 src/queue/processors.ts | 31 +++++++++----------------------
 test/unit/queue.test.ts | 23 +++++++++++++++++++++--
 2 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/src/queue/processors.ts b/src/queue/processors.ts
index 63a93aa22e..f472abfcda 100644
--- a/src/queue/processors.ts
+++ b/src/queue/processors.ts
@@ -2310,10 +2310,12 @@ async function putTransientKey(
 // would read the same stale-but-still-"current" state, both pass their own freshness checks, and both
 // independently fire a mutating call. This is a lightweight interim mutex (a full per-PR Durable Object /
 // SubmissionLock is a separate, more-involved follow-up — see the TODO in env.d.ts) built on the SAME transient
-// cache used for CI-completion coalescing above, claimed ATOMICALLY (see claimPrActuationLock) so two racing
+// cache used for CI-completion coalescing above, claimed ATOMICALLY (see claimTransientLock) so two racing
 // deliveries can never both win the claim — a short TTL, best-effort release. A lock-contended caller fails
 // OPEN (returns false / skips this pass) rather than blocking — the delivery holding the lock is evaluating
-// the SAME PR, and the periodic sweep is the backstop if this specific trigger is dropped.
+// the SAME PR, and the periodic sweep is the backstop if this specific trigger is dropped. A cache adapter with
+// no claim() primitive gets NO exclusivity at all (every call proceeds) rather than a get-then-set pair that
+// only *looks* atomic — see claimTransientLock's doc comment for why that fallback was removed.
 //
 // KNOWN LIMITATION: the lock value is a constant, not a per-holder ownership token, so release does not verify
 // it still owns the key — if a holder ran past the TTL, a later claimer's live lock could be deleted by the
@@ -2331,26 +2333,11 @@ export async function claimPrActuationLock(
   repoFullName: string,
   prNumber: number,
 ): Promise<boolean> {
-  const key = prActuationLockKey(repoFullName, prNumber);
-  // Atomic claim (#2129, mirroring claimAgentMaintenanceLock): a get-then-set pair has a window between the
-  // read and the write where two concurrent deliveries for the SAME PR can both observe an absent key and both
-  // claim it, defeating this mutex entirely. env.SELFHOST_TRANSIENT_CACHE.claim performs the check-and-set as
-  // one operation (Redis SET NX server-side), closing that window. Falls back to the non-atomic get/set pair
-  // only for a cache adapter that hasn't implemented claim yet — strictly no worse than this function's prior
-  // behavior.
-  if (env.SELFHOST_TRANSIENT_CACHE?.claim) {
-    try {
-      return await env.SELFHOST_TRANSIENT_CACHE.claim(key, "1", PR_ACTUATION_LOCK_TTL_SECONDS);
-    } catch {
-      return true; // fail open — see the doc comment above.
-    }
-  }
-  // getTransientKey/putTransientKey already fail open internally (a missing cache or a thrown read/write error
-  // both resolve rather than throw), so this never needs its own try/catch — a cache fault surfaces here as
-  // "no lock held", which correctly falls through to claiming it.
-  if (await getTransientKey(env, key)) return false;
-  await putTransientKey(env, key, "1", PR_ACTUATION_LOCK_TTL_SECONDS);
-  return true;
+  return claimTransientLock(
+    env,
+    prActuationLockKey(repoFullName, prNumber),
+    PR_ACTUATION_LOCK_TTL_SECONDS,
+  );
 }
 export async function releasePrActuationLock(
   env: Env,
diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts
index 44bbb4655e..634b81c0e7 100644
--- a/test/unit/queue.test.ts
+++ b/test/unit/queue.test.ts
@@ -3732,7 +3732,10 @@ describe("queue processors", () => {
     expect(calls).toEqual(["claim"]); // never falls through to the racy get/set pair when claim is available
   });
 
-  it("claimPrActuationLock falls back to the get/set pair and still denies a held key when the cache has no claim() (#2135)", async () => {
+  it("claimPrActuationLock returns true unconditionally when the cache has no claim() — no false exclusivity guarantee (#2135, review round 2)", async () => {
+    // Mirrors claimAgentMaintenanceLock's #confirmed-bug fix: a get-then-set pair (even with a re-read) is not
+    // a real exclusivity guarantee under concurrent load, so a cache without claim() now gets NO exclusivity at
+    // all rather than a fallback that only looks atomic.
     const values = new Map<string, string>();
     const env = createTestEnv({
       SELFHOST_TRANSIENT_CACHE: {
@@ -3741,7 +3744,23 @@ describe("queue processors", () => {
       },
     });
     expect(await claimPrActuationLock(env, "owner/act-repo", 7)).toBe(true);
-    expect(await claimPrActuationLock(env, "owner/act-repo", 7)).toBe(false);
+    expect(await claimPrActuationLock(env, "owner/act-repo", 7)).toBe(true);
+  });
+
+  it("REGRESSION (#2135, review round 2): claimPrActuationLock does not falsely claim exclusivity for two genuinely concurrent callers when the cache has no claim()", async () => {
+    const values = new Map<string, string>();
+    const yieldThenRun = <T,>(fn: () => T): Promise<T> => new Promise((resolve) => queueMicrotask(() => resolve(fn())));
+    const env = createTestEnv({
+      SELFHOST_TRANSIENT_CACHE: {
+        get: (key: string) => yieldThenRun(() => values.get(key) ?? null),
+        set: (key: string, value: string) => yieldThenRun(() => { values.set(key, value); }),
+      },
+    });
+    const [first, second] = await Promise.all([
+      claimPrActuationLock(env, "owner/act-repo", 7),
+      claimPrActuationLock(env, "owner/act-repo", 7),
+    ]);
+    expect([first, second]).toEqual([true, true]);
   });
 
   it("INVARIANT (#2129 per-PR lock): a maintenance pass defers when another pass already holds the PR's lock", async () => {

From 2469124f7003bf6d8ef58f6281ebb01ac0696fac Mon Sep 17 00:00:00 2001
From: JSONbored <49853598+JSONbored@users.noreply.github.com>
Date: Wed, 1 Jul 2026 17:33:25 -0700
Subject: [PATCH 6/7] fix(queue): stop the reopen-reclose webhook pass on
 actuation-lock contention
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

maybeRecloseDisallowedReopen returned a plain false on lock contention,
which the caller's boolean contract read as 'not blocked, proceed to
normal re-review' — a contended webhook could still evaluate/mutate
the same PR the lock holder owns. Replace the boolean with a tri-state
ReopenRecloseOutcome (reclosed / allowed / lock_contended) so the
caller skips the re-review on contention too.
---
 src/queue/processors.ts | 49 ++++++++++++++++++++++++-----------------
 test/unit/queue.test.ts |  6 +++++
 2 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/src/queue/processors.ts b/src/queue/processors.ts
index f472abfcda..a9bcd97a05 100644
--- a/src/queue/processors.ts
+++ b/src/queue/processors.ts
@@ -3782,19 +3782,21 @@ async function processGitHubWebhook(
       // Reopen-prevention (#one-shot-reopen): a CONTRIBUTOR may not reopen a PR that gittensory or a maintainer
       // closed — closes are one-shot (resubmit, don't reopen). If a non-maintainer reopened a PR whose last close
       // was by the bot / repo owner / admin, re-close it and skip the re-review. Self-closes (the contributor
-      // closed their own PR) stay reopenable; the bot's own nightly-re-review reopens are exempt.
-      if (
-        payload.action === "reopened" &&
-        installationId &&
-        (await maybeRecloseDisallowedReopen(
-          env,
-          deliveryId,
-          installationId,
-          repoFullName,
-          pr,
-          payload,
-        ).catch(() => false))
-      ) {
+      // closed their own PR) stay reopenable; the bot's own nightly-re-review reopens are exempt. A contended
+      // actuation lock ALSO skips the re-review (#2135, review round 3) — the winning delivery already owns
+      // this PR, so this pass must not evaluate/mutate it concurrently under a false "not blocked" reading.
+      const reopenOutcome: ReopenRecloseOutcome =
+        payload.action === "reopened" && installationId
+          ? await maybeRecloseDisallowedReopen(
+              env,
+              deliveryId,
+              installationId,
+              repoFullName,
+              pr,
+              payload,
+            ).catch(() => "allowed" as const)
+          : "allowed";
+      if (reopenOutcome === "reclosed" || reopenOutcome === "lock_contended") {
         // Stamp the delivery processed like every other owning path — the early return otherwise leaves the
         // webhook_events row stuck at "queued"/its body hash, mis-reporting the delivery as un-acked (#review-audit).
         await recordWebhookEvent(env, {
@@ -7962,12 +7964,18 @@ async function closeDraftDodgeAttemptIfBlocked(
   }
 }
 
+/** Outcome of {@link maybeRecloseDisallowedReopen}: "reclosed" and "lock_contended" both mean the caller must
+ *  skip the normal re-review pass — a plain boolean can't distinguish "evaluated, not blocked" from "never
+ *  evaluated, another delivery owns this PR", and conflating them let a contended pass fall through to a
+ *  concurrent re-review (#2135, review round 3). */
+type ReopenRecloseOutcome = "reclosed" | "allowed" | "lock_contended";
+
 /** Reopen-prevention (#one-shot-reopen): re-close a contributor's reopen of a PR that gittensory / a maintainer
- *  closed (closes are one-shot). Returns true when it re-closed (caller skips the re-review). Exempt: the bot's
- *  own re-review reopens, owner/admin reopens, and a contributor reopening a PR they CLOSED THEMSELVES.
+ *  closed (closes are one-shot). Returns "reclosed" when it re-closed (caller skips the re-review). Exempt: the
+ *  bot's own re-review reopens, owner/admin reopens, and a contributor reopening a PR they CLOSED THEMSELVES.
  *  Per-PR actuation-locked (#2135): a concurrent delivery for the same PR (e.g. a check_suite completion racing
- *  this reopen) must not evaluate + potentially mutate this PR at the same time. Lock-contended fails open
- *  (returns false, falls through to normal re-review) — the delivery holding the lock is handling this PR. */
+ *  this reopen) must not evaluate + potentially mutate this PR at the same time. Lock-contended returns
+ *  "lock_contended" — the caller skips its own re-review too, since the delivery holding the lock owns this PR. */
 async function maybeRecloseDisallowedReopen(
   env: Env,
   deliveryId: string,
@@ -7975,10 +7983,10 @@ async function maybeRecloseDisallowedReopen(
   repoFullName: string,
   pr: PullRequestRecord,
   payload: GitHubWebhookPayload,
-): Promise<boolean> {
-  if (!(await claimPrActuationLock(env, repoFullName, pr.number))) return false;
+): Promise<ReopenRecloseOutcome> {
+  if (!(await claimPrActuationLock(env, repoFullName, pr.number))) return "lock_contended";
   try {
-    return await recloseDisallowedReopenIfNeeded(
+    const reclosed = await recloseDisallowedReopenIfNeeded(
       env,
       deliveryId,
       installationId,
@@ -7986,6 +7994,7 @@ async function maybeRecloseDisallowedReopen(
       pr,
       payload,
     );
+    return reclosed ? "reclosed" : "allowed";
   } finally {
     await releasePrActuationLock(env, repoFullName, pr.number);
   }
diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts
index 634b81c0e7..46dc72421f 100644
--- a/test/unit/queue.test.ts
+++ b/test/unit/queue.test.ts
@@ -4,6 +4,7 @@ import { clearInstallationTokenCacheForTest } from "../../src/github/app";
 import { PR_PANEL_COMMENT_MARKER } from "../../src/github/comments";
 import * as backfillModule from "../../src/github/backfill";
 import * as repositoriesModule from "../../src/db/repositories";
+import * as repositorySettingsModule from "../../src/settings/repository-settings";
 import * as sentryModule from "../../src/selfhost/sentry";
 import {
   listCollisionEdges,
@@ -11823,6 +11824,10 @@ describe("one-shot reopen prevention", () => {
     // Simulates a DIFFERENT concurrent delivery for the same PR already in flight (e.g. the draft-dodge sibling
     // racing this reopen) — the lock key it would hold is pre-claimed here.
     await env.SELFHOST_TRANSIENT_CACHE?.set("pr-actuation-lock:jsonbored/gittensory#42", "1", 60);
+    // REGRESSION (#2135, review round 3): a contended lock previously returned `false`, which the caller's old
+    // boolean contract read as "not blocked, proceed to normal re-review" -- this spy proves that no longer
+    // happens; the webhook path must stop BEFORE resolveRepositorySettings, the first call the re-review makes.
+    const resolveSettingsSpy = vi.spyOn(repositorySettingsModule, "resolveRepositorySettings");
 
     await processJob(env, {
       type: "github-webhook",
@@ -11834,6 +11839,7 @@ describe("one-shot reopen prevention", () => {
     expect(calls.some((call) => call.method === "PATCH" && call.url.endsWith("/pulls/42"))).toBe(false);
     const audit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?").bind("github_app.reopen_reclosed").first<{ n: number }>();
     expect(audit?.n).toBe(0); // no decision recorded either way — the in-flight delivery owns this pass
+    expect(resolveSettingsSpy).not.toHaveBeenCalled(); // the normal re-review pass never started
   });
 
   it("does NOT re-close a disallowed reopen on an OBSERVE-only / un-opted-in repo (autonomy floor, #review-audit)", async () => {

From 1e70418dd63afd0cb0dc59747f8718657a7b81ac Mon Sep 17 00:00:00 2001
From: JSONbored <49853598+JSONbored@users.noreply.github.com>
Date: Wed, 1 Jul 2026 17:58:08 -0700
Subject: [PATCH 7/7] fix(queue): remove the unreachable catch masking a
 disallowed reopen
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The outer .catch() around maybeRecloseDisallowedReopen could never
actually fire — the lock claim/release fail open and every step in
recloseDisallowedReopenIfNeeded already catches its own errors — so
codecov/patch flagged it as an uncoverable line. Swallowing an
unexpected error there into a silent 'allowed' would have re-permitted
the exact disallowed reopen this guard exists to stop, so removing it
is also the safer behavior: let it propagate and retry.
---
 src/queue/processors.ts | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/queue/processors.ts b/src/queue/processors.ts
index a9bcd97a05..7f03845843 100644
--- a/src/queue/processors.ts
+++ b/src/queue/processors.ts
@@ -3785,6 +3785,11 @@ async function processGitHubWebhook(
       // closed their own PR) stay reopenable; the bot's own nightly-re-review reopens are exempt. A contended
       // actuation lock ALSO skips the re-review (#2135, review round 3) — the winning delivery already owns
       // this PR, so this pass must not evaluate/mutate it concurrently under a false "not blocked" reading.
+      // Deliberately UNCAUGHT here: every step inside maybeRecloseDisallowedReopen already fails safe on its own
+      // (the lock claim/release fail open; recloseDisallowedReopenIfNeeded's own operations all .catch()), so a
+      // swallowing catch at this call site could only ever mask a genuinely unexpected error into a silent
+      // "allowed" — which would re-permit exactly the disallowed reopen this guard exists to stop. Let it
+      // propagate and retry instead, same reasoning as the draft-dodge sibling's uncaught getInstallation read.
       const reopenOutcome: ReopenRecloseOutcome =
         payload.action === "reopened" && installationId
           ? await maybeRecloseDisallowedReopen(
@@ -3794,7 +3799,7 @@ async function processGitHubWebhook(
               repoFullName,
               pr,
               payload,
-            ).catch(() => "allowed" as const)
+            )
           : "allowed";
       if (reopenOutcome === "reclosed" || reopenOutcome === "lock_contended") {
         // Stamp the delivery processed like every other owning path — the early return otherwise leaves the