fix(ACI): Fix adding sentry app action to a workflow #103790

ceorourke · 2025-11-20T23:25:54Z

Fixes adding and updating sentry app actions on a workflow and requires settings to be passed for a sentry app action if it has a UI component. If it has no component, settings does not need to be passed.

Adds support for a target_type of either sentry_app_id or sentry_app_installation_uuid until we can migrate the data to only have sentry_app_id.

Add tests for adding and updating sentry app actions.

Fixes https://sentry.sentry.io/issues/6980690322

codecov · 2025-11-20T23:45:16Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
30050	1	30049	241

View the top 1 failed test(s) by shortest run time

tests.sentry.core.endpoints.test_organization_index.OrganizationsCreateTest::test_valid_slugs

Stack Traces | 1.62s run time

#x1B[1m#x1B[.../core/endpoints/test_organization_index.py#x1B[0m:190: in test_valid_slugs
    response = self.get_success_response(name=input_slug, slug=input_slug)
#x1B[1m#x1B[.../sentry/testutils/cases.py#x1B[0m:628: in get_success_response
    assert_status_code(response, 200, 300)
#x1B[1m#x1B[.../sentry/testutils/asserts.py#x1B[0m:47: in assert_status_code
    assert minimum <= response.status_code < maximum, (
#x1B[1m#x1B[31mE   AssertionError: (500, b'{"detail":"Internal Error","errorId":null}')#x1B[0m
#x1B[1m#x1B[31mE   assert 500 < 300#x1B[0m
#x1B[1m#x1B[31mE    +  where 500 = <Response status_code=500, "application/json">.status_code#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

ceorourke · 2025-11-21T00:56:26Z

tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py

+                            "sentryAppIdentifier": SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID,
+                            "targetIdentifier": self.sentry_app_installation.uuid,
+                            "targetType": ActionType.SENTRY_APP,


@ameliahsu I was only able to get this test to pass if I used this data which differs from what was passed in https://sentry.sentry.io/issues/6980690322 which was

{ "sentry_app_identifier":"'sentry_app_id'", "target_identifier":"'72013'", "target_type":"'sentry_app'" }

Do you know if the data in the Sentry issue is what's expected to be passed all the time or only sometimes? I'm not sure when we pass one versus the other, but if we set sentryAppIdentifier to "sentry_app" instead of "sentry_app_installation" (SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID) we go down a slightly different code path and set sentryAppInstallationUuid to None (code here) which leads to a SentryAppInstallation.DoesNotExist error (here).

I truly don't know if this is a bug or how it's intended to work, but it doesn't currently work if you do not pass the installation data. I did verify that even an internal unpublished sentry app has an installation object, so maybe it's intended for us to always pass that data?

@GabeVillalobos might be able to help here, i believe some existing app actions use the sentry app UUID instead of the sentry app ID, which is why we have to specify sentry_app_identifier? but we would like for all newly created actions going forward to use sentry app ID. not sure if that's why we throw an error tho 😅

Short answer here is that sentry_app_installation is some legacy data we need to migrate away from and the front end will be passing sentry_app_id for new actions. I updated the code to support both until we can run the migration so that old action with the uuid can be updated.

src/sentry/notifications/notification_action/action_handler_registry/sentry_app_handler.py

src/sentry/notifications/notification_action/action_validation.py

ceorourke · 2025-11-25T01:06:53Z

src/sentry/notifications/notification_action/action_validation.py

+        if settings:
+            try:
+                # Only call creator for Sentry Apps with UI Components (settings) for actions


I copied this logic from how we do it today - in project_rule_details.py and project_rules.py we call trigger_sentry_app_action_creators_for_issues (here) which skips over the validation if there is no UI component

ceorourke · 2025-11-25T01:07:22Z

src/sentry/sentry_apps/services/app/impl.py

+        except SentryAppInstallation.DoesNotExist:
+            return None
+
+    def get_installation_by_uuid(


We only need this temporarily until we can migrate the action data to not have the installation uuid

cathteng

Generally looks good

cathteng · 2025-11-25T22:06:18Z

src/sentry/notifications/notification_action/action_validation.py

+            installation = app_service.get_installation_by_sentry_app_id(
+                sentry_app_id=int(target_identifier), organization_id=self.organization.id


Adding a new RPC function will need to be deployed in a separate PR from the PR where you use it

cathteng · 2025-11-25T22:07:56Z

src/sentry/notifications/notification_action/action_validation.py

-            return self.validated_data
-        except SentryAppBaseError as e:
-            raise ValidationError(e.message) from e
+        return self.validated_data


Should we be updating the validated data here to only save sentry app actions with sentry_app_identifier = "sentry_app_id"?

We have actions with the uuid saved in the db today so we don't want to block being able to update those. After we run the migration maybe part of the cleanup can be enforcing only that identifier.

If somebody is trying to update an action with a uuid we could update that action to save it with the sentry app id so we don't have any more actions being saved with uuid

updated 👍

tests/sentry/workflow_engine/endpoints/validators/actions/test_sentry_app.py

src/sentry/notifications/notification_action/action_validation.py

tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py

cathteng · 2025-11-25T23:57:54Z

src/sentry/notifications/notification_action/action_validation.py

+        installations = None
+        installation = None


Is typing gonna complain this needs to be RpcSentryAppInstallation | None = None?

tests/sentry/workflow_engine/endpoints/validators/actions/test_sentry_app.py

src/sentry/notifications/notification_action/action_validation.py

ceorourke · 2025-11-26T00:44:17Z

src/sentry/notifications/notification_action/action_validation.py

+            )
+        else:
+            installations = app_service.get_many(
+                filter=dict(app_ids=[int(target_identifier)], organization_id=self.organization.id)


good thing it is a number

src/sentry/notifications/notification_action/action_validation.py

cursor · 2025-11-26T00:52:14Z

src/sentry/notifications/notification_action/action_validation.py

+            )
+        else:
+            installations = app_service.get_many(
+                filter=dict(app_ids=[int(target_identifier)], organization_id=self.organization.id)


Bug: Unhandled ValueError when converting target identifier to int

When sentry_app_identifier is SENTRY_APP_ID, the code converts target_identifier to an integer without error handling. If a user passes a non-numeric string (like a UUID) as the target_identifier, this will raise an unhandled ValueError instead of returning a proper validation error, causing a 500 error rather than a user-friendly 400 validation error.

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 20, 2025

vercel bot deployed to Preview November 20, 2025 23:28 View deployment

vercel bot deployed to Preview November 21, 2025 00:50 View deployment

ceorourke commented Nov 21, 2025

View reviewed changes

vercel bot deployed to Preview November 21, 2025 01:12 View deployment

vercel bot deployed to Preview November 21, 2025 01:17 View deployment

vercel bot deployed to Preview November 21, 2025 21:52 View deployment

ceorourke commented Nov 21, 2025

View reviewed changes

src/sentry/notifications/notification_action/action_handler_registry/sentry_app_handler.py Outdated Show resolved Hide resolved

vercel bot deployed to Preview November 22, 2025 01:24 View deployment

vercel bot deployed to Preview November 22, 2025 01:33 View deployment

vercel bot deployed to Preview November 24, 2025 23:49 View deployment

vercel bot deployed to Preview November 24, 2025 23:55 View deployment

vercel bot deployed to Preview November 25, 2025 00:50 View deployment

ceorourke commented Nov 25, 2025

View reviewed changes

src/sentry/notifications/notification_action/action_validation.py Outdated Show resolved Hide resolved

ceorourke commented Nov 25, 2025

View reviewed changes

vercel bot deployed to Preview November 25, 2025 01:07 View deployment

ceorourke commented Nov 25, 2025

View reviewed changes

ceorourke force-pushed the ceorourke/fix-sentry-app-actions branch from 2966483 to 0ddf637 Compare November 25, 2025 01:22

vercel bot deployed to Preview November 25, 2025 01:25 View deployment

ceorourke force-pushed the ceorourke/fix-sentry-app-actions branch from 20f0117 to f78da0a Compare November 25, 2025 01:40

vercel bot deployed to Preview November 25, 2025 01:43 View deployment

vercel bot deployed to Preview November 25, 2025 18:02 View deployment

ceorourke force-pushed the ceorourke/fix-sentry-app-actions branch from 933836f to 4472102 Compare November 25, 2025 18:23

vercel bot deployed to Preview November 25, 2025 18:26 View deployment

ceorourke added 5 commits November 25, 2025 11:17

fix(ACI): Fix adding sentry app action to a workflow

3bab3af

require settings, add tests

69520e9

Add settings to test

a790a41

update more tests

5113166

fix test

22b269b

ceorourke requested review from a team as code owners November 25, 2025 19:29

cathteng reviewed Nov 25, 2025

View reviewed changes

ceorourke mentioned this pull request Nov 25, 2025

feat(ACI): Add RPC methods for sentryapp installations #104027

Closed

use get_many

4cb7e7e

cursor bot reviewed Nov 25, 2025

View reviewed changes

tests/sentry/workflow_engine/endpoints/validators/actions/test_sentry_app.py Outdated Show resolved Hide resolved

vercel bot deployed to Preview November 25, 2025 22:41 View deployment

update uuid to use app id

85742c9

cursor bot reviewed Nov 25, 2025

View reviewed changes

src/sentry/notifications/notification_action/action_validation.py Outdated Show resolved Hide resolved

ceorourke added 2 commits November 25, 2025 14:55

fix test

89b5435

fix get_many usage

0f5922d

vercel bot deployed to Preview November 25, 2025 22:59 View deployment

cursor bot reviewed Nov 25, 2025

View reviewed changes

tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py Outdated Show resolved Hide resolved

update test

7228774

ceorourke requested a review from cathteng November 25, 2025 23:57

vercel bot deployed to Preview November 25, 2025 23:57 View deployment

cathteng approved these changes Nov 26, 2025

View reviewed changes

fix test, add another for sentry app id

a603836

vercel bot deployed to Preview November 26, 2025 00:21 View deployment

cursor bot reviewed Nov 26, 2025

View reviewed changes

src/sentry/notifications/notification_action/action_validation.py Show resolved Hide resolved

ceorourke added 2 commits November 25, 2025 16:22

convert to int

e79e8a2

only check alert-rule-action components

47ce60d

sentry bot reviewed Nov 26, 2025

View reviewed changes

vercel bot deployed to Preview November 26, 2025 00:35 View deployment

appease typing

ea7ebbd

vercel bot deployed to Preview November 26, 2025 00:49 View deployment

sentry bot reviewed Nov 26, 2025

View reviewed changes

src/sentry/notifications/notification_action/action_validation.py Show resolved Hide resolved

cursor bot reviewed Nov 26, 2025

View reviewed changes

		installation = app_service.get_installation_by_sentry_app_id(
		sentry_app_id=int(target_identifier), organization_id=self.organization.id

Uh oh!

fix(ACI): Fix adding sentry app action to a workflow #103790

Are you sure you want to change the base?

fix(ACI): Fix adding sentry app action to a workflow #103790

Conversation

ceorourke commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 1 Tests Failed:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cathteng left a comment

Choose a reason for hiding this comment

Uh oh!

cathteng Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor bot Nov 26, 2025

Choose a reason for hiding this comment

Bug: Unhandled ValueError when converting target identifier to int

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ceorourke commented Nov 20, 2025 •

edited

Loading

codecov bot commented Nov 20, 2025 •

edited

Loading

cathteng Nov 25, 2025 •

edited

Loading