fix(autofix): Dual-delete Seer preferences for disabled repositories

src/sentry/integrations/services/repository/impl.py

@@ -128,15 +128,60 @@ def update_repositories(self, *, organization_id: int, updates: list[RpcReposito
 
             Repository.objects.bulk_update(repositories, fields=list(fields_to_update))
 
+    def _cleanup_seer_project_repositories(
+        self,
+        organization_id: int,
+        repo_tuples: list[tuple[int, str | None, str | None]],
+    ) -> None:
+        """Delete SeerProjectRepository rows and schedule Seer API cleanup.
+
+        Must be called inside a transaction.atomic() block. The SeerProjectRepository
+        delete participates in the transaction, while the Seer API task is dispatched
+        via on_commit so it only fires if the transaction succeeds.
+
+        repo_tuples: list of (repo_id, external_id, provider) from Repository.values_list().
+        """
+        try:
+            organization = Organization.objects.get_from_cache(id=organization_id)
+            if features.has("organizations:seer-project-settings-dual-write", organization):
+                SeerProjectRepository.objects.filter(
+                    repository_id__in=[repo_id for repo_id, _, _ in repo_tuples]
+                ).delete()
+        except Organization.DoesNotExist:
+            pass
+
+        repos_to_clean = [
+            {"repo_external_id": external_id, "repo_provider": repo_provider}
+            for _, external_id, repo_provider in repo_tuples
+            if external_id and repo_provider
+        ]
+        if repos_to_clean:
+            transaction.on_commit(
+                lambda: bulk_cleanup_seer_repository_preferences.apply_async(
+                    kwargs={
+                        "organization_id": organization_id,
+                        "repos": repos_to_clean,
+                    }
+                ),
+                using=router.db_for_write(Repository),
+            )
+
     def disable_repositories_for_integration(
         self, *, organization_id: int, integration_id: int, provider: str
     ) -> None:
         with transaction.atomic(router.db_for_write(Repository)):
-            Repository.objects.filter(
-                organization_id=organization_id,
-                integration_id=integration_id,
-                provider=provider,
-            ).update(status=ObjectStatus.DISABLED)
+            repos = list(
+                Repository.objects.filter(
+                    organization_id=organization_id,
+                    integration_id=integration_id,
+                    provider=provider,
+                ).values_list("id", "external_id", "provider")
+            )
+            repo_ids = [repo_id for repo_id, _, _ in repos]
+
+            if repo_ids:
+                Repository.objects.filter(id__in=repo_ids).update(status=ObjectStatus.DISABLED)
+                self._cleanup_seer_project_repositories(organization_id, repos)
 
     def disable_repositories_by_external_ids(
         self,
@@ -147,13 +192,20 @@ def disable_repositories_by_external_ids(
         external_ids: list[str],
     ) -> None:
         with transaction.atomic(router.db_for_write(Repository)):
-            Repository.objects.filter(
-                organization_id=organization_id,
-                integration_id=integration_id,
-                provider=provider,
-                external_id__in=external_ids,
-                status=ObjectStatus.ACTIVE,
-            ).update(status=ObjectStatus.DISABLED)
+            repos = list(
+                Repository.objects.filter(
+                    organization_id=organization_id,
+                    integration_id=integration_id,
+                    provider=provider,
+                    external_id__in=external_ids,
+                    status=ObjectStatus.ACTIVE,
+                ).values_list("id", "external_id", "provider")
+            )
+            repo_ids = [repo_id for repo_id, _, _ in repos]
+
+            if repo_ids:
+                Repository.objects.filter(id__in=repo_ids).update(status=ObjectStatus.DISABLED)
+                self._cleanup_seer_project_repositories(organization_id, repos)
 
     def disassociate_organization_integration(
         self,
@@ -169,21 +221,10 @@ def disassociate_organization_integration(
                 ).values_list("id", "external_id", "provider")
             )
             repo_ids = [repo_id for repo_id, _, _ in repos]
-
             if repo_ids:
                 # Disassociate repos from the organization integration being deleted
                 Repository.objects.filter(id__in=repo_ids).update(integration_id=None)
-
-                # Delete Seer project preferences for the affected repos.
-                # Organization may already be deleted if org deletion and integration
-                # uninstall overlap; skip SeerProjectRepository cleanup in that case
-                # since cascades will handle it.
-                try:
-                    organization = Organization.objects.get_from_cache(id=organization_id)
-                    if features.has("organizations:seer-project-settings-dual-write", organization):
-                        SeerProjectRepository.objects.filter(repository_id__in=repo_ids).delete()
-                except Organization.DoesNotExist:
-                    pass
+                self._cleanup_seer_project_repositories(organization_id, repos)
 
             # Delete Code Owners with a Code Mapping using the OrganizationIntegration
             ProjectCodeOwners.objects.filter(
@@ -196,17 +237,3 @@ def disassociate_organization_integration(
             RepositoryProjectPathConfig.objects.filter(
                 organization_integration_id=organization_integration_id
             ).delete()
-
-        # Delete Seer project preferences for the affected repos via Seer API
-        repos_to_clean = [
-            {"repo_external_id": external_id, "repo_provider": provider}
-            for _, external_id, provider in repos
-            if external_id and provider
-        ]
-        if repos_to_clean:
-            bulk_cleanup_seer_repository_preferences.apply_async(
-                kwargs={
-                    "organization_id": organization_id,
-                    "repos": repos_to_clean,
-                }
-            )

tests/sentry/integrations/github/test_webhook.py

@@ -455,7 +455,8 @@ def test_end_to_end_repos_added(self) -> None:
         assert repos[0].provider == "integrations:github"
         assert repos[1].name == "getsentry/snuba"
 
-    def test_end_to_end_repos_removed(self) -> None:
+    @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences")
+    def test_end_to_end_repos_removed(self, mock_seer_cleanup: MagicMock) -> None:
         """Full end-to-end: webhook URL → handler → task → Repository disabled."""
         future_expires = datetime.now().replace(microsecond=0) + timedelta(minutes=5)
         integration = self.create_integration(

tests/sentry/integrations/services/repository/test_impl.py

@@ -1,7 +1,14 @@
+from unittest.mock import MagicMock, patch
+
+import pytest
+
 from sentry.constants import ObjectStatus
+from sentry.integrations.models.repository_project_path_config import RepositoryProjectPathConfig
 from sentry.integrations.services.repository.service import repository_service
 from sentry.models.repository import Repository
+from sentry.seer.models.project_repository import SeerProjectRepository
 from sentry.testutils.cases import TestCase
+from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.silo import cell_silo_test
 
 
@@ -15,7 +22,8 @@ def setUp(self) -> None:
         )
         self.provider = "integrations:github"
 
-    def test_disables_matching_active_repos(self) -> None:
+    @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences")
+    def test_disables_matching_active_repos(self, mock_seer_cleanup: MagicMock) -> None:
         repo1 = Repository.objects.create(
             organization_id=self.organization.id,
             name="getsentry/sentry",
@@ -111,7 +119,8 @@ def test_does_not_affect_repos_from_other_orgs(self) -> None:
         repo.refresh_from_db()
         assert repo.status == ObjectStatus.ACTIVE
 
-    def test_only_disables_specified_external_ids(self) -> None:
+    @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences")
+    def test_only_disables_specified_external_ids(self, mock_seer_cleanup: MagicMock) -> None:
         repo_to_disable = Repository.objects.create(
             organization_id=self.organization.id,
             name="getsentry/sentry",
@@ -160,3 +169,192 @@ def test_empty_external_ids_is_noop(self) -> None:
 
         repo.refresh_from_db()
         assert repo.status == ObjectStatus.ACTIVE
+
+    @with_feature("organizations:seer-project-settings-dual-write")
+    @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences")
+    def test_cleans_up_seer_preferences(self, mock_cleanup: MagicMock) -> None:
+        project = self.create_project(organization=self.organization)
+        repo = Repository.objects.create(
+            organization_id=self.organization.id,
+            name="getsentry/sentry",
+            external_id="100",
+            provider=self.provider,
+            integration_id=self.integration.id,
+            status=ObjectStatus.ACTIVE,
+        )
+        SeerProjectRepository.objects.create(project=project, repository_id=repo.id)
+
+        repository_service.disable_repositories_by_external_ids(
+            organization_id=self.organization.id,
+            integration_id=self.integration.id,
+            provider=self.provider,
+            external_ids=["100"],
+        )
+
+        repo.refresh_from_db()
+        assert repo.status == ObjectStatus.DISABLED
+
+        assert not SeerProjectRepository.objects.filter(repository_id=repo.id).exists()
+        mock_cleanup.apply_async.assert_called_once_with(
+            kwargs={
+                "organization_id": self.organization.id,
+                "repos": [{"repo_external_id": "100", "repo_provider": self.provider}],
+            }
+        )
+
+
+@cell_silo_test
+class DisableRepositoriesForIntegrationTest(TestCase):
+    def setUp(self) -> None:
+        self.integration = self.create_integration(
+            organization=self.organization,
+            external_id="1",
+            provider="github",
+        )
+        self.provider = "integrations:github"
+
+    @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences")
+    def test_disables_matching_active_repos(self, mock_seer_cleanup: MagicMock) -> None:
+        repo = Repository.objects.create(
+            organization_id=self.organization.id,
+            name="getsentry/sentry",
+            external_id="100",
+            provider=self.provider,
+            integration_id=self.integration.id,
+            status=ObjectStatus.ACTIVE,
+        )
+
+        repository_service.disable_repositories_for_integration(
+            organization_id=self.organization.id,
+            integration_id=self.integration.id,
+            provider=self.provider,
+        )
+
+        repo.refresh_from_db()
+        assert repo.status == ObjectStatus.DISABLED
+
+    @with_feature("organizations:seer-project-settings-dual-write")
+    @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences")
+    def test_cleans_up_seer_preferences(self, mock_cleanup: MagicMock) -> None:
+        project = self.create_project(organization=self.organization)
+        repo = Repository.objects.create(
+            organization_id=self.organization.id,
+            name="getsentry/sentry",
+            external_id="100",
+            provider=self.provider,
+            integration_id=self.integration.id,
+            status=ObjectStatus.ACTIVE,
+        )
+        SeerProjectRepository.objects.create(project=project, repository_id=repo.id)
+
+        repository_service.disable_repositories_for_integration(
+            organization_id=self.organization.id,
+            integration_id=self.integration.id,
+            provider=self.provider,
+        )
+
+        repo.refresh_from_db()
+        assert repo.status == ObjectStatus.DISABLED
+
+        assert not SeerProjectRepository.objects.filter(repository_id=repo.id).exists()
+        mock_cleanup.apply_async.assert_called_once_with(
+            kwargs={
+                "organization_id": self.organization.id,
+                "repos": [{"repo_external_id": "100", "repo_provider": self.provider}],
+            }
+        )
+
+
+@cell_silo_test
+class DisassociateOrganizationIntegrationTest(TestCase):
+    def setUp(self) -> None:
+        self.integration = self.create_integration(
+            organization=self.organization,
+            external_id="1",
+            provider="github",
+        )
+        self.provider = "integrations:github"
+        self.org_integration = self.integration.organizationintegration_set.first()
+
+    @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences")
+    def test_disassociates_repos(self, mock_cleanup: MagicMock) -> None:
+        repo = Repository.objects.create(
+            organization_id=self.organization.id,
+            name="getsentry/sentry",
+            external_id="100",
+            provider=self.provider,
+            integration_id=self.integration.id,
+            status=ObjectStatus.ACTIVE,
+        )
+
+        repository_service.disassociate_organization_integration(
+            organization_id=self.organization.id,
+            organization_integration_id=self.org_integration.id,
+            integration_id=self.integration.id,
+        )
+
+        repo.refresh_from_db()
+        assert repo.integration_id is None
+        mock_cleanup.apply_async.assert_called_once()
+
+    @with_feature("organizations:seer-project-settings-dual-write")
+    @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences")
+    def test_cleans_up_seer_preferences(self, mock_cleanup: MagicMock) -> None:
+        project = self.create_project(organization=self.organization)
+        repo = Repository.objects.create(
+            organization_id=self.organization.id,
+            name="getsentry/sentry",
+            external_id="100",
+            provider=self.provider,
+            integration_id=self.integration.id,
+            status=ObjectStatus.ACTIVE,
+        )
+        SeerProjectRepository.objects.create(project=project, repository_id=repo.id)
+
+        repository_service.disassociate_organization_integration(
+            organization_id=self.organization.id,
+            organization_integration_id=self.org_integration.id,
+            integration_id=self.integration.id,
+        )
+
+        repo.refresh_from_db()
+        assert repo.integration_id is None
+        assert not SeerProjectRepository.objects.filter(repository_id=repo.id).exists()
+        mock_cleanup.apply_async.assert_called_once_with(
+            kwargs={
+                "organization_id": self.organization.id,
+                "repos": [{"repo_external_id": "100", "repo_provider": self.provider}],
+            }
+        )
+
+    @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences")
+    def test_transaction_rollback_does_not_dispatch_seer_cleanup(
+        self, mock_cleanup: MagicMock
+    ) -> None:
+        repo = Repository.objects.create(
+            organization_id=self.organization.id,
+            name="getsentry/sentry",
+            external_id="100",
+            provider=self.provider,
+            integration_id=self.integration.id,
+            status=ObjectStatus.ACTIVE,
+        )
+
+        with patch.object(
+            RepositoryProjectPathConfig.objects,
+            "filter",
+            side_effect=RuntimeError("simulated failure"),
+        ):
+            with pytest.raises(RuntimeError):
+                repository_service.disassociate_organization_integration(
+                    organization_id=self.organization.id,
+                    organization_integration_id=self.org_integration.id,
+                    integration_id=self.integration.id,
+                )
+
+        # Transaction rolled back: repo should still have its integration
+        repo.refresh_from_db()
+        assert repo.integration_id == self.integration.id
+
+        # Task should not have been dispatched
+        mock_cleanup.apply_async.assert_not_called()