Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2361,9 +2361,17 @@ export async function releasePrActuationLock(
}
}

class PrActuationLockContendedError extends Error {
// A plain thrown Error still reaches the queue's retry path (this call site is deliberately uncaught, same as
// maybeRecloseDisallowedReopen's other error paths), but it only gets the queue's generic default backoff — far
// slower than the near-instant window a per-PR actuation lock is actually held for. Extending RetryableJobError
// gives lock contention its own fast, deterministic retry plus a distinct retryKind for observability, without
// changing the uncaught-and-propagate shape either call site already relies on (#2135/#2447).
class PrActuationLockContendedError extends RetryableJobError {
constructor(repoFullName: string, prNumber: number, policy: string) {
super(`pr actuation lock contended for ${repoFullName}#${prNumber} during ${policy}`);
super(`pr actuation lock contended for ${repoFullName}#${prNumber} during ${policy}`, {
retryAfterMs: 5_000,
retryKind: "pr_actuation_lock_contended",
});
this.name = "PrActuationLockContendedError";
}
}
Expand Down
25 changes: 15 additions & 10 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11889,22 +11889,27 @@ 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.
// A contended lock must still stop before resolveRepositorySettings, the first call the normal re-review makes,
// but must NOT stamp this reopen delivery processed: the lock holder may be an unrelated same-PR guard that
// no-ops, so the queue needs to retry this reopen guard once the lock clears.
const resolveSettingsSpy = vi.spyOn(repositorySettingsModule, "resolveRepositorySettings");

await expect(processJob(env, {
type: "github-webhook",
deliveryId: "reopen-lock-contended",
eventName: "pull_request",
payload: reopenedPayload("contributor"),
})).rejects.toThrow("pr actuation lock contended");
await expect(
processJob(env, {
type: "github-webhook",
deliveryId: "reopen-lock-contended",
eventName: "pull_request",
payload: reopenedPayload("contributor"),
}),
).rejects.toMatchObject({ retryKind: "pr_actuation_lock_contended" });

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 queue retry owns the deferred decision
expect(audit?.n).toBe(0); // no decision recorded either way — retry owns the eventual reopen decision
expect(resolveSettingsSpy).not.toHaveBeenCalled(); // the normal re-review pass never started
const webhookRow = await env.DB.prepare("select status, error_summary from webhook_events where delivery_id = ?").bind("reopen-lock-contended").first<{ status: string; error_summary: string }>();
expect(webhookRow?.status).toBe("error");
expect(webhookRow?.error_summary).toContain("pr actuation lock contended");
});

it("does NOT re-close a disallowed reopen on an OBSERVE-only / un-opted-in repo (autonomy floor, #review-audit)", async () => {
Expand Down
Loading