diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py index 22dd76b31bd0..2df5468cfee6 100644 --- a/src/sentry/features/temporary.py +++ b/src/sentry/features/temporary.py @@ -342,6 +342,8 @@ def register_temporary_features(manager: FeatureManager) -> None: manager.add("organizations:autofix-pr-iteration-review-request", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Assign the triggering user and post a status comment once a Seer PR exhausts its CI-fix iteration cap manager.add("organizations:autofix-pr-iteration-cap-assign", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) + # Keep PR iteration turns on their PR's latest commit, and retry a push the branch moved under + manager.add("organizations:autofix-pr-iteration-pin-to-pr-head", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Enable the pr content verification prior to opening a PR manager.add("organizations:autofix-verify-pr-content", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Steer autofix to run the repo's linter and tests over its changes diff --git a/src/sentry/seer/agent/client_utils.py b/src/sentry/seer/agent/client_utils.py index 02fa31d8c4dc..3e1ef09d5450 100644 --- a/src/sentry/seer/agent/client_utils.py +++ b/src/sentry/seer/agent/client_utils.py @@ -144,6 +144,7 @@ class AgentRunOptions(TypedDict): enable_tool_summary: NotRequired[bool] embed_widgets: NotRequired[list[dict[str, Any]] | None] enable_streaming: NotRequired[bool] + pin_to_pr_head: NotRequired[bool] class SeerFeatureRunRequest(TypedDict): diff --git a/src/sentry/seer/autofix/feature/dispatch.py b/src/sentry/seer/autofix/feature/dispatch.py index f8a115e1ebe7..0f8a04755f08 100644 --- a/src/sentry/seer/autofix/feature/dispatch.py +++ b/src/sentry/seer/autofix/feature/dispatch.py @@ -153,6 +153,10 @@ def trigger_autofix_feature( ) if features.has("organizations:seer-autofix-code-mode", group.organization): agent_run_options["enable_code_mode_tools"] = "only" + if args.step == AutofixStep.PR_ITERATION and features.has( + "organizations:autofix-pr-iteration-pin-to-pr-head", group.organization + ): + agent_run_options["pin_to_pr_head"] = True extras: dict[str, Any] = { "referrer": args.referrer.value, diff --git a/src/sentry/seer/autofix/pr_iteration/completion.py b/src/sentry/seer/autofix/pr_iteration/completion.py index d21c4941d462..99d37c79e937 100644 --- a/src/sentry/seer/autofix/pr_iteration/completion.py +++ b/src/sentry/seer/autofix/pr_iteration/completion.py @@ -14,12 +14,18 @@ from sentry.seer.autofix.autofix_agent import trigger_push_changes from sentry.seer.autofix.commit_author import SeerCommitAuthor, parse_commit_author from sentry.seer.autofix.constants import AutofixReferrer +from sentry.seer.autofix.feature.dispatch import AutofixFeatureArgs, trigger_autofix_feature +from sentry.seer.autofix.feature.models import PrIterationStepArgs +from sentry.seer.autofix.pr_iteration.current_iteration import triggered_iteration_id from sentry.seer.autofix.pr_iteration.emit import ( PrIterationOutcome, complete_pr_iteration_details, outcome_for_failed_run, ) -from sentry.seer.autofix.pr_iteration.feedback import latest_iteration_feedback_kind +from sentry.seer.autofix.pr_iteration.feedback import ( + automated_iteration_allowed, + latest_iteration_feedback_kind, +) from sentry.seer.autofix.pr_iteration.feedback_sources.base import ConsumeTriggerSource from sentry.seer.autofix.pr_iteration.iterations import ( get_iterations, @@ -35,6 +41,7 @@ ) from sentry.seer.autofix.pr_iteration.tracing import set_pr_iteration_attributes from sentry.seer.autofix.pr_ready_for_review import format_pull_requests_payload +from sentry.seer.autofix.steps import AutofixStep from sentry.tasks.seer.pr_iteration import consume_queued_autofix_feedback from sentry.utils import metrics @@ -43,6 +50,11 @@ logger = logging.getLogger(__name__) +BRANCH_MOVED_RETRY_PROMPT = ( + "Your last push failed because the pull request's branch has new commits. Make sure your " + "changes are integrated with the latest code on the branch." +) + def _iteration_repo_states(state: SeerRunState) -> list[dict[str, Any]]: return [ @@ -390,6 +402,8 @@ def pr_iteration_push_outcome( return PrIterationOutcome.ALREADY_PUSHED errored_repos = iteration_terminal_errored_repos(state) + if errored_repos and _push_failed_on_moved_branch(group, state, errored_repos): + return _retry_push_on_moved_branch(log_ctx, group, run_id, state) if errored_repos: log_ctx.info( "autofix.pr_iteration.push", @@ -461,3 +475,57 @@ def _push_iteration_changes( log_ctx.info("autofix.pr_iteration.push", outcome="pushed", reason="ok") return True + + +def _push_failed_on_moved_branch( + group: Group, state: SeerRunState, errored_repos: list[str] +) -> bool: + return features.has( + "organizations:autofix-pr-iteration-pin-to-pr-head", group.organization + ) and any( + state.repo_pr_states[repo].pr_creation_error_code == "branch_moved" + for repo in errored_repos + ) + + +@traces.trace +def _retry_push_on_moved_branch( + log_ctx: PrIterationLogContext, + group: Group, + run_id: int, + state: SeerRunState, +) -> PrIterationOutcome | None: + """Run another turn of the same iteration. It starts on the PR's latest commit, so the push + that follows it can land on top.""" + decision = automated_iteration_allowed(state) + if not decision.ok: + log_ctx.info( + "autofix.pr_iteration.push", + outcome="not_pushed", + reason="branch_moved_retry_blocked", + decision=decision.reason, + ) + return PrIterationOutcome.PR_CREATION_ERRORED + + iteration = get_iterations(state)[-1] + metadata = iteration.blocks[0].message.metadata or {} + trigger_autofix_feature( + group, + AutofixFeatureArgs( + step=AutofixStep.PR_ITERATION, + referrer=AutofixReferrer.ON_COMPLETION_HOOK, + existing_run_id=run_id, + user_context=BRANCH_MOVED_RETRY_PROMPT, + step_args=PrIterationStepArgs( + iteration_index=iteration.index, + iteration_id=triggered_iteration_id(state), + feedback=metadata.get("feedback"), + commit_author=metadata.get("commit_author"), + pr_urls={ + pr.repo_name: pr.pr_url for pr in state.repo_pr_states.values() if pr.pr_url + }, + ), + ), + ) + log_ctx.info("autofix.pr_iteration.push", outcome="retrying", reason="branch_moved") + return None 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 36da7721f770..7d65dddaa0ef 100644 --- a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py +++ b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py @@ -18,6 +18,7 @@ from sentry.seer.autofix.autofix_agent import _group_and_referrer_from_run, get_current_step from sentry.seer.autofix.commit_author import SeerCommitAuthor from sentry.seer.autofix.constants import AutofixReferrer +from sentry.seer.autofix.feature.models import LEGACY_FEATURE_ID from sentry.seer.autofix.on_completion_hook import ( PIPELINE_ORDER, STOPPING_POINT_TO_STEP, @@ -25,6 +26,7 @@ _stopping_point_from_run, ) from sentry.seer.autofix.pr_iteration.completion import ( + BRANCH_MOVED_RETRY_PROMPT, continue_pr_iteration, iteration_completed_webhook_fields, iteration_log_context, @@ -999,6 +1001,46 @@ def test_a_terminally_errored_repo_reaches_completion_details_as_that_outcome( == PrIterationOutcome.PR_CREATION_ERRORED.value ) + def test_a_push_refused_because_the_branch_moved_retries_the_iteration_pinned(self): + run = self.create_seer_run(organization=self.organization, seer_run_state_id=123) + self.create_seer_agent_run( + run=run, source=LEGACY_FEATURE_ID, group=self.group, project=self.project + ) + state = self._unsynced() + state.blocks[0].message.metadata = { + **(state.blocks[0].message.metadata or {}), + "iteration_id": "41", + "feedback": "[]", + } + pr_state = state.repo_pr_states["test-repo"] + pr_state.pr_creation_status = "error" + pr_state.pr_creation_error_code = "branch_moved" + + with ( + self.feature("organizations:autofix-pr-iteration-pin-to-pr-head"), + patch("sentry.seer.autofix.feature.dispatch.SeerAgentClient") as mock_client_cls, + patch(f"{COMPLETION_PATH}.trigger_push_changes") as mock_push, + ): + mock_client_cls.return_value.continue_feature_run.return_value = run + outcome = pr_iteration_push_outcome( + iteration_log_context(self.organization, self.group, state), + self.group, + 123, + state, + AutofixReferrer.ON_COMPLETION_HOOK, + ) + + assert outcome is None + mock_push.assert_not_called() + kwargs = mock_client_cls.return_value.continue_feature_run.call_args.kwargs + assert kwargs["agent_run_options"]["pin_to_pr_head"] is True + payload = kwargs["payload"] + assert payload["step"] == AutofixStep.PR_ITERATION + assert payload["user_context"] == BRANCH_MOVED_RETRY_PROMPT + assert payload["step_args"]["iteration_index"] == 1 + assert payload["step_args"]["iteration_id"] == 41 + assert payload["step_args"]["feedback"] == "[]" + @patch(f"{COMPLETION_PATH}.complete_pr_iteration_details") @patch(f"{COMPLETION_PATH}.consume_queued_feedback") @patch(f"{COMPLETION_PATH}.trigger_push_changes")