From b9cc7f4d2ebcfd20730e748cbf8d7f9359cff36e Mon Sep 17 00:00:00 2001 From: joseph-sentry Date: Fri, 6 Nov 2026 16:42:57 -0500 Subject: [PATCH 1/2] feat(seer): Report an unconnected SCM account or missing repo on lookups we're adding more types of failures on repo lookups: - missing repo - unconnected account (there's no integration that matches the provided SCM org) this is so when the seer agent calls these tools with these misconfigurations we can offer accurate information to the user: - missing repo: are you sure your integration has access to this repo, or is the agent using the wrong slug - unconnected account: please make sure to install the app on this org, or again correct the agent's call arg --- src/sentry/seer/agent/tools.py | 44 +++++++++++++++--- src/sentry/seer/sentry_data_models.py | 3 +- tests/sentry/seer/agent/test_tools.py | 65 ++++++++++++++++++++++++--- 3 files changed, 99 insertions(+), 13 deletions(-) diff --git a/src/sentry/seer/agent/tools.py b/src/sentry/seer/agent/tools.py index 46adc0fd3d8b..0c681a2e203b 100644 --- a/src/sentry/seer/agent/tools.py +++ b/src/sentry/seer/agent/tools.py @@ -25,7 +25,7 @@ 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.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 +932,23 @@ def rpc_get_profile_flamegraph( ) +def _name_segments(name: str) -> list[str]: + return [segment.strip().casefold() for segment in name.split("/") if segment.strip()] + + +def _is_under_account(repo_segments: list[str], integration: RpcIntegration) -> bool: + """Whether a repository name starts with the integration's account, by the account's name or + its URL path. GitLab needs both: its repository names can start with the group's display name + ("My Group / Sub") or its path ("my-group/sub").""" + domain_name = integration.metadata.get("domain_name", "") + url_path = domain_name.removeprefix("https://").partition("/")[2] + for account_name in (integration.name, url_path): + account = _name_segments(account_name) + if 0 < len(account) < len(repo_segments) and repo_segments[: len(account)] == account: + return True + return False + + def _find_repository( organization_id: int, repo_full_name: str, providers: list[str] ) -> Repository | None: @@ -952,7 +969,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 +981,20 @@ def _repository_lookup_failure( ) if not integrations: return RepositoryDefinitionFailure(failure_reason="no_scm_integration") - return None + + repo_segments = _name_segments(repo_full_name) + # A name with no owner can't be under any account, so there's nothing to say about it. + if len(repo_segments) < 2: + return None + under_connected_account = any( + _is_under_account(repo_segments, integration) for integration in integrations + ) + return RepositoryDefinitionFailure( + failure_reason=( + "repo_not_in_sentry" if under_connected_account else "scm_account_not_connected" + ), + connected_accounts=[integration.name for integration in integrations], + ) def get_repository_definition( @@ -988,8 +1018,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 +1063,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..8b2d88b4c423 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,52 @@ 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=[name] + ) + not_in_sentry = RepositoryDefinitionFailure( + failure_reason="repo_not_in_sentry", connected_accounts=[name] + ) 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 + # 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" + for repo_name in ("my-group/other/app", "my-group/app"): + result = self._lookup(self.organization, repo_name) + assert result.failure_reason == "scm_account_not_connected" + + 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" + assert set(result.connected_accounts) == {"acme-labs", "My Group / Sub"} + + result = self._lookup(self.organization, "other-org/app") + assert result.failure_reason == "scm_account_not_connected" def test_get_repository_definition_not_found(self) -> None: """Test that nonexistent repository returns None""" From 077540d4f55a66436243e64cad6fbf95a791bc40 Mon Sep 17 00:00:00 2001 From: joseph-sentry Date: Fri, 13 Nov 2026 13:21:32 -0500 Subject: [PATCH 2/2] fix(seer): Give repo lookup failure reasons only for names the lookup reads The failure reason trimmed spaces and ignored case, but the lookup doesn't. So a repository Sentry has could get the wrong reason: "My Group/Sub/App" got repo_not_in_sentry, and "github.com/acme-labs/app" got scm_account_not_connected. Now a name that matches an account only after trimming spaces or case, or after dropping a host in front, gets no reason. A host in front of an account that isn't connected still gets scm_account_not_connected. connected_accounts now lists what the agent can retry with: the group path on GitLab ("my-group/sub", not "My Group / Sub"), and the account name elsewhere. Each appears once, sorted. Also drop test cases that test_failure_for_each_provider already covers. --- src/sentry/seer/agent/tools.py | 65 ++++++++++++++++++++------- tests/sentry/seer/agent/test_tools.py | 21 +++++---- 2 files changed, 62 insertions(+), 24 deletions(-) diff --git a/src/sentry/seer/agent/tools.py b/src/sentry/seer/agent/tools.py index 0c681a2e203b..1d1205d9134b 100644 --- a/src/sentry/seer/agent/tools.py +++ b/src/sentry/seer/agent/tools.py @@ -26,6 +26,7 @@ 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 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,19 +933,48 @@ def rpc_get_profile_flamegraph( ) -def _name_segments(name: str) -> list[str]: - return [segment.strip().casefold() for segment in name.split("/") if segment.strip()] +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 _is_under_account(repo_segments: list[str], integration: RpcIntegration) -> bool: - """Whether a repository name starts with the integration's account, by the account's name or - its URL path. GitLab needs both: its repository names can start with the group's display name - ("My Group / Sub") or its path ("my-group/sub").""" - domain_name = integration.metadata.get("domain_name", "") - url_path = domain_name.removeprefix("https://").partition("/")[2] - for account_name in (integration.name, url_path): - account = _name_segments(account_name) - if 0 < len(account) < len(repo_segments) and repo_segments[: len(account)] == account: +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 @@ -982,18 +1012,23 @@ def _repository_lookup_failure( if not integrations: return RepositoryDefinitionFailure(failure_reason="no_scm_integration") - repo_segments = _name_segments(repo_full_name) # A name with no owner can't be under any account, so there's nothing to say about it. - if len(repo_segments) < 2: + if len(_loose_segments(repo_full_name)) < 2: return None under_connected_account = any( - _is_under_account(repo_segments, integration) for integration in integrations + _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" ), - connected_accounts=[integration.name for integration in integrations], + # 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}), ) diff --git a/tests/sentry/seer/agent/test_tools.py b/tests/sentry/seer/agent/test_tools.py index 8b2d88b4c423..3e2a88b39a36 100644 --- a/tests/sentry/seer/agent/test_tools.py +++ b/tests/sentry/seer/agent/test_tools.py @@ -3206,10 +3206,10 @@ def test_failure_for_each_provider(self) -> None: organization = self.create_organization() self._connect(organization, provider, name, domain_name) not_connected = RepositoryDefinitionFailure( - failure_reason="scm_account_not_connected", connected_accounts=[name] + failure_reason="scm_account_not_connected", connected_accounts=[path] ) not_in_sentry = RepositoryDefinitionFailure( - failure_reason="repo_not_in_sentry", connected_accounts=[name] + failure_reason="repo_not_in_sentry", connected_accounts=[path] ) with self.subTest(provider=provider): assert self._lookup(organization, "other-org/app") == not_connected @@ -3217,6 +3217,10 @@ def test_failure_for_each_provider(self) -> None: 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 @@ -3226,9 +3230,10 @@ def test_failure_for_gitlab_group_names(self) -> None: 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" - for repo_name in ("my-group/other/app", "my-group/app"): - result = self._lookup(self.organization, repo_name) - assert result.failure_reason == "scm_account_not_connected" + # 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") @@ -3243,10 +3248,8 @@ def test_failure_checks_every_connected_account(self) -> None: result = self._lookup(self.organization, "my-group/sub/app") assert result.failure_reason == "repo_not_in_sentry" - assert set(result.connected_accounts) == {"acme-labs", "My Group / Sub"} - - result = self._lookup(self.organization, "other-org/app") - assert result.failure_reason == "scm_account_not_connected" + # 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"""