Parent: #1936
Problem
enqueueWebhookByEnv's dedup (src/github/webhook.ts:159) only suppresses a redelivery of the same X-GitHub-Delivery ID with a matching payload hash. It does not prevent two different legitimate deliveries for the same PR (e.g. a reopened event and a concurrent check_suite completed event) from being processed concurrently by separate queue workers — nothing in the queue-claim layer acquires a per-repoFullName#pullNumber lock before evaluating gate state and firing a mutation. This is a broader, more general version of the concurrency gap already filed for the webhook-vs-sweep case (src/selfhost/queue-common.ts) — that issue covers coalesce-key-shape mismatches specifically; this one covers the absence of any per-PR mutex at all, including within identically-shaped event types.
Failure scenario: two webhook deliveries for the same PR are dequeued by different workers at nearly the same time. Both read the same stale-but-still-"current" head SHA, both pass their respective freshness checks (neither has yet mutated the PR), and both independently call closePullRequest/mergePullRequest. GitHub's PATCH/PUT calls are individually idempotent-ish (a second close on an already-closed PR is a no-op 200, a second merge attempt 405s), so the practical damage is usually duplicate audit events and duplicate close-comment spam rather than a corrupted final state — but it remains a genuine TOCTOU window with no explicit mutex.
Requirements
- Two webhook-triggered jobs for the same PR must not both reach live-read-and-actuate concurrently, regardless of event type.
Deliverables
- Ship the already-planned fix noted in the codebase's own TODO: a per-PR
SubmissionLock Durable Object (or equivalent D1/SQLite row-level advisory lock keyed by repoFullName#pullNumber) acquired before evaluating gate state and released after the mutation (or its no-op decision) completes, with a short lease/timeout so a crashed worker can't deadlock it.
- As a lower-effort interim step (can ship ahead of the full lock and coordinate with the sibling sweep-vs-webhook issue): extend the queue's coalesce lookup to also match
status = 'processing' rows, not just pending, so a second arrival for an in-flight PR defers rather than claims a parallel slot.
- Add a regression test simulating two concurrent deliveries for the same PR and asserting only one reaches the mutation.
Acceptance criteria
- Two concurrently-arriving webhook events for the same PR are serialized before either reaches a mutating GitHub call.
- No regression to overall queue throughput for the common case of unrelated PRs processing in parallel.
Expected outcome
Actuation for any single PR is always serialized, eliminating duplicate-mutation races and their audit-log/comment-spam side effects, and closing the last piece of the concurrency TODO already on record.
Parent: #1936
Problem
enqueueWebhookByEnv's dedup (src/github/webhook.ts:159) only suppresses a redelivery of the sameX-GitHub-DeliveryID with a matching payload hash. It does not prevent two different legitimate deliveries for the same PR (e.g. areopenedevent and a concurrentcheck_suite completedevent) from being processed concurrently by separate queue workers — nothing in the queue-claim layer acquires a per-repoFullName#pullNumberlock before evaluating gate state and firing a mutation. This is a broader, more general version of the concurrency gap already filed for the webhook-vs-sweep case (src/selfhost/queue-common.ts) — that issue covers coalesce-key-shape mismatches specifically; this one covers the absence of any per-PR mutex at all, including within identically-shaped event types.Failure scenario: two webhook deliveries for the same PR are dequeued by different workers at nearly the same time. Both read the same stale-but-still-"current" head SHA, both pass their respective freshness checks (neither has yet mutated the PR), and both independently call
closePullRequest/mergePullRequest. GitHub's PATCH/PUT calls are individually idempotent-ish (a second close on an already-closed PR is a no-op 200, a second merge attempt 405s), so the practical damage is usually duplicate audit events and duplicate close-comment spam rather than a corrupted final state — but it remains a genuine TOCTOU window with no explicit mutex.Requirements
Deliverables
SubmissionLockDurable Object (or equivalent D1/SQLite row-level advisory lock keyed byrepoFullName#pullNumber) acquired before evaluating gate state and released after the mutation (or its no-op decision) completes, with a short lease/timeout so a crashed worker can't deadlock it.status = 'processing'rows, not justpending, so a second arrival for an in-flight PR defers rather than claims a parallel slot.Acceptance criteria
Expected outcome
Actuation for any single PR is always serialized, eliminating duplicate-mutation races and their audit-log/comment-spam side effects, and closing the last piece of the concurrency TODO already on record.