Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 35 additions & 1 deletion src/sentry/seer/autofix/autofix_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
read_preference_from_sentry_db,
)
from sentry.seer.entrypoints.operator import SeerAutofixOperator, process_autofix_updates
from sentry.seer.models import SeerRepoDefinition
from sentry.seer.models import SeerApiError, SeerRepoDefinition
from sentry.seer.models.run import SeerRun
from sentry.seer.models.seer_api_models import SeerPermissionError
from sentry.sentry_apps.event_types import SentryAppEventType
Expand Down Expand Up @@ -531,6 +531,40 @@ def trigger_autofix_agent(
if not has_budget:
raise NoSeerQuotaException()

use_seer_rca_feature = features.has(
"organizations:autofix-rca-in-seer", group.organization, actor=user
)
if step == AutofixStep.ROOT_CAUSE and run_id is None and use_seer_rca_feature:
# Local import avoids a circular import (dispatch imports this module).
from sentry.seer.autofix_rca.dispatch import trigger_autofix_rca_feature

feature_run = trigger_autofix_rca_feature(
group, referrer=referrer, user_context=user_context
)
feature_run_id = feature_run.seer_run_state_id
if feature_run_id is None:
# flush=True populates this on success; guard defensively.
raise SeerApiError("autofix_rca feature run has no run id", 500)
Comment on lines +544 to +547

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The new SeerApiError raised by trigger_autofix_agent is not handled by several callers in async tasks, which will lead to silent task failures.
Severity: MEDIUM

Suggested Fix

Wrap the calls to trigger_autofix_agent in on_completion_hook.py, issue_summary.py, and pr_iteration.py with a try...except SeerApiError block to properly handle this new exception. This will prevent silent task failures and allow for explicit error logging or handling.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/seer/autofix/autofix_agent.py#L541-L544

Potential issue: The function `trigger_autofix_agent` can now raise a `SeerApiError` if
a `feature_run` object lacks a `seer_run_state_id`. This new exception is not handled in
three locations where the function is called within asynchronous tasks:
`on_completion_hook.py`, `issue_summary.py`, and `pr_iteration.py`. Consequently, if
this error condition occurs, the async tasks will fail. While the task framework may log
the exception, the application logic does not handle it explicitly, leading to silent
failures that are difficult to diagnose and obscure the root cause of the problem.

Also affects:

  • src/sentry/seer/autofix/on_completion_hook.py:571
  • src/sentry/seer/autofix/issue_summary.py:182
  • src/sentry/tasks/seer/pr_iteration.py:211

Did we get this right? 👍 / 👎 to inform future reviews.


logger.info(
"autofix.trigger.routed_to_rca_feature",
extra={
"group_id": group.id,
"organization_id": group.organization.id,
"run_id": feature_run_id,
"referrer": referrer.value,
},
)

handle_step_started_events(
group,
AutofixStep.ROOT_CAUSE,
feature_run_id,
str(feature_run.uuid),
referrer,
)
return feature_run_id

config = STEP_CONFIGS[step]

pr_iteration_enabled = features.has("organizations:autofix-pr-iteration", group.organization)
Expand Down
102 changes: 102 additions & 0 deletions tests/sentry/seer/autofix/test_autofix_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,108 @@ def test_trigger_autofix_agent_passes_project_and_group_to_client(
assert call_kwargs["project"] == self.group.project
assert call_kwargs["group"] == self.group

@patch("sentry.quotas.backend.record_seer_run")
@patch("sentry.quotas.backend.check_seer_quota", return_value=True)
@patch("sentry.seer.autofix.autofix_agent.broadcast_webhooks_for_organization.delay")
@patch("sentry.seer.autofix_rca.dispatch.trigger_autofix_rca_feature")
@patch("sentry.seer.autofix.autofix_agent.SeerAgentClient")
def test_root_cause_routes_to_rca_feature_when_flagged(
self, mock_client_class, mock_feature, mock_broadcast, mock_check_quota, mock_record_run
):
"""With the flag on, a new root-cause run dispatches the autofix_rca feature
instead of a legacy explorer run, still emits the started webhook, and
returns the feature run's id."""
feature_run = self.create_seer_run(
organization=self.group.organization, type="feature_run", seer_run_state_id=777
)
mock_feature.return_value = feature_run

with self.feature("organizations:autofix-rca-in-seer"):
result = trigger_autofix_agent(
group=self.group,
step=AutofixStep.ROOT_CAUSE,
referrer=AutofixReferrer.UNKNOWN,
run_id=None,
)

assert result == 777
mock_feature.assert_called_once()
assert mock_feature.call_args.args[0] == self.group
# legacy explorer run is not started for a flagged root-cause kickoff
mock_client_class.return_value.start_run.assert_not_called()
# the started webhook still fires, pointing at the feature run
mock_broadcast.assert_called_once()
payload = mock_broadcast.call_args.kwargs["payload"]
assert (
mock_broadcast.call_args.kwargs["event_name"] == SeerActionType.ROOT_CAUSE_STARTED.value
)
assert payload["run_id"] == 777
assert payload["sentry_run_id"] == str(feature_run.uuid)

@patch("sentry.quotas.backend.record_seer_run")
@patch("sentry.quotas.backend.check_seer_quota", return_value=True)
@patch("sentry.seer.autofix_rca.dispatch.trigger_autofix_rca_feature")
@patch("sentry.seer.autofix.autofix_agent.SeerAgentClient")
def test_solution_step_does_not_route_to_rca_feature(
self, mock_client_class, mock_feature, mock_check_quota, mock_record_run
):
"""Only the root-cause step routes; solution kickoffs stay on the legacy flow."""
mock_client = MagicMock()
mock_client_class.return_value = mock_client
mock_client.start_run.return_value = MagicMock(seer_run_state_id=5)

with self.feature("organizations:autofix-rca-in-seer"):
trigger_autofix_agent(
group=self.group,
step=AutofixStep.SOLUTION,
referrer=AutofixReferrer.UNKNOWN,
run_id=None,
)

mock_feature.assert_not_called()
mock_client.start_run.assert_called_once()

@patch("sentry.quotas.backend.record_seer_run")
@patch("sentry.quotas.backend.check_seer_quota", return_value=True)
@patch("sentry.seer.autofix_rca.dispatch.trigger_autofix_rca_feature")
@patch("sentry.seer.autofix.autofix_agent.SeerAgentClient")
def test_root_cause_uses_legacy_flow_without_flag(
self, mock_client_class, mock_feature, mock_check_quota, mock_record_run
):
"""Without the flag, root-cause kickoffs use the legacy explorer run."""
mock_client = MagicMock()
mock_client_class.return_value = mock_client
mock_client.start_run.return_value = MagicMock(seer_run_state_id=9)

result = trigger_autofix_agent(
group=self.group,
step=AutofixStep.ROOT_CAUSE,
referrer=AutofixReferrer.UNKNOWN,
run_id=None,
)

assert result == 9
mock_feature.assert_not_called()
mock_client.start_run.assert_called_once()

@patch("sentry.quotas.backend.check_seer_quota", return_value=False)
@patch("sentry.seer.autofix_rca.dispatch.trigger_autofix_rca_feature")
@patch("sentry.seer.autofix.autofix_agent.SeerAgentClient")
def test_flagged_root_cause_still_enforces_quota(
self, mock_client_class, mock_feature, mock_check_quota
):
"""The upstream quota check runs before routing to the feature."""
with self.feature("organizations:autofix-rca-in-seer"):
with pytest.raises(NoSeerQuotaException):
trigger_autofix_agent(
group=self.group,
step=AutofixStep.ROOT_CAUSE,
referrer=AutofixReferrer.UNKNOWN,
run_id=None,
)

mock_feature.assert_not_called()

@patch("sentry.quotas.backend.record_seer_run")
@patch("sentry.quotas.backend.check_seer_quota", return_value=True)
@patch("sentry.seer.autofix.autofix_agent.broadcast_webhooks_for_organization.delay")
Expand Down
Loading