Skip to content

chore(repositories): Switch over queries for SeerProjectRepository to use ProjectRepository#115456

Open
wedamija wants to merge 3 commits into
masterfrom
danf/repository-project-seerprojectrepo
Open

chore(repositories): Switch over queries for SeerProjectRepository to use ProjectRepository#115456
wedamija wants to merge 3 commits into
masterfrom
danf/repository-project-seerprojectrepo

Conversation

@wedamija
Copy link
Copy Markdown
Member

@wedamija wedamija commented May 12, 2026

This adds a feature flag that gates switching SeerProjectRepository queries to start using the new ProjectRepository table.

…to use `ProjectRepository`

This adds a feature flag that gates switching queries to start using the new `ProjectRepository` table.
@wedamija wedamija requested a review from a team May 12, 2026 23:14
@wedamija wedamija requested review from a team as code owners May 12, 2026 23:14
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 12, 2026
…transfer_to cleanup

- Remove duplicate ModelRelation(ProjectRepository) in
  RepositoryDeletionTask.get_child_relations since the shared
  _get_repository_child_relations helper already includes it.
- Add ProjectRepository cleanup to Project.transfer_to so project-repo
  links are removed when a project moves to a different org.
@wedamija wedamija requested review from a team as code owners May 12, 2026 23:16
Comment thread src/sentry/seer/autofix/utils.py
Comment thread src/sentry/seer/autofix/utils.py
repository_id__in=repo_ids,
project__organization_id=organization_id,
).delete()
org = Organization.objects.get(id=organization_id)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Organization.objects.get() raises DoesNotExist and aborts integration cleanup transaction

Line 247 introduces an unguarded Organization.objects.get(id=organization_id) solely to evaluate a feature flag. If the organization has been deleted or is mid-deletion when this RPC runs (e.g., during cascading integration cleanup), it raises Organization.DoesNotExist, which rolls back the surrounding transaction.atomic block and skips the remaining cleanup of ProjectCodeOwners, RepositoryProjectPathConfig, ProjectRepository, and ProjectOption. This leaves orphaned integration data behind.

Verification

Read the full disassociate_organization_integration method (lines 229-291). Confirmed (1) the method is wrapped in transaction.atomic, so a DoesNotExist exception bubbles up and rolls back unrelated cleanup work performed later in the same block; (2) organization_id originates from RPC caller input with no prior validation in this function; (3) features.has() accepts an Organization object — using Organization.objects.filter(id=organization_id).first() with a None fallback would still allow flag evaluation (defaulting to False for unknown orgs) without aborting cleanup. (4) The caller in organizationintegration.py:44-46 explicitly documents this scenario can happen ("This can happen when an organization has been deleted already") and catches CellMappingNotFound, but does not catch Organization.DoesNotExist, confirming this exception will abort the deletion task.

Identified by Warden sentry-backend-bugs · P3Z-ATR

- _write_preferences_to_sentry_db delete: filter by
  project_repository__project_id and project_repository__repository__status
- organization_seer_onboarding_check: filter by
  project_repository__project__organization_id and status fields
- night_shift cron: add select_related for project_repository__project
  (query filter stays on direct FK since cron is cross-org)
project_repository__project=project,
project_repository__repository__status=ObjectStatus.ACTIVE,
)
.select_related("project_repository", "project_repository__repository")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

N+1 queries from missing select_related("repository") on new path

Medium Severity

When the feature flag is enabled, the new query paths use select_related("project_repository", "project_repository__repository") but the downstream build_repo_definition_from_project_repo function accesses seer_project_repo.repository — the direct FK, not the one traversed through project_repository. Since the direct repository FK is not included in select_related, every iteration triggers a lazy-load DB query, causing an N+1 regression. The old code path correctly used select_related("repository") to eagerly load this relation.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a49b4db. Configure here.

ModelRelation(Commit, {"repository_id": instance.id}),
ModelRelation(PullRequest, {"repository_id": instance.id}),
ModelRelation(RepositoryProjectPathConfig, {"repository_id": instance.id}),
ModelRelation(ProjectRepository, {"repository_id": instance.id}),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hidden repo cascades into unintended SeerProjectRepository deletion

High Severity

ProjectRepository is added to _get_repository_child_relations, which is shared between RepositoryDeletionTask and repository_cascade_delete_on_hide. The comment on line 37-38 explicitly states that SeerProjectRepository records must NOT be deleted when a repo is merely hidden. However, deleting ProjectRepository during a hide operation triggers ProjectRepositoryDeletionTask, whose child relations include SeerProjectRepository (by project_repository_id), effectively cascade-deleting the very records intended to be preserved. ProjectRepository likely belongs in RepositoryDeletionTask.get_child_relations only, alongside SeerProjectRepository.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a49b4db. Configure here.

Comment on lines +659 to +667
if features.has("organizations:project-repository-fk-reads", project.organization):
seer_project_repo_qs = (
SeerProjectRepository.objects.filter(
project_repository__project=project,
project_repository__repository__status=ObjectStatus.ACTIVE,
)
.select_related("project_repository", "project_repository__repository")
.prefetch_related("branch_overrides")
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The new feature-flagged code path in read_preference_from_sentry_db and bulk_read_preferences_from_sentry_db is missing select_related("repository"), causing an N+1 query issue.
Severity: MEDIUM

Suggested Fix

Add "repository" to the select_related(...) call in the querysets for read_preference_from_sentry_db and bulk_read_preferences_from_sentry_db within the new feature-flagged code path. This will ensure the repository data is prefetched, resolving the N+1 query.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/seer/autofix/utils.py#L659-L667

Potential issue: When the `organizations:project-repository-fk-reads` feature flag is
enabled, the `read_preference_from_sentry_db` and `bulk_read_preferences_from_sentry_db`
functions do not prefetch the `repository` object via `select_related`. However, the
helper function `build_repo_definition_from_project_repo` accesses
`seer_project_repo.repository`, which triggers an additional database query for each
`SeerProjectRepository` being processed. This introduces a classic N+1 query problem in
the new code path, which can lead to significant performance degradation when handling
multiple repositories.

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 173ac04. Configure here.

SeerProjectRepository.objects.filter(
project_repository__project_id__in=project_ids,
project_repository__repository__status=ObjectStatus.ACTIVE,
).delete()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Flag-gated delete misses records with null FK

Medium Severity

When the feature flag is on, the delete query filters through the nullable project_repository FK (project_repository__project_id__in=project_ids). Any SeerProjectRepository rows where project_repository is NULL won't match this join and will survive the delete. The subsequent bulk_create then attempts to insert rows with the same (project, repository) pair, violating the unique_together constraint and raising an IntegrityError that rolls back the entire transaction. No backfill migration was found to ensure project_repository is populated before the flag is enabled.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 173ac04. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant