From 92cf3a94e75a7e351850817819b48df7bef96aea Mon Sep 17 00:00:00 2001 From: srest2021 Date: Tue, 14 Apr 2026 10:23:42 -0700 Subject: [PATCH 1/4] fix(seer): Make Seer repository cleanup synchronous to prevent stale dual-writes Extract cleanup logic from Celery tasks into synchronous helpers in autofix/utils.py and call them directly from impl.py and organization_repository_details.py. This closes the race window where async Seer cleanup allowed stale preferences (with disabled/hidden repos) to be read back from Seer and dual-written into SeerProjectRepository before the cleanup task ran. The tasks are kept as thin wrappers (renamed with _task suffix) for any future async callers. Co-Authored-By: Claude Opus 4.6 --- .../organization_repository_details.py | 12 +-- .../integrations/services/repository/impl.py | 10 +- src/sentry/seer/autofix/utils.py | 85 ++++++++++++++++- src/sentry/tasks/seer/cleanup.py | 95 ++----------------- .../test_organization_repository_details.py | 48 ++++++---- .../services/repository/test_impl.py | 30 +++--- tests/sentry/tasks/seer/test_cleanup.py | 14 +-- 7 files changed, 149 insertions(+), 145 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_repository_details.py b/src/sentry/integrations/api/endpoints/organization_repository_details.py index 25df9ba13509d9..8ca89530b41093 100644 --- a/src/sentry/integrations/api/endpoints/organization_repository_details.py +++ b/src/sentry/integrations/api/endpoints/organization_repository_details.py @@ -19,8 +19,8 @@ from sentry.models.commit import Commit from sentry.models.organization import Organization from sentry.models.repository import Repository +from sentry.seer.autofix.utils import cleanup_seer_repository_preferences from sentry.tasks.repository import repository_cascade_delete_on_hide -from sentry.tasks.seer.cleanup import cleanup_seer_repository_preferences class RepositorySerializer(serializers.Serializer): @@ -100,12 +100,10 @@ def put(self, request: Request, organization: Organization, repo_id) -> Response repository_cascade_delete_on_hide.apply_async(kwargs={"repo_id": repo.id}) if repo.external_id and repo.provider: - cleanup_seer_repository_preferences.apply_async( - kwargs={ - "organization_id": repo.organization_id, - "repo_external_id": repo.external_id, - "repo_provider": repo.provider, - } + cleanup_seer_repository_preferences( + organization_id=repo.organization_id, + repo_external_id=repo.external_id, + repo_provider=repo.provider, ) return Response(serialize(repo, request.user)) diff --git a/src/sentry/integrations/services/repository/impl.py b/src/sentry/integrations/services/repository/impl.py index 7a6bbe2e6ab168..36d7affb6303fb 100644 --- a/src/sentry/integrations/services/repository/impl.py +++ b/src/sentry/integrations/services/repository/impl.py @@ -15,8 +15,8 @@ from sentry.models.organization import Organization from sentry.models.projectcodeowners import ProjectCodeOwners from sentry.models.repository import Repository +from sentry.seer.autofix.utils import bulk_cleanup_seer_repository_preferences from sentry.seer.models.project_repository import SeerProjectRepository -from sentry.tasks.seer.cleanup import bulk_cleanup_seer_repository_preferences from sentry.users.services.user.model import RpcUser @@ -157,11 +157,9 @@ def _cleanup_seer_project_repositories( ] if repos_to_clean: transaction.on_commit( - lambda: bulk_cleanup_seer_repository_preferences.apply_async( - kwargs={ - "organization_id": organization_id, - "repos": repos_to_clean, - } + lambda: bulk_cleanup_seer_repository_preferences( + organization_id=organization_id, + repos=repos_to_clean, ), using=router.db_for_write(Repository), ) diff --git a/src/sentry/seer/autofix/utils.py b/src/sentry/seer/autofix/utils.py index 125949640c91b4..199ba6809d74b6 100644 --- a/src/sentry/seer/autofix/utils.py +++ b/src/sentry/seer/autofix/utils.py @@ -50,7 +50,15 @@ SeerProjectRepository, SeerProjectRepositoryBranchOverride, ) -from sentry.seer.signed_seer_api import SeerViewerContext, make_signed_seer_api_request +from sentry.seer.signed_seer_api import ( + BulkRemoveRepositoriesRequest, + RemoveRepositoryRequest, + RepoIdentifier, + SeerViewerContext, + make_bulk_remove_repositories_request, + make_remove_repository_request, + make_signed_seer_api_request, +) from sentry.utils.cache import cache from sentry.utils.outcomes import Outcome, track_outcome @@ -1255,3 +1263,78 @@ def update_coding_agent_state( "response": response.data.decode("utf-8"), }, ) + + +def cleanup_seer_repository_preferences( + organization_id: int, repo_external_id: str, repo_provider: str +) -> None: + """Remove a single repository from Seer project preferences via the Seer API.""" + body = RemoveRepositoryRequest( + organization_id=organization_id, + repo_provider=repo_provider, + repo_external_id=repo_external_id, + ) + + viewer_context = SeerViewerContext(organization_id=organization_id) + try: + response = make_remove_repository_request(body, viewer_context=viewer_context) + if response.status >= 400: + raise SeerApiError("Seer request failed", response.status) + logger.info( + "cleanup_seer_repository_preferences.success", + extra={ + "organization_id": organization_id, + "repo_external_id": repo_external_id, + "repo_provider": repo_provider, + }, + ) + except Exception as e: + logger.exception( + "cleanup_seer_repository_preferences.failed", + extra={ + "organization_id": organization_id, + "repo_external_id": repo_external_id, + "repo_provider": repo_provider, + "error": str(e), + }, + ) + raise + + +def bulk_cleanup_seer_repository_preferences( + organization_id: int, repos: list[dict[str, str]] +) -> None: + """Remove multiple repositories from Seer project preferences via the Seer API.""" + body = BulkRemoveRepositoriesRequest( + organization_id=organization_id, + repositories=[ + RepoIdentifier( + repo_provider=repo["repo_provider"], + repo_external_id=repo["repo_external_id"], + ) + for repo in repos + ], + ) + + viewer_context = SeerViewerContext(organization_id=organization_id) + try: + response = make_bulk_remove_repositories_request(body, viewer_context=viewer_context) + if response.status >= 400: + raise SeerApiError("Seer request failed", response.status) + logger.info( + "bulk_cleanup_seer_repository_preferences.success", + extra={ + "organization_id": organization_id, + "repo_count": len(repos), + }, + ) + except Exception as e: + logger.exception( + "bulk_cleanup_seer_repository_preferences.failed", + extra={ + "organization_id": organization_id, + "repo_count": len(repos), + "error": str(e), + }, + ) + raise diff --git a/src/sentry/tasks/seer/cleanup.py b/src/sentry/tasks/seer/cleanup.py index 06406fe3a18f1c..c908755cb264e1 100644 --- a/src/sentry/tasks/seer/cleanup.py +++ b/src/sentry/tasks/seer/cleanup.py @@ -1,22 +1,13 @@ from __future__ import annotations -import logging - -from sentry.seer.models import SeerApiError -from sentry.seer.signed_seer_api import ( - BulkRemoveRepositoriesRequest, - RemoveRepositoryRequest, - RepoIdentifier, - SeerViewerContext, - make_bulk_remove_repositories_request, - make_remove_repository_request, +from sentry.seer.autofix.utils import ( + bulk_cleanup_seer_repository_preferences, + cleanup_seer_repository_preferences, ) from sentry.silo.base import SiloMode from sentry.tasks.base import instrumented_task from sentry.taskworker.namespaces import seer_tasks -logger = logging.getLogger(__name__) - @instrumented_task( name="sentry.tasks.seer.cleanup_seer_repository_preferences", @@ -24,47 +15,16 @@ processing_deadline_duration=60 * 5, silo_mode=SiloMode.CELL, ) -def cleanup_seer_repository_preferences( +def cleanup_seer_repository_preferences_task( organization_id: int, repo_external_id: str, repo_provider: str ) -> None: - """ - Clean up Seer preferences for a deleted repository. - - This task removes a repository from Seer project preferences when the repository - is deleted from an organization's integration. - """ - # Call Seer API to remove repository from project preferences - body = RemoveRepositoryRequest( + """Task wrapper for cleaning up Seer preferences for a single repository.""" + cleanup_seer_repository_preferences( organization_id=organization_id, - repo_provider=repo_provider, repo_external_id=repo_external_id, + repo_provider=repo_provider, ) - viewer_context = SeerViewerContext(organization_id=organization_id) - try: - response = make_remove_repository_request(body, viewer_context=viewer_context) - if response.status >= 400: - raise SeerApiError("Seer request failed", response.status) - logger.info( - "cleanup_seer_repository_preferences.success", - extra={ - "organization_id": organization_id, - "repo_external_id": repo_external_id, - "repo_provider": repo_provider, - }, - ) - except Exception as e: - logger.exception( - "cleanup_seer_repository_preferences.failed", - extra={ - "organization_id": organization_id, - "repo_external_id": repo_external_id, - "repo_provider": repo_provider, - "error": str(e), - }, - ) - raise - @instrumented_task( name="sentry.tasks.seer.bulk_cleanup_seer_repository_preferences", @@ -72,43 +32,8 @@ def cleanup_seer_repository_preferences( processing_deadline_duration=60 * 10, silo_mode=SiloMode.CELL, ) -def bulk_cleanup_seer_repository_preferences( +def bulk_cleanup_seer_repository_preferences_task( organization_id: int, repos: list[dict[str, str]] ) -> None: - """ - Removes multiple repositories from Seer project preferences when the repository - is deleted from an organization's integration. - """ - body = BulkRemoveRepositoriesRequest( - organization_id=organization_id, - repositories=[ - RepoIdentifier( - repo_provider=repo["repo_provider"], - repo_external_id=repo["repo_external_id"], - ) - for repo in repos - ], - ) - - viewer_context = SeerViewerContext(organization_id=organization_id) - try: - response = make_bulk_remove_repositories_request(body, viewer_context=viewer_context) - if response.status >= 400: - raise SeerApiError("Seer request failed", response.status) - logger.info( - "bulk_cleanup_seer_repository_preferences.success", - extra={ - "organization_id": organization_id, - "repo_count": len(repos), - }, - ) - except Exception as e: - logger.exception( - "bulk_cleanup_seer_repository_preferences.failed", - extra={ - "organization_id": organization_id, - "repo_count": len(repos), - "error": str(e), - }, - ) - raise + """Task wrapper for removing multiple repositories from Seer project preferences.""" + bulk_cleanup_seer_repository_preferences(organization_id=organization_id, repos=repos) diff --git a/tests/sentry/integrations/api/endpoints/test_organization_repository_details.py b/tests/sentry/integrations/api/endpoints/test_organization_repository_details.py index e11527b5fc9597..55b866e0f4365b 100644 --- a/tests/sentry/integrations/api/endpoints/test_organization_repository_details.py +++ b/tests/sentry/integrations/api/endpoints/test_organization_repository_details.py @@ -262,7 +262,9 @@ def test_put_cancel_deletion(self) -> None: organization_id=org.id, key=build_pending_deletion_key(repo) ).exists() - @patch("sentry.tasks.seer.cleanup.cleanup_seer_repository_preferences.apply_async") + @patch( + "sentry.integrations.api.endpoints.organization_repository_details.cleanup_seer_repository_preferences" + ) def test_put_hide_repo(self, mock_cleanup_task: MagicMock) -> None: self.login_as(user=self.user) @@ -286,14 +288,14 @@ def test_put_hide_repo(self, mock_cleanup_task: MagicMock) -> None: # Verify the cleanup task was called mock_cleanup_task.assert_called_once_with( - kwargs={ - "organization_id": org.id, - "repo_external_id": "uuid-external-id", - "repo_provider": "github", - } + organization_id=org.id, + repo_external_id="uuid-external-id", + repo_provider="github", ) - @patch("sentry.tasks.seer.cleanup.cleanup_seer_repository_preferences.apply_async") + @patch( + "sentry.integrations.api.endpoints.organization_repository_details.cleanup_seer_repository_preferences" + ) def test_put_hide_repo_with_commits(self, mock_cleanup_task: MagicMock) -> None: self.login_as(user=self.user) @@ -315,11 +317,9 @@ def test_put_hide_repo_with_commits(self, mock_cleanup_task: MagicMock) -> None: # Verify the cleanup task was called mock_cleanup_task.assert_called_once_with( - kwargs={ - "organization_id": org.id, - "repo_external_id": "abc123", - "repo_provider": "github", - } + organization_id=org.id, + repo_external_id="abc123", + repo_provider="github", ) def test_put_rejects_integration_id(self) -> None: @@ -347,7 +347,9 @@ def test_put_rejects_integration_id(self) -> None: assert repo.provider == "integrations:github" assert repo.integration_id is None - @patch("sentry.tasks.seer.cleanup.cleanup_seer_repository_preferences.apply_async") + @patch( + "sentry.integrations.api.endpoints.organization_repository_details.cleanup_seer_repository_preferences" + ) def test_put_hide_repo_triggers_cleanup(self, mock_cleanup_task: MagicMock) -> None: """Test that hiding a repository triggers Seer cleanup task.""" self.login_as(user=self.user) @@ -371,14 +373,14 @@ def test_put_hide_repo_triggers_cleanup(self, mock_cleanup_task: MagicMock) -> N # Verify the cleanup task was called with correct parameters mock_cleanup_task.assert_called_once_with( - kwargs={ - "organization_id": org.id, - "repo_external_id": "github-123", - "repo_provider": "github", - } + organization_id=org.id, + repo_external_id="github-123", + repo_provider="github", ) - @patch("sentry.tasks.seer.cleanup.cleanup_seer_repository_preferences.apply_async") + @patch( + "sentry.integrations.api.endpoints.organization_repository_details.cleanup_seer_repository_preferences" + ) def test_put_hide_repo_no_cleanup_when_null_fields(self, mock_cleanup_task: MagicMock) -> None: """Test that hiding a repository with null external_id/provider does not trigger Seer cleanup.""" self.login_as(user=self.user) @@ -403,7 +405,9 @@ def test_put_hide_repo_no_cleanup_when_null_fields(self, mock_cleanup_task: Magi # Verify the cleanup task was NOT called mock_cleanup_task.assert_not_called() - @patch("sentry.tasks.seer.cleanup.cleanup_seer_repository_preferences.apply_async") + @patch( + "sentry.integrations.api.endpoints.organization_repository_details.cleanup_seer_repository_preferences" + ) def test_put_hide_repo_no_cleanup_when_external_id_null( self, mock_cleanup_task: MagicMock ) -> None: @@ -430,7 +434,9 @@ def test_put_hide_repo_no_cleanup_when_external_id_null( # Verify the cleanup task was NOT called mock_cleanup_task.assert_not_called() - @patch("sentry.tasks.seer.cleanup.cleanup_seer_repository_preferences.apply_async") + @patch( + "sentry.integrations.api.endpoints.organization_repository_details.cleanup_seer_repository_preferences" + ) def test_put_hide_repo_no_cleanup_when_provider_null( self, mock_cleanup_task: MagicMock ) -> None: diff --git a/tests/sentry/integrations/services/repository/test_impl.py b/tests/sentry/integrations/services/repository/test_impl.py index 896850e192c36e..d2fff658c2f46f 100644 --- a/tests/sentry/integrations/services/repository/test_impl.py +++ b/tests/sentry/integrations/services/repository/test_impl.py @@ -195,11 +195,9 @@ def test_cleans_up_seer_preferences(self, mock_cleanup: MagicMock) -> None: 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}], - } + mock_cleanup.assert_called_once_with( + organization_id=self.organization.id, + repos=[{"repo_external_id": "100", "repo_provider": self.provider}], ) @@ -257,11 +255,9 @@ def test_cleans_up_seer_preferences(self, mock_cleanup: MagicMock) -> None: 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}], - } + mock_cleanup.assert_called_once_with( + organization_id=self.organization.id, + repos=[{"repo_external_id": "100", "repo_provider": self.provider}], ) @@ -295,7 +291,7 @@ def test_disassociates_repos(self, mock_cleanup: MagicMock) -> None: repo.refresh_from_db() assert repo.integration_id is None - mock_cleanup.apply_async.assert_called_once() + mock_cleanup.assert_called_once() @with_feature("organizations:seer-project-settings-dual-write") @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences") @@ -320,11 +316,9 @@ def test_cleans_up_seer_preferences(self, mock_cleanup: MagicMock) -> None: 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}], - } + mock_cleanup.assert_called_once_with( + 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") @@ -356,5 +350,5 @@ def test_transaction_rollback_does_not_dispatch_seer_cleanup( repo.refresh_from_db() assert repo.integration_id == self.integration.id - # Task should not have been dispatched - mock_cleanup.apply_async.assert_not_called() + # Cleanup should not have been called + mock_cleanup.assert_not_called() diff --git a/tests/sentry/tasks/seer/test_cleanup.py b/tests/sentry/tasks/seer/test_cleanup.py index a614066e46f525..2b2abac11f6201 100644 --- a/tests/sentry/tasks/seer/test_cleanup.py +++ b/tests/sentry/tasks/seer/test_cleanup.py @@ -4,11 +4,11 @@ import pytest -from sentry.seer.models import SeerApiError -from sentry.tasks.seer.cleanup import ( +from sentry.seer.autofix.utils import ( bulk_cleanup_seer_repository_preferences, cleanup_seer_repository_preferences, ) +from sentry.seer.models import SeerApiError from sentry.testutils.cases import TestCase @@ -19,7 +19,7 @@ def setUp(self) -> None: self.repo_external_id = "12345" self.repo_provider = "github" - @patch("sentry.tasks.seer.cleanup.make_remove_repository_request") + @patch("sentry.seer.autofix.utils.make_remove_repository_request") def test_cleanup_seer_repository_preferences_success(self, mock_request: MagicMock) -> None: """Test successful cleanup of Seer repository preferences.""" mock_request.return_value.status = 200 @@ -38,7 +38,7 @@ def test_cleanup_seer_repository_preferences_success(self, mock_request: MagicMo "repo_external_id": self.repo_external_id, } - @patch("sentry.tasks.seer.cleanup.make_remove_repository_request") + @patch("sentry.seer.autofix.utils.make_remove_repository_request") def test_cleanup_seer_repository_preferences_api_error(self, mock_request: MagicMock) -> None: """Test handling of Seer API errors.""" mock_request.return_value.status = 500 @@ -50,7 +50,7 @@ def test_cleanup_seer_repository_preferences_api_error(self, mock_request: Magic repo_provider=self.repo_provider, ) - @patch("sentry.tasks.seer.cleanup.make_remove_repository_request") + @patch("sentry.seer.autofix.utils.make_remove_repository_request") def test_cleanup_seer_repository_preferences_organization_not_found( self, mock_request: MagicMock ) -> None: @@ -82,7 +82,7 @@ def setUp(self) -> None: {"repo_external_id": "456", "repo_provider": "github"}, ] - @patch("sentry.tasks.seer.cleanup.make_bulk_remove_repositories_request") + @patch("sentry.seer.autofix.utils.make_bulk_remove_repositories_request") def test_bulk_cleanup_success(self, mock_request: MagicMock) -> None: """Test successful bulk cleanup of Seer repository preferences.""" mock_request.return_value.status = 200 @@ -99,7 +99,7 @@ def test_bulk_cleanup_success(self, mock_request: MagicMock) -> None: assert body["repositories"][0] == {"repo_provider": "github", "repo_external_id": "123"} assert body["repositories"][1] == {"repo_provider": "github", "repo_external_id": "456"} - @patch("sentry.tasks.seer.cleanup.make_bulk_remove_repositories_request") + @patch("sentry.seer.autofix.utils.make_bulk_remove_repositories_request") def test_bulk_cleanup_api_error(self, mock_request: MagicMock) -> None: """Test handling of Seer API errors.""" mock_request.return_value.status = 500 From 755c68246326f60aabe5edce853297b9936de8d3 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Tue, 14 Apr 2026 10:34:39 -0700 Subject: [PATCH 2/4] cleanup on transaction commit --- .../organization_repository_details.py | 11 +- src/sentry/tasks/seer/cleanup.py | 39 ------ .../sentry/seer/autofix/test_autofix_utils.py | 97 ++++++++++++++- tests/sentry/tasks/seer/test_cleanup.py | 111 ------------------ 4 files changed, 103 insertions(+), 155 deletions(-) delete mode 100644 src/sentry/tasks/seer/cleanup.py delete mode 100644 tests/sentry/tasks/seer/test_cleanup.py diff --git a/src/sentry/integrations/api/endpoints/organization_repository_details.py b/src/sentry/integrations/api/endpoints/organization_repository_details.py index 8ca89530b41093..dc8bf4d6bc58e2 100644 --- a/src/sentry/integrations/api/endpoints/organization_repository_details.py +++ b/src/sentry/integrations/api/endpoints/organization_repository_details.py @@ -100,10 +100,13 @@ def put(self, request: Request, organization: Organization, repo_id) -> Response repository_cascade_delete_on_hide.apply_async(kwargs={"repo_id": repo.id}) if repo.external_id and repo.provider: - cleanup_seer_repository_preferences( - organization_id=repo.organization_id, - repo_external_id=repo.external_id, - repo_provider=repo.provider, + transaction.on_commit( + lambda: cleanup_seer_repository_preferences( + organization_id=repo.organization_id, + repo_external_id=repo.external_id, + repo_provider=repo.provider, + ), + using=router.db_for_write(Repository), ) return Response(serialize(repo, request.user)) diff --git a/src/sentry/tasks/seer/cleanup.py b/src/sentry/tasks/seer/cleanup.py deleted file mode 100644 index c908755cb264e1..00000000000000 --- a/src/sentry/tasks/seer/cleanup.py +++ /dev/null @@ -1,39 +0,0 @@ -from __future__ import annotations - -from sentry.seer.autofix.utils import ( - bulk_cleanup_seer_repository_preferences, - cleanup_seer_repository_preferences, -) -from sentry.silo.base import SiloMode -from sentry.tasks.base import instrumented_task -from sentry.taskworker.namespaces import seer_tasks - - -@instrumented_task( - name="sentry.tasks.seer.cleanup_seer_repository_preferences", - namespace=seer_tasks, - processing_deadline_duration=60 * 5, - silo_mode=SiloMode.CELL, -) -def cleanup_seer_repository_preferences_task( - organization_id: int, repo_external_id: str, repo_provider: str -) -> None: - """Task wrapper for cleaning up Seer preferences for a single repository.""" - cleanup_seer_repository_preferences( - organization_id=organization_id, - repo_external_id=repo_external_id, - repo_provider=repo_provider, - ) - - -@instrumented_task( - name="sentry.tasks.seer.bulk_cleanup_seer_repository_preferences", - namespace=seer_tasks, - processing_deadline_duration=60 * 10, - silo_mode=SiloMode.CELL, -) -def bulk_cleanup_seer_repository_preferences_task( - organization_id: int, repos: list[dict[str, str]] -) -> None: - """Task wrapper for removing multiple repositories from Seer project preferences.""" - bulk_cleanup_seer_repository_preferences(organization_id=organization_id, repos=repos) diff --git a/tests/sentry/seer/autofix/test_autofix_utils.py b/tests/sentry/seer/autofix/test_autofix_utils.py index 1aedaabcee4317..9a3dc232da6578 100644 --- a/tests/sentry/seer/autofix/test_autofix_utils.py +++ b/tests/sentry/seer/autofix/test_autofix_utils.py @@ -1,5 +1,5 @@ from typing import Any -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch import orjson import pytest @@ -11,8 +11,10 @@ AutofixState, AutofixTriggerSource, CodingAgentStatus, + bulk_cleanup_seer_repository_preferences, bulk_read_preferences_from_sentry_db, bulk_write_preferences_to_sentry_db, + cleanup_seer_repository_preferences, deduplicate_repositories, get_autofix_prompt, get_coding_agent_prompt, @@ -1582,3 +1584,96 @@ def test_invalid_stopping_point_falls_back_to_default(self): ) stopping_point, _ = get_org_default_seer_automation_handoff(self.organization) assert stopping_point == SEER_AUTOMATED_RUN_STOPPING_POINT_DEFAULT + + +class TestCleanupSeerRepositoryPreferences(TestCase): + def setUp(self) -> None: + self.organization = self.create_organization() + self.project = self.create_project(organization=self.organization) + self.repo_external_id = "12345" + self.repo_provider = "github" + + @patch("sentry.seer.autofix.utils.make_remove_repository_request") + def test_cleanup_success(self, mock_request: MagicMock) -> None: + """Test successful cleanup of Seer repository preferences.""" + mock_request.return_value.status = 200 + + cleanup_seer_repository_preferences( + organization_id=self.organization.id, + repo_external_id=self.repo_external_id, + repo_provider=self.repo_provider, + ) + + mock_request.assert_called_once() + body = mock_request.call_args[0][0] + assert body == { + "organization_id": self.organization.id, + "repo_provider": self.repo_provider, + "repo_external_id": self.repo_external_id, + } + + @patch("sentry.seer.autofix.utils.make_remove_repository_request") + def test_cleanup_api_error(self, mock_request: MagicMock) -> None: + """Test handling of Seer API errors.""" + mock_request.return_value.status = 500 + + with pytest.raises(SeerApiError): + cleanup_seer_repository_preferences( + organization_id=self.organization.id, + repo_external_id=self.repo_external_id, + repo_provider=self.repo_provider, + ) + + @patch("sentry.seer.autofix.utils.make_remove_repository_request") + def test_cleanup_organization_not_found(self, mock_request: MagicMock) -> None: + """Test handling when organization doesn't exist.""" + mock_request.return_value.status = 200 + + nonexistent_organization_id = 99999 + + cleanup_seer_repository_preferences( + organization_id=nonexistent_organization_id, + repo_external_id=self.repo_external_id, + repo_provider=self.repo_provider, + ) + + mock_request.assert_called_once() + body = mock_request.call_args[0][0] + assert body == { + "organization_id": nonexistent_organization_id, + "repo_provider": self.repo_provider, + "repo_external_id": self.repo_external_id, + } + + @patch("sentry.seer.autofix.utils.make_bulk_remove_repositories_request") + def test_bulk_cleanup_success(self, mock_request: MagicMock) -> None: + """Test successful bulk cleanup of Seer repository preferences.""" + mock_request.return_value.status = 200 + + repos = [ + {"repo_external_id": "123", "repo_provider": "github"}, + {"repo_external_id": "456", "repo_provider": "github"}, + ] + + bulk_cleanup_seer_repository_preferences( + organization_id=self.organization.id, + repos=repos, + ) + + mock_request.assert_called_once() + body = mock_request.call_args[0][0] + assert body["organization_id"] == self.organization.id + assert len(body["repositories"]) == 2 + assert body["repositories"][0] == {"repo_provider": "github", "repo_external_id": "123"} + assert body["repositories"][1] == {"repo_provider": "github", "repo_external_id": "456"} + + @patch("sentry.seer.autofix.utils.make_bulk_remove_repositories_request") + def test_bulk_cleanup_api_error(self, mock_request: MagicMock) -> None: + """Test handling of Seer API errors.""" + mock_request.return_value.status = 500 + + with pytest.raises(SeerApiError): + bulk_cleanup_seer_repository_preferences( + organization_id=self.organization.id, + repos=[{"repo_external_id": "123", "repo_provider": "github"}], + ) diff --git a/tests/sentry/tasks/seer/test_cleanup.py b/tests/sentry/tasks/seer/test_cleanup.py deleted file mode 100644 index 2b2abac11f6201..00000000000000 --- a/tests/sentry/tasks/seer/test_cleanup.py +++ /dev/null @@ -1,111 +0,0 @@ -from __future__ import annotations - -from unittest.mock import MagicMock, patch - -import pytest - -from sentry.seer.autofix.utils import ( - bulk_cleanup_seer_repository_preferences, - cleanup_seer_repository_preferences, -) -from sentry.seer.models import SeerApiError -from sentry.testutils.cases import TestCase - - -class TestSeerRepositoryCleanup(TestCase): - def setUp(self) -> None: - self.organization = self.create_organization() - self.project = self.create_project(organization=self.organization) - self.repo_external_id = "12345" - self.repo_provider = "github" - - @patch("sentry.seer.autofix.utils.make_remove_repository_request") - def test_cleanup_seer_repository_preferences_success(self, mock_request: MagicMock) -> None: - """Test successful cleanup of Seer repository preferences.""" - mock_request.return_value.status = 200 - - cleanup_seer_repository_preferences( - organization_id=self.organization.id, - repo_external_id=self.repo_external_id, - repo_provider=self.repo_provider, - ) - - mock_request.assert_called_once() - body = mock_request.call_args[0][0] - assert body == { - "organization_id": self.organization.id, - "repo_provider": self.repo_provider, - "repo_external_id": self.repo_external_id, - } - - @patch("sentry.seer.autofix.utils.make_remove_repository_request") - def test_cleanup_seer_repository_preferences_api_error(self, mock_request: MagicMock) -> None: - """Test handling of Seer API errors.""" - mock_request.return_value.status = 500 - - with pytest.raises(SeerApiError): - cleanup_seer_repository_preferences( - organization_id=self.organization.id, - repo_external_id=self.repo_external_id, - repo_provider=self.repo_provider, - ) - - @patch("sentry.seer.autofix.utils.make_remove_repository_request") - def test_cleanup_seer_repository_preferences_organization_not_found( - self, mock_request: MagicMock - ) -> None: - """Test handling when organization doesn't exist.""" - mock_request.return_value.status = 200 - - nonexistent_organization_id = 99999 - - cleanup_seer_repository_preferences( - organization_id=nonexistent_organization_id, - repo_external_id=self.repo_external_id, - repo_provider=self.repo_provider, - ) - - mock_request.assert_called_once() - body = mock_request.call_args[0][0] - assert body == { - "organization_id": nonexistent_organization_id, - "repo_provider": self.repo_provider, - "repo_external_id": self.repo_external_id, - } - - -class TestBulkSeerRepositoryCleanup(TestCase): - def setUp(self) -> None: - self.organization = self.create_organization() - self.repos = [ - {"repo_external_id": "123", "repo_provider": "github"}, - {"repo_external_id": "456", "repo_provider": "github"}, - ] - - @patch("sentry.seer.autofix.utils.make_bulk_remove_repositories_request") - def test_bulk_cleanup_success(self, mock_request: MagicMock) -> None: - """Test successful bulk cleanup of Seer repository preferences.""" - mock_request.return_value.status = 200 - - bulk_cleanup_seer_repository_preferences( - organization_id=self.organization.id, - repos=self.repos, - ) - - mock_request.assert_called_once() - body = mock_request.call_args[0][0] - assert body["organization_id"] == self.organization.id - assert len(body["repositories"]) == 2 - assert body["repositories"][0] == {"repo_provider": "github", "repo_external_id": "123"} - assert body["repositories"][1] == {"repo_provider": "github", "repo_external_id": "456"} - - @patch("sentry.seer.autofix.utils.make_bulk_remove_repositories_request") - def test_bulk_cleanup_api_error(self, mock_request: MagicMock) -> None: - """Test handling of Seer API errors.""" - mock_request.return_value.status = 500 - - with pytest.raises(SeerApiError): - bulk_cleanup_seer_repository_preferences( - organization_id=self.organization.id, - repos=self.repos, - ) From 45cf72054fc3199515ddb7a34cfb432ce02df6b4 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Tue, 14 Apr 2026 10:59:28 -0700 Subject: [PATCH 3/4] fixes --- .../organization_repository_details.py | 4 +-- src/sentry/seer/autofix/utils.py | 2 -- .../test_sync_repos_on_install_change.py | 6 ++-- .../source_code_management/test_sync_repos.py | 3 +- .../sentry/seer/autofix/test_autofix_utils.py | 28 +++++++++---------- 5 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_repository_details.py b/src/sentry/integrations/api/endpoints/organization_repository_details.py index dc8bf4d6bc58e2..207de09d6411a8 100644 --- a/src/sentry/integrations/api/endpoints/organization_repository_details.py +++ b/src/sentry/integrations/api/endpoints/organization_repository_details.py @@ -103,8 +103,8 @@ def put(self, request: Request, organization: Organization, repo_id) -> Response transaction.on_commit( lambda: cleanup_seer_repository_preferences( organization_id=repo.organization_id, - repo_external_id=repo.external_id, - repo_provider=repo.provider, + repo_external_id=repo.external_id, # type: ignore[arg-type] + repo_provider=repo.provider, # type: ignore[arg-type] ), using=router.db_for_write(Repository), ) diff --git a/src/sentry/seer/autofix/utils.py b/src/sentry/seer/autofix/utils.py index 199ba6809d74b6..c58ebe65e7fe9b 100644 --- a/src/sentry/seer/autofix/utils.py +++ b/src/sentry/seer/autofix/utils.py @@ -1298,7 +1298,6 @@ def cleanup_seer_repository_preferences( "error": str(e), }, ) - raise def bulk_cleanup_seer_repository_preferences( @@ -1337,4 +1336,3 @@ def bulk_cleanup_seer_repository_preferences( "error": str(e), }, ) - raise diff --git a/tests/sentry/integrations/github/tasks/test_sync_repos_on_install_change.py b/tests/sentry/integrations/github/tasks/test_sync_repos_on_install_change.py index 390184025afcce..4e0d6a8b46038b 100644 --- a/tests/sentry/integrations/github/tasks/test_sync_repos_on_install_change.py +++ b/tests/sentry/integrations/github/tasks/test_sync_repos_on_install_change.py @@ -59,7 +59,8 @@ def test_repos_added(self, _: MagicMock) -> None: ) assert entries.count() == 2 - def test_repos_removed(self, _: MagicMock) -> None: + @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences") + def test_repos_removed(self, mock_seer_cleanup: MagicMock, _: MagicMock) -> None: with assume_test_silo_mode(SiloMode.CELL): repo = Repository.objects.create( organization_id=self.organization.id, @@ -89,7 +90,8 @@ def test_repos_removed(self, _: MagicMock) -> None: event=audit_log.get_event_id("REPO_DISABLED"), ).exists() - def test_mixed_add_and_remove(self, _: MagicMock) -> None: + @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences") + def test_mixed_add_and_remove(self, mock_seer_cleanup: MagicMock, _: MagicMock) -> None: with assume_test_silo_mode(SiloMode.CELL): old_repo = Repository.objects.create( organization_id=self.organization.id, diff --git a/tests/sentry/integrations/source_code_management/test_sync_repos.py b/tests/sentry/integrations/source_code_management/test_sync_repos.py index c33ceb824a26f9..b5f4c06e5d83fe 100644 --- a/tests/sentry/integrations/source_code_management/test_sync_repos.py +++ b/tests/sentry/integrations/source_code_management/test_sync_repos.py @@ -70,7 +70,8 @@ def test_creates_new_repos(self, _: MagicMock) -> None: assert entries.count() == 2 @responses.activate - def test_disables_removed_repos(self, _: MagicMock) -> None: + @patch("sentry.integrations.services.repository.impl.bulk_cleanup_seer_repository_preferences") + def test_disables_removed_repos(self, mock_seer_cleanup: MagicMock, _: MagicMock) -> None: with assume_test_silo_mode(SiloMode.CELL): repo = Repository.objects.create( organization_id=self.organization.id, diff --git a/tests/sentry/seer/autofix/test_autofix_utils.py b/tests/sentry/seer/autofix/test_autofix_utils.py index 9a3dc232da6578..0d51d3e8d05e4d 100644 --- a/tests/sentry/seer/autofix/test_autofix_utils.py +++ b/tests/sentry/seer/autofix/test_autofix_utils.py @@ -1613,16 +1613,15 @@ def test_cleanup_success(self, mock_request: MagicMock) -> None: } @patch("sentry.seer.autofix.utils.make_remove_repository_request") - def test_cleanup_api_error(self, mock_request: MagicMock) -> None: - """Test handling of Seer API errors.""" + def test_cleanup_api_error_does_not_raise(self, mock_request: MagicMock) -> None: + """Test that Seer API errors are logged but not propagated.""" mock_request.return_value.status = 500 - with pytest.raises(SeerApiError): - cleanup_seer_repository_preferences( - organization_id=self.organization.id, - repo_external_id=self.repo_external_id, - repo_provider=self.repo_provider, - ) + cleanup_seer_repository_preferences( + organization_id=self.organization.id, + repo_external_id=self.repo_external_id, + repo_provider=self.repo_provider, + ) @patch("sentry.seer.autofix.utils.make_remove_repository_request") def test_cleanup_organization_not_found(self, mock_request: MagicMock) -> None: @@ -1668,12 +1667,11 @@ def test_bulk_cleanup_success(self, mock_request: MagicMock) -> None: assert body["repositories"][1] == {"repo_provider": "github", "repo_external_id": "456"} @patch("sentry.seer.autofix.utils.make_bulk_remove_repositories_request") - def test_bulk_cleanup_api_error(self, mock_request: MagicMock) -> None: - """Test handling of Seer API errors.""" + def test_bulk_cleanup_api_error_does_not_raise(self, mock_request: MagicMock) -> None: + """Test that Seer API errors are logged but not propagated.""" mock_request.return_value.status = 500 - with pytest.raises(SeerApiError): - bulk_cleanup_seer_repository_preferences( - organization_id=self.organization.id, - repos=[{"repo_external_id": "123", "repo_provider": "github"}], - ) + bulk_cleanup_seer_repository_preferences( + organization_id=self.organization.id, + repos=[{"repo_external_id": "123", "repo_provider": "github"}], + ) From b5a556e1f5910aa4dee33679f3238ca8770bef24 Mon Sep 17 00:00:00 2001 From: srest2021 Date: Tue, 14 Apr 2026 11:10:44 -0700 Subject: [PATCH 4/4] remove task --- src/sentry/conf/server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 6bbb2650b790c8..8970ee8a8b6775 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -974,7 +974,6 @@ def SOCIAL_AUTH_DEFAULT_USERNAME() -> str: "sentry.tasks.repository", "sentry.tasks.reprocessing2", "sentry.tasks.scim.privilege_sync", - "sentry.tasks.seer.cleanup", "sentry.tasks.statistical_detectors", "sentry.tasks.store", "sentry.tasks.summaries.weekly_reports",