fix(scm): Make PR lifecycle writes monotonic across reordered webhooks

[vaind]

SCM webhooks arrive out of order, and every pull-request / merge-request payload is a full snapshot of the PR — so an older delivery silently overwrites a newer one. Both the GitHub and GitLab handlers wrote theirs through an unguarded update_or_create.

T+0:00  synchronize     → transient cell 500, rescheduled ~4 min out
T+1:00  closed (merged) → delivered: state=merged, merged_at, closed_at set
T+4:00  synchronize     → retry lands: state=open, merged_at=None

The PR is then shown as open in Sentry while it is merged upstream, permanently. The same write also rolls back closed_at, so run_deferred_emission reads the PR as reopened and cancels its metrics emission; flips open ↔ non-open, so pull_request_state_changing writes a spurious "reopened this pull request" onto every issue the PR resolves; and re-derives group links from the stale body, deleting the GroupLink.

Two independent sources of reordering, both addressed here: retry backoff (schedule_next_attempt — and for skip_on_failure_providers the drain skips the failed message outright), and concurrency — _run_parallel_delivery_batch hands the next hybridcloud.webhookpayload.worker_threads payloads from one mailbox to a single threadpool (16 in production), and GitHub buckets every pull_request event for a repo into one mailbox.

The fix

Staleness is decided on the provider's own last-modified time, stored in a new nullable PullRequest.provider_updated_at — migration 1150_pullrequest_provider_updated_at, additive and nullable, no backfill. date_added/date_updated are arrival time and can't order anything, and no existing column carried provider time. The provider_ prefix marks that the column holds someone else's clock: bare updated_at is the usual ORM convention for a row-modification timestamp, so it would read as Sentry-local time. The same prefix is used by the sibling guards on ExternalIssue (provider_status_updated_at in #121084, provider_assignee_updated_at in #121157) — those three are read together, so a naming divergence between them would look meaningful when it isn't.

A stale snapshot is dropped whole rather than field by field: every mutable column comes from the same point in time, and a state-only guard would still fabricate the timeline entries and delete the issue links listed above. A second rule — merged is terminal — covers what timestamps can't: rows predating the column (no backfill, so all of them at first) and GitHub's one-second resolution. It isn't sufficient alone, because closed → open is a real transition that only provider time separates from a replay. Equal timestamps are not stale.

The row is locked with select_for_update across the decision, otherwise two concurrent deliveries for one PR both read the pre-write row and the older write lands last. update_or_create already takes FOR UPDATE internally, immediately after this read — the lock is widened, not introduced.

pr_metrics.handle_metrics writes PullRequestMetrics from the same payload and is held to the same verdict. Without that, select_verdict (src/sentry/pr_metrics/emit.py:175-178) reads zeroed discussion counts off a PR that had real reviewer engagement and emits a CLOSED_UNMERGED that _claim_terminal_event makes permanent.

Skips report scm.webhook.pull_request.stale_snapshot (tagged by provider) and pr_metrics.metrics.stale_snapshot.

Landing order

Prerequisite for #121057, which widens skip_on_failure_providers to github_enterprise, bitbucket, and bitbucket_server. GitHub Enterprise inherits this guard by subclassing the GitHub handler.

Delivery and retry semantics in deliver_webhooks.py are untouched.

[github-actions]

This PR has a migration; here is the generated SQL for src/sentry/migrations/1149_pullrequest_updated_at.py

for 1149_pullrequest_updated_at in sentry

--
-- Add field updated_at to pullrequest
--
ALTER TABLE "sentry_pull_request" ADD COLUMN "updated_at" timestamp with time zone NULL;

[github-actions]

This PR has a migration; here is the generated SQL for src/sentry/migrations/1150_pullrequest_updated_at.py

for 1150_pullrequest_updated_at in sentry

--
-- Add field updated_at to pullrequest
--
ALTER TABLE "sentry_pull_request" ADD COLUMN "updated_at" timestamp with time zone NULL;

[github-actions]

This PR has a migration; here is the generated SQL for src/sentry/migrations/1150_pullrequest_scm_updated_at.py

for 1150_pullrequest_scm_updated_at in sentry

--
-- Add field scm_updated_at to pullrequest
--
ALTER TABLE "sentry_pull_request" ADD COLUMN "scm_updated_at" timestamp with time zone NULL;

[github-actions]

This PR has a migration; here is the generated SQL for src/sentry/migrations/1150_pullrequest_provider_updated_at.py

for 1150_pullrequest_provider_updated_at in sentry

--
-- Add field provider_updated_at to pullrequest
--
ALTER TABLE "sentry_pull_request" ADD COLUMN "provider_updated_at" timestamp with time zone NULL;

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit ac4a7ad. Configure here.}

[cursor]

Create race skips staleness guard

High Severity

select_for_update only serializes when a PullRequest row already exists. Concurrent first deliveries both see no row, skip is_stale_pull_request_snapshot, and call update_or_create; Django then applies the loser's defaults onto the winner's insert. An older open snapshot can still overwrite a newer merged one under the parallel mailbox delivery this change aims to fix.

 

^{Reviewed by Cursor Bugbot for commit ac4a7ad. Configure here.}