From 9b0e863af9bdf275527c80da0804e7b7ffcb5a8f Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Thu, 17 Sep 2026 15:41:07 -0700 Subject: [PATCH 1/2] feat(cursor-origin): Track release commits Implement `compare_commits` so releases built from Origin repositories include their commits, authors, and file changes. Origin's comparison only returns a commit count, so ranges use `aheadBy` to fetch commits from the head. The first release fetches the 20 most recent commits. This is exact for linear history, but can miss commits when merging a long-lived branch. Commit files require a separate 5-point call, so `cursor-origin-app.fetch-commits.max-compare-commits` caps the range at 500 commits and logs when truncation occurs. `_paginate` now accepts a `limit` to avoid fetching more commits than needed. --- .../integrations/cursor_origin/client.py | 19 +- .../integrations/cursor_origin/repository.py | 87 ++++++- src/sentry/options/defaults.py | 6 + .../cursor_origin/test_client_reads.py | 14 ++ .../cursor_origin/test_repository.py | 218 +++++++++++++++++- 5 files changed, 331 insertions(+), 13 deletions(-) diff --git a/src/sentry/integrations/cursor_origin/client.py b/src/sentry/integrations/cursor_origin/client.py index ac51feb26901..e0812466183a 100644 --- a/src/sentry/integrations/cursor_origin/client.py +++ b/src/sentry/integrations/cursor_origin/client.py @@ -356,18 +356,25 @@ def track_response_data( super().track_response_data(code, error, resp, extra) def _paginate[T]( - self, path: str, collection_key: str, params: dict[str, Any] | None = None + self, + path: str, + collection_key: str, + params: dict[str, Any] | None = None, + limit: int | None = None, ) -> list[T]: results: list[T] = [] page_token: str | None = None + page_size = min(PAGE_SIZE, limit) if limit else PAGE_SIZE for _ in range(self.page_number_limit): - request_params: dict[str, Any] = {"pageSize": PAGE_SIZE, **(params or {})} + request_params: dict[str, Any] = {"pageSize": page_size, **(params or {})} if page_token: request_params["pageToken"] = page_token response = self.get(path, params=request_params) results.extend(response[collection_key]) + if limit is not None and len(results) >= limit: + return results[:limit] # Present on every page; empty on the last one. page_token = response["nextPageToken"] @@ -386,10 +393,14 @@ def get_repo(self, repo_full_name: str) -> OriginRepository: def get_branches(self, repo_full_name: str) -> list[OriginBranch]: return self._paginate(f"/repos/{repo_full_name}/branches", "branches") - def get_commits(self, repo_full_name: str, sha: str | None = None) -> list[OriginCommit]: + def get_commits( + self, repo_full_name: str, sha: str | None = None, limit: int | None = None + ) -> list[OriginCommit]: """Return commits from `sha`, newest first, or from the default branch.""" params = {"sha": sha} if sha else None - return self._paginate(f"/repos/{repo_full_name}/commits", "commits", params=params) + return self._paginate( + f"/repos/{repo_full_name}/commits", "commits", params=params, limit=limit + ) def get_commit(self, repo_full_name: str, sha: str) -> OriginCommit: """Return a commit with aggregate stats.""" diff --git a/src/sentry/integrations/cursor_origin/repository.py b/src/sentry/integrations/cursor_origin/repository.py index dcc5b61c8686..7acb99c5608e 100644 --- a/src/sentry/integrations/cursor_origin/repository.py +++ b/src/sentry/integrations/cursor_origin/repository.py @@ -4,6 +4,12 @@ from collections.abc import Mapping, MutableMapping, Sequence from typing import Any +from sentry import options +from sentry.integrations.cursor_origin.client import ( + CursorOriginApiClient, + OriginCommit, + OriginCommitFile, +) from sentry.integrations.cursor_origin.constants import CURSOR_ORIGIN_WEB_BASE_URL from sentry.integrations.cursor_origin.integration import CursorOriginIntegration from sentry.integrations.types import IntegrationProviderSlug @@ -17,6 +23,9 @@ logger = logging.getLogger("sentry.integrations.cursor_origin") +MAX_COMPARE_COMMITS_OPTION_KEY = "cursor-origin-app.fetch-commits.max-compare-commits" +RECENT_COMMIT_COUNT = 20 + class CursorOriginRepositoryProvider(IntegrationRepositoryProvider[CursorOriginIntegration]): name = "Cursor Origin" @@ -57,11 +66,79 @@ def repository_external_slug(self, repo: Repository) -> str: def compare_commits( self, repo: Repository, start_sha: str | None, end_sha: str ) -> Sequence[Mapping[str, Any]]: - # Origin does expose List Commits and Compare Commits, but wiring release commit - # tracking to them is its own change. Raising rather than returning [] keeps this - # visible in Sentry's own errors: an empty list is indistinguishable from a - # repository that genuinely has no new commits. - raise NotImplementedError("Cursor Origin commit tracking is not implemented yet") + installation = self.get_installation(repo.integration_id, repo.organization_id) + client = installation.get_client() + name = repo.config["name"] + + try: + if start_sha is None: + commits = client.get_commits(name, sha=end_sha, limit=RECENT_COMMIT_COUNT) + else: + commits = self._commits_in_range(client, repo, name, start_sha, end_sha) + return [self._format_commit(client, name, commit) for commit in reversed(commits)] + except Exception as e: + installation.raise_error(e) + + def _commits_in_range( + self, + client: CursorOriginApiClient, + repo: Repository, + name: str, + start_sha: str, + end_sha: str, + ) -> list[OriginCommit]: + """Return commits in the range, newest first""" + comparison = client.compare_commits(name, start_sha, end_sha) + ahead_by = comparison["aheadBy"] + if ahead_by <= 0: + return [] + + max_commits = options.get(MAX_COMPARE_COMMITS_OPTION_KEY) + if max_commits and ahead_by > max_commits: + logger.info( + "cursor_origin.fetch_commits.truncated", + extra={ + "organization_id": repo.organization_id, + "repository": repo.name, + "start_sha": start_sha, + "end_sha": end_sha, + "ahead_by": ahead_by, + "truncated_count": max_commits, + }, + ) + ahead_by = max_commits + + return client.get_commits(name, sha=end_sha, limit=ahead_by) + + def _format_commit( + self, client: CursorOriginApiClient, name: str, commit: OriginCommit + ) -> Mapping[str, Any]: + author = commit["commit"]["author"] + return { + "id": commit["sha"], + "repository": name, + "author_email": author["email"], + "author_name": author["name"][:128], + "message": commit["commit"]["message"], + "timestamp": self.format_date(author["date"]), + "patch_set": self._patch_set(client.get_commit_files(name, commit["sha"])), + } + + def _patch_set(self, files: Sequence[OriginCommitFile]) -> Sequence[Mapping[str, str]]: + """File changes in the shape `Release.set_commits` expects.""" + changes: list[Mapping[str, str]] = [] + for file in files: + status = file["status"] + if status == "modified": + changes.append({"path": file["filename"], "type": "M"}) + elif status in ("added", "copied"): + changes.append({"path": file["filename"], "type": "A"}) + elif status == "removed": + changes.append({"path": file["filename"], "type": "D"}) + elif status == "renamed": + changes.append({"path": file["previousFilename"], "type": "D"}) + changes.append({"path": file["filename"], "type": "A"}) + return changes def pull_request_url(self, repo: Repository, pull_request: PullRequest) -> str: return f"{CURSOR_ORIGIN_WEB_BASE_URL}/{repo.name}/pull/{pull_request.key}" diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py index 8b944d792bd0..655fb18f4d58 100644 --- a/src/sentry/options/defaults.py +++ b/src/sentry/options/defaults.py @@ -775,6 +775,12 @@ # Cursor Origin Integration register("cursor-origin-app.id", default="", flags=FLAG_AUTOMATOR_MODIFIABLE) register("cursor-origin-app.private-key", default="", flags=FLAG_CREDENTIAL | FLAG_PRIORITIZE_DISK) +register( + "cursor-origin-app.fetch-commits.max-compare-commits", + type=Int, + default=500, + flags=FLAG_AUTOMATOR_MODIFIABLE, +) # Github Enterprise Integration register( diff --git a/tests/sentry/integrations/cursor_origin/test_client_reads.py b/tests/sentry/integrations/cursor_origin/test_client_reads.py index d9e61cb76bbb..af7f6b45b787 100644 --- a/tests/sentry/integrations/cursor_origin/test_client_reads.py +++ b/tests/sentry/integrations/cursor_origin/test_client_reads.py @@ -151,6 +151,20 @@ def test_get_commits_starts_from_a_ref(self) -> None: assert [commit["sha"] for commit in commits] == ["abc"] assert "sha=main" in responses.calls[0].request.url + @responses.activate + def test_a_limited_read_asks_for_no_more_than_it_wants(self) -> None: + responses.add( + responses.GET, + f"{CURSOR_ORIGIN_API_BASE_URL}/repos/{REPO}/commits", + json={"commits": [{"sha": f"c{i}"} for i in range(20)], "nextPageToken": "page-2"}, + ) + + commits = self.origin_client.get_commits(REPO, sha="main", limit=20) + + assert len(commits) == 20 + assert "pageSize=20" in responses.calls[0].request.url + assert len(responses.calls) == 1 + @responses.activate def test_get_commits_defaults_to_the_default_branch(self) -> None: """Origin reads an absent `sha` as the repository's default branch.""" diff --git a/tests/sentry/integrations/cursor_origin/test_repository.py b/tests/sentry/integrations/cursor_origin/test_repository.py index b7db9c73bb3b..fef6e218da1f 100644 --- a/tests/sentry/integrations/cursor_origin/test_repository.py +++ b/tests/sentry/integrations/cursor_origin/test_repository.py @@ -1,13 +1,21 @@ from __future__ import annotations +import re +from datetime import UTC, datetime, timedelta from typing import Any from unittest import mock +from urllib.parse import parse_qs, urlparse import pytest +import responses from sentry.constants import ObjectStatus from sentry.integrations.cursor_origin.client import CursorOriginApiClient -from sentry.integrations.cursor_origin.repository import CursorOriginRepositoryProvider +from sentry.integrations.cursor_origin.constants import CURSOR_ORIGIN_API_BASE_URL, PAGE_SIZE +from sentry.integrations.cursor_origin.repository import ( + MAX_COMPARE_COMMITS_OPTION_KEY, + CursorOriginRepositoryProvider, +) from sentry.models.pullrequest import PullRequest from sentry.models.repository import Repository from sentry.plugins.base import bindings @@ -89,6 +97,208 @@ def test_pull_request_url(self) -> None: assert url == f"{WEB}/{REPO}/pull/7" - def test_commit_tracking_is_not_implemented_yet(self) -> None: - with pytest.raises(NotImplementedError): - self.provider.compare_commits(Repository(name=REPO), "abc", "def") + +def _commit(sha: str, message: str = "a change", email: str = "dev@example.com") -> dict[str, Any]: + return { + "sha": sha, + "commit": { + "author": {"name": "A Dev", "email": email, "date": "2026-09-16T12:00:00Z"}, + "committer": {"name": "A Dev", "email": email, "date": "2026-09-16T12:00:00Z"}, + "message": message, + }, + "parents": [], + } + + +def _file(filename: str, status: str, previous: str | None = None) -> dict[str, Any]: + file: dict[str, Any] = { + "filename": filename, + "status": status, + "additions": 1, + "deletions": 0, + "changes": 1, + "patch": "", + } + if previous is not None: + file["previousFilename"] = previous + return file + + +@control_silo_test +class CompareCommitsTest(TestCase): + def setUp(self) -> None: + super().setUp() + self.integration = self.create_integration( + organization=self.organization, + provider="cursor_origin", + name="acme", + external_id=INSTALLATION_ID, + metadata={ + "access_token": "oit_stored", + "expires_at": (datetime.now(UTC) + timedelta(minutes=14)) + .isoformat() + .replace("+00:00", "Z"), + }, + status=ObjectStatus.ACTIVE, + ) + self.provider = CursorOriginRepositoryProvider("integrations:cursor_origin") + self.repo = Repository( + organization_id=self.organization.id, + name=REPO, + provider="integrations:cursor_origin", + integration_id=self.integration.id, + external_id="r_01example", + config={"name": REPO, "default_branch": "main"}, + ) + + def _stub_compare(self, ahead_by: int, status_name: str = "ahead") -> None: + responses.add( + responses.GET, + f"{CURSOR_ORIGIN_API_BASE_URL}/repos/{REPO}/compare/a...b", + json={"status": status_name, "aheadBy": ahead_by, "behindBy": 0}, + ) + + def _stub_commits(self, *commits: dict[str, Any], sha: str = "b") -> None: + responses.add( + responses.GET, + f"{CURSOR_ORIGIN_API_BASE_URL}/repos/{REPO}/commits", + json={"commits": list(commits), "nextPageToken": ""}, + ) + + def _stub_files(self, sha: str, *files: dict[str, Any]) -> None: + responses.add( + responses.GET, + f"{CURSOR_ORIGIN_API_BASE_URL}/repos/{REPO}/commits/{sha}/files", + json={"files": list(files), "nextPageToken": ""}, + ) + + def _query(self, index: int) -> dict[str, list[str]]: + return parse_qs(urlparse(responses.calls[index].request.url).query) + + @responses.activate + def test_a_first_release_reports_recent_commits(self) -> None: + """With no previous release there is nothing to compare against.""" + self._stub_commits(_commit("b"), _commit("a")) + self._stub_files("b") + self._stub_files("a") + + commits = self.provider.compare_commits(self.repo, None, "b") + + assert [c["id"] for c in commits] == ["a", "b"] + # Recent commits are read from the head, with no comparison call. + assert self._query(0) == {"sha": ["b"], "pageSize": ["20"]} + assert "/compare/" not in "".join(call.request.url for call in responses.calls) + + @responses.activate + def test_a_range_walks_back_as_far_as_the_comparison_counts(self) -> None: + self._stub_compare(ahead_by=2) + self._stub_commits(_commit("c"), _commit("b")) + self._stub_files("c") + self._stub_files("b") + + commits = self.provider.compare_commits(self.repo, "a", "b") + + assert [c["id"] for c in commits] == ["b", "c"] + assert self._query(1) == {"sha": ["b"], "pageSize": ["2"]} + + @responses.activate + def test_an_unchanged_range_reads_no_commits(self) -> None: + self._stub_compare(ahead_by=0, status_name="identical") + + assert self.provider.compare_commits(self.repo, "a", "b") == [] + + assert len(responses.calls) == 1 + + @responses.activate + def test_a_long_range_is_capped(self) -> None: + """Each commit's file list costs 5 points of an installation's 3,000 per minute.""" + self._stub_compare(ahead_by=900) + # A page is PAGE_SIZE at most however large the range is, so the cap can only be + # seen in where the walk stops. + for token in ("page-2", ""): + responses.add( + responses.GET, + f"{CURSOR_ORIGIN_API_BASE_URL}/repos/{REPO}/commits", + json={ + "commits": [_commit(f"{token}-{i}") for i in range(PAGE_SIZE)], + "nextPageToken": token, + }, + ) + responses.add( + responses.GET, + re.compile(rf"{re.escape(CURSOR_ORIGIN_API_BASE_URL)}/repos/.+/commits/.+/files"), + json={"files": [], "nextPageToken": ""}, + ) + + with self.options({MAX_COMPARE_COMMITS_OPTION_KEY: 150}): + commits = self.provider.compare_commits(self.repo, "a", "b") + + assert len(commits) == 150 + commit_reads = [ + call for call in responses.calls if call.request.url.split("?")[0].endswith("/commits") + ] + assert len(commit_reads) == 2 + assert parse_qs(urlparse(commit_reads[0].request.url).query)["pageSize"] == [str(PAGE_SIZE)] + + @responses.activate + def test_the_patch_set_carries_every_kind_of_change(self) -> None: + self._stub_compare(ahead_by=1) + self._stub_commits(_commit("b")) + self._stub_files( + "b", + _file("src/edited.py", "modified"), + _file("src/added.py", "added"), + _file("src/gone.py", "removed"), + _file("src/new_name.py", "renamed", previous="src/old_name.py"), + _file("src/copy.py", "copied"), + ) + + commits = self.provider.compare_commits(self.repo, "a", "b") + + assert commits[0]["patch_set"] == [ + {"path": "src/edited.py", "type": "M"}, + {"path": "src/added.py", "type": "A"}, + {"path": "src/gone.py", "type": "D"}, + {"path": "src/old_name.py", "type": "D"}, + {"path": "src/new_name.py", "type": "A"}, + {"path": "src/copy.py", "type": "A"}, + ] + + @responses.activate + def test_a_commit_carries_the_author_and_message(self) -> None: + self._stub_compare(ahead_by=1) + self._stub_commits(_commit("b", message="fix: a thing")) + self._stub_files("b") + + commit = self.provider.compare_commits(self.repo, "a", "b")[0] + + assert commit["id"] == "b" + assert commit["repository"] == REPO + assert commit["author_email"] == "dev@example.com" + assert commit["author_name"] == "A Dev" + assert commit["message"] == "fix: a thing" + assert commit["timestamp"].isoformat() == "2026-09-16T12:00:00+00:00" + + @responses.activate + def test_a_long_author_name_is_truncated_to_the_column(self) -> None: + """`set_commits` truncates the email but not the name, and the column stops at 128.""" + long_name = "A" * 200 + commit = _commit("b") + commit["commit"]["author"]["name"] = long_name + self._stub_compare(ahead_by=1) + self._stub_commits(commit) + self._stub_files("b") + + assert self.provider.compare_commits(self.repo, "a", "b")[0]["author_name"] == "A" * 128 + + @responses.activate + def test_a_failure_is_raised_as_an_integration_error(self) -> None: + responses.add( + responses.GET, + f"{CURSOR_ORIGIN_API_BASE_URL}/repos/{REPO}/compare/a...b", + json={"code": 5, "message": "not found"}, + status=404, + ) + + with pytest.raises(IntegrationError): + self.provider.compare_commits(self.repo, "a", "b") From 7b78ab5bbc11636eec4adb1453705d86976bf63d Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 18 Sep 2026 11:37:34 -0700 Subject: [PATCH 2/2] ref(cursor-origin): Type the commits compare_commits returns Add `CommitData` and `CommitPatchFile` beside `RepositoryConfig`, and annotate `compare_commits`, `_format_commit` and `_patch_set` with them. The base signature stays `Sequence[Mapping[str, Any]]`, so no other provider changes. `Literal["A", "D", "M"]` matches `CommitFileChange.type`. Typing the timestamp also caught that `format_date` returns None for an empty date, which `set_commits` sorts on, so it would raise rather than store a null. --- .../integrations/cursor_origin/repository.py | 19 ++++++++++----- .../providers/integration_repository.py | 23 +++++++++++++++++-- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/sentry/integrations/cursor_origin/repository.py b/src/sentry/integrations/cursor_origin/repository.py index 7acb99c5608e..e4f0cc0a39b5 100644 --- a/src/sentry/integrations/cursor_origin/repository.py +++ b/src/sentry/integrations/cursor_origin/repository.py @@ -4,6 +4,8 @@ from collections.abc import Mapping, MutableMapping, Sequence from typing import Any +from django.utils import timezone + from sentry import options from sentry.integrations.cursor_origin.client import ( CursorOriginApiClient, @@ -18,7 +20,11 @@ from sentry.models.repository import Repository from sentry.organizations.services.organization.model import RpcOrganization from sentry.plugins.providers import IntegrationRepositoryProvider -from sentry.plugins.providers.integration_repository import RepositoryConfig +from sentry.plugins.providers.integration_repository import ( + CommitData, + CommitPatchFile, + RepositoryConfig, +) from sentry.shared_integrations.exceptions import ApiError, IntegrationError logger = logging.getLogger("sentry.integrations.cursor_origin") @@ -65,7 +71,7 @@ def repository_external_slug(self, repo: Repository) -> str: def compare_commits( self, repo: Repository, start_sha: str | None, end_sha: str - ) -> Sequence[Mapping[str, Any]]: + ) -> Sequence[CommitData]: installation = self.get_installation(repo.integration_id, repo.organization_id) client = installation.get_client() name = repo.config["name"] @@ -112,7 +118,7 @@ def _commits_in_range( def _format_commit( self, client: CursorOriginApiClient, name: str, commit: OriginCommit - ) -> Mapping[str, Any]: + ) -> CommitData: author = commit["commit"]["author"] return { "id": commit["sha"], @@ -120,13 +126,14 @@ def _format_commit( "author_email": author["email"], "author_name": author["name"][:128], "message": commit["commit"]["message"], - "timestamp": self.format_date(author["date"]), + # `format_date` gives None for an empty date, and `set_commits` sorts on this. + "timestamp": self.format_date(author["date"]) or timezone.now(), "patch_set": self._patch_set(client.get_commit_files(name, commit["sha"])), } - def _patch_set(self, files: Sequence[OriginCommitFile]) -> Sequence[Mapping[str, str]]: + def _patch_set(self, files: Sequence[OriginCommitFile]) -> list[CommitPatchFile]: """File changes in the shape `Release.set_commits` expects.""" - changes: list[Mapping[str, str]] = [] + changes: list[CommitPatchFile] = [] for file in files: status = file["status"] if status == "modified": diff --git a/src/sentry/plugins/providers/integration_repository.py b/src/sentry/plugins/providers/integration_repository.py index 6fc49568d503..bf93cfe8107d 100644 --- a/src/sentry/plugins/providers/integration_repository.py +++ b/src/sentry/plugins/providers/integration_repository.py @@ -2,8 +2,8 @@ import logging from collections.abc import Mapping, MutableMapping, Sequence -from datetime import timezone -from typing import Any, ClassVar, Generic, TypedDict, TypeVar, cast +from datetime import datetime, timezone +from typing import Any, ClassVar, Generic, Literal, TypedDict, TypeVar, cast from dateutil.parser import parse as parse_date from rest_framework import status @@ -38,6 +38,25 @@ class RepositoryConfig(TypedDict): integration_id: int +class CommitPatchFile(TypedDict): + """One file a commit touched. `type` is a `CommitFileChange.type` choice.""" + + path: str + type: Literal["A", "D", "M"] + + +class CommitData(TypedDict): + """A commit in the shape `Release.set_commits` consumes.""" + + id: str + repository: str + author_email: str + author_name: str + message: str + timestamp: datetime + patch_set: Sequence[CommitPatchFile] + + class RepoExistsError(SentryAPIException): status_code = status.HTTP_400_BAD_REQUEST code = "repo_exists"