-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(autofix): Add pr_iteration endpoint step and operator activity #117218
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.
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
Changes from 13 commits
31aceb2
5a0f49b
05f9d82
332411d
ced1e52
7ad6eb1
9a7551b
356611b
1e45d15
47bd549
cbeb38b
09b1bb2
d36c8a5
037fc8d
6255a59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
| import logging | ||
| import re | ||
| from enum import StrEnum | ||
| from typing import TYPE_CHECKING, Any, Literal, TypedDict, cast | ||
| from typing import TYPE_CHECKING, Any, Literal, NotRequired, TypedDict, cast | ||
|
|
||
| from django.utils import timezone | ||
| from pydantic import BaseModel | ||
|
|
@@ -71,6 +71,13 @@ | |
| # use the same stable key (`user_id`) that `GroupSeen` uses to track which | ||
| # users have viewed an issue. | ||
| user_id: int | ||
| # The publicly serialized user, resolved at write time so the read path | ||
| # doesn't have to hydrate it. ``None`` if the user could not be serialized. | ||
| # This is serialized as an anonymous viewer (never as the requester) so the | ||
| # payload never includes the user's full email list, options, or flags: it | ||
|
Check warning on line 77 in src/sentry/seer/autofix/autofix_agent.py
|
||
| # is embedded in Seer prompt metadata and round-tripped back to any org | ||
| # member with group-read access. | ||
| user: NotRequired[Any] | ||
|
|
||
|
|
||
| # Discriminated on ``type``. Add new TypedDict variants to this union as more | ||
|
|
@@ -87,6 +94,14 @@ | |
| pass | ||
|
|
||
|
|
||
| class PrIterationNoPullRequestException(Exception): | ||
| pass | ||
|
|
||
|
|
||
| class PrIterationNotEnabledException(Exception): | ||
| pass | ||
|
|
||
|
|
||
| class AutofixStep(StrEnum): | ||
| """Available autofix steps.""" | ||
|
|
||
|
|
@@ -274,6 +289,11 @@ | |
| ) | ||
|
|
||
|
|
||
| def get_autofix_run_state(group: Group, run_id: int) -> SeerRunState: | ||
| client = get_autofix_agent_client(group) | ||
| return _get_group_run_state(client, group, run_id) | ||
|
|
||
|
|
||
| def _validate_run_belongs_to_group(state: SeerRunState, group: Group) -> None: | ||
| group_id = state.metadata.get("group_id") if state.metadata else None | ||
| if group_id != group.id: | ||
|
|
@@ -374,7 +394,11 @@ | |
| pr_iteration_enabled = run_state.metadata.get("pr_iteration_enabled", pr_iteration_enabled) | ||
|
|
||
| iteration_index: int | None = None | ||
| if step == AutofixStep.PR_ITERATION and run_state is not None: | ||
| if step == AutofixStep.PR_ITERATION: | ||
|
Comment on lines
396
to
+397
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The Suggested FixAdd validation to ensure Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews. |
||
| if not pr_iteration_enabled: | ||
| raise PrIterationNotEnabledException() | ||
| if run_state is None or not run_state.repo_pr_states: | ||
| raise PrIterationNoPullRequestException() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PR iteration allows non-created PRsMedium Severity
Reviewed by Cursor Bugbot for commit 8b754c1. Configure here. |
||
| if insert_index is not None: | ||
| iteration_index = get_iteration_for_insert_index(run_state, insert_index) | ||
| else: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,10 @@ | |
| from sentry.seer.autofix.autofix_agent import ( | ||
| UNKNOWN_RUN_ID_FOR_GROUP, | ||
| AutofixStep, | ||
| Feedback, | ||
| NoSeerQuotaException, | ||
| PrIterationNoPullRequestException, | ||
| PrIterationNotEnabledException, | ||
| get_autofix_agent_state, | ||
| trigger_autofix_agent, | ||
| trigger_coding_agent_handoff, | ||
|
|
@@ -65,6 +68,7 @@ | |
| from sentry.seer.endpoints.utils import get_seer_run, resolve_seer_run | ||
| from sentry.seer.models import SeerPermissionError | ||
| from sentry.types.ratelimit import RateLimit, RateLimitCategory | ||
| from sentry.users.services.user.service import user_service | ||
|
Check warning on line 71 in src/sentry/seer/endpoints/group_ai_autofix.py
|
||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
@@ -94,6 +98,7 @@ | |
| "root_cause", | ||
| "solution", | ||
| "code_changes", | ||
| "pr_iteration", | ||
|
Check warning on line 101 in src/sentry/seer/endpoints/group_ai_autofix.py
|
||
|
sentry-warden[bot] marked this conversation as resolved.
|
||
| "open_pr", | ||
| "coding_agent_handoff", | ||
| ], | ||
|
|
@@ -295,18 +300,52 @@ | |
| } | ||
| return Response(open_pr_body, status=status.HTTP_202_ACCEPTED) | ||
|
|
||
| if step == "pr_iteration": | ||
| if resolved_run_id is None: | ||
| return Response( | ||
| {"detail": "run_id is required for pr_iteration"}, | ||
| status=status.HTTP_400_BAD_REQUEST, | ||
| ) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing endpoint-level feature flag check for pr_iterationMedium Severity The endpoint checks that Reviewed by Cursor Bugbot for commit d36c8a5. Configure here. |
||
|
|
||
| # Handle all built-in Seer steps. A missing run_id means this call starts a new | ||
| # autofix run (the kickoff); a provided run_id is advancing an existing run. | ||
| is_autofix_kickoff = resolved_run_id is None | ||
| user_context = data.get("user_context") | ||
| feedback = None | ||
| if ( | ||
| step == "pr_iteration" | ||
| and user_context is not None | ||
| and request.user | ||
| and request.user.is_authenticated | ||
| ): | ||
| # Serialize the user here on write so the read path (GET) doesn't have | ||
|
Comment on lines
+317
to
+321
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The check Suggested FixChange the condition at line 317 from Prompt for AI Agent |
||
| # to hydrate it from the stored user_id on every fetch. Serialize as | ||
| # an anonymous viewer (no ``as_user``) so the result is the public | ||
| # user representation rather than the self representation, which would | ||
| # leak the user's full email list, options, and flags. This payload is | ||
| # embedded in Seer prompt metadata and readable by any org member with | ||
| # group-read access. | ||
| serialized_users = user_service.serialize_many( | ||
| filter={"user_ids": [request.user.id]}, | ||
| ) | ||
| feedback = Feedback( | ||
| message=user_context, | ||
| source={ | ||
| "type": "user-ui", | ||
| "user_id": request.user.id, | ||
|
Check warning on line 335 in src/sentry/seer/endpoints/group_ai_autofix.py
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a thought but why not just store the user here and avoid having to hydrate the user later? This is manual process so my expectations are that this wont take up that much storage anyways |
||
| "user": serialized_users[0] if serialized_users else None, | ||
|
Check warning on line 336 in src/sentry/seer/endpoints/group_ai_autofix.py
|
||
| }, | ||
| ) | ||
| try: | ||
| run_id = trigger_autofix_agent( | ||
| group=group, | ||
| step=AutofixStep(step), | ||
| referrer=_parse_autofix_referrer(data.get("referrer")), | ||
| stopping_point=AutofixStoppingPoint(stopping_point) if stopping_point else None, | ||
| run_id=resolved_run_id, | ||
| user_context=data.get("user_context"), | ||
| user_context=user_context, | ||
| insert_index=data.get("insert_index"), | ||
| feedback=feedback, | ||
| ) | ||
| if is_autofix_kickoff: | ||
| # Record the trigger action only on kickoff, not on each subsequent | ||
|
|
@@ -334,6 +373,16 @@ | |
| return Response(kickoff_body, status=status.HTTP_202_ACCEPTED) | ||
| except NoSeerQuotaException: | ||
| return Response("No budget for Seer Autofix.", status=status.HTTP_402_PAYMENT_REQUIRED) | ||
| except PrIterationNotEnabledException: | ||
| return Response( | ||
| {"detail": "PR iteration is not enabled for this organization"}, | ||
| status=status.HTTP_403_FORBIDDEN, | ||
| ) | ||
| except PrIterationNoPullRequestException: | ||
| return Response( | ||
| {"detail": "Cannot iterate on a PR before one has been created"}, | ||
| status=status.HTTP_400_BAD_REQUEST, | ||
| ) | ||
| except SeerPermissionError as e: | ||
| if _is_unknown_run_id_error(e): | ||
| return Response(status=status.HTTP_404_NOT_FOUND) | ||
|
|
@@ -393,13 +442,14 @@ | |
| ) | ||
|
|
||
| run = get_seer_run(state.run_id, group.organization) | ||
| blocks = [block.dict() for block in state.blocks] | ||
| return Response( | ||
| { | ||
| "autofix": { | ||
| "run_id": state.run_id, | ||
| "sentry_run_id": str(run.uuid) if run else None, | ||
| "status": state.status, | ||
| "blocks": [block.dict() for block in state.blocks], | ||
| "blocks": blocks, | ||
| "updated_at": state.updated_at, | ||
| "pending_user_input": ( | ||
| state.pending_user_input.dict() if state.pending_user_input else None | ||
|
|
@@ -410,6 +460,9 @@ | |
| "coding_agents": { | ||
| agent_id: agent.dict() for agent_id, agent in state.coding_agents.items() | ||
| }, | ||
| "pr_iteration_enabled": bool( | ||
| state.metadata.get("pr_iteration_enabled") if state.metadata else False | ||
| ), | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
| } | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -587,6 +587,16 @@ def _create_seer_activity( | |
| pull_requests = event_payload.get("pull_requests", []) | ||
| if pull_requests: | ||
| activity_data["pull_requests"] = pull_requests | ||
| elif event_type == SentryAppEventType.SEER_ITERATION_COMPLETED: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain what this activity does?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mostly here for parity with the other steps having activities, but i don't think this ends up doing anything right now there is code that seems like it'll handle activities in the future, that's behind a feature flag and there's currently no handlers implemented for these activities there's also an activity log in the UI where we show these, that's behind a feature flag |
||
| pull_requests = event_payload.get("pull_requests", []) | ||
| if pull_requests: | ||
| activity_data["pull_requests"] = pull_requests | ||
| code_changes = event_payload.get("code_changes") | ||
| if code_changes: | ||
| activity_data["code_changes"] = code_changes | ||
| iteration_index = event_payload.get("iteration_index") | ||
| if iteration_index is not None: | ||
| activity_data["iteration_index"] = iteration_index | ||
|
|
||
| Activity.objects.create_group_activity( | ||
| group, | ||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-serialized user (all emails, options, flags) stored in Seer feedback metadata and returned to other org members
When submitting
pr_iterationfeedback, the endpoint serializes the requesting user withas_user=serialize_generic_user(request.user). Becauseas_userequals the target user,UserSerializer._user_is_requesteris True and the result is aUserSerializerResponseSelfcontaining the user's full registered email list, user options, and internal flags. This payload is stored infeedback.source["user"], embedded into the Seer message'sprompt_metadata["feedback"], and round-tripped back throughblock.dict()in the GET endpoint, where any authenticated org member with group-read access can read another member's personal emails, options, and flags. Only a minimal attribution subset (id, name, avatarUrl) is needed.Evidence
group_ai_autofix.py:329-331callsuser_service.serialize_many(filter={"user_ids":[request.user.id]}, as_user=serialize_generic_user(request.user)); sinceas_useris the same user,UserSerializer._user_is_requesterreturns True.users/api/serializers/user.py:193-203then populatesemails(allUserEmailrows),options, andflags, producingUserSerializerResponseSelfrather than the public response.feedback.source["user"](Feedbackinautofix_agent.py) and embedded viaprompt_metadata["feedback"] = json.dumps({... "source": feedback.source ...})atautofix_agent.py:418.Message.metadataisdict[str, str] | None(agent/client_models.py:33) and is part ofMemoryBlock; the GET handler returnsblocks = [block.dict() for block in state.blocks]atgroup_ai_autofix.py:442, surfacingmessage.metadata.feedbackto any org member with group-read access.Also found at 2 additional locations
src/sentry/seer/endpoints/group_ai_autofix.py:68-69src/sentry/seer/endpoints/group_ai_autofix.py:326-342Identified by Warden wrdn-data-exfil, wrdn-pii · GE3-KTX