Skip to content
3 changes: 0 additions & 3 deletions src/sentry/seer/autofix/autofix_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from enum import StrEnum
from typing import TYPE_CHECKING, Literal

from django.utils import timezone
from pydantic import BaseModel

from sentry.seer.autofix.artifact_schemas import (
Expand Down Expand Up @@ -246,8 +245,6 @@ def trigger_autofix_explorer(
artifact_schema=artifact_schema,
)

group.update(seer_explorer_autofix_last_triggered=timezone.now())

payload = {
"run_id": run_id,
"group_id": group.id,
Expand Down
85 changes: 47 additions & 38 deletions src/sentry/seer/autofix/on_completion_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import logging
from typing import TYPE_CHECKING

from django.utils import timezone

from sentry import features
from sentry.models.group import Group
from sentry.models.organization import Organization
Expand Down Expand Up @@ -76,13 +78,25 @@
)
return

group_id = state.metadata.get("group_id") if state.metadata else None
if group_id is None:
group = None
else:
try:
group = Group.objects.get(id=group_id)
except Group.DoesNotExist:
group = None

# Send webhook for the completed step
cls._send_step_webhook(organization, run_id, state)
cls._send_step_webhook(organization, run_id, state, group)

cls._maybe_trigger_supergroups_embedding(organization, run_id, state)
cls._maybe_trigger_supergroups_embedding(organization, run_id, state, group)

# Continue the automated pipeline if stopping_point hasn't been reached
cls._maybe_continue_pipeline(organization, run_id, state)
cls._maybe_continue_pipeline(organization, run_id, state, group)

if group is not None:
group.update(seer_explorer_autofix_last_triggered=timezone.now())
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Outdated

@classmethod
def find_latest_artifact_for_step(cls, state: SeerRunState, key: str) -> Artifact | None:
Expand All @@ -95,19 +109,27 @@
return None

@classmethod
def _send_step_webhook(cls, organization, run_id, state: SeerRunState):
def _send_step_webhook(
cls,
organization: Organization,
run_id: int,
state: SeerRunState,
group: Group | None,
):
"""
Send webhook for the completed step.

Determines which step just completed and sends the appropriate webhook event.
"""
current_step = cls._get_current_step(state)
if group is None:
return

webhook_payload = {"run_id": run_id}
current_step = cls._get_current_step(state)

group_id = state.metadata.get("group_id") if state.metadata else None
if group_id is not None:
webhook_payload["group_id"] = group_id
webhook_payload = {
"run_id": run_id,
"group_id": group.id,
}

# Iterate through blocks in reverse order (most recent first)
# to find which step just completed
Expand Down Expand Up @@ -213,25 +235,21 @@
organization: Organization,
run_id: int,
state: SeerRunState,
group: Group | None,
) -> None:
"""Trigger supergroups embedding if feature flag is enabled."""
current_step = cls._get_current_step(state)
if current_step != AutofixStep.ROOT_CAUSE:
return

group_id = state.metadata.get("group_id") if state.metadata else None
if group_id is None:
return

try:
group = Group.objects.get(id=group_id)
except Group.DoesNotExist:
if group is None:
group_id = state.metadata.get("group_id") if state.metadata else None
logger.warning(
"autofix.supergroup_embedding.group_not_found",
extra={"group_id": group_id},
)
return

Check warning on line 252 in src/sentry/seer/autofix/on_completion_hook.py

View check run for this annotation

@sentry/warden / warden: sentry-security

Organization scoping removed from Group fetch - defense-in-depth weakened

The code change removes organization scoping from the Group.objects.get() call. Previously in `_maybe_continue_pipeline`, the group was fetched with `Group.objects.get(id=group_id, project__organization=organization)`. Now in `execute()` at line 86, it uses `Group.objects.get(id=group_id)` without verifying the group belongs to the organization. While this is an internal Seer webhook (not user-facing), removing the org scope weakens defense-in-depth. If Seer's metadata were ever corrupted or buggy, operations (webhook broadcasts, group timestamp updates, autofix triggers) could affect groups from other organizations.
if not features.has("projects:supergroup-embeddings-explorer", group.project):
return

Expand All @@ -242,7 +260,7 @@
try:
trigger_supergroups_embedding(
organization_id=organization.id,
group_id=group_id,
group_id=group.id,
project_id=group.project_id,
artifact_data=root_cause_artifact.data,
)
Expand All @@ -252,7 +270,7 @@
extra={
"run_id": run_id,
"organization_id": organization.id,
"group_id": group_id,
"group_id": group.id,
},
)

Expand Down Expand Up @@ -289,6 +307,7 @@
organization: Organization,
run_id: int,
state: SeerRunState,
group: Group | None,
) -> None:
"""
Continue to the next step if stopping_point hasn't been reached.
Expand All @@ -307,12 +326,16 @@
return

stopping_point = AutofixStoppingPoint(metadata["stopping_point"])
group_id = metadata.get("group_id")

if not group_id:
if group is None:
group_id = state.metadata.get("group_id") if state.metadata else None
logger.warning(
"autofix.on_completion_hook.no_group_id_in_metadata",
extra={"run_id": run_id, "organization_id": organization.id},
"autofix.on_completion_hook.group_not_found",
extra={
"run_id": run_id,
"organization_id": organization.id,
"group_id": group_id,
},
)
return

Expand All @@ -331,10 +354,10 @@

# Check if we should trigger coding agent handoff instead of continuing
handoff_config = cls._get_handoff_config_if_applicable(
stopping_point, current_step, group_id
stopping_point, current_step, group.id
)
if handoff_config:
cls._trigger_coding_agent_handoff(organization, run_id, group_id, handoff_config)
cls._trigger_coding_agent_handoff(organization, run_id, group.id, handoff_config)
return

# Special case: if stopping_point is open_pr and we just finished code_changes, push changes
Expand Down Expand Up @@ -363,20 +386,6 @@
)
return

# Get the group
try:
group = Group.objects.get(id=group_id, project__organization=organization)
except Group.DoesNotExist:
logger.warning(
"autofix.on_completion_hook.group_not_found",
extra={
"run_id": run_id,
"organization_id": organization.id,
"group_id": group_id,
},
)
return

# Trigger the next step
logger.info(
"autofix.on_completion_hook.continuing_pipeline",
Expand Down
24 changes: 0 additions & 24 deletions tests/sentry/seer/autofix/test_autofix_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,30 +354,6 @@ def test_trigger_autofix_explorer_passes_group_id_in_metadata(
call_kwargs = mock_client.start_run.call_args.kwargs
assert call_kwargs["metadata"] == {"group_id": self.group.id, "referrer": "unknown"}

@patch("sentry.seer.autofix.autofix_agent.broadcast_webhooks_for_organization.delay")
@patch("sentry.seer.autofix.autofix_agent.SeerExplorerClient")
def test_trigger_autofix_explorer_updates_explorer_last_triggered(
self, mock_client_class, mock_broadcast
):
"""trigger_autofix_explorer sets seer_explorer_autofix_last_triggered on the group."""
mock_client = MagicMock()
mock_client_class.return_value = mock_client
mock_client.start_run.return_value = 123

assert self.group.seer_explorer_autofix_last_triggered is None

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

self.group.refresh_from_db()

assert self.group.seer_autofix_last_triggered is None
assert self.group.seer_explorer_autofix_last_triggered is not None


class TestTriggerCodingAgentHandoff(TestCase):
"""Tests for trigger_coding_agent_handoff function."""
Expand Down
36 changes: 22 additions & 14 deletions tests/sentry/seer/autofix/test_autofix_on_completion_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,14 @@ def setUp(self):
def test_maybe_continue_pipeline_no_metadata(self, mock_trigger):
"""Does not continue when metadata is missing."""
state = run_state(blocks=[root_cause_memory_block()])
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)
mock_trigger.assert_not_called()

@patch("sentry.seer.autofix.on_completion_hook.trigger_autofix_explorer")
def test_maybe_continue_pipeline_no_stopping_point_in_metadata(self, mock_trigger):
"""Does not continue when stopping_point is missing from metadata."""
state = run_state(blocks=[root_cause_memory_block()], metadata={"group_id": self.group.id})
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)
mock_trigger.assert_not_called()

@patch("sentry.seer.autofix.on_completion_hook.trigger_autofix_explorer")
Expand All @@ -219,7 +219,7 @@ def test_maybe_continue_pipeline_at_stopping_point(self, mock_trigger):
"stopping_point": AutofixStoppingPoint.ROOT_CAUSE.value,
},
)
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)
mock_trigger.assert_not_called()

@patch("sentry.seer.autofix.on_completion_hook.get_project_seer_preferences")
Expand All @@ -236,7 +236,7 @@ def test_maybe_continue_pipeline_continues_to_next_step(self, mock_trigger, mock
"stopping_point": AutofixStoppingPoint.CODE_CHANGES.value,
},
)
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)
mock_trigger.assert_called_once()
call_kwargs = mock_trigger.call_args.kwargs
assert call_kwargs["group"].id == self.group.id
Expand All @@ -260,7 +260,7 @@ def test_maybe_continue_pipeline_pushes_changes_for_open_pr(self, mock_client_cl
"stopping_point": AutofixStoppingPoint.OPEN_PR.value,
},
)
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)
mock_client.push_changes.assert_called_once_with(123, blocking=False)


Expand All @@ -287,6 +287,8 @@ class TestAutofixOnCompletionHookWebhooks(TestCase):
def setUp(self):
super().setUp()
self.organization = self.create_organization()
self.project = self.create_project(organization=self.organization)
self.group = self.create_group(project=self.project)

@patch("sentry.seer.autofix.on_completion_hook.broadcast_webhooks_for_organization.delay")
def test_send_step_webhook_artifact_types(self, mock_broadcast):
Expand Down Expand Up @@ -325,7 +327,7 @@ class TestCaseDict(TypedDict):
for i, test_case in enumerate(test_cases):
mock_broadcast.reset_mock()
state = run_state(blocks=[test_case["block"]])
AutofixOnCompletionHook._send_step_webhook(self.organization, run_id, state)
AutofixOnCompletionHook._send_step_webhook(self.organization, run_id, state, self.group)

mock_broadcast.assert_called_once()
call_kwargs = mock_broadcast.call_args.kwargs
Expand All @@ -349,7 +351,7 @@ def test_send_step_webhook_coding(self, mock_broadcast):
code_changes_memory_block(),
]
)
AutofixOnCompletionHook._send_step_webhook(self.organization, 123, state)
AutofixOnCompletionHook._send_step_webhook(self.organization, 123, state, self.group)

mock_broadcast.assert_called_once()
call_kwargs = mock_broadcast.call_args.kwargs
Expand All @@ -368,7 +370,7 @@ def test_send_step_webhook_no_artifacts_no_webhook(self, mock_broadcast):
artifacts=[],
)
state = run_state(blocks=[block])
AutofixOnCompletionHook._send_step_webhook(self.organization, 123, state)
AutofixOnCompletionHook._send_step_webhook(self.organization, 123, state, self.group)

mock_broadcast.assert_not_called()

Expand All @@ -388,7 +390,9 @@ def test_triggers_embedding_on_root_cause(self, mock_trigger_sg):
"""Triggers supergroups embedding when root cause completes with feature flag enabled."""
block = root_cause_memory_block()
state = run_state(blocks=[block], metadata={"group_id": self.group.id})
AutofixOnCompletionHook._maybe_trigger_supergroups_embedding(self.organization, 123, state)
AutofixOnCompletionHook._maybe_trigger_supergroups_embedding(
self.organization, 123, state, self.group
)

mock_trigger_sg.assert_called_once_with(
organization_id=self.organization.id,
Expand All @@ -404,15 +408,19 @@ def test_skips_embedding_when_flag_disabled(self, mock_trigger_sg):
blocks=[root_cause_memory_block()],
metadata={"group_id": self.group.id},
)
AutofixOnCompletionHook._maybe_trigger_supergroups_embedding(self.organization, 123, state)
AutofixOnCompletionHook._maybe_trigger_supergroups_embedding(
self.organization, 123, state, self.group
)

mock_trigger_sg.assert_not_called()

@patch("sentry.seer.autofix.on_completion_hook.trigger_supergroups_embedding")
def test_skips_embedding_when_no_group_id(self, mock_trigger_sg):
"""Does not trigger supergroups embedding when group_id is missing from metadata."""
def test_skips_embedding_when_no_group(self, mock_trigger_sg):
"""Does not trigger supergroups embedding when group is None."""
state = run_state(blocks=[root_cause_memory_block()])
AutofixOnCompletionHook._maybe_trigger_supergroups_embedding(self.organization, 123, state)
AutofixOnCompletionHook._maybe_trigger_supergroups_embedding(
self.organization, 123, state, None
)
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated

mock_trigger_sg.assert_not_called()

Expand Down Expand Up @@ -556,7 +564,7 @@ def test_maybe_continue_pipeline_triggers_handoff_when_configured(
},
)

AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)

mock_trigger_handoff.assert_called_once()

Expand Down
Loading