Skip to content

fix(seer): Make Seer repo preferences cleanup synchronous to prevent stale dual-writes #112949

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

Draft
srest2021 wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

fix(seer): Make Seer repo preferences cleanup synchronous to prevent stale dual-writes #112949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
92cf3a9
fix(seer): Make Seer repository cleanup synchronous to prevent stale …
srest2021 Apr 14, 2026
755c682
cleanup on transaction commit
srest2021 Apr 14, 2026
45cf720
fixes
srest2021 Apr 14, 2026
b5a556e
remove task
srest2021 Apr 14, 2026
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
1 change: 0 additions & 1 deletion src/sentry/conf/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 8 additions & 7 deletions src/sentry/integrations/api/endpoints/organization_repository_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -100,12 +100,13 @@ def put(self, request: Request, organization: Organization, repo_id) -> Response
repository_cascade_delete_on_hide.apply_async(kwargs={"repo_id": repo.id})
Copy link
Copy Markdown
Member Author

@srest2021 srest2021 Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Note that this thing which deletes associated SeerProjectRepository is still asynchronous.

If read flag is off: we read and write existing prefs from Seer which are now updated synchronously. ✅

If read flag is on: we read and write existing prefs from Sentry which may be stale, just like all the other Repository child relations. However they will be eventually deleted from Sentry via the above task, and from Seer via its own cleanup cron. ✅

So I think we are ok here.


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,
}
transaction.on_commit(
lambda: cleanup_seer_repository_preferences(
organization_id=repo.organization_id,
repo_external_id=repo.external_id, # type: ignore[arg-type]
repo_provider=repo.provider, # type: ignore[arg-type]
Copy link
Copy Markdown
Member Author

@srest2021 srest2021 Apr 14, 2026

Choose a reason for hiding this comment

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

mypy keeps complaining even though these can't be none

),
using=router.db_for_write(Repository),
)

return Response(serialize(repo, request.user))
Expand Down
10 changes: 4 additions & 6 deletions src/sentry/integrations/services/repository/impl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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),
)
Expand Down
83 changes: 82 additions & 1 deletion src/sentry/seer/autofix/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -1255,3 +1263,76 @@ 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),
},
)


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),
},
)
114 changes: 0 additions & 114 deletions src/sentry/tasks/seer/cleanup.py
View file
Open in desktop

This file was deleted.

48 changes: 27 additions & 21 deletions tests/sentry/integrations/api/endpoints/test_organization_repository_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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:
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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:
Expand All @@ -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:
Expand Down
6 changes: 4 additions & 2 deletions tests/sentry/integrations/github/tasks/test_sync_repos_on_install_change.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Loading
Loading