diff --git a/src/sentry/integrations/cursor_origin/authors.py b/src/sentry/integrations/cursor_origin/authors.py new file mode 100644 index 000000000000..3e8707f4affc --- /dev/null +++ b/src/sentry/integrations/cursor_origin/authors.py @@ -0,0 +1,17 @@ +from __future__ import annotations + +from sentry.models.commitauthor import CommitAuthor + +MAX_AUTHOR_EMAIL_LENGTH = 75 + + +def get_or_create_commit_author(organization_id: int, email: str, name: str) -> CommitAuthor | None: + if not email or len(email) > MAX_AUTHOR_EMAIL_LENGTH: + return None + + author, _ = CommitAuthor.objects.get_or_create( + organization_id=organization_id, + email=email, + defaults={"name": name[:128]}, + ) + return author diff --git a/src/sentry/integrations/cursor_origin/pull_request.py b/src/sentry/integrations/cursor_origin/pull_request.py new file mode 100644 index 000000000000..084e7eeb9c93 --- /dev/null +++ b/src/sentry/integrations/cursor_origin/pull_request.py @@ -0,0 +1,96 @@ +from __future__ import annotations + +import logging +from collections.abc import Mapping, Sequence +from typing import Any + +from sentry.integrations.cursor_origin.authors import get_or_create_commit_author +from sentry.integrations.cursor_origin.handlers import WebhookEventHandler +from sentry.integrations.cursor_origin.repository import active_repositories +from sentry.integrations.cursor_origin.webhook_types import PullRequestEvent +from sentry.integrations.services.integration.model import ( + RpcIntegration, + RpcOrganizationIntegration, +) +from sentry.integrations.types import IntegrationProviderSlug +from sentry.integrations.utils.metrics import IntegrationWebhookEventType +from sentry.models.pullrequest import PullRequestLifecycleState +from sentry.models.repository import Repository +from sentry.pr_metrics.lifecycle_mapping import update_pull_request_from_scm_snapshot +from sentry.utils import metrics + +logger = logging.getLogger("sentry.integrations.cursor_origin") + + +def lifecycle_state(event: PullRequestEvent) -> str: + if event.pull_request.merged: + return PullRequestLifecycleState.MERGED + if event.pull_request.state == "closed": + return PullRequestLifecycleState.CLOSED + return PullRequestLifecycleState.OPEN + + +class PullRequestLifecycleHandler(WebhookEventHandler): + """Record a pull request from any of Origin's snapshot events.""" + + EVENT_TYPE = IntegrationWebhookEventType.MERGE_REQUEST + + def __call__( + self, + payload: Mapping[str, Any], + delivery_id: str, + integration: RpcIntegration, + org_integrations: Sequence[RpcOrganizationIntegration], + ) -> None: + event = PullRequestEvent.from_payload(payload) + + repositories = active_repositories(event.repository_id, org_integrations) + if not repositories: + logger.info( + "cursor_origin.pull_request.unknown_repository", + extra={"delivery_id": delivery_id, "external_id": event.repository_id}, + ) + metrics.incr("cursor_origin.pull_request.unknown_repository", sample_rate=1.0) + return + + for repo in repositories: + self._record(repo, event, delivery_id) + + def _record(self, repo: Repository, event: PullRequestEvent, delivery_id: str) -> None: + pull_request = event.pull_request + state = lifecycle_state(event) + email, name = pull_request.author.email_and_name() + author = get_or_create_commit_author(repo.organization_id, email, name) + + _, created = update_pull_request_from_scm_snapshot( + provider=IntegrationProviderSlug.CURSOR_ORIGIN.value, + organization_id=repo.organization_id, + repository_id=repo.id, + key=pull_request.number, + defaults={ + "organization_id": repo.organization_id, + "title": pull_request.title, + "author": author, + "message": pull_request.body, + "merge_commit_sha": pull_request.merge_commit_sha or None, + "head_commit_sha": pull_request.head.sha, + "opened_at": pull_request.created_at, + "closed_at": pull_request.closed_at, + "merged_at": pull_request.merged_at, + "provider_updated_at": pull_request.updated_at, + "state": state, + "draft": pull_request.draft, + }, + event_state=state, + event_updated_at=pull_request.updated_at, + ) + + if created: + logger.info( + "cursor_origin.pull_request.created", + extra={ + "delivery_id": delivery_id, + "repository_id": repo.id, + "key": pull_request.number, + }, + ) diff --git a/src/sentry/integrations/cursor_origin/push.py b/src/sentry/integrations/cursor_origin/push.py index 8e832f7dd45e..d9fbe5051a02 100644 --- a/src/sentry/integrations/cursor_origin/push.py +++ b/src/sentry/integrations/cursor_origin/push.py @@ -9,6 +9,7 @@ from sentry import options from sentry.constants import ObjectStatus +from sentry.integrations.cursor_origin.authors import get_or_create_commit_author from sentry.integrations.cursor_origin.client import CursorOriginApiClient from sentry.integrations.cursor_origin.handlers import WebhookEventHandler from sentry.integrations.cursor_origin.repository import ( @@ -23,7 +24,6 @@ ) from sentry.integrations.utils.metrics import IntegrationWebhookEventType from sentry.models.commit import Commit -from sentry.models.commitauthor import CommitAuthor from sentry.models.commitfilechange import CommitFileChange, post_bulk_create from sentry.models.repository import Repository from sentry.plugins.providers.integration_repository import IntegrationRepositoryProvider @@ -33,8 +33,6 @@ EMPTY_SHA = "0" * 40 -MAX_AUTHOR_EMAIL_LENGTH = 75 - class RepositoryPushedHandler(WebhookEventHandler): """Record the commits a push added. @@ -155,7 +153,9 @@ def _record( if Commit.objects.filter(repository_id=repo.id, key=commit.sha).exists(): return - author = self._author(repo, commit) + author = get_or_create_commit_author( + repo.organization_id, commit.author_email, commit.author_name + ) changes = file_changes_from(client.get_commit_files(repo.config["name"], commit.sha)) with transaction.atomic(router.db_for_write(Commit)): @@ -184,14 +184,3 @@ def _record( if rows: CommitFileChange.objects.bulk_create(rows, ignore_conflicts=True) post_bulk_create(rows) - - def _author(self, repo: Repository, commit: PushedCommit) -> CommitAuthor | None: - if not commit.author_email or len(commit.author_email) > MAX_AUTHOR_EMAIL_LENGTH: - return None - - commit_author, _ = CommitAuthor.objects.get_or_create( - organization_id=repo.organization_id, - email=commit.author_email, - defaults={"name": commit.author_name[:128]}, - ) - return commit_author diff --git a/src/sentry/integrations/cursor_origin/webhook.py b/src/sentry/integrations/cursor_origin/webhook.py index c9d5104f2da9..a8abc10cb54b 100644 --- a/src/sentry/integrations/cursor_origin/webhook.py +++ b/src/sentry/integrations/cursor_origin/webhook.py @@ -32,6 +32,7 @@ WebhookEventHandler, ) from sentry.integrations.cursor_origin.keys import signing_keys_for +from sentry.integrations.cursor_origin.pull_request import PullRequestLifecycleHandler from sentry.integrations.cursor_origin.push import RepositoryPushedHandler from sentry.integrations.cursor_origin.repository_events import RepositoryMetadataUpdatedHandler from sentry.integrations.cursor_origin.webhook_types import OriginPayloadError @@ -140,6 +141,12 @@ def verify_delivery(request: HttpRequest, body: bytes) -> Verification: "installation.suspended": InstallationRemovedHandler, "installation.unsuspended": InstallationRestoredHandler, "installation.updated": InstallationUpdatedHandler, + "pull_request.closed": PullRequestLifecycleHandler, + "pull_request.created": PullRequestLifecycleHandler, + "pull_request.merged": PullRequestLifecycleHandler, + "pull_request.metadata.updated": PullRequestLifecycleHandler, + "pull_request.published": PullRequestLifecycleHandler, + "pull_request.reopened": PullRequestLifecycleHandler, "repository.metadata.updated": RepositoryMetadataUpdatedHandler, "repository.pushed": RepositoryPushedHandler, } diff --git a/src/sentry/integrations/cursor_origin/webhook_types.py b/src/sentry/integrations/cursor_origin/webhook_types.py index ad1905ab0742..765826a0b9f3 100644 --- a/src/sentry/integrations/cursor_origin/webhook_types.py +++ b/src/sentry/integrations/cursor_origin/webhook_types.py @@ -2,7 +2,7 @@ from collections.abc import Mapping from datetime import datetime -from typing import Any +from typing import Any, Literal from pydantic import BaseModel, Field, ValidationError, validator @@ -111,3 +111,77 @@ def from_payload(cls, payload: Mapping[str, Any]) -> RepositoryMetadataEvent: return cls.parse_obj(payload) except ValidationError as e: raise OriginPayloadError(str(e)) from e + + +class PullRequestHead(OriginModel): + sha: str = Field(min_length=1) + + +class PullRequestUser(OriginModel): + email: str = Field(min_length=1) + display_name: str = Field(default="", alias="displayName") + + +class PullRequestApp(OriginModel): + id: str = Field(min_length=1) + display_name: str = Field(default="", alias="displayName") + + +class PullRequestServiceAccount(OriginModel): + id: str = Field(min_length=1) + + +class PullRequestAuthor(OriginModel): + """Exactly one of a user, an app or a service account.""" + + user: PullRequestUser | None = None + app: PullRequestApp | None = None + service_account: PullRequestServiceAccount | None = Field(default=None, alias="serviceAccount") + + def email_and_name(self) -> tuple[str, str]: + if self.user is not None: + return self.user.email, self.user.display_name + if self.app is not None: + return f"{self.app.id}@localhost", self.app.display_name or self.app.id + assert self.service_account is not None + return f"{self.service_account.id}@localhost", self.service_account.id + + +class PullRequest(OriginModel): + """Origin's own id is not kept: it is a prefixed string, and + `PullRequest.external_id` is an integer column.""" + + number: str = Field(min_length=1) + title: str + body: str + state: Literal["open", "closed"] + draft: bool + merged: bool + head: PullRequestHead + merge_commit_sha: str = Field(alias="mergeCommitSha") + author: PullRequestAuthor + created_at: datetime | None = Field(..., alias="createdAt") + updated_at: datetime | None = Field(..., alias="updatedAt") + closed_at: datetime | None = Field(..., alias="closedAt") + merged_at: datetime | None = Field(..., alias="mergedAt") + + @validator("created_at", "updated_at", "closed_at", "merged_at", pre=True) + def _absent_date(cls, value: Any) -> Any: + """Origin sends an empty string for an unset date.""" + return value or None + + +class PullRequestEvent(OriginModel): + repository: Repository + pull_request: PullRequest = Field(alias="pullRequest") + + @property + def repository_id(self) -> str: + return self.repository.id + + @classmethod + def from_payload(cls, payload: Mapping[str, Any]) -> PullRequestEvent: + try: + return cls.parse_obj(payload) + except ValidationError as e: + raise OriginPayloadError(str(e)) from e diff --git a/tests/sentry/integrations/cursor_origin/test_pull_request.py b/tests/sentry/integrations/cursor_origin/test_pull_request.py new file mode 100644 index 000000000000..4c788baf828b --- /dev/null +++ b/tests/sentry/integrations/cursor_origin/test_pull_request.py @@ -0,0 +1,204 @@ +from __future__ import annotations + +from typing import Any + +import pytest + +from sentry.constants import ObjectStatus +from sentry.integrations.cursor_origin.pull_request import PullRequestLifecycleHandler +from sentry.integrations.cursor_origin.webhook_types import OriginPayloadError, PullRequestEvent +from sentry.integrations.services.integration import integration_service +from sentry.models.pullrequest import PullRequest, PullRequestLifecycleState +from sentry.models.repository import Repository +from sentry.testutils.cases import TestCase +from sentry.testutils.silo import cell_silo_test + +INSTALLATION_ID = "i_01example" +REPO = "acme/rocket" +REPO_EXTERNAL_ID = "r_01example" +DELIVERY_ID = "whd_01example" + + +def _payload(**overrides: Any) -> dict[str, Any]: + pull_request: dict[str, Any] = { + "id": "pr_01example", + "number": "17", + "state": "open", + "draft": False, + "merged": False, + "title": "Add launch telemetry", + "body": "Adds structured launch telemetry.", + "head": {"ref": "add-telemetry", "sha": "9a41f0c3"}, + "base": {"ref": "main", "sha": "3b1f9c2d"}, + "author": {"user": {"id": "user_01example", "email": "jane@example.com"}}, + "createdAt": "2026-08-01T09:30:00Z", + "updatedAt": "2026-08-01T10:00:00Z", + "closedAt": "", + "mergedAt": "", + "mergeCommitSha": "", + } + pull_request.update(overrides) + return { + "pullRequest": pull_request, + "repository": {"id": REPO_EXTERNAL_ID, "name": "rocket"}, + } + + +@cell_silo_test +class PullRequestLifecycleHandlerTest(TestCase): + def setUp(self) -> None: + super().setUp() + self.integration = self.create_integration( + organization=self.organization, + provider="cursor_origin", + name="acme", + external_id=INSTALLATION_ID, + status=ObjectStatus.ACTIVE, + ) + self.repo = Repository.objects.create( + organization_id=self.organization.id, + name=REPO, + provider="integrations:cursor_origin", + integration_id=self.integration.id, + external_id=REPO_EXTERNAL_ID, + config={"name": REPO}, + ) + context = integration_service.organization_contexts( + provider="cursor_origin", external_id=INSTALLATION_ID + ) + assert context.integration is not None + self.rpc_integration = context.integration + self.org_integrations = context.organization_integrations + + def _handle(self, payload: dict[str, Any]) -> None: + PullRequestLifecycleHandler()( + payload, DELIVERY_ID, self.rpc_integration, self.org_integrations + ) + + def _pull_requests(self) -> list[PullRequest]: + return list(PullRequest.objects.filter(repository_id=self.repo.id).order_by("key")) + + def test_an_opened_pull_request_is_recorded(self) -> None: + self._handle(_payload()) + + pull_request = self._pull_requests()[0] + assert pull_request.key == "17" + assert pull_request.title == "Add launch telemetry" + assert pull_request.message == "Adds structured launch telemetry." + assert pull_request.state == PullRequestLifecycleState.OPEN + assert pull_request.head_commit_sha == "9a41f0c3" + assert pull_request.author is not None + assert pull_request.author.email == "jane@example.com" + + def test_a_merge_is_stored_as_merged_rather_than_closed(self) -> None: + """Origin reports a merged pull request as closed with a separate flag.""" + self._handle( + _payload( + state="closed", + merged=True, + mergeCommitSha="ffff0000", + closedAt="2026-08-02T09:30:00Z", + mergedAt="2026-08-02T09:30:00Z", + updatedAt="2026-08-02T09:30:00Z", + ) + ) + + pull_request = self._pull_requests()[0] + assert pull_request.state == PullRequestLifecycleState.MERGED + assert pull_request.merge_commit_sha == "ffff0000" + assert pull_request.merged_at is not None + + def test_a_close_without_a_merge_is_stored_as_closed(self) -> None: + self._handle( + _payload( + state="closed", closedAt="2026-08-02T09:30:00Z", updatedAt="2026-08-02T09:30:00Z" + ) + ) + + assert self._pull_requests()[0].state == PullRequestLifecycleState.CLOSED + + def test_a_later_event_updates_the_same_row(self) -> None: + self._handle(_payload()) + self._handle( + _payload(title="Add launch telemetry, take two", updatedAt="2026-08-01T11:00:00Z") + ) + + pull_requests = self._pull_requests() + assert len(pull_requests) == 1 + assert pull_requests[0].title == "Add launch telemetry, take two" + + def test_a_stale_snapshot_is_dropped(self) -> None: + """Deliveries can arrive out of order, so the shared upsert compares timestamps.""" + self._handle( + _payload( + state="closed", + merged=True, + mergedAt="2026-08-02T09:30:00Z", + closedAt="2026-08-02T09:30:00Z", + updatedAt="2026-08-02T09:30:00Z", + ) + ) + self._handle(_payload(updatedAt="2026-08-01T10:00:00Z")) + + assert self._pull_requests()[0].state == PullRequestLifecycleState.MERGED + + def test_a_draft_is_recorded_as_open(self) -> None: + self._handle(_payload(draft=True)) + + pull_request = self._pull_requests()[0] + assert pull_request.draft is True + assert pull_request.state == PullRequestLifecycleState.OPEN + + def test_an_app_author_is_kept_under_a_localhost_email(self) -> None: + payload = _payload() + payload["pullRequest"]["author"] = {"app": {"id": "app_01example", "displayName": "Sentry"}} + + self._handle(payload) + + author = self._pull_requests()[0].author + assert author is not None + assert (author.email, author.name) == ("app_01example@localhost", "Sentry") + + def test_a_service_account_author_is_named_by_its_id(self) -> None: + payload = _payload() + payload["pullRequest"]["author"] = {"serviceAccount": {"id": "sa_01example"}} + + self._handle(payload) + + author = self._pull_requests()[0].author + assert author is not None + assert (author.email, author.name) == ("sa_01example@localhost", "sa_01example") + + def test_an_unknown_repository_is_ignored(self) -> None: + payload = _payload() + payload["repository"]["id"] = "r_01nope" + + self._handle(payload) + + assert self._pull_requests() == [] + + +class PullRequestEventTest(TestCase): + def test_an_empty_date_is_absent_rather_than_invalid(self) -> None: + """Origin sends an empty string for a date that is unset, as `closedAt` is while open.""" + assert PullRequestEvent.from_payload(_payload()).pull_request.closed_at is None + + def test_a_field_origin_always_sends_is_required(self) -> None: + payload = _payload() + del payload["pullRequest"]["title"] + + with pytest.raises(OriginPayloadError, match="pullRequest -> title"): + PullRequestEvent.from_payload(payload) + + def test_a_state_origin_does_not_document_is_refused(self) -> None: + with pytest.raises(OriginPayloadError, match="pullRequest -> state"): + PullRequestEvent.from_payload(_payload(state="locked")) + + def test_an_author_display_name_is_kept_when_origin_sends_one(self) -> None: + payload = _payload() + payload["pullRequest"]["author"]["user"]["displayName"] = "Jane Roe" + + user = PullRequestEvent.from_payload(payload).pull_request.author.user + + assert user is not None + assert user.display_name == "Jane Roe"