From c10f9c109549a9345ede87e2706e436bb357be3e Mon Sep 17 00:00:00 2001 From: Giovanni M Guidini Date: Tue, 7 Jul 2026 15:30:38 +0200 Subject: [PATCH] feat(pr-metrics): Record sentry-app PR attribution on close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit handle_attribution only wrote the SENTRY_APP author signal on the PR 'opened' event. A Sentry-authored PR that missed its open webhook (or was opened before the attribution flag was enabled) then had no attribution row, so it was never tracked for emission at close. Re-check the author signal on 'closed' as well. record_attribution_signal is keyed on (pull_request, signal_type, source), so re-running when the row already exists won't duplicate it — it refreshes signal_details in place (e.g. resolved_group_ids may return more at close than at open), which is acceptable here. handle_attribution runs before handle_emission in the same delivery, so a PR first attributed at close is tracked in time to emit. Scoped to SENTRY_APP for now: MCP and delegated-agent attribution stay open-only. --- src/sentry/pr_metrics/webhooks.py | 5 ++++- tests/sentry/pr_metrics/test_webhooks.py | 26 ++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/sentry/pr_metrics/webhooks.py b/src/sentry/pr_metrics/webhooks.py index 5bb9a5b2d9d0..03762273ec60 100644 --- a/src/sentry/pr_metrics/webhooks.py +++ b/src/sentry/pr_metrics/webhooks.py @@ -161,7 +161,10 @@ def handle_attribution( if pr is None: return - if action == "opened": + # SENTRY_APP author attribution is recorded on open and re-checked on close. + # This is for the unlikely event that we missed the open webhook or for cases + # where the PR open and closes super fast and the webhooks might be out of order + if action in ("opened", "closed"): pr_url = (pull_request or {}).get("html_url") or None _write_author_attribution(pr, github_user, pr_url=pr_url, group_ids=resolved_group_ids(pr)) if features.has("organizations:mcp-issue-view-attribution", organization): diff --git a/tests/sentry/pr_metrics/test_webhooks.py b/tests/sentry/pr_metrics/test_webhooks.py index e677e3ebc176..17bb3193f3c9 100644 --- a/tests/sentry/pr_metrics/test_webhooks.py +++ b/tests/sentry/pr_metrics/test_webhooks.py @@ -102,7 +102,7 @@ def test_unknown_user_no_attribution_created(self) -> None: assert not PullRequestAttribution.objects.filter(pull_request=self.pr).exists() - def test_app_attribution_only_written_on_opened(self) -> None: + def test_app_attribution_not_written_on_non_terminal_actions(self) -> None: self._call(action="synchronize", user_id=settings.SEER_AUTOFIX_GITHUB_APP_USER_ID) self._call(action="labeled", user_id=settings.SEER_AUTOFIX_GITHUB_APP_USER_ID) @@ -111,12 +111,34 @@ def test_app_attribution_only_written_on_opened(self) -> None: signal_type=PullRequestAttributionSignalType.SENTRY_APP, ).exists() + def test_app_attribution_written_on_closed(self) -> None: + # A Sentry-authored PR that never got a row at open (e.g. opened before the + # flag, or resolving no issues) is still attributed at the terminal close + # event so it can be tracked for emission. + self._call(action="closed", user_id=settings.SENTRY_GITHUB_APP_USER_ID) + + attr = PullRequestAttribution.objects.get(pull_request=self.pr) + assert attr.signal_type == PullRequestAttributionSignalType.SENTRY_APP + assert attr.source == PullRequestAttributionSource.WEBHOOK_DATA + assert attr.is_valid is True + + def test_app_attribution_idempotent_across_open_then_close(self) -> None: + self._call(action="opened", user_id=settings.SENTRY_GITHUB_APP_USER_ID) + self._call(action="closed", user_id=settings.SENTRY_GITHUB_APP_USER_ID) + + assert ( + PullRequestAttribution.objects.filter( + pull_request=self.pr, + signal_type=PullRequestAttributionSignalType.SENTRY_APP, + ).count() + == 1 + ) + # --- Action gate --- def test_irrelevant_actions_skipped(self) -> None: for action in ( "synchronize", - "closed", "labeled", "assigned", ):