Skip to content

feat(seer): Report an unconnected SCM account or missing repo on lookups #130316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
joseph-sentry wants to merge 1 commit into from
+99 −13
Open

feat(seer): Report an unconnected SCM account or missing repo on lookups #130316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 38 additions & 6 deletions src/sentry/seer/agent/tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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(
Expand All @@ -963,7 +980,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"
Comment on lines +984 to +993

@billyvg billyvg Nov 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Classifier normalizes names more loosely than the lookup, so misses get wrong reasons

_name_segments strips whitespace around each / and casefolds. _find_repository does neither: it matches name exactly and compares a url suffix. So a lookup that misses only because of how the name is written still gets a confident failure reason, and the reason is wrong:

  • GitLab display name without the spaces. Take My Group/Sub/App, where Sentry stores My Group / Sub / App with URL path my-group/sub/app. _find_repository misses it, because Q(name=...) is exact and the URL doesn't end with that string. Here, ["my group", "sub", "app"] starts with the integration name My Group / Sub, so the response is repo_not_in_sentry, but the repo is in Sentry. This input is realistic. Seer's own _normalize_repo_full_name (seer/automation/explorer/tools/utils.py) says the LLM passes the stripped form, and that Seer only recovers from it when it already has an external_id hint.
  • Host-qualified name. Take github.com/acme-labs/app. url__iendswith matches the repo as a candidate, but the Python check rejects it. Then ["github.com", "acme-labs", "app"] doesn't start with acme-labs, so the response is scm_account_not_connected. The agent would then ask the user to install the app on an account where it's already installed, for a repo Sentry already has. That is the misleading advice this PR is meant to prevent.

Fix: normalize the requested name once and use the result for both the lookup and the classification.

  • Whitespace: _find_repository could also try Q(name__iexact=" / ".join(segments)), since GitLab's name_with_namespace always joins with /, and compare segment-normalized names in its Python check.
  • Hosts: strip a leading scheme, and a first segment equal to a connected integration's host, before the lookup.
  • Smaller fix: if that's out of scope here, make _repository_lookup_failure return None when the first segment is a connected integration's host. In that case the reason isn't actually known, and None matches the function's "when it's known" contract.

Written by an AI agent and included in this review by its author.

),
connected_accounts=[integration.name for integration in integrations],

@billyvg billyvg Nov 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 connected_accounts gives GitLab display names, duplicates and no provider

The description says this list should help the agent correct its call argument. As written, it doesn't help much:

  • GitLab names: for GitLab, integration.name is the group's display name (My Group / Sub). It isn't the path (my-group/sub), which is what get_repository_definition returns as owner on success and what a retry should pass. An agent that builds My Group / Sub/app from this list still misses in _find_repository.
  • Duplicates: an account appears once per integration. The new test at test_tools.py:3228 compares a set because the list holds acme-labs three times. GitHub and Cursor Origin connected to the same account is a plausible real setup.
  • No provider: with the provider dropped, GitHub's acme-labs and Cursor Origin's acme-labs look the same.

Fix: report the prefix the agent should pass: the URL path for GitLab, and integration.name elsewhere. Don't derive it from domain_name for Cursor Origin, whose path is codebase/<slug>. Then deduplicate the list, add the provider if you want it, and make the test assert the exact list rather than a set.

Written by an AI agent and included in this review by its author.

@billyvg billyvg Nov 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 GitLab connected_accounts gives the agent a display name it can't reuse

connected_accounts lists integration.name. For GitLab that is the group's full_name, a display name like My Group / Sub (see gitlab/integration.py:799). The point of this field is to let the agent fix its slug and retry. The form it can retry with is the URL path (my-group/sub): RepositoryDefinitionResponse builds owner/name from get_repo_url_path, and _find_repository matches display names only by exact, case-sensitive name=. So My Group / Sub/app and my group / sub / app both still miss.

For GitHub, GHE and Cursor Origin, name already equals the path segment, so only GitLab is affected. You can't simply switch every provider to the path taken from domain_name, though. Cursor Origin's domain_name is https://cursor.com/codebase/<slug>, so the generic parse on L944 yields codebase/<slug> there. That also means the URL-path branch of _is_under_account never matches for Cursor Origin, which is classified through its name alone. Options:

  • report the group path for GitLab (metadata["domain_name"].partition("/")[2], the same parse GitlabIntegration.source_url_matches uses), or
  • report both forms, e.g. connected_accounts=[{"name": ..., "path": ...}], so the user-facing message can show the display name and the agent can retry with the path.

Seer doesn't read this field yet (no report_failure_reason callers on seer main), so the shape is still cheap to change.

Written by an AI agent and included in this review by its author.

)


def get_repository_definition(
Expand All @@ -987,8 +1017,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
Expand Down Expand Up @@ -1030,7 +1062,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.
Expand Down
3 changes: 2 additions & 1 deletion src/sentry/seer/sentry_data_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
65 changes: 59 additions & 6 deletions tests/sentry/seer/agent/test_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down Expand Up @@ -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")
Expand All @@ -3170,12 +3183,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"
Comment on lines +3211 to +3213

@billyvg billyvg Nov 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Not-connected GitLab cases repeat test_failure_for_each_provider

These two cases don't catch any mutant that test_failure_for_each_provider misses. Its GitLab subtest already checks my-group/sub-2/app → scm_account_not_connected. That fails on the same wrong implementations these would catch: comparing only the first segment, dropping the last account segment, or a plain startswith on the joined strings. The two repo_not_in_sentry cases above them do add coverage (the display-name form tests .strip(), and nested/app tests deeper repos), so keep those and drop this loop.

The same applies to the final other-org/app assertion in test_failure_checks_every_connected_account (L3230–3231). That test earns its place with the first assertion, which fails if only one integration is checked.

Written by an AI agent and included in this review by its author.


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_only_reports_the_reason_when_asked(self) -> None:
assert (
Expand Down
Loading