From 15abd620aac32d2a4409c2eb5e61bc8f5f7f924e Mon Sep 17 00:00:00 2001 From: Alex Sohn Date: Tue, 8 Sep 2026 15:58:08 -0400 Subject: [PATCH 1/6] fix(seer): Skip the iteration push when the PR is closed An iteration triggered by a comment on a closed PR pushed its changes anyway, so a close no longer stopped Seer from writing to the PR. Read the PR state before pushing and stop when every PR on the run is closed. A PR we cannot read counts as open, so a transient failure does not drop the changes. Fixes CW-1998 Claude-Session: https://claude.ai/code/session_017A4BhakBapTmqxiuspM29x --- src/sentry/seer/autofix/on_completion_hook.py | 49 ++++++++++++++++++ .../test_autofix_on_completion_hook.py | 51 +++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/src/sentry/seer/autofix/on_completion_hook.py b/src/sentry/seer/autofix/on_completion_hook.py index 90c40a43fa13..a3654ec111e4 100644 --- a/src/sentry/seer/autofix/on_completion_hook.py +++ b/src/sentry/seer/autofix/on_completion_hook.py @@ -8,7 +8,9 @@ from django.db import router, transaction from django.utils import timezone from pydantic import ValidationError +from scm import actions as scm_actions from scm.manager import SourceCodeManager +from scm.types import GetPullRequestProtocol from sentry import analytics, features from sentry.analytics.events.autofix_events import ( @@ -125,6 +127,49 @@ def _iteration_repo_states(state: SeerRunState) -> list[dict[str, Any]]: ] +def _iteration_prs_all_closed(organization: Organization, state: SeerRunState) -> bool: + """True when the run has PRs and every one of them reads back as closed. + + A PR we cannot read (no number, repo gone, unsupported provider, API error) + counts as open: a transient read failure should not silently drop an + iteration's changes. + """ + checked_any = False + + for repo_name, pr_state in state.repo_pr_states.items(): + pr_number = pr_state.pr_number + if pr_number is None: + return False + + repo, _resolution = Repository.objects.resolve_active( + organization_id=organization.id, + name=repo_name, + normalized_provider=None, + ) + if repo is None: + return False + + try: + scm = make_scm(organization.id, repo.id, referrer="seer") + except Exception: + return False + + if not isinstance(scm, GetPullRequestProtocol): + return False + + try: + pull_request = scm_actions.get_pull_request(scm, str(pr_number)) + except Exception: + return False + + if pull_request["data"]["state"] != "closed": + return False + + checked_any = True + + return checked_any + + def _stopping_point_from_run(organization: Organization, run_id: int) -> str | None: return ( SeerAgentRun.objects.filter( @@ -1123,6 +1168,10 @@ def _push_iteration_changes( ) return False + if _iteration_prs_all_closed(group.organization, state): + log_ctx.info("autofix.pr_iteration.push", outcome="not_pushed", reason="pr_closed") + return False + try: trigger_push_changes( group, diff --git a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py index 148b1cca730d..ba52538822b0 100644 --- a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py +++ b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py @@ -2,6 +2,7 @@ from unittest.mock import MagicMock, patch from sentry.models.activity import Activity +from sentry.models.repository import Repository from sentry.seer.agent.client_models import ( AgentFilePatch, Artifact, @@ -754,6 +755,56 @@ def test_a_repo_whose_pr_creation_errored_stops_the_push(self, mock_push): assert pushed is False mock_push.assert_not_called() + def _github_repo(self) -> None: + Repository.objects.create( + organization_id=self.organization.id, + name="test-repo", + provider="integrations:github", + external_id="1", + ) + + @patch(f"{HOOK_PATH}.GetPullRequestProtocol", object) + @patch(f"{HOOK_PATH}.scm_actions.get_pull_request") + @patch(f"{HOOK_PATH}.make_scm") + @patch(f"{HOOK_PATH}.trigger_push_changes") + def test_a_closed_pr_stops_the_push(self, mock_push, mock_make_scm, mock_get_pull_request): + """Closing the PR is the stop signal; pushing into it would talk past it.""" + self._github_repo() + mock_get_pull_request.return_value = {"data": {"state": "closed"}} + + pushed = self._push(self._unsynced()) + + assert pushed is False + mock_push.assert_not_called() + + @patch(f"{HOOK_PATH}.GetPullRequestProtocol", object) + @patch(f"{HOOK_PATH}.scm_actions.get_pull_request") + @patch(f"{HOOK_PATH}.make_scm") + @patch(f"{HOOK_PATH}.trigger_push_changes") + def test_an_open_pr_still_pushes(self, mock_push, mock_make_scm, mock_get_pull_request): + self._github_repo() + mock_get_pull_request.return_value = {"data": {"state": "open"}} + + pushed = self._push(self._unsynced()) + + assert pushed is True + mock_push.assert_called_once() + + @patch(f"{HOOK_PATH}.GetPullRequestProtocol", object) + @patch(f"{HOOK_PATH}.scm_actions.get_pull_request", side_effect=ValueError("boom")) + @patch(f"{HOOK_PATH}.make_scm") + @patch(f"{HOOK_PATH}.trigger_push_changes") + def test_a_pr_we_cannot_read_still_pushes( + self, mock_push, mock_make_scm, mock_get_pull_request + ): + """A transient read failure must not silently drop the iteration's changes.""" + self._github_repo() + + pushed = self._push(self._unsynced()) + + assert pushed is True + mock_push.assert_called_once() + @patch(f"{HOOK_PATH}.trigger_push_changes", side_effect=ValueError("boom")) def test_a_failed_push_is_swallowed(self, mock_push): state = self._unsynced() From 4982808ee19e5ba5dd295f179625b773f920be7a Mon Sep 17 00:00:00 2001 From: Alex Sohn Date: Tue, 8 Sep 2026 16:03:38 -0400 Subject: [PATCH 2/6] fix(seer): Skip the iteration itself when the PR is closed The completion-hook block still spent a whole agent run before finding the PR closed. Check at the front of the consume task too, so a closed PR costs one API read instead of an iteration. The queue is left intact rather than cleared: if the PR reopens, the feedback that arrived while it was closed is still worth draining. Claude-Session: https://claude.ai/code/session_017A4BhakBapTmqxiuspM29x --- src/sentry/seer/autofix/on_completion_hook.py | 48 +-------------- .../seer/autofix/pr_iteration/pr_state.py | 59 ++++++++++++++++++ src/sentry/tasks/seer/pr_iteration.py | 9 +++ .../test_autofix_on_completion_hook.py | 19 +++--- tests/sentry/tasks/seer/test_pr_iteration.py | 60 +++++++++++++++++++ 5 files changed, 140 insertions(+), 55 deletions(-) create mode 100644 src/sentry/seer/autofix/pr_iteration/pr_state.py diff --git a/src/sentry/seer/autofix/on_completion_hook.py b/src/sentry/seer/autofix/on_completion_hook.py index a3654ec111e4..abb5fc9814c8 100644 --- a/src/sentry/seer/autofix/on_completion_hook.py +++ b/src/sentry/seer/autofix/on_completion_hook.py @@ -8,9 +8,7 @@ from django.db import router, transaction from django.utils import timezone from pydantic import ValidationError -from scm import actions as scm_actions from scm.manager import SourceCodeManager -from scm.types import GetPullRequestProtocol from sentry import analytics, features from sentry.analytics.events.autofix_events import ( @@ -50,6 +48,7 @@ ) from sentry.seer.autofix.pr_iteration.logs import PrIterationLogContext from sentry.seer.autofix.pr_iteration.pause import PauseReason, pause_pr_iteration +from sentry.seer.autofix.pr_iteration.pr_state import iteration_prs_all_closed from sentry.seer.autofix.pr_ready_for_review import ( emit_pr_ready_for_review, format_pull_requests_payload, @@ -127,49 +126,6 @@ def _iteration_repo_states(state: SeerRunState) -> list[dict[str, Any]]: ] -def _iteration_prs_all_closed(organization: Organization, state: SeerRunState) -> bool: - """True when the run has PRs and every one of them reads back as closed. - - A PR we cannot read (no number, repo gone, unsupported provider, API error) - counts as open: a transient read failure should not silently drop an - iteration's changes. - """ - checked_any = False - - for repo_name, pr_state in state.repo_pr_states.items(): - pr_number = pr_state.pr_number - if pr_number is None: - return False - - repo, _resolution = Repository.objects.resolve_active( - organization_id=organization.id, - name=repo_name, - normalized_provider=None, - ) - if repo is None: - return False - - try: - scm = make_scm(organization.id, repo.id, referrer="seer") - except Exception: - return False - - if not isinstance(scm, GetPullRequestProtocol): - return False - - try: - pull_request = scm_actions.get_pull_request(scm, str(pr_number)) - except Exception: - return False - - if pull_request["data"]["state"] != "closed": - return False - - checked_any = True - - return checked_any - - def _stopping_point_from_run(organization: Organization, run_id: int) -> str | None: return ( SeerAgentRun.objects.filter( @@ -1168,7 +1124,7 @@ def _push_iteration_changes( ) return False - if _iteration_prs_all_closed(group.organization, state): + if iteration_prs_all_closed(group.organization, state): log_ctx.info("autofix.pr_iteration.push", outcome="not_pushed", reason="pr_closed") return False diff --git a/src/sentry/seer/autofix/pr_iteration/pr_state.py b/src/sentry/seer/autofix/pr_iteration/pr_state.py new file mode 100644 index 000000000000..9233d0a199e7 --- /dev/null +++ b/src/sentry/seer/autofix/pr_iteration/pr_state.py @@ -0,0 +1,59 @@ +"""Read the live state of the pull requests an Autofix run opened. + +Closing a PR is how someone tells Seer to stop working on it. Both ends of an +iteration ask here: the consume task before it spends an agent run, and the +completion hook before it pushes what that run produced. +""" + +from __future__ import annotations + +from scm import actions as scm_actions +from scm.types import GetPullRequestProtocol + +from sentry.models.organization import Organization +from sentry.models.repository import Repository +from sentry.scm.factory import new as make_scm +from sentry.seer.agent.client_models import SeerRunState + + +def iteration_prs_all_closed(organization: Organization, state: SeerRunState) -> bool: + """True when the run has PRs and every one of them reads back as closed. + + A PR we cannot read (no number, repo gone, unsupported provider, API error) + counts as open: a transient read failure should not silently drop an + iteration's work. + """ + checked_any = False + + for repo_name, pr_state in state.repo_pr_states.items(): + pr_number = pr_state.pr_number + if pr_number is None: + return False + + repo, _resolution = Repository.objects.resolve_active( + organization_id=organization.id, + name=repo_name, + normalized_provider=None, + ) + if repo is None: + return False + + try: + scm = make_scm(organization.id, repo.id, referrer="seer") + except Exception: + return False + + if not isinstance(scm, GetPullRequestProtocol): + return False + + try: + pull_request = scm_actions.get_pull_request(scm, str(pr_number)) + except Exception: + return False + + if pull_request["data"]["state"] != "closed": + return False + + checked_any = True + + return checked_any diff --git a/src/sentry/tasks/seer/pr_iteration.py b/src/sentry/tasks/seer/pr_iteration.py index 3bc5b5936faa..3a0c50b58207 100644 --- a/src/sentry/tasks/seer/pr_iteration.py +++ b/src/sentry/tasks/seer/pr_iteration.py @@ -96,6 +96,7 @@ pause_pr_iteration, record_pause_blocked, ) +from sentry.seer.autofix.pr_iteration.pr_state import iteration_prs_all_closed from sentry.seer.autofix.pr_iteration.queue import ( QueuedAutofixFeedback, clear_queued_autofix_feedback, @@ -418,6 +419,14 @@ def consume_queued_autofix_feedback( activation_id=task_state.id if task_state else None, ) + if iteration_prs_all_closed(organization, state): + log_ctx.info( + "autofix.pr_iteration.consume_feedback.skipped", + trigger_id=trigger_id, + reason="pr_closed", + ) + return + try: _drain_queued_autofix_feedback( log_ctx=log_ctx, diff --git a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py index ba52538822b0..3af78f149e5b 100644 --- a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py +++ b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py @@ -600,6 +600,7 @@ def test_push_changes_pushes_when_any_unsynced_repo_not_errored(self, mock_push_ HOOK_PATH = "sentry.seer.autofix.on_completion_hook" +PR_STATE_PATH = "sentry.seer.autofix.pr_iteration.pr_state" class TestPrIterationCompletionHook(TestCase): @@ -763,9 +764,9 @@ def _github_repo(self) -> None: external_id="1", ) - @patch(f"{HOOK_PATH}.GetPullRequestProtocol", object) - @patch(f"{HOOK_PATH}.scm_actions.get_pull_request") - @patch(f"{HOOK_PATH}.make_scm") + @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) + @patch(f"{PR_STATE_PATH}.scm_actions.get_pull_request") + @patch(f"{PR_STATE_PATH}.make_scm") @patch(f"{HOOK_PATH}.trigger_push_changes") def test_a_closed_pr_stops_the_push(self, mock_push, mock_make_scm, mock_get_pull_request): """Closing the PR is the stop signal; pushing into it would talk past it.""" @@ -777,9 +778,9 @@ def test_a_closed_pr_stops_the_push(self, mock_push, mock_make_scm, mock_get_pul assert pushed is False mock_push.assert_not_called() - @patch(f"{HOOK_PATH}.GetPullRequestProtocol", object) - @patch(f"{HOOK_PATH}.scm_actions.get_pull_request") - @patch(f"{HOOK_PATH}.make_scm") + @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) + @patch(f"{PR_STATE_PATH}.scm_actions.get_pull_request") + @patch(f"{PR_STATE_PATH}.make_scm") @patch(f"{HOOK_PATH}.trigger_push_changes") def test_an_open_pr_still_pushes(self, mock_push, mock_make_scm, mock_get_pull_request): self._github_repo() @@ -790,9 +791,9 @@ def test_an_open_pr_still_pushes(self, mock_push, mock_make_scm, mock_get_pull_r assert pushed is True mock_push.assert_called_once() - @patch(f"{HOOK_PATH}.GetPullRequestProtocol", object) - @patch(f"{HOOK_PATH}.scm_actions.get_pull_request", side_effect=ValueError("boom")) - @patch(f"{HOOK_PATH}.make_scm") + @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) + @patch(f"{PR_STATE_PATH}.scm_actions.get_pull_request", side_effect=ValueError("boom")) + @patch(f"{PR_STATE_PATH}.make_scm") @patch(f"{HOOK_PATH}.trigger_push_changes") def test_a_pr_we_cannot_read_still_pushes( self, mock_push, mock_make_scm, mock_get_pull_request diff --git a/tests/sentry/tasks/seer/test_pr_iteration.py b/tests/sentry/tasks/seer/test_pr_iteration.py index 40c6fa5cbf84..3677c134bad1 100644 --- a/tests/sentry/tasks/seer/test_pr_iteration.py +++ b/tests/sentry/tasks/seer/test_pr_iteration.py @@ -8,6 +8,7 @@ from scm.types import ReviewComment from sentry.models.pullrequest import PullRequest +from sentry.models.repository import Repository from sentry.seer.agent.client_models import MemoryBlock, Message, RepoPRState, SeerRunState from sentry.seer.autofix.autofix_agent import ( PrIterationNoPullRequestException, @@ -65,6 +66,7 @@ TASK_PATH = "sentry.tasks.seer.pr_iteration" CHECK_SUITE_SOURCE_PATH = "sentry.seer.autofix.pr_iteration.feedback_sources.check_suite" PAUSE_PATH = "sentry.seer.autofix.pr_iteration.pause" +PR_STATE_PATH = "sentry.seer.autofix.pr_iteration.pr_state" class _CommentScmStub: @@ -910,6 +912,64 @@ def _state_on_head(self, **kwargs: Any) -> SeerRunState: def _call(self) -> None: consume_queued_autofix_feedback(run_id=67890, organization_id=self.organization.id) + def _state_with_open_pr(self) -> SeerRunState: + state = self._state() + state.repo_pr_states = { + "owner/repo": RepoPRState(repo_name="owner/repo", pr_number=7, commit_sha="abc") + } + Repository.objects.create( + organization_id=self.organization.id, + name="owner/repo", + provider="integrations:github", + external_id="1", + ) + return state + + @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) + @patch(f"{PR_STATE_PATH}.scm_actions.get_pull_request") + @patch(f"{PR_STATE_PATH}.make_scm") + @patch(f"{TASK_PATH}.trigger_autofix_agent") + @patch(f"{TASK_PATH}.pop_queued_autofix_feedback") + @patch(f"{TASK_PATH}.fetch_run_status") + def test_a_closed_pr_stops_the_iteration_before_it_starts( + self, + mock_fetch: MagicMock, + mock_pop: MagicMock, + mock_trigger: MagicMock, + mock_make_scm: MagicMock, + mock_get_pull_request: MagicMock, + ) -> None: + """No agent run, and the queue is left alone in case the PR reopens.""" + mock_fetch.return_value = self._state_with_open_pr() + mock_get_pull_request.return_value = {"data": {"state": "closed"}} + + self._call() + + mock_trigger.assert_not_called() + mock_pop.assert_not_called() + + @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) + @patch(f"{PR_STATE_PATH}.scm_actions.get_pull_request") + @patch(f"{PR_STATE_PATH}.make_scm") + @patch(f"{TASK_PATH}.trigger_autofix_agent") + @patch(f"{TASK_PATH}.pop_queued_autofix_feedback") + @patch(f"{TASK_PATH}.fetch_run_status") + def test_an_open_pr_still_iterates( + self, + mock_fetch: MagicMock, + mock_pop: MagicMock, + mock_trigger: MagicMock, + mock_make_scm: MagicMock, + mock_get_pull_request: MagicMock, + ) -> None: + mock_fetch.return_value = self._state_with_open_pr() + mock_pop.return_value = [self._ui_queued()] + mock_get_pull_request.return_value = {"data": {"state": "open"}} + + self._call() + + mock_trigger.assert_called_once() + @patch(f"{TASK_PATH}.trigger_autofix_agent") @patch(f"{TASK_PATH}.pop_queued_autofix_feedback") @patch(f"{TASK_PATH}.fetch_run_status") From 1965d752ba90f3dc229664d9f63db825f37fd6a0 Mon Sep 17 00:00:00 2001 From: Alex Sohn Date: Wed, 9 Sep 2026 10:40:27 -0400 Subject: [PATCH 3/6] ref(seer): Stop the iteration when any PR on the run is closed A push serves every repo on the run at once, so it cannot honor a close on one PR while still writing to the others. Treat one closed PR as the stop signal for the run rather than waiting for all of them to close. Claude-Session: https://claude.ai/code/session_017A4BhakBapTmqxiuspM29x --- src/sentry/seer/autofix/on_completion_hook.py | 4 +-- .../seer/autofix/pr_iteration/pr_state.py | 30 ++++++++-------- src/sentry/tasks/seer/pr_iteration.py | 4 +-- .../test_autofix_on_completion_hook.py | 36 +++++++++++++++++-- 4 files changed, 52 insertions(+), 22 deletions(-) diff --git a/src/sentry/seer/autofix/on_completion_hook.py b/src/sentry/seer/autofix/on_completion_hook.py index abb5fc9814c8..16f7e1a9f688 100644 --- a/src/sentry/seer/autofix/on_completion_hook.py +++ b/src/sentry/seer/autofix/on_completion_hook.py @@ -48,7 +48,7 @@ ) from sentry.seer.autofix.pr_iteration.logs import PrIterationLogContext from sentry.seer.autofix.pr_iteration.pause import PauseReason, pause_pr_iteration -from sentry.seer.autofix.pr_iteration.pr_state import iteration_prs_all_closed +from sentry.seer.autofix.pr_iteration.pr_state import iteration_prs_any_closed from sentry.seer.autofix.pr_ready_for_review import ( emit_pr_ready_for_review, format_pull_requests_payload, @@ -1124,7 +1124,7 @@ def _push_iteration_changes( ) return False - if iteration_prs_all_closed(group.organization, state): + if iteration_prs_any_closed(group.organization, state): log_ctx.info("autofix.pr_iteration.push", outcome="not_pushed", reason="pr_closed") return False diff --git a/src/sentry/seer/autofix/pr_iteration/pr_state.py b/src/sentry/seer/autofix/pr_iteration/pr_state.py index 9233d0a199e7..ff728db60043 100644 --- a/src/sentry/seer/autofix/pr_iteration/pr_state.py +++ b/src/sentry/seer/autofix/pr_iteration/pr_state.py @@ -16,19 +16,21 @@ from sentry.seer.agent.client_models import SeerRunState -def iteration_prs_all_closed(organization: Organization, state: SeerRunState) -> bool: - """True when the run has PRs and every one of them reads back as closed. +def iteration_prs_any_closed(organization: Organization, state: SeerRunState) -> bool: + """True when any PR on the run reads back as closed. + + One closed PR stops the whole run: an iteration pushes to every repo at + once, so there is no way to serve the open PRs while leaving the closed one + alone. A PR we cannot read (no number, repo gone, unsupported provider, API error) - counts as open: a transient read failure should not silently drop an + is passed over: a transient read failure should not silently drop an iteration's work. """ - checked_any = False - for repo_name, pr_state in state.repo_pr_states.items(): pr_number = pr_state.pr_number if pr_number is None: - return False + continue repo, _resolution = Repository.objects.resolve_active( organization_id=organization.id, @@ -36,24 +38,22 @@ def iteration_prs_all_closed(organization: Organization, state: SeerRunState) -> normalized_provider=None, ) if repo is None: - return False + continue try: scm = make_scm(organization.id, repo.id, referrer="seer") except Exception: - return False + continue if not isinstance(scm, GetPullRequestProtocol): - return False + continue try: pull_request = scm_actions.get_pull_request(scm, str(pr_number)) except Exception: - return False - - if pull_request["data"]["state"] != "closed": - return False + continue - checked_any = True + if pull_request["data"]["state"] == "closed": + return True - return checked_any + return False diff --git a/src/sentry/tasks/seer/pr_iteration.py b/src/sentry/tasks/seer/pr_iteration.py index 3a0c50b58207..798a83ca26fa 100644 --- a/src/sentry/tasks/seer/pr_iteration.py +++ b/src/sentry/tasks/seer/pr_iteration.py @@ -96,7 +96,7 @@ pause_pr_iteration, record_pause_blocked, ) -from sentry.seer.autofix.pr_iteration.pr_state import iteration_prs_all_closed +from sentry.seer.autofix.pr_iteration.pr_state import iteration_prs_any_closed from sentry.seer.autofix.pr_iteration.queue import ( QueuedAutofixFeedback, clear_queued_autofix_feedback, @@ -419,7 +419,7 @@ def consume_queued_autofix_feedback( activation_id=task_state.id if task_state else None, ) - if iteration_prs_all_closed(organization, state): + if iteration_prs_any_closed(organization, state): log_ctx.info( "autofix.pr_iteration.consume_feedback.skipped", trigger_id=trigger_id, diff --git a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py index 3af78f149e5b..104a3c8bb096 100644 --- a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py +++ b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py @@ -756,12 +756,12 @@ def test_a_repo_whose_pr_creation_errored_stops_the_push(self, mock_push): assert pushed is False mock_push.assert_not_called() - def _github_repo(self) -> None: + def _github_repo(self, name: str = "test-repo", external_id: str = "1") -> None: Repository.objects.create( organization_id=self.organization.id, - name="test-repo", + name=name, provider="integrations:github", - external_id="1", + external_id=external_id, ) @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) @@ -791,6 +791,36 @@ def test_an_open_pr_still_pushes(self, mock_push, mock_make_scm, mock_get_pull_r assert pushed is True mock_push.assert_called_once() + @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) + @patch(f"{PR_STATE_PATH}.scm_actions.get_pull_request") + @patch(f"{PR_STATE_PATH}.make_scm") + @patch(f"{HOOK_PATH}.trigger_push_changes") + def test_one_closed_pr_stops_a_multi_repo_push( + self, mock_push, mock_make_scm, mock_get_pull_request + ): + """A push serves every repo at once, so it cannot skip just the closed one.""" + self._github_repo() + self._github_repo("other-repo", external_id="2") + state = self._unsynced() + state.repo_pr_states["other-repo"] = RepoPRState( + repo_name="other-repo", + provider="github", + pr_id=88, + pr_number=8, + pr_url="https://example.com/pull/8", + pr_creation_status="completed", + commit_sha="stale-sha", + ) + mock_get_pull_request.side_effect = [ + {"data": {"state": "open"}}, + {"data": {"state": "closed"}}, + ] + + pushed = self._push(state) + + assert pushed is False + mock_push.assert_not_called() + @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) @patch(f"{PR_STATE_PATH}.scm_actions.get_pull_request", side_effect=ValueError("boom")) @patch(f"{PR_STATE_PATH}.make_scm") From 2a1009d163b631aa46ed726a022894fb63c8dc4c Mon Sep 17 00:00:00 2001 From: Alex Sohn Date: Wed, 9 Sep 2026 10:59:28 -0400 Subject: [PATCH 4/6] feat(seer): Pause the run when a PR it iterates on is closed Both gates now write the pause marker, so a closed PR costs one read once rather than a read on every later trigger. The pause is one-way, as we have no signal to lift it on: nothing tells us a PR reopened. Add autofix.pr_iteration.pr_closed, tagged with the gate that caught it, so we can see how often a close lands mid-iteration versus before one starts. Claude-Session: https://claude.ai/code/session_017A4BhakBapTmqxiuspM29x --- src/sentry/seer/autofix/on_completion_hook.py | 11 ++++++++++- src/sentry/seer/autofix/pr_iteration/pause.py | 1 + src/sentry/seer/autofix/pr_iteration/pr_state.py | 13 +++++++++++++ src/sentry/seer/endpoints/group_ai_autofix.py | 1 + src/sentry/tasks/seer/pr_iteration.py | 11 ++++++++++- .../seer/autofix/test_autofix_on_completion_hook.py | 8 +++++++- tests/sentry/tasks/seer/test_pr_iteration.py | 8 +++++++- 7 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/sentry/seer/autofix/on_completion_hook.py b/src/sentry/seer/autofix/on_completion_hook.py index 16f7e1a9f688..0797ff217e82 100644 --- a/src/sentry/seer/autofix/on_completion_hook.py +++ b/src/sentry/seer/autofix/on_completion_hook.py @@ -48,7 +48,10 @@ ) from sentry.seer.autofix.pr_iteration.logs import PrIterationLogContext from sentry.seer.autofix.pr_iteration.pause import PauseReason, pause_pr_iteration -from sentry.seer.autofix.pr_iteration.pr_state import iteration_prs_any_closed +from sentry.seer.autofix.pr_iteration.pr_state import ( + iteration_prs_any_closed, + record_pr_closed, +) from sentry.seer.autofix.pr_ready_for_review import ( emit_pr_ready_for_review, format_pull_requests_payload, @@ -1125,6 +1128,12 @@ def _push_iteration_changes( return False if iteration_prs_any_closed(group.organization, state): + record_pr_closed("push") + pause_pr_iteration( + run_id=run_id, + organization_id=group.organization.id, + reason=PauseReason.PR_CLOSED, + ) log_ctx.info("autofix.pr_iteration.push", outcome="not_pushed", reason="pr_closed") return False diff --git a/src/sentry/seer/autofix/pr_iteration/pause.py b/src/sentry/seer/autofix/pr_iteration/pause.py index c06404436587..60ef1f18ed8c 100644 --- a/src/sentry/seer/autofix/pr_iteration/pause.py +++ b/src/sentry/seer/autofix/pr_iteration/pause.py @@ -27,6 +27,7 @@ class PauseReason(StrEnum): USER_STOP = "user_stop" RUN_ERRORED = "run_errored" + PR_CLOSED = "pr_closed" def _get_seer_run(run_id: int, organization_id: int) -> SeerRun | None: diff --git a/src/sentry/seer/autofix/pr_iteration/pr_state.py b/src/sentry/seer/autofix/pr_iteration/pr_state.py index ff728db60043..9d50abe932f8 100644 --- a/src/sentry/seer/autofix/pr_iteration/pr_state.py +++ b/src/sentry/seer/autofix/pr_iteration/pr_state.py @@ -7,6 +7,8 @@ from __future__ import annotations +from typing import Literal + from scm import actions as scm_actions from scm.types import GetPullRequestProtocol @@ -14,6 +16,17 @@ from sentry.models.repository import Repository from sentry.scm.factory import new as make_scm from sentry.seer.agent.client_models import SeerRunState +from sentry.utils import metrics + +PR_CLOSED_METRIC = "autofix.pr_iteration.pr_closed" + +# Where we caught it: before the agent run, or before the push. The only tag on +# the metric, and closed so it stays two time series. +PrClosedGate = Literal["consume", "push"] + + +def record_pr_closed(gate: PrClosedGate) -> None: + metrics.incr(PR_CLOSED_METRIC, tags={"gate": gate}) def iteration_prs_any_closed(organization: Organization, state: SeerRunState) -> bool: diff --git a/src/sentry/seer/endpoints/group_ai_autofix.py b/src/sentry/seer/endpoints/group_ai_autofix.py index 121998ccadcd..32b61308497c 100644 --- a/src/sentry/seer/endpoints/group_ai_autofix.py +++ b/src/sentry/seer/endpoints/group_ai_autofix.py @@ -98,6 +98,7 @@ PAUSED_PR_ITERATION_DETAIL = { PauseReason.USER_STOP: "Iteration was stopped for this pull request", PauseReason.RUN_ERRORED: "Seer can no longer iterate on this pull request", + PauseReason.PR_CLOSED: "This pull request is closed, so Seer stopped iterating on it", } diff --git a/src/sentry/tasks/seer/pr_iteration.py b/src/sentry/tasks/seer/pr_iteration.py index 798a83ca26fa..d23d03ecd706 100644 --- a/src/sentry/tasks/seer/pr_iteration.py +++ b/src/sentry/tasks/seer/pr_iteration.py @@ -96,7 +96,10 @@ pause_pr_iteration, record_pause_blocked, ) -from sentry.seer.autofix.pr_iteration.pr_state import iteration_prs_any_closed +from sentry.seer.autofix.pr_iteration.pr_state import ( + iteration_prs_any_closed, + record_pr_closed, +) from sentry.seer.autofix.pr_iteration.queue import ( QueuedAutofixFeedback, clear_queued_autofix_feedback, @@ -420,6 +423,12 @@ def consume_queued_autofix_feedback( ) if iteration_prs_any_closed(organization, state): + record_pr_closed("consume") + pause_pr_iteration( + run_id=run_id, + organization_id=organization_id, + reason=PauseReason.PR_CLOSED, + ) log_ctx.info( "autofix.pr_iteration.consume_feedback.skipped", trigger_id=trigger_id, diff --git a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py index 104a3c8bb096..d77dec811f88 100644 --- a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py +++ b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py @@ -764,11 +764,15 @@ def _github_repo(self, name: str = "test-repo", external_id: str = "1") -> None: external_id=external_id, ) + @patch(f"{PR_STATE_PATH}.metrics.incr") + @patch(f"{HOOK_PATH}.pause_pr_iteration") @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) @patch(f"{PR_STATE_PATH}.scm_actions.get_pull_request") @patch(f"{PR_STATE_PATH}.make_scm") @patch(f"{HOOK_PATH}.trigger_push_changes") - def test_a_closed_pr_stops_the_push(self, mock_push, mock_make_scm, mock_get_pull_request): + def test_a_closed_pr_stops_the_push( + self, mock_push, mock_make_scm, mock_get_pull_request, mock_pause, mock_incr + ): """Closing the PR is the stop signal; pushing into it would talk past it.""" self._github_repo() mock_get_pull_request.return_value = {"data": {"state": "closed"}} @@ -777,6 +781,8 @@ def test_a_closed_pr_stops_the_push(self, mock_push, mock_make_scm, mock_get_pul assert pushed is False mock_push.assert_not_called() + assert mock_pause.call_args.kwargs["reason"] == PauseReason.PR_CLOSED + mock_incr.assert_any_call("autofix.pr_iteration.pr_closed", tags={"gate": "push"}) @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) @patch(f"{PR_STATE_PATH}.scm_actions.get_pull_request") diff --git a/tests/sentry/tasks/seer/test_pr_iteration.py b/tests/sentry/tasks/seer/test_pr_iteration.py index 3677c134bad1..80207cc268f7 100644 --- a/tests/sentry/tasks/seer/test_pr_iteration.py +++ b/tests/sentry/tasks/seer/test_pr_iteration.py @@ -925,6 +925,8 @@ def _state_with_open_pr(self) -> SeerRunState: ) return state + @patch(f"{PR_STATE_PATH}.metrics.incr") + @patch(f"{TASK_PATH}.pause_pr_iteration") @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) @patch(f"{PR_STATE_PATH}.scm_actions.get_pull_request") @patch(f"{PR_STATE_PATH}.make_scm") @@ -938,8 +940,10 @@ def test_a_closed_pr_stops_the_iteration_before_it_starts( mock_trigger: MagicMock, mock_make_scm: MagicMock, mock_get_pull_request: MagicMock, + mock_pause: MagicMock, + mock_incr: MagicMock, ) -> None: - """No agent run, and the queue is left alone in case the PR reopens.""" + """The run is paused rather than left to re-check on every later trigger.""" mock_fetch.return_value = self._state_with_open_pr() mock_get_pull_request.return_value = {"data": {"state": "closed"}} @@ -947,6 +951,8 @@ def test_a_closed_pr_stops_the_iteration_before_it_starts( mock_trigger.assert_not_called() mock_pop.assert_not_called() + assert mock_pause.call_args.kwargs["reason"] == PauseReason.PR_CLOSED + mock_incr.assert_any_call("autofix.pr_iteration.pr_closed", tags={"gate": "consume"}) @patch(f"{PR_STATE_PATH}.GetPullRequestProtocol", object) @patch(f"{PR_STATE_PATH}.scm_actions.get_pull_request") From c985ea2f87bbda6b339a4c54c0da1a2ecffcb2d1 Mon Sep 17 00:00:00 2001 From: Alex Sohn Date: Fri, 11 Sep 2026 12:15:04 -0400 Subject: [PATCH 5/6] fix(seer): Narrow PR iteration repo lookup to GitHub The closed-PR gate resolved the repo by name with no provider narrowing, so an org with the same repo name on two providers resolved as ambiguous and the PR was skipped. PR iteration only runs on GitHub, so narrow the lookup to that provider. Claude-Session: https://claude.ai/code/session_01345bPhEwEAN5uQHJYXar9T --- src/sentry/seer/autofix/pr_iteration/pr_state.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sentry/seer/autofix/pr_iteration/pr_state.py b/src/sentry/seer/autofix/pr_iteration/pr_state.py index 9d50abe932f8..0e4a5332a24a 100644 --- a/src/sentry/seer/autofix/pr_iteration/pr_state.py +++ b/src/sentry/seer/autofix/pr_iteration/pr_state.py @@ -16,6 +16,7 @@ from sentry.models.repository import Repository from sentry.scm.factory import new as make_scm from sentry.seer.agent.client_models import SeerRunState +from sentry.seer.autofix.pr_iteration.constants import PR_ITERATION_PROVIDER_SLUG from sentry.utils import metrics PR_CLOSED_METRIC = "autofix.pr_iteration.pr_closed" @@ -48,7 +49,9 @@ def iteration_prs_any_closed(organization: Organization, state: SeerRunState) -> repo, _resolution = Repository.objects.resolve_active( organization_id=organization.id, name=repo_name, - normalized_provider=None, + # Narrowed to GitHub: PR iteration runs nowhere else, and an + # unnarrowed name that repeats across providers reads as ambiguous. + normalized_provider=PR_ITERATION_PROVIDER_SLUG, ) if repo is None: continue From c4cb91486801995b88340b6e7e4a965786646e5a Mon Sep 17 00:00:00 2001 From: Alex Sohn <44201357+alexsohn1126@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:38:19 -0400 Subject: [PATCH 6/6] Update pr_state.py --- src/sentry/seer/autofix/pr_iteration/pr_state.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/sentry/seer/autofix/pr_iteration/pr_state.py b/src/sentry/seer/autofix/pr_iteration/pr_state.py index 0e4a5332a24a..505387d3fd27 100644 --- a/src/sentry/seer/autofix/pr_iteration/pr_state.py +++ b/src/sentry/seer/autofix/pr_iteration/pr_state.py @@ -49,8 +49,6 @@ def iteration_prs_any_closed(organization: Organization, state: SeerRunState) -> repo, _resolution = Repository.objects.resolve_active( organization_id=organization.id, name=repo_name, - # Narrowed to GitHub: PR iteration runs nowhere else, and an - # unnarrowed name that repeats across providers reads as ambiguous. normalized_provider=PR_ITERATION_PROVIDER_SLUG, ) if repo is None: