diff --git a/src/sentry/seer/agent/tools.py b/src/sentry/seer/agent/tools.py index 46adc0fd3d8b..1d1205d9134b 100644 --- a/src/sentry/seer/agent/tools.py +++ b/src/sentry/seer/agent/tools.py @@ -25,7 +25,8 @@ from sentry.api.serializers.models.groupactionlogentry import get_serialized_activity_items from sentry.api.utils import MAX_STATS_PERIOD, default_start_end_dates, get_date_range_from_params from sentry.constants import ALL_ACCESS_PROJECT_ID, ObjectStatus -from sentry.integrations.services.integration import integration_service +from sentry.integrations.services.integration import RpcIntegration, integration_service +from sentry.integrations.types import IntegrationProviderSlug from sentry.issues.action_log.types import GroupActionType, GroupActorType from sentry.issues.formatting.formatter import Format from sentry.issues.formatting.limits import LIMITS_DEFAULT, LIMITS_LOW @@ -932,6 +933,52 @@ def rpc_get_profile_flamegraph( ) +def _account_path(integration: RpcIntegration) -> str: + """The path an account's repository paths start with, so what a lookup should be given: + "acme-labs" for "acme-labs/app". It's the account's name, except on GitLab, where the name is + the group's display name ("My Group / Sub") and the path is "my-group/sub".""" + if integration.provider == IntegrationProviderSlug.GITLAB.value: + return integration.metadata.get("domain_name", "").partition("/")[2] + return integration.name + + +def _starts_with(segments: list[str], prefix: list[str]) -> bool: + """Whether ``segments`` start with all of ``prefix``, with more after it.""" + return 0 < len(prefix) < len(segments) and segments[: len(prefix)] == prefix + + +def _loose_segments(name: str) -> list[str]: + """A name's segments, ignoring a leading "https://", spaces around each "/", and case.""" + without_scheme = name.split("://", 1)[-1] + return [segment.strip().casefold() for segment in without_scheme.split("/") if segment.strip()] + + +def _is_under_account(repo_full_name: str, integration: RpcIntegration) -> bool: + """Whether a repository name is under the integration's account, written the way a lookup + reads names: the account's path in any case ("ACME-Labs/app"), or on GitLab, the group's + display name as it is ("My Group / Sub / App"). A lookup that misses such a name means Sentry + has no such repository.""" + account_path = _account_path(integration).casefold().split("/") + if _starts_with(repo_full_name.casefold().split("/"), account_path): + return True + return integration.provider == IntegrationProviderSlug.GITLAB.value and _starts_with( + repo_full_name.split(" / "), integration.name.split(" / ") + ) + + +def _may_be_under_account(repo_full_name: str, integration: RpcIntegration) -> bool: + """Whether a repository name is under the integration's account once spaces and case are + ignored ("My Group/Sub/App"), or past the account's host ("github.com/acme-labs/app"). A lookup + misses names written like this even when Sentry has the repository.""" + repo_segments = _loose_segments(repo_full_name) + url = _loose_segments(integration.metadata.get("domain_name", "")) + # The URL with its host ("github.com/acme-labs") and without it ("acme-labs"). + for account in (_loose_segments(integration.name), url, url[1:]): + if _starts_with(repo_segments, account): + return True + return False + + def _find_repository( organization_id: int, repo_full_name: str, providers: list[str] ) -> Repository | None: @@ -952,7 +999,7 @@ def _find_repository( def _repository_lookup_failure( - organization_id: int, supported_providers: list[str] + organization_id: int, repo_full_name: str, supported_providers: list[str] ) -> RepositoryDefinitionFailure | None: """Why a lookup found nothing, when it's known.""" integrations = integration_service.get_integrations( @@ -964,7 +1011,25 @@ def _repository_lookup_failure( ) if not integrations: return RepositoryDefinitionFailure(failure_reason="no_scm_integration") - return None + + # A name with no owner can't be under any account, so there's nothing to say about it. + if len(_loose_segments(repo_full_name)) < 2: + return None + under_connected_account = any( + _is_under_account(repo_full_name, integration) for integration in integrations + ) + # The lookup may have missed only because of how the name is written, so the reason isn't known. + if not under_connected_account and any( + _may_be_under_account(repo_full_name, integration) for integration in integrations + ): + return None + return RepositoryDefinitionFailure( + failure_reason=( + "repo_not_in_sentry" if under_connected_account else "scm_account_not_connected" + ), + # What the agent can retry with, once each. Sorted: integrations come in no set order. + connected_accounts=sorted({_account_path(integration) for integration in integrations}), + ) def get_repository_definition( @@ -988,8 +1053,10 @@ def get_repository_definition( external_id: Optional external repository ID (e.g., GitHub repo ID). Stable across renames. If provided, this is used for lookup instead of name. report_failure_reason: When nothing is found, say why in the response when it's known, - instead of returning None. Opt-in because callers that don't ask would read - that response as a repository. + instead of returning None: no source control connected, the repository's + account isn't connected, or its account is connected but the repository isn't + in Sentry. Opt-in because callers that don't ask would read that response as a + repository. Returns: dict with RepoDefinition fields if found, None otherwise. Includes external_id @@ -1031,7 +1098,7 @@ def get_repository_definition( }, ) if report_failure_reason: - return _repository_lookup_failure(organization_id, supported_providers) + return _repository_lookup_failure(organization_id, repo_full_name, supported_providers) return None # Use the actual repo name from the database, not the requested name. diff --git a/src/sentry/seer/sentry_data_models.py b/src/sentry/seer/sentry_data_models.py index a0a5c12f338e..e5f98a355808 100644 --- a/src/sentry/seer/sentry_data_models.py +++ b/src/sentry/seer/sentry_data_models.py @@ -276,7 +276,8 @@ class RepositoryDefinitionResponse(BaseModel): class RepositoryDefinitionFailure(BaseModel): - failure_reason: Literal["no_scm_integration"] + failure_reason: Literal["no_scm_integration", "scm_account_not_connected", "repo_not_in_sentry"] + connected_accounts: list[str] = [] class EventFilterKeyEntry(BaseModel): diff --git a/tests/sentry/seer/agent/test_tools.py b/tests/sentry/seer/agent/test_tools.py index 7e8947b2234b..3e2a88b39a36 100644 --- a/tests/sentry/seer/agent/test_tools.py +++ b/tests/sentry/seer/agent/test_tools.py @@ -3049,6 +3049,16 @@ def test_get_recommended_event_fallback_if_no_events_with_spans_in_clamped_range assert result.event_id == event2.event_id +# Each provider Seer reads code from: its integration's name and domain, and the path its +# repository names start with. +SEER_SCM_ACCOUNTS = [ + ("github", "acme-labs", "github.com/acme-labs", "acme-labs"), + ("github_enterprise", "acme-labs", "ghe.example.com/acme-labs", "acme-labs"), + ("gitlab", "My Group / Sub", "gitlab.com/my-group/sub", "my-group/sub"), + ("cursor_origin", "acme-labs", "https://cursor.com/codebase/acme-labs", "acme-labs"), +] + + class TestGetRepositoryDefinition(APITransactionTestCase): def test_get_repository_definition_success(self) -> None: """Test successful repository lookup""" @@ -3144,12 +3154,15 @@ def _lookup(self, organization: Organization, repo_full_name: str) -> Any: report_failure_reason=True, ) - def _connect(self, organization: Organization, provider: str) -> None: + def _connect( + self, organization: Organization, provider: str, name: str = "", domain_name: str = "" + ) -> None: self.create_integration( organization=organization, provider=provider, external_id=f"{provider}:{organization.id}", - name=provider, + name=name, + metadata={"domain_name": domain_name}, ) @with_feature("organizations:seer-cursor-origin-support") @@ -3188,12 +3201,55 @@ def test_cursor_origin_does_not_count_without_seer_support(self) -> None: ) @with_feature("organizations:seer-cursor-origin-support") - def test_no_failure_reason_when_source_control_is_connected(self) -> None: - for provider in ("github", "github_enterprise", "gitlab", "cursor_origin"): + def test_failure_for_each_provider(self) -> None: + for provider, name, domain_name, path in SEER_SCM_ACCOUNTS: organization = self.create_organization() - self._connect(organization, provider) + self._connect(organization, provider, name, domain_name) + not_connected = RepositoryDefinitionFailure( + failure_reason="scm_account_not_connected", connected_accounts=[path] + ) + not_in_sentry = RepositoryDefinitionFailure( + failure_reason="repo_not_in_sentry", connected_accounts=[path] + ) with self.subTest(provider=provider): - assert self._lookup(organization, "acme-labs/app") is None + assert self._lookup(organization, "other-org/app") == not_connected + # One that only starts the same isn't that account. + assert self._lookup(organization, f"{path}-2/app") == not_connected + assert self._lookup(organization, f"{path}/app") == not_in_sentry + assert self._lookup(organization, f"{path.upper()}/app") == not_in_sentry + # The lookup misses a name with the host in front even when Sentry has the + # repository, so it can't say why. Another account's is still not connected. + assert self._lookup(organization, f"{domain_name}/app") is None + assert self._lookup(organization, f"{domain_name}-2/app") == not_connected + # A name with no owner can't be under any account. + assert self._lookup(organization, "invalid-format") is None + + def test_failure_for_gitlab_group_names(self) -> None: + self._connect(self.organization, "gitlab", "My Group / Sub", "gitlab.com/my-group/sub") + + for repo_name in ("My Group / Sub / App", "my-group/sub/nested/app"): + result = self._lookup(self.organization, repo_name) + assert result.failure_reason == "repo_not_in_sentry" + # The lookup reads a display name only as it is, so it misses these even when Sentry has + # the repository, and can't say why. + for repo_name in ("My Group/Sub/App", "my group / sub / app"): + assert self._lookup(self.organization, repo_name) is None + + def test_failure_when_the_account_has_no_url(self) -> None: + self._connect(self.organization, "github", "acme-labs") + + result = self._lookup(self.organization, "other-org/app") + assert result.failure_reason == "scm_account_not_connected" + + @with_feature("organizations:seer-cursor-origin-support") + def test_failure_checks_every_connected_account(self) -> None: + for provider, name, domain_name, _ in SEER_SCM_ACCOUNTS: + self._connect(self.organization, provider, name, domain_name) + + result = self._lookup(self.organization, "my-group/sub/app") + assert result.failure_reason == "repo_not_in_sentry" + # Each account once, as the path a lookup reads. + assert result.connected_accounts == ["acme-labs", "my-group/sub"] def test_get_repository_definition_not_found(self) -> None: """Test that nonexistent repository returns None"""