From f6388b89dbebaab0418451af6342a33fc8dd19df Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Wed, 15 Apr 2026 16:23:53 -0400 Subject: [PATCH 1/9] initial pass, moving routing to control --- src/sentry/features/manager.py | 3 +- .../integrations/slack/requests/event.py | 85 +++++++++++++++- .../integrations/slack/webhooks/event.py | 98 +++---------------- .../middleware/integrations/parsers/slack.py | 54 +++++++++- .../seer/entrypoints/slack/entrypoint.py | 3 +- .../seer/entrypoints/slack/messaging.py | 2 + src/sentry/seer/explorer/client_utils.py | 4 +- src/sentry/seer/seer_setup.py | 7 +- 8 files changed, 165 insertions(+), 91 deletions(-) diff --git a/src/sentry/features/manager.py b/src/sentry/features/manager.py index e539ed580bcb..2f6dbc1015d5 100644 --- a/src/sentry/features/manager.py +++ b/src/sentry/features/manager.py @@ -14,6 +14,7 @@ from sentry import options from sentry.options.rollout import in_random_rollout +from sentry.organizations.services.organization.model import RpcOrganization from sentry.users.services.user.model import RpcUser from sentry.utils import metrics from sentry.utils.flag import record_feature_flag @@ -321,7 +322,7 @@ def batch_has( feature_names: Sequence[str], actor: User | RpcUser | AnonymousUser | None = None, projects: Sequence[Project] | None = None, - organization: Organization | None = None, + organization: RpcOrganization | Organization | None = None, ) -> dict[str, dict[str, bool | None]] | None: """ Determine if multiple features are enabled. Unhandled flags will not be in diff --git a/src/sentry/integrations/slack/requests/event.py b/src/sentry/integrations/slack/requests/event.py index fa80982f7a6e..b392f42c45d8 100644 --- a/src/sentry/integrations/slack/requests/event.py +++ b/src/sentry/integrations/slack/requests/event.py @@ -1,14 +1,24 @@ from __future__ import annotations from collections.abc import Mapping -from typing import Any +from typing import Any, NamedTuple +from sentry.constants import ObjectStatus +from sentry.integrations.messaging.metrics import SeerSlackHaltReason +from sentry.integrations.services.integration import integration_service from sentry.integrations.slack.requests.base import SlackDMRequest, SlackRequestError from sentry.integrations.slack.unfurl.handlers import match_link from sentry.integrations.slack.unfurl.types import LinkType from sentry.integrations.slack.utils.constants import SlackScope +from sentry.integrations.types import IntegrationProviderSlug +from sentry.models.organization import OrganizationStatus +from sentry.organizations.services.organization.service import organization_service +from sentry.seer.entrypoints.slack.entrypoint import SlackExplorerEntrypoint +from sentry.seer.entrypoints.slack.messaging import send_identity_link_prompt +from sentry.silo.base import all_silo_function COMMANDS = ["link", "unlink", "link team", "unlink team"] +SLACK_PROVIDERS = [IntegrationProviderSlug.SLACK, IntegrationProviderSlug.SLACK_STAGING] def has_discover_links(links: list[str]) -> bool: @@ -23,6 +33,11 @@ def is_event_challenge(data: Mapping[str, Any]) -> bool: return data.get("type", "") == "url_verification" +class SeerResolutionResult(NamedTuple): + organization_id: int | None + error_reason: SeerSlackHaltReason | None + + class SlackEventRequest(SlackDMRequest): """ An Event request sent from Slack. @@ -55,6 +70,74 @@ def is_challenge(self) -> bool: """We need to call this before validation.""" return is_event_challenge(self.request.data) + @property + def is_seer_explorer_request(self) -> bool: + return ( + self.type == "app_mention" + or self.type == "assistant_thread_started" + or (self.dm_data.get("type") == "message" and self.has_assistant_scope) + ) + + @all_silo_function + def resolve_seer_organization(self) -> SeerResolutionResult: + """ + Resolve and validate an organization/user for a Seer Slack event. + + If the initiating user is not linked, we will reply with a prompt to link their identity. + + Then we search for an active, organization with Seer Explorer access. If the user does not + belong to any matched organization, their request will be dropped. + + Note: There is a limitation here of only grabbing the first organization belonging to the user + with access to Seer. If a Slack installation corresponds to multiple organizations with Seer + access, this will not work as expected. This will be revisited. + """ + identity_user = self.get_identity_user() + if not identity_user: + send_identity_link_prompt( + integration=self.integration, + slack_user_id=self.user_id, + channel_id=self.channel_id, + thread_ts=self.thread_ts or None, + is_welcome_message=self.is_assistant_thread_event, + ) + return SeerResolutionResult( + organization_id=None, error_reason=SeerSlackHaltReason.IDENTITY_NOT_LINKED + ) + + ois = integration_service.get_organization_integrations( + integration_id=self.integration.id, + status=ObjectStatus.ACTIVE, + providers=SLACK_PROVIDERS, + ) + if not ois: + return SeerResolutionResult( + organization_id=None, error_reason=SeerSlackHaltReason.NO_VALID_INTEGRATION + ) + + for oi in ois: + organization_id = oi.organization_id + ctx = organization_service.get_organization_by_id( + id=oi.organization_id, user_id=identity_user.id + ) + if ctx is None: + continue + + if ctx.organization.status != OrganizationStatus.ACTIVE: + continue + + if not SlackExplorerEntrypoint.has_access(ctx.organization): + continue + + if ctx.member is None: + continue + + return SeerResolutionResult(organization_id=organization_id, error_reason=None) + + return SeerResolutionResult( + organization_id=None, error_reason=SeerSlackHaltReason.NO_VALID_ORGANIZATION + ) + @property def dm_data(self) -> Mapping[str, Any]: return self.data.get("event", {}) diff --git a/src/sentry/integrations/slack/webhooks/event.py b/src/sentry/integrations/slack/webhooks/event.py index fa560e4423f2..3c9cb6ed5395 100644 --- a/src/sentry/integrations/slack/webhooks/event.py +++ b/src/sentry/integrations/slack/webhooks/event.py @@ -15,7 +15,6 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import all_silo_endpoint -from sentry.constants import ObjectStatus from sentry.integrations.messaging.metrics import ( MessagingInteractionEvent, MessagingInteractionType, @@ -34,11 +33,8 @@ from sentry.integrations.slack.unfurl.types import LinkType, UnfurlableUrl from sentry.integrations.slack.views.link_identity import build_linking_url from sentry.integrations.types import IntegrationProviderSlug -from sentry.models.organization import Organization, OrganizationStatus from sentry.organizations.services.organization import organization_service from sentry.organizations.services.organization.model import RpcOrganization -from sentry.seer.entrypoints.slack.entrypoint import SlackExplorerEntrypoint -from sentry.seer.entrypoints.slack.messaging import send_identity_link_prompt from sentry.seer.entrypoints.slack.tasks import process_mention_for_slack from .base import SlackDMEndpoint @@ -368,74 +364,6 @@ def on_link_shared(self, request: Request, slack_request: SlackDMRequest) -> boo return True - def _resolve_seer_organization(self, slack_request: SlackEventRequest) -> SeerResolutionResult: - """ - Resolve and validate an organization/user for a Seer Slack event. - - If the initiating user is not linked, we will reply with a prompt to link their identity. - - Then we search for an active, organization with Seer Explorer access. If the user does not - belong to any matched organization, their request will be dropped. - - Note: There is a limitation here of only grabbing the first organization belonging to the user - with access to Seer. If a Slack installation corresponds to multiple organizations with Seer - access, this will not work as expected. This will be revisited. - """ - result: SeerResolutionResult = { - "organization_id": None, - "installation": None, - "error_reason": None, - } - - identity_user = slack_request.get_identity_user() - if not identity_user: - result["error_reason"] = SeerSlackHaltReason.IDENTITY_NOT_LINKED - send_identity_link_prompt( - integration=slack_request.integration, - slack_user_id=slack_request.user_id, - channel_id=slack_request.channel_id, - thread_ts=slack_request.thread_ts or None, - is_welcome_message=slack_request.is_assistant_thread_event, - ) - return result - - ois = integration_service.get_organization_integrations( - integration_id=slack_request.integration.id, - status=ObjectStatus.ACTIVE, - providers=SLACK_PROVIDERS, - ) - if not ois: - result["error_reason"] = SeerSlackHaltReason.NO_VALID_INTEGRATION - return result - - for oi in ois: - organization_id = oi.organization_id - try: - organization = Organization.objects.get_from_cache(id=organization_id) - except Organization.DoesNotExist: - continue - - if organization.status != OrganizationStatus.ACTIVE: - continue - - if not SlackExplorerEntrypoint.has_access(organization): - continue - - if not organization.has_access(identity_user): - continue - - installation = slack_request.integration.get_installation( - organization_id=organization_id - ) - assert isinstance(installation, SlackIntegration) - - result["organization_id"] = organization_id - result["installation"] = installation - return result - - result["error_reason"] = SeerSlackHaltReason.NO_VALID_ORGANIZATION - return result - def _handle_seer_prompt( self, slack_request: SlackEventRequest, @@ -461,16 +389,17 @@ def _handle_seer_prompt( } ) - result = self._resolve_seer_organization(slack_request) - if result["error_reason"]: - lifecycle.record_halt(result["error_reason"]) + organization_id, error_reason = slack_request.resolve_seer_organization() + if error_reason: + lifecycle.record_halt(error_reason) return self.respond() - if not result["organization_id"] or not result["installation"]: + if not organization_id: return self.respond() - organization_id = result["organization_id"] - installation = result["installation"] + installation: SlackIntegration = slack_request.integration.get_installation( + organization_id=organization_id + ) if not channel_id or not text or not ts or not slack_request.user_id: lifecycle.record_halt(SeerSlackHaltReason.MISSING_EVENT_DATA) @@ -523,15 +452,17 @@ def on_assistant_thread_started(self, slack_request: SlackEventRequest) -> Respo spec=SlackMessagingSpec(), ).capture() as lifecycle: lifecycle.add_extra("integration_id", slack_request.integration.id) - result = self._resolve_seer_organization(slack_request) - if result["error_reason"]: - lifecycle.record_halt(result["error_reason"]) + organization_id, error_reason = slack_request.resolve_seer_organization() + if error_reason: + lifecycle.record_halt(error_reason) return self.respond() - if not result["installation"]: + if not organization_id: return self.respond() - installation = result["installation"] + installation: SlackIntegration = slack_request.integration.get_installation( + organization_id=organization_id + ) channel_id = slack_request.channel_id thread_ts = slack_request.thread_ts @@ -542,6 +473,7 @@ def on_assistant_thread_started(self, slack_request: SlackEventRequest) -> Respo "channel_id": channel_id, "thread_ts": thread_ts, "context": assistant_thread.get("context"), + "organization_id": organization_id, } ) diff --git a/src/sentry/middleware/integrations/parsers/slack.py b/src/sentry/middleware/integrations/parsers/slack.py index 06f26baf8273..9202e565f223 100644 --- a/src/sentry/middleware/integrations/parsers/slack.py +++ b/src/sentry/middleware/integrations/parsers/slack.py @@ -23,7 +23,7 @@ from sentry.integrations.models.integration import Integration from sentry.integrations.slack.message_builder.routing import SlackRoutingData, decode_action_id from sentry.integrations.slack.requests.base import SlackRequestError -from sentry.integrations.slack.requests.event import is_event_challenge +from sentry.integrations.slack.requests.event import SlackEventRequest, is_event_challenge from sentry.integrations.slack.sdk_client import SlackSdkClient from sentry.integrations.slack.views import SALT from sentry.integrations.slack.views.link_identity import SlackLinkIdentityView @@ -41,7 +41,8 @@ from sentry.integrations.slack.webhooks.options_load import SlackOptionsLoadEndpoint from sentry.integrations.types import EXTERNAL_PROVIDERS, ExternalProviders from sentry.middleware.integrations.tasks import convert_to_async_slack_response -from sentry.types.cell import Cell +from sentry.models.organizationmapping import OrganizationMapping +from sentry.types.cell import Cell, CellResolutionError, get_cell_by_name from sentry.utils import json from sentry.utils.signing import unsign @@ -293,6 +294,50 @@ def filter_organizations_from_request( ) return organizations + def _maybe_get_response_from_event_request(self) -> HttpResponseBase | None: + if self.view_class != SlackEventEndpoint: + return None + drf_request: Request = SlackDMEndpoint().initialize_request(self.request) + slack_request: SlackEventRequest = self.view_class.slack_request_class(drf_request) + + if not slack_request.is_seer_explorer_request: + return None + + organization, error_reason = slack_request.resolve_seer_organization() + if not organization or error_reason: + logger.info( + "slack.control.seer_event.organization.not_found", + extra={ + "organization_id": organization.id, + "error_reason": error_reason, + }, + ) + return None + + try: + mapping = OrganizationMapping.objects.get(organization_id=organization.id) + except OrganizationMapping.DoesNotExist: + logger.info( + "slack.control.seer_event.organization.mapping.not_found", + extra={ + "organization_id": organization.id, + }, + ) + return None + + try: + cell = get_cell_by_name(mapping.cell_name) + except CellResolutionError: + logger.info( + "slack.control.seer_event.organization.cell.not_found", + extra={ + "organization_id": organization.id, + "cell_name": mapping.cell_name, + }, + ) + return None + return self.get_response_from_cell_silo(cell=cell) + def get_response(self) -> HttpResponseBase: """ Slack Webhook Requests all require synchronous responses. @@ -322,6 +367,7 @@ def get_response(self) -> HttpResponseBase: # this request until it succeeds. return HttpResponse(status=status.HTTP_202_ACCEPTED) + drf_request: Request if self.view_class == SlackActionEndpoint: drf_request: Request = SlackDMEndpoint().initialize_request(self.request) slack_request = self.view_class.slack_request_class(drf_request) @@ -337,6 +383,10 @@ def get_response(self) -> HttpResponseBase: else self.get_response_from_all_cells() ) + event_response = self._maybe_get_response_from_event_request() + if event_response: + return event_response + # Slack webhooks can only receive one synchronous call/response, as there are many # places where we post to slack on their webhook request. This would cause multiple # calls back to slack for every cell we forward to. diff --git a/src/sentry/seer/entrypoints/slack/entrypoint.py b/src/sentry/seer/entrypoints/slack/entrypoint.py index 25f81fe6d268..5f5e132df841 100644 --- a/src/sentry/seer/entrypoints/slack/entrypoint.py +++ b/src/sentry/seer/entrypoints/slack/entrypoint.py @@ -15,6 +15,7 @@ SeerExplorerResponse, ) from sentry.notifications.utils.actions import BlockKitMessageAction +from sentry.organizations.services.organization.model import RpcOrganization from sentry.seer.autofix.utils import AutofixStoppingPoint from sentry.seer.entrypoints.cache import SeerOperatorAutofixCache from sentry.seer.entrypoints.registry import ( @@ -420,7 +421,7 @@ def __init__( self.slack_user_id = slack_user_id @staticmethod - def has_access(organization: Organization) -> bool: + def has_access(organization: Organization | RpcOrganization) -> bool: has_seer_slack_feature_flag = features.has( "organizations:seer-slack-explorer", organization ) diff --git a/src/sentry/seer/entrypoints/slack/messaging.py b/src/sentry/seer/entrypoints/slack/messaging.py index 4521eb421d8e..e312439132fa 100644 --- a/src/sentry/seer/entrypoints/slack/messaging.py +++ b/src/sentry/seer/entrypoints/slack/messaging.py @@ -29,6 +29,7 @@ SlackEntrypointInteractionType, ) from sentry.shared_integrations.exceptions import IntegrationConfigurationError, IntegrationError +from sentry.silo.base import all_silo_function from sentry.tasks.base import instrumented_task from sentry.taskworker.namespaces import integrations_tasks from sentry.utils.registry import NoRegistrationExistsError @@ -265,6 +266,7 @@ def remove_all_buttons_transformer(_elem: dict[str, Any]) -> dict[str, Any] | No lifecycle.record_halt(halt_reason=e) +@all_silo_function def send_identity_link_prompt( *, integration: RpcIntegration, diff --git a/src/sentry/seer/explorer/client_utils.py b/src/sentry/seer/explorer/client_utils.py index 53b7f10a2360..e6f315ed454f 100644 --- a/src/sentry/seer/explorer/client_utils.py +++ b/src/sentry/seer/explorer/client_utils.py @@ -24,6 +24,7 @@ from sentry.models.organizationmember import OrganizationMember from sentry.models.project import Project from sentry.net.http import connection_from_url +from sentry.organizations.services.organization.model import RpcOrganization from sentry.seer.explorer.client_models import SeerRunState from sentry.seer.models import SeerApiError from sentry.seer.seer_setup import has_seer_access_with_detail @@ -184,7 +185,8 @@ def get_explorer_state_from_pr_id( def has_seer_explorer_access_with_detail( - organization: Organization, actor: SentryUser | AnonymousUser | RpcUser | None = None + organization: Organization | RpcOrganization, + actor: SentryUser | AnonymousUser | RpcUser | None = None, ) -> tuple[bool, str | None]: """ Check if the actor has access to Seer Explorer. diff --git a/src/sentry/seer/seer_setup.py b/src/sentry/seer/seer_setup.py index 3bcba3541fec..944375854f9c 100644 --- a/src/sentry/seer/seer_setup.py +++ b/src/sentry/seer/seer_setup.py @@ -2,12 +2,14 @@ from sentry import features from sentry.models.organization import Organization +from sentry.organizations.services.organization.model import RpcOrganization from sentry.users.models.user import User from sentry.users.services.user.model import RpcUser def has_seer_access( - organization: Organization, actor: User | AnonymousUser | RpcUser | None = None + organization: Organization | RpcOrganization, + actor: User | AnonymousUser | RpcUser | None = None, ) -> bool: return features.has("organizations:gen-ai-features", organization, actor=actor) and not bool( organization.get_option("sentry:hide_ai_features") @@ -15,7 +17,8 @@ def has_seer_access( def has_seer_access_with_detail( - organization: Organization, actor: User | AnonymousUser | RpcUser | None = None + organization: Organization | RpcOrganization, + actor: User | AnonymousUser | RpcUser | None = None, ) -> tuple[bool, str | None]: if not features.has("organizations:gen-ai-features", organization, actor=actor): return False, "Feature flag not enabled" From 7a3efea5741fd0503be2e0a34cf98f456559d424 Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Wed, 15 Apr 2026 16:26:29 -0400 Subject: [PATCH 2/9] =?UTF-8?q?=F0=9F=9A=A7=20fix=20imports,=20add=20some?= =?UTF-8?q?=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/sentry/features/manager.py | 2 +- .../middleware/integrations/parsers/slack.py | 12 +- .../slack/webhooks/events/test_app_mention.py | 4 +- .../events/test_assistant_thread_started.py | 4 +- .../webhooks/events/test_direct_message.py | 4 +- .../integrations/parsers/test_slack.py | 117 ++++++++++++++++++ 6 files changed, 130 insertions(+), 13 deletions(-) diff --git a/src/sentry/features/manager.py b/src/sentry/features/manager.py index 2f6dbc1015d5..6c30432325c8 100644 --- a/src/sentry/features/manager.py +++ b/src/sentry/features/manager.py @@ -14,7 +14,6 @@ from sentry import options from sentry.options.rollout import in_random_rollout -from sentry.organizations.services.organization.model import RpcOrganization from sentry.users.services.user.model import RpcUser from sentry.utils import metrics from sentry.utils.flag import record_feature_flag @@ -29,6 +28,7 @@ from sentry.features.handler import FeatureHandler from sentry.models.organization import Organization from sentry.models.project import Project + from sentry.organizations.services.organization.model import RpcOrganization from sentry.users.models.user import User diff --git a/src/sentry/middleware/integrations/parsers/slack.py b/src/sentry/middleware/integrations/parsers/slack.py index 9202e565f223..7784da09ca4b 100644 --- a/src/sentry/middleware/integrations/parsers/slack.py +++ b/src/sentry/middleware/integrations/parsers/slack.py @@ -303,24 +303,24 @@ def _maybe_get_response_from_event_request(self) -> HttpResponseBase | None: if not slack_request.is_seer_explorer_request: return None - organization, error_reason = slack_request.resolve_seer_organization() - if not organization or error_reason: + organization_id, error_reason = slack_request.resolve_seer_organization() + if not organization_id or error_reason: logger.info( "slack.control.seer_event.organization.not_found", extra={ - "organization_id": organization.id, + "organization_id": organization_id, "error_reason": error_reason, }, ) return None try: - mapping = OrganizationMapping.objects.get(organization_id=organization.id) + mapping = OrganizationMapping.objects.get(organization_id=organization_id) except OrganizationMapping.DoesNotExist: logger.info( "slack.control.seer_event.organization.mapping.not_found", extra={ - "organization_id": organization.id, + "organization_id": organization_id, }, ) return None @@ -331,7 +331,7 @@ def _maybe_get_response_from_event_request(self) -> HttpResponseBase | None: logger.info( "slack.control.seer_event.organization.cell.not_found", extra={ - "organization_id": organization.id, + "organization_id": organization_id, "cell_name": mapping.cell_name, }, ) diff --git a/tests/sentry/integrations/slack/webhooks/events/test_app_mention.py b/tests/sentry/integrations/slack/webhooks/events/test_app_mention.py index 0795fd0a6314..c79df31c169c 100644 --- a/tests/sentry/integrations/slack/webhooks/events/test_app_mention.py +++ b/tests/sentry/integrations/slack/webhooks/events/test_app_mention.py @@ -80,7 +80,7 @@ def test_app_mention_empty_text(self, mock_apply_async, mock_record): def test_app_mention_no_integration(self, mock_apply_async, mock_record): """When the integration has no org integrations, we should not dispatch.""" with patch( - "sentry.integrations.slack.webhooks.event.integration_service.get_organization_integrations", + "sentry.integrations.slack.requests.event.integration_service.get_organization_integrations", return_value=[], ): with self.feature(SEER_EXPLORER_FEATURES): @@ -100,7 +100,7 @@ def test_app_mention_no_explorer_access(self, mock_apply_async, mock_record): assert_halt_metric(mock_record, SeerSlackHaltReason.NO_VALID_ORGANIZATION) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") - @patch("sentry.integrations.slack.webhooks.event.send_identity_link_prompt") + @patch("sentry.integrations.slack.requests.event.send_identity_link_prompt") @patch("sentry.seer.entrypoints.slack.tasks.process_mention_for_slack.apply_async") def test_app_mention_no_identity_prompt_linkage( self, mock_apply_async, mock_send_link, mock_record diff --git a/tests/sentry/integrations/slack/webhooks/events/test_assistant_thread_started.py b/tests/sentry/integrations/slack/webhooks/events/test_assistant_thread_started.py index 473b6f292816..6162343148fc 100644 --- a/tests/sentry/integrations/slack/webhooks/events/test_assistant_thread_started.py +++ b/tests/sentry/integrations/slack/webhooks/events/test_assistant_thread_started.py @@ -54,7 +54,7 @@ def test_prompt_titles_and_messages(self, mock_set_prompts): assert prompt["message"] @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") - @patch("sentry.integrations.slack.webhooks.event.send_identity_link_prompt") + @patch("sentry.integrations.slack.requests.event.send_identity_link_prompt") @patch("sentry.integrations.slack.integration.SlackIntegration.set_suggested_prompts") def test_identity_not_linked(self, mock_set_prompts, mock_send_link, mock_record): self.unlink_identity() @@ -79,7 +79,7 @@ def test_feature_flag_disabled(self, mock_set_prompts, mock_record): @patch("sentry.integrations.slack.integration.SlackIntegration.set_suggested_prompts") def test_no_integration(self, mock_set_prompts, mock_record): with patch( - "sentry.integrations.slack.webhooks.event.integration_service.get_organization_integrations", + "sentry.integrations.slack.requests.event.integration_service.get_organization_integrations", return_value=[], ): with self.feature(SEER_EXPLORER_FEATURES): diff --git a/tests/sentry/integrations/slack/webhooks/events/test_direct_message.py b/tests/sentry/integrations/slack/webhooks/events/test_direct_message.py index e7f5b623ea4b..a03c937fce93 100644 --- a/tests/sentry/integrations/slack/webhooks/events/test_direct_message.py +++ b/tests/sentry/integrations/slack/webhooks/events/test_direct_message.py @@ -73,7 +73,7 @@ def test_dm_threaded_dispatches_task(self, mock_apply_async): assert kwargs["thread_ts"] == THREADED_MESSAGE_DM_EVENT["thread_ts"] @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") - @patch("sentry.integrations.slack.webhooks.event.send_identity_link_prompt") + @patch("sentry.integrations.slack.requests.event.send_identity_link_prompt") @patch("sentry.seer.entrypoints.slack.tasks.process_mention_for_slack.apply_async") def test_dm_identity_not_linked(self, mock_apply_async, mock_send_link, mock_record): """When no identity is linked, send a link prompt and halt.""" @@ -100,7 +100,7 @@ def test_dm_feature_flag_disabled(self, mock_apply_async, mock_record): @patch("sentry.seer.entrypoints.slack.tasks.process_mention_for_slack.apply_async") def test_dm_no_integration(self, mock_apply_async, mock_record): with patch( - "sentry.integrations.slack.webhooks.event.integration_service.get_organization_integrations", + "sentry.integrations.slack.requests.event.integration_service.get_organization_integrations", return_value=[], ): with self.feature(SEER_EXPLORER_FEATURES): diff --git a/tests/sentry/middleware/integrations/parsers/test_slack.py b/tests/sentry/middleware/integrations/parsers/test_slack.py index b2946f1df2ea..a29199ac5d7e 100644 --- a/tests/sentry/middleware/integrations/parsers/test_slack.py +++ b/tests/sentry/middleware/integrations/parsers/test_slack.py @@ -3,6 +3,7 @@ from unittest.mock import MagicMock, patch from urllib.parse import urlencode +import orjson import responses from django.db import router, transaction from django.http import HttpRequest, HttpResponse @@ -11,13 +12,16 @@ from rest_framework import status from sentry.hybridcloud.models.outbox import outbox_context +from sentry.integrations.messaging.metrics import SeerSlackHaltReason from sentry.integrations.middleware.hybrid_cloud.parser import create_async_request_payload from sentry.integrations.models.organization_integration import OrganizationIntegration from sentry.integrations.slack.message_builder.routing import encode_action_id from sentry.integrations.slack.message_builder.types import SlackAction +from sentry.integrations.slack.requests.event import SeerResolutionResult from sentry.integrations.slack.utils.auth import _encode_data from sentry.integrations.slack.views import SALT from sentry.middleware.integrations.parsers.slack import SlackRequestParser +from sentry.models.organizationmapping import OrganizationMapping from sentry.testutils.cases import TestCase from sentry.testutils.outbox import assert_no_webhook_payloads from sentry.testutils.silo import assume_test_silo_mode_of, control_silo_test, create_test_cells @@ -344,3 +348,116 @@ def test_targeting_issue_actions(self) -> None: assert len(organization_ids) == 2 assert self.organization.id in organization_ids assert other_organization.id in organization_ids + + +@control_silo_test(cells=create_test_cells("us")) +class SlackRequestParserSeerEventRoutingTest(TestCase): + """Tests for _maybe_get_response_from_event_request cell routing.""" + + factory = RequestFactory() + + def setUp(self) -> None: + self.user = self.create_user() + self.organization = self.create_organization(owner=self.user) + self.integration = self.create_integration( + organization=self.organization, external_id="TXXXXXXX1", provider="slack" + ) + + def get_response(self, request: HttpRequest) -> HttpResponse: + return HttpResponse(status=200, content="passthrough") + + def _make_event_request(self, event_type: str = "app_mention"): + data = { + "type": "event_callback", + "team_id": self.integration.external_id, + "api_app_id": "AXXXXXXXX1", + "event_id": "E1", + "event": { + "type": event_type, + "channel": "C1234567890", + "user": "U1234567890", + "text": "hello", + "ts": "1234567890.123456", + }, + } + return self.factory.post( + reverse("sentry-integration-slack-event"), + data=orjson.dumps(data), + content_type="application/json", + ) + + def test_returns_none_for_non_event_endpoint(self): + data = urlencode({"team_id": self.integration.external_id}).encode("utf-8") + request = self.factory.post( + path=reverse("sentry-integration-slack-commands"), + data=data, + content_type="application/x-www-form-urlencoded", + ) + parser = SlackRequestParser(request, self.get_response) + assert parser._maybe_get_response_from_event_request() is None + + @patch( + "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", + return_value=True, + ) + def test_returns_none_for_non_seer_event(self, mock_signing): + request = self._make_event_request(event_type="link_shared") + parser = SlackRequestParser(request, self.get_response) + assert parser._maybe_get_response_from_event_request() is None + + @patch( + "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", + return_value=True, + ) + @patch( + "sentry.integrations.slack.requests.event.SlackEventRequest.resolve_seer_organization", + ) + def test_routes_seer_event_to_correct_cell(self, mock_resolve, mock_signing): + mock_resolve.return_value = SeerResolutionResult( + organization_id=self.organization.id, error_reason=None + ) + mapping = OrganizationMapping.objects.get(organization_id=self.organization.id) + assert mapping.cell_name == "us" + + cell_response = HttpResponse(status=200, content="cell_response") + request = self._make_event_request(event_type="app_mention") + parser = SlackRequestParser(request, self.get_response) + + with patch.object( + parser, "get_response_from_cell_silo", return_value=cell_response + ) as mock_cell: + result = parser._maybe_get_response_from_event_request() + + assert result is not None + assert result.status_code == 200 + mock_cell.assert_called_once() + cell_arg = mock_cell.call_args[1]["cell"] + assert cell_arg.name == "us" + + @patch( + "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", + return_value=True, + ) + @patch( + "sentry.integrations.slack.requests.event.SlackEventRequest.resolve_seer_organization", + ) + def test_returns_none_on_org_resolution_error(self, mock_resolve, mock_signing): + mock_resolve.return_value = SeerResolutionResult( + organization_id=None, error_reason=SeerSlackHaltReason.NO_VALID_ORGANIZATION + ) + request = self._make_event_request(event_type="app_mention") + parser = SlackRequestParser(request, self.get_response) + assert parser._maybe_get_response_from_event_request() is None + + @patch( + "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", + return_value=True, + ) + @patch( + "sentry.integrations.slack.requests.event.SlackEventRequest.resolve_seer_organization", + ) + def test_returns_none_on_missing_org_mapping(self, mock_resolve, mock_signing): + mock_resolve.return_value = SeerResolutionResult(organization_id=99999, error_reason=None) + request = self._make_event_request(event_type="app_mention") + parser = SlackRequestParser(request, self.get_response) + assert parser._maybe_get_response_from_event_request() is None From 507f38dbe1baece4e926b68d9541cccff6f821dc Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Thu, 16 Apr 2026 11:31:40 -0400 Subject: [PATCH 3/9] ref(slack): Integrate Seer event org resolution into standard routing flow Replace the separate cell-resolution code path for Seer Slack events with organization filtering in the standard routing flow. Instead of directly resolving to a cell in _maybe_get_response_from_event_request, the new _filter_organizations_for_seer_event method narrows the organization list so normal cell routing handles it. Also reuse the validated slack_request from get_integration_from_request instead of re-initializing it in multiple places, add RpcOrganization support to feature handler type hints, and rename is_seer_explorer_request to is_seer_agent_request. Refs ISWF-2467 Co-Authored-By: Claude Opus 4.6 --- src/sentry/features/handler.py | 7 +- .../integrations/slack/requests/event.py | 2 +- .../integrations/slack/webhooks/event.py | 16 ++- .../middleware/integrations/parsers/slack.py | 112 ++++++++---------- 4 files changed, 62 insertions(+), 75 deletions(-) diff --git a/src/sentry/features/handler.py b/src/sentry/features/handler.py index 2dec9b537f5b..f25dc30ef1f4 100644 --- a/src/sentry/features/handler.py +++ b/src/sentry/features/handler.py @@ -11,6 +11,7 @@ from sentry.features.manager import FeatureCheckBatch from sentry.models.organization import Organization from sentry.models.project import Project + from sentry.organizations.services.organization.model import RpcOrganization from sentry.users.models.user import User from sentry.users.services.user import RpcUser @@ -59,7 +60,7 @@ def batch_has( feature_names: Sequence[str], actor: User | RpcUser | AnonymousUser | None, projects: Sequence[Project] | None = None, - organization: Organization | None = None, + organization: Organization | RpcOrganization | None = None, batch: bool = True, ) -> dict[str, dict[str, bool | None]] | None: raise NotImplementedError @@ -67,7 +68,7 @@ def batch_has( def batch_has_for_organizations( self, feature_name: str, - organizations: Sequence[Organization], + organizations: Sequence[Organization | RpcOrganization], ) -> dict[str, bool] | None: raise NotImplementedError @@ -97,7 +98,7 @@ class BatchFeatureHandler(FeatureHandler): def _check_for_batch( self, feature_name: str, - entity: Organization | User | RpcUser | AnonymousUser | None, + entity: Organization | RpcOrganization | User | RpcUser | AnonymousUser | None, actor: User | RpcUser | AnonymousUser | None, ) -> bool | None: raise NotImplementedError diff --git a/src/sentry/integrations/slack/requests/event.py b/src/sentry/integrations/slack/requests/event.py index b392f42c45d8..35edd91cbeb5 100644 --- a/src/sentry/integrations/slack/requests/event.py +++ b/src/sentry/integrations/slack/requests/event.py @@ -71,7 +71,7 @@ def is_challenge(self) -> bool: return is_event_challenge(self.request.data) @property - def is_seer_explorer_request(self) -> bool: + def is_seer_agent_request(self) -> bool: return ( self.type == "app_mention" or self.type == "assistant_thread_started" diff --git a/src/sentry/integrations/slack/webhooks/event.py b/src/sentry/integrations/slack/webhooks/event.py index 3c9cb6ed5395..eee3815f65e4 100644 --- a/src/sentry/integrations/slack/webhooks/event.py +++ b/src/sentry/integrations/slack/webhooks/event.py @@ -3,7 +3,7 @@ import logging from collections import defaultdict from collections.abc import Mapping -from typing import Any, TypedDict +from typing import Any import orjson import sentry_sdk @@ -73,12 +73,6 @@ SLACK_PROVIDERS = [IntegrationProviderSlug.SLACK, IntegrationProviderSlug.SLACK_STAGING] -class SeerResolutionResult(TypedDict): - organization_id: int | None - installation: SlackIntegration | None - error_reason: SeerSlackHaltReason | None - - @all_silo_endpoint # Only challenge verification is handled at control class SlackEventEndpoint(SlackDMEndpoint): owner = ApiOwner.ECOSYSTEM @@ -397,9 +391,11 @@ def _handle_seer_prompt( if not organization_id: return self.respond() - installation: SlackIntegration = slack_request.integration.get_installation( + installation = slack_request.integration.get_installation( organization_id=organization_id ) + if not isinstance(installation, SlackIntegration): + return self.respond() if not channel_id or not text or not ts or not slack_request.user_id: lifecycle.record_halt(SeerSlackHaltReason.MISSING_EVENT_DATA) @@ -460,9 +456,11 @@ def on_assistant_thread_started(self, slack_request: SlackEventRequest) -> Respo if not organization_id: return self.respond() - installation: SlackIntegration = slack_request.integration.get_installation( + installation = slack_request.integration.get_installation( organization_id=organization_id ) + if not isinstance(installation, SlackIntegration): + return self.respond() channel_id = slack_request.channel_id thread_ts = slack_request.thread_ts diff --git a/src/sentry/middleware/integrations/parsers/slack.py b/src/sentry/middleware/integrations/parsers/slack.py index 7784da09ca4b..0e0c2a443d8f 100644 --- a/src/sentry/middleware/integrations/parsers/slack.py +++ b/src/sentry/middleware/integrations/parsers/slack.py @@ -22,7 +22,9 @@ ) from sentry.integrations.models.integration import Integration from sentry.integrations.slack.message_builder.routing import SlackRoutingData, decode_action_id -from sentry.integrations.slack.requests.base import SlackRequestError +from sentry.integrations.slack.requests.action import SlackActionRequest +from sentry.integrations.slack.requests.base import SlackRequest, SlackRequestError +from sentry.integrations.slack.requests.command import SlackCommandRequest from sentry.integrations.slack.requests.event import SlackEventRequest, is_event_challenge from sentry.integrations.slack.sdk_client import SlackSdkClient from sentry.integrations.slack.views import SALT @@ -41,8 +43,7 @@ from sentry.integrations.slack.webhooks.options_load import SlackOptionsLoadEndpoint from sentry.integrations.types import EXTERNAL_PROVIDERS, ExternalProviders from sentry.middleware.integrations.tasks import convert_to_async_slack_response -from sentry.models.organizationmapping import OrganizationMapping -from sentry.types.cell import Cell, CellResolutionError, get_cell_by_name +from sentry.types.cell import Cell from sentry.utils import json from sentry.utils.signing import unsign @@ -56,6 +57,7 @@ class SlackRequestParser(BaseRequestParser): webhook_identifier = WebhookProviderIdentifier.SLACK response_url: str | None = None action_option: str | None = None + slack_request: SlackRequest | None = None control_classes = [ SlackLinkIdentityView, @@ -204,6 +206,7 @@ def get_integration_from_request(self) -> Integration | None: "slack.validation_error", extra={"path": self.request.path, "error": error} ) return None + self.slack_request = slack_request self.response_url = slack_request.response_url return Integration.objects.filter(id=slack_request.integration.id).first() @@ -223,11 +226,13 @@ def filter_organizations_from_request( as an additional argument. If not, we'll pick from all the organizations, which might fail. """ - drf_request: Request - if self.view_class == SlackCommandsEndpoint: - drf_request = SlackDMEndpoint().initialize_request(self.request) - slack_request = self.view_class.slack_request_class(drf_request) - cmd_input = slack_request.get_command_input() + if not self.slack_request: + return organizations + + if self.view_class == SlackCommandsEndpoint and isinstance( + self.slack_request, SlackCommandRequest + ): + cmd_input = self.slack_request.get_command_input() # For both linking/unlinking teams, the organization slug is found in the same place link_input = None @@ -250,15 +255,13 @@ def filter_organizations_from_request( return [linking_organization] elif self.view_class in [SlackActionEndpoint, SlackOptionsLoadEndpoint]: - drf_request = SlackDMEndpoint().initialize_request(self.request) - slack_request = self.view_class.slack_request_class(drf_request) if self.view_class == SlackActionEndpoint: - actions = slack_request.data.get("actions", []) + actions = self.slack_request.data.get("actions", []) action_ids: list[str] = [ action["action_id"] for action in actions if action.get("action_id") ] elif self.view_class == SlackOptionsLoadEndpoint: - action_ids = [slack_request.data.get("action_id", "")] + action_ids = [self.slack_request.data.get("action_id", "")] decoded_actions: list[SlackRoutingData] = [ decode_action_id(action_id) for action_id in action_ids @@ -272,7 +275,7 @@ def filter_organizations_from_request( logger.info( "slack.control.multiple_organizations", extra={ - "integration_id": slack_request.integration.id, + "integration_id": self.slack_request.integration.id, "organization_ids": list(decoded_organization_ids), "action_ids": action_ids, }, @@ -288,55 +291,45 @@ def filter_organizations_from_request( ) return [action_organization] - logger.info( - "slack.control.could_not_route", - extra={"view_class": self.view_class}, - ) - return organizations + return self._filter_organizations_for_seer_event(organizations) + + def _filter_organizations_for_seer_event( + self, organizations: list[RpcOrganizationMapping] + ) -> list[RpcOrganizationMapping]: + """ + If the request is a Seer Agent event, attempt to resolve a specific organization. + Fallback to the original organizations if there's no resolution. + """ + if not self.slack_request: + return organizations + + if self.view_class != SlackEventEndpoint or not isinstance( + self.slack_request, SlackEventRequest + ): + return organizations - def _maybe_get_response_from_event_request(self) -> HttpResponseBase | None: - if self.view_class != SlackEventEndpoint: - return None - drf_request: Request = SlackDMEndpoint().initialize_request(self.request) - slack_request: SlackEventRequest = self.view_class.slack_request_class(drf_request) + if not self.slack_request.is_seer_agent_request: + return organizations - if not slack_request.is_seer_explorer_request: - return None + organization_id, error_reason = self.slack_request.resolve_seer_organization() + logger_ctx = {"organization_id": organization_id, "error_reason": error_reason} - organization_id, error_reason = slack_request.resolve_seer_organization() if not organization_id or error_reason: logger.info( - "slack.control.seer_event.organization.not_found", - extra={ - "organization_id": organization_id, - "error_reason": error_reason, - }, + "slack.control.filter_seer_event.organization_not_resolved", extra=logger_ctx ) - return None + return organizations - try: - mapping = OrganizationMapping.objects.get(organization_id=organization_id) - except OrganizationMapping.DoesNotExist: + mapping = next((org for org in organizations if org.id == organization_id), None) + if not mapping: logger.info( - "slack.control.seer_event.organization.mapping.not_found", - extra={ - "organization_id": organization_id, - }, + "slack.control.filter_seer_event.organization_mapping_not_found", + extra=logger_ctx, ) - return None + return organizations - try: - cell = get_cell_by_name(mapping.cell_name) - except CellResolutionError: - logger.info( - "slack.control.seer_event.organization.cell.not_found", - extra={ - "organization_id": organization_id, - "cell_name": mapping.cell_name, - }, - ) - return None - return self.get_response_from_cell_silo(cell=cell) + logger.info("slack.control.filter_seer_event.routed", extra=logger_ctx) + return [mapping] def get_response(self) -> HttpResponseBase: """ @@ -367,13 +360,12 @@ def get_response(self) -> HttpResponseBase: # this request until it succeeds. return HttpResponse(status=status.HTTP_202_ACCEPTED) - drf_request: Request - if self.view_class == SlackActionEndpoint: - drf_request: Request = SlackDMEndpoint().initialize_request(self.request) - slack_request = self.view_class.slack_request_class(drf_request) - self.response_url = slack_request.response_url + if self.view_class == SlackActionEndpoint and isinstance( + self.slack_request, SlackActionRequest + ): + self.response_url = self.slack_request.response_url self.action_option, self.action_id = SlackActionEndpoint.get_action_option( - slack_request=slack_request + slack_request=self.slack_request ) # All actions other than those below are sent to every cell if self.action_option not in ACTIONS_ENDPOINT_ALL_SILOS_ACTIONS: @@ -383,10 +375,6 @@ def get_response(self) -> HttpResponseBase: else self.get_response_from_all_cells() ) - event_response = self._maybe_get_response_from_event_request() - if event_response: - return event_response - # Slack webhooks can only receive one synchronous call/response, as there are many # places where we post to slack on their webhook request. This would cause multiple # calls back to slack for every cell we forward to. From b798aaa7c46ebfee7019a18b0075ea3288167a77 Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Thu, 16 Apr 2026 12:11:48 -0400 Subject: [PATCH 4/9] test(slack): Update and add tests for Slack event request routing Adjust SlackRequestParserTest to match the refactored routing flow: call get_integration_from_request() before get_organizations_from_integration so self.slack_request is populated. Move signing secret mock to setUp via addCleanup and consolidate SlackRequestParserSeerEventRoutingTest into the main test class. Add tests for SlackEventRequest properties: is_seer_agent_request, is_assistant_thread_event, and the assistant-thread-aware channel_id, user_id, and thread_ts accessors. Refs ISWF-2467 Co-Authored-By: Claude Opus 4.6 --- .../integrations/slack/test_requests.py | 80 ++++++++- .../integrations/parsers/test_slack.py | 168 +++++++----------- 2 files changed, 145 insertions(+), 103 deletions(-) diff --git a/tests/sentry/integrations/slack/test_requests.py b/tests/sentry/integrations/slack/test_requests.py index a065fd55bba5..63a4e30c051e 100644 --- a/tests/sentry/integrations/slack/test_requests.py +++ b/tests/sentry/integrations/slack/test_requests.py @@ -7,13 +7,16 @@ from django.utils.functional import cached_property from sentry import options +from sentry.integrations.models.integration import Integration +from sentry.integrations.services.integration.service import integration_service from sentry.integrations.slack.requests.action import SlackActionRequest from sentry.integrations.slack.requests.base import SlackRequest, SlackRequestError from sentry.integrations.slack.requests.event import SlackEventRequest from sentry.integrations.slack.utils.auth import set_signing_secret +from sentry.integrations.slack.utils.constants import SlackScope from sentry.testutils.cases import TestCase from sentry.testutils.helpers import override_options -from sentry.testutils.silo import control_silo_test +from sentry.testutils.silo import assume_test_silo_mode_of, control_silo_test @control_silo_test @@ -116,6 +119,9 @@ class SlackEventRequestTest(TestCase): def setUp(self) -> None: super().setUp() + self.integration = self.create_integration( + organization=self.organization, external_id="T001", provider="slack" + ) self.request = mock.Mock() self.request.data = { "type": "foo", @@ -194,6 +200,78 @@ def test_use_verification_token(self) -> None: self.slack_request.validate() + def test_is_seer_agent_request(self) -> None: + self.request.data["event"] = {"type": "app_mention"} + assert SlackEventRequest(self.request).is_seer_agent_request is True + + self.request.data["event"] = {"type": "assistant_thread_started"} + assert SlackEventRequest(self.request).is_seer_agent_request is True + + self.request.data["event"] = {"type": "link_shared"} + assert SlackEventRequest(self.request).is_seer_agent_request is False + + def test_is_seer_agent_request_dm_checks_assistant_scope(self) -> None: + self.request.data["event"] = {"type": "message"} + + with assume_test_silo_mode_of(Integration): + self.integration.metadata["scopes"] = [SlackScope.ASSISTANT_WRITE.value] + self.integration.save() + rpc_integration = integration_service.get_integration(integration_id=self.integration.id) + + req = SlackEventRequest(self.request) + req._integration = rpc_integration + assert req.is_seer_agent_request is True + + with assume_test_silo_mode_of(Integration): + self.integration.metadata["scopes"] = [] + self.integration.save() + rpc_integration = integration_service.get_integration(integration_id=self.integration.id) + + req = SlackEventRequest(self.request) + req._integration = rpc_integration + assert req.is_seer_agent_request is False + + def test_is_assistant_thread_event(self) -> None: + self.request.data["event"] = {"type": "assistant_thread_started"} + assert SlackEventRequest(self.request).is_assistant_thread_event is True + + self.request.data["event"] = {"type": "app_mention"} + assert SlackEventRequest(self.request).is_assistant_thread_event is False + + def test_channel_id(self) -> None: + self.request.data["event"] = { + "type": "message", + "channel": "C_REGULAR", + "assistant_thread": {"channel_id": "C_ASSISTANT", "user_id": "U1", "thread_ts": "1.0"}, + } + assert SlackEventRequest(self.request).channel_id == "C_REGULAR" + self.request.data["event"]["type"] = "assistant_thread_started" + assert SlackEventRequest(self.request).channel_id == "C_ASSISTANT" + + def test_user_id(self) -> None: + self.request.data["event"] = { + "type": "message", + "user": "U_REGULAR", + "assistant_thread": { + "channel_id": "C1", + "user_id": "U_ASSISTANT", + "thread_ts": "1.0", + }, + } + assert SlackEventRequest(self.request).user_id == "U_REGULAR" + self.request.data["event"]["type"] = "assistant_thread_started" + assert SlackEventRequest(self.request).user_id == "U_ASSISTANT" + + def test_thread_ts(self) -> None: + self.request.data["event"] = { + "type": "app_mention", + "thread_ts": "111.222", + "assistant_thread": {"channel_id": "C1", "user_id": "U1", "thread_ts": "333.444"}, + } + assert SlackEventRequest(self.request).thread_ts == "111.222" + self.request.data["event"]["type"] = "assistant_thread_started" + assert SlackEventRequest(self.request).thread_ts == "333.444" + class SlackActionRequestTest(TestCase): def setUp(self) -> None: diff --git a/tests/sentry/middleware/integrations/parsers/test_slack.py b/tests/sentry/middleware/integrations/parsers/test_slack.py index a29199ac5d7e..82b8273c2dad 100644 --- a/tests/sentry/middleware/integrations/parsers/test_slack.py +++ b/tests/sentry/middleware/integrations/parsers/test_slack.py @@ -12,6 +12,8 @@ from rest_framework import status from sentry.hybridcloud.models.outbox import outbox_context +from sentry.hybridcloud.services.organization_mapping.model import RpcOrganizationMapping +from sentry.hybridcloud.services.organization_mapping.service import organization_mapping_service from sentry.integrations.messaging.metrics import SeerSlackHaltReason from sentry.integrations.middleware.hybrid_cloud.parser import create_async_request_payload from sentry.integrations.models.organization_integration import OrganizationIntegration @@ -21,7 +23,6 @@ from sentry.integrations.slack.utils.auth import _encode_data from sentry.integrations.slack.views import SALT from sentry.middleware.integrations.parsers.slack import SlackRequestParser -from sentry.models.organizationmapping import OrganizationMapping from sentry.testutils.cases import TestCase from sentry.testutils.outbox import assert_no_webhook_payloads from sentry.testutils.silo import assume_test_silo_mode_of, control_silo_test, create_test_cells @@ -40,10 +41,38 @@ def setUp(self) -> None: self.integration = self.create_integration( organization=self.organization, external_id="TXXXXXXX1", provider="slack" ) + self.org_mapping = organization_mapping_service.get(organization_id=self.organization.id) + patcher = patch( + "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", + return_value=True, + ) + patcher.start() + self.addCleanup(patcher.stop) def get_response(self, request: HttpRequest) -> HttpResponse: return HttpResponse(status=200, content="passthrough") + def _make_parser_with_seer_event(self, event_type: str = "app_mention"): + data = { + "type": "event_callback", + "team_id": self.integration.external_id, + "api_app_id": "AXXXXXXXX1", + "event_id": "E1", + "event": { + "type": event_type, + "channel": "C1234567890", + "user": "U1234567890", + "text": "hello", + "ts": "1234567890.123456", + }, + } + request = self.factory.post( + reverse("sentry-integration-slack-event"), + data=orjson.dumps(data), + content_type="application/json", + ) + return SlackRequestParser(request, self.get_response) + @responses.activate @patch( "slack_sdk.signature.SignatureVerifier.is_valid", @@ -114,14 +143,8 @@ def test_django_view(self) -> None: assert len(responses.calls) == 0 assert_no_webhook_payloads() - @patch( - "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", - return_value=True, - ) @patch("sentry.middleware.integrations.parsers.slack.convert_to_async_slack_response") - def test_triggers_async_response( - self, mock_slack_task: MagicMock, mock_signing_secret: MagicMock - ) -> None: + def test_triggers_async_response(self, mock_slack_task: MagicMock) -> None: response_url = "https://hooks.slack.com/commands/TXXXXXXX1/1234567890123/something" data = { "payload": json.dumps( @@ -140,14 +163,8 @@ def test_triggers_async_response( ) assert response.status_code == status.HTTP_200_OK - @patch( - "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", - return_value=True, - ) @patch("sentry.middleware.integrations.parsers.slack.convert_to_async_slack_response") - def test_skips_async_response_if_org_integration_missing( - self, mock_slack_task, mock_signing_secret - ): + def test_skips_async_response_if_org_integration_missing(self, mock_slack_task): response_url = "https://hooks.slack.com/commands/TXXXXXXX1/1234567890123/something" data = { "payload": json.dumps( @@ -207,6 +224,7 @@ def test_targeting_all_orgs(self) -> None: content_type="application/x-www-form-urlencoded", ) parser = SlackRequestParser(request, self.get_response) + parser.get_integration_from_request() organizations = parser.get_organizations_from_integration(self.integration) organization_ids = {org.id for org in organizations} assert len(organization_ids) == 2 @@ -232,6 +250,7 @@ def test_targeting_specific_org(self) -> None: content_type="application/x-www-form-urlencoded", ) parser = SlackRequestParser(request, self.get_response) + parser.get_integration_from_request() organizations = parser.get_organizations_from_integration(self.integration) assert len(organizations) == 1 @@ -258,6 +277,7 @@ def test_targeting_irrelevant_org(self) -> None: content_type="application/x-www-form-urlencoded", ) parser = SlackRequestParser(request, self.get_response) + parser.get_integration_from_request() organizations = parser.get_organizations_from_integration(self.integration) organization_ids = {org.id for org in organizations} assert len(organization_ids) == 2 @@ -285,6 +305,7 @@ def test_targeting_issue_actions(self) -> None: content_type="application/x-www-form-urlencoded", ) parser = SlackRequestParser(request, self.get_response) + parser.get_integration_from_request() organizations = parser.get_organizations_from_integration(self.integration) organization_ids = {org.id for org in organizations} assert len(organization_ids) == 2 @@ -314,6 +335,7 @@ def test_targeting_issue_actions(self) -> None: content_type="application/x-www-form-urlencoded", ) parser = SlackRequestParser(request, self.get_response) + parser.get_integration_from_request() organizations = parser.get_organizations_from_integration(self.integration) organization_ids = {org.id for org in organizations} assert len(organization_ids) == 1 @@ -343,121 +365,63 @@ def test_targeting_issue_actions(self) -> None: content_type="application/x-www-form-urlencoded", ) parser = SlackRequestParser(request, self.get_response) + parser.get_integration_from_request() organizations = parser.get_organizations_from_integration(self.integration) organization_ids = {org.id for org in organizations} assert len(organization_ids) == 2 assert self.organization.id in organization_ids assert other_organization.id in organization_ids - -@control_silo_test(cells=create_test_cells("us")) -class SlackRequestParserSeerEventRoutingTest(TestCase): - """Tests for _maybe_get_response_from_event_request cell routing.""" - - factory = RequestFactory() - - def setUp(self) -> None: - self.user = self.create_user() - self.organization = self.create_organization(owner=self.user) - self.integration = self.create_integration( - organization=self.organization, external_id="TXXXXXXX1", provider="slack" - ) - - def get_response(self, request: HttpRequest) -> HttpResponse: - return HttpResponse(status=200, content="passthrough") - - def _make_event_request(self, event_type: str = "app_mention"): - data = { - "type": "event_callback", - "team_id": self.integration.external_id, - "api_app_id": "AXXXXXXXX1", - "event_id": "E1", - "event": { - "type": event_type, - "channel": "C1234567890", - "user": "U1234567890", - "text": "hello", - "ts": "1234567890.123456", - }, - } - return self.factory.post( - reverse("sentry-integration-slack-event"), - data=orjson.dumps(data), - content_type="application/json", - ) - - def test_returns_none_for_non_event_endpoint(self): - data = urlencode({"team_id": self.integration.external_id}).encode("utf-8") + def test_seer_filter_returns_orgs_unchanged_when_no_slack_request(self): request = self.factory.post( path=reverse("sentry-integration-slack-commands"), - data=data, + data=urlencode({"team_id": self.integration.external_id}).encode("utf-8"), content_type="application/x-www-form-urlencoded", ) parser = SlackRequestParser(request, self.get_response) - assert parser._maybe_get_response_from_event_request() is None + orgs = [self.org_mapping] + assert parser._filter_organizations_for_seer_event(orgs) is orgs - @patch( - "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", - return_value=True, - ) - def test_returns_none_for_non_seer_event(self, mock_signing): - request = self._make_event_request(event_type="link_shared") - parser = SlackRequestParser(request, self.get_response) - assert parser._maybe_get_response_from_event_request() is None + def test_seer_filter_returns_orgs_unchanged_for_non_seer_event(self): + parser = self._make_parser_with_seer_event(event_type="link_shared") + parser.get_integration_from_request() + orgs = [self.org_mapping] + assert parser._filter_organizations_for_seer_event(orgs) is orgs - @patch( - "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", - return_value=True, - ) @patch( "sentry.integrations.slack.requests.event.SlackEventRequest.resolve_seer_organization", ) - def test_routes_seer_event_to_correct_cell(self, mock_resolve, mock_signing): + def test_seer_filter_filters_to_resolved_organization(self, mock_resolve): mock_resolve.return_value = SeerResolutionResult( organization_id=self.organization.id, error_reason=None ) - mapping = OrganizationMapping.objects.get(organization_id=self.organization.id) - assert mapping.cell_name == "us" - - cell_response = HttpResponse(status=200, content="cell_response") - request = self._make_event_request(event_type="app_mention") - parser = SlackRequestParser(request, self.get_response) + other_org = RpcOrganizationMapping(id=99999, slug="other") + parser = self._make_parser_with_seer_event(event_type="app_mention") + parser.get_integration_from_request() - with patch.object( - parser, "get_response_from_cell_silo", return_value=cell_response - ) as mock_cell: - result = parser._maybe_get_response_from_event_request() + result = parser._filter_organizations_for_seer_event([other_org, self.org_mapping]) + assert result == [self.org_mapping] - assert result is not None - assert result.status_code == 200 - mock_cell.assert_called_once() - cell_arg = mock_cell.call_args[1]["cell"] - assert cell_arg.name == "us" - - @patch( - "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", - return_value=True, - ) @patch( "sentry.integrations.slack.requests.event.SlackEventRequest.resolve_seer_organization", ) - def test_returns_none_on_org_resolution_error(self, mock_resolve, mock_signing): + def test_seer_filter_returns_orgs_unchanged_on_resolution_error(self, mock_resolve): mock_resolve.return_value = SeerResolutionResult( organization_id=None, error_reason=SeerSlackHaltReason.NO_VALID_ORGANIZATION ) - request = self._make_event_request(event_type="app_mention") - parser = SlackRequestParser(request, self.get_response) - assert parser._maybe_get_response_from_event_request() is None + parser = self._make_parser_with_seer_event(event_type="app_mention") + parser.get_integration_from_request() + orgs = [self.org_mapping] + assert parser._filter_organizations_for_seer_event(orgs) is orgs - @patch( - "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", - return_value=True, - ) @patch( "sentry.integrations.slack.requests.event.SlackEventRequest.resolve_seer_organization", ) - def test_returns_none_on_missing_org_mapping(self, mock_resolve, mock_signing): - mock_resolve.return_value = SeerResolutionResult(organization_id=99999, error_reason=None) - request = self._make_event_request(event_type="app_mention") - parser = SlackRequestParser(request, self.get_response) - assert parser._maybe_get_response_from_event_request() is None + def test_seer_filter_returns_orgs_unchanged_when_mapping_not_in_list(self, mock_resolve): + mock_resolve.return_value = SeerResolutionResult( + organization_id=self.organization.id, error_reason=None + ) + parser = self._make_parser_with_seer_event(event_type="app_mention") + parser.get_integration_from_request() + other_orgs = [RpcOrganizationMapping(id=99999, slug="other")] + assert parser._filter_organizations_for_seer_event(other_orgs) is other_orgs From ad5749094a2259ccac61069fd07d822feec58a0a Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Thu, 16 Apr 2026 12:31:02 -0400 Subject: [PATCH 5/9] test(slack): Add resolve_seer_organization tests Add SlackEventRequestSeerResolutionTest covering all exit paths: identity not linked, no org integrations, org not found, org inactive, no Seer access, user not a member, and successful resolution. Uses RequestFactory with SlackDMEndpoint().initialize_request() to exercise the real DRF request and validation code paths rather than mocking request internals. Refs ISWF-2467 Co-Authored-By: Claude Opus 4.6 --- .../integrations/slack/test_requests.py | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/tests/sentry/integrations/slack/test_requests.py b/tests/sentry/integrations/slack/test_requests.py index 63a4e30c051e..a44a937f9470 100644 --- a/tests/sentry/integrations/slack/test_requests.py +++ b/tests/sentry/integrations/slack/test_requests.py @@ -4,16 +4,21 @@ import orjson import pytest +from django.test import RequestFactory from django.utils.functional import cached_property from sentry import options +from sentry.integrations.messaging.metrics import SeerSlackHaltReason from sentry.integrations.models.integration import Integration +from sentry.integrations.models.organization_integration import OrganizationIntegration from sentry.integrations.services.integration.service import integration_service from sentry.integrations.slack.requests.action import SlackActionRequest from sentry.integrations.slack.requests.base import SlackRequest, SlackRequestError from sentry.integrations.slack.requests.event import SlackEventRequest from sentry.integrations.slack.utils.auth import set_signing_secret from sentry.integrations.slack.utils.constants import SlackScope +from sentry.integrations.slack.webhooks.base import SlackDMEndpoint +from sentry.models.organization import OrganizationStatus from sentry.testutils.cases import TestCase from sentry.testutils.helpers import override_options from sentry.testutils.silo import assume_test_silo_mode_of, control_silo_test @@ -273,6 +278,107 @@ def test_thread_ts(self) -> None: assert SlackEventRequest(self.request).thread_ts == "333.444" +class SlackEventRequestSeerResolutionTest(TestCase): + factory = RequestFactory() + + def setUp(self) -> None: + self.slack_user = self.create_user() + self.organization = self.create_organization(owner=self.slack_user) + self.integration = self.create_integration( + organization=self.organization, external_id="T_SEER", provider="slack" + ) + self.identity_provider = self.create_identity_provider(integration=self.integration) + self.identity = self.create_identity( + user=self.slack_user, + identity_provider=self.identity_provider, + external_id="U_SLACK", + ) + patcher = patch( + "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", + return_value=True, + ) + patcher.start() + self.addCleanup(patcher.stop) + + data = { + "type": "event_callback", + "team_id": "T_SEER", + "event_id": "E1", + "api_app_id": "A1", + "event": { + "type": "app_mention", + "channel": "C1", + "user": "U_SLACK", + "ts": "1.0", + }, + } + request = self.factory.post( + "/extensions/slack/event/", + data=orjson.dumps(data), + content_type="application/json", + ) + drf_request = SlackDMEndpoint().initialize_request(request) + self.slack_request = SlackEventRequest(drf_request) + self.slack_request.authorize() + self.slack_request.validate_integration() + + @patch( + "sentry.integrations.slack.requests.event.send_identity_link_prompt", + ) + def test_identity_not_linked(self, mock_prompt): + with assume_test_silo_mode_of(self.identity): + self.identity.delete() + result = self.slack_request.resolve_seer_organization() + assert result.organization_id is None + assert result.error_reason == SeerSlackHaltReason.IDENTITY_NOT_LINKED + mock_prompt.assert_called_once() + + def test_no_organization_integrations(self): + with assume_test_silo_mode_of(OrganizationIntegration): + OrganizationIntegration.objects.filter(integration_id=self.integration.id).delete() + result = self.slack_request.resolve_seer_organization() + assert result.organization_id is None + assert result.error_reason == SeerSlackHaltReason.NO_VALID_INTEGRATION + + def test_org_not_found(self): + self.organization.delete() + result = self.slack_request.resolve_seer_organization() + assert result.organization_id is None + assert result.error_reason == SeerSlackHaltReason.NO_VALID_ORGANIZATION + + def test_org_not_active(self): + self.organization.update(status=OrganizationStatus.PENDING_DELETION) + result = self.slack_request.resolve_seer_organization() + assert result.organization_id is None + assert result.error_reason == SeerSlackHaltReason.NO_VALID_ORGANIZATION + + @patch( + "sentry.integrations.slack.requests.event.SlackExplorerEntrypoint.has_access", + return_value=False, + ) + def test_org_no_seer_access(self, mock_access): + result = self.slack_request.resolve_seer_organization() + assert result.organization_id is None + assert result.error_reason == SeerSlackHaltReason.NO_VALID_ORGANIZATION + + def test_user_not_member(self): + non_member = self.create_user() + with assume_test_silo_mode_of(self.identity): + self.identity.update(user=non_member) + result = self.slack_request.resolve_seer_organization() + assert result.organization_id is None + assert result.error_reason == SeerSlackHaltReason.NO_VALID_ORGANIZATION + + @patch( + "sentry.integrations.slack.requests.event.SlackExplorerEntrypoint.has_access", + return_value=True, + ) + def test_resolves_valid_organization(self, mock_access): + result = self.slack_request.resolve_seer_organization() + assert result.organization_id == self.organization.id + assert result.error_reason is None + + class SlackActionRequestTest(TestCase): def setUp(self) -> None: super().setUp() From e628eb8acd55ee0e9dad3068a2c62a0eec3200b3 Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Thu, 16 Apr 2026 12:34:28 -0400 Subject: [PATCH 6/9] fix(slack): Validate request data before logging in SlackRequest Move _validate_data() before _log_request() in validate() so that invalid request data raises SlackRequestError(400) instead of crashing in _log_request when it accesses self.request.data. Also switch logging_data to read from self.data (the validated copy) rather than self.request.data directly. Co-Authored-By: Claude Opus 4.6 --- src/sentry/integrations/slack/requests/base.py | 4 ++-- tests/sentry/integrations/slack/test_requests.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/sentry/integrations/slack/requests/base.py b/src/sentry/integrations/slack/requests/base.py index e8267f76c0e6..db1cda053a0b 100644 --- a/src/sentry/integrations/slack/requests/base.py +++ b/src/sentry/integrations/slack/requests/base.py @@ -86,10 +86,10 @@ def validate(self) -> None: """ Ensure everything is present to properly process this request """ + self._validate_data() self._log_request() self._get_context() self.authorize() - self._validate_data() self.validate_integration() def is_bot(self) -> bool: @@ -154,7 +154,7 @@ def data(self) -> Mapping[str, Any]: @property def logging_data(self) -> Mapping[str, str]: - _data = self.request.data + _data = self.data data = { "slack_team_id": _get_field_id_option(_data, "team"), "slack_channel_id": _get_field_id_option(_data, "channel"), diff --git a/tests/sentry/integrations/slack/test_requests.py b/tests/sentry/integrations/slack/test_requests.py index a44a937f9470..7f1f43b3e902 100644 --- a/tests/sentry/integrations/slack/test_requests.py +++ b/tests/sentry/integrations/slack/test_requests.py @@ -78,7 +78,6 @@ def test_disregards_None_logging_values(self) -> None: "slack_user_id": "2", } - @pytest.mark.xfail(strict=True, reason="crashes in _log_request before validation can occur") def test_returns_400_on_invalid_data(self) -> None: type(self.request).data = mock.PropertyMock(side_effect=ValueError()) From 288a10ae75e053545d1aa213711fe1a0875574ae Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Thu, 16 Apr 2026 14:26:36 -0400 Subject: [PATCH 7/9] fix(slack): Send Seer identity link prompt from region silo only resolve_seer_organization is invoked in both the control-silo parser (for cell routing) and the region-silo endpoint (for request handling). Sending the link prompt as a side effect of resolution caused unlinked users to receive two ephemeral link prompts per request. Move the send_identity_link_prompt call out of resolve_seer_organization and into the region-silo webhook endpoint, so it fires exactly once when the halt reason is IDENTITY_NOT_LINKED. --- src/sentry/integrations/slack/requests/event.py | 14 ++------------ src/sentry/integrations/slack/webhooks/event.py | 16 ++++++++++++++++ tests/sentry/integrations/slack/test_requests.py | 6 +----- .../slack/webhooks/events/test_app_mention.py | 4 ++-- .../events/test_assistant_thread_started.py | 2 +- .../slack/webhooks/events/test_direct_message.py | 2 +- .../integrations/parsers/test_slack.py | 4 +++- 7 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/sentry/integrations/slack/requests/event.py b/src/sentry/integrations/slack/requests/event.py index 35edd91cbeb5..36096ecb37d6 100644 --- a/src/sentry/integrations/slack/requests/event.py +++ b/src/sentry/integrations/slack/requests/event.py @@ -14,7 +14,6 @@ from sentry.models.organization import OrganizationStatus from sentry.organizations.services.organization.service import organization_service from sentry.seer.entrypoints.slack.entrypoint import SlackExplorerEntrypoint -from sentry.seer.entrypoints.slack.messaging import send_identity_link_prompt from sentry.silo.base import all_silo_function COMMANDS = ["link", "unlink", "link team", "unlink team"] @@ -83,10 +82,8 @@ def resolve_seer_organization(self) -> SeerResolutionResult: """ Resolve and validate an organization/user for a Seer Slack event. - If the initiating user is not linked, we will reply with a prompt to link their identity. - - Then we search for an active, organization with Seer Explorer access. If the user does not - belong to any matched organization, their request will be dropped. + We require a linked identity, then search for an active, organization they belong to with + Seer Agent access. Note: There is a limitation here of only grabbing the first organization belonging to the user with access to Seer. If a Slack installation corresponds to multiple organizations with Seer @@ -94,13 +91,6 @@ def resolve_seer_organization(self) -> SeerResolutionResult: """ identity_user = self.get_identity_user() if not identity_user: - send_identity_link_prompt( - integration=self.integration, - slack_user_id=self.user_id, - channel_id=self.channel_id, - thread_ts=self.thread_ts or None, - is_welcome_message=self.is_assistant_thread_event, - ) return SeerResolutionResult( organization_id=None, error_reason=SeerSlackHaltReason.IDENTITY_NOT_LINKED ) diff --git a/src/sentry/integrations/slack/webhooks/event.py b/src/sentry/integrations/slack/webhooks/event.py index eee3815f65e4..d651240ca765 100644 --- a/src/sentry/integrations/slack/webhooks/event.py +++ b/src/sentry/integrations/slack/webhooks/event.py @@ -35,6 +35,7 @@ from sentry.integrations.types import IntegrationProviderSlug from sentry.organizations.services.organization import organization_service from sentry.organizations.services.organization.model import RpcOrganization +from sentry.seer.entrypoints.slack.messaging import send_identity_link_prompt from sentry.seer.entrypoints.slack.tasks import process_mention_for_slack from .base import SlackDMEndpoint @@ -386,6 +387,13 @@ def _handle_seer_prompt( organization_id, error_reason = slack_request.resolve_seer_organization() if error_reason: lifecycle.record_halt(error_reason) + if error_reason == SeerSlackHaltReason.IDENTITY_NOT_LINKED: + send_identity_link_prompt( + integration=slack_request.integration, + slack_user_id=slack_request.user_id, + channel_id=slack_request.channel_id, + thread_ts=slack_request.thread_ts or None, + ) return self.respond() if not organization_id: @@ -451,6 +459,14 @@ def on_assistant_thread_started(self, slack_request: SlackEventRequest) -> Respo organization_id, error_reason = slack_request.resolve_seer_organization() if error_reason: lifecycle.record_halt(error_reason) + if error_reason == SeerSlackHaltReason.IDENTITY_NOT_LINKED: + send_identity_link_prompt( + integration=slack_request.integration, + slack_user_id=slack_request.user_id, + channel_id=slack_request.channel_id, + thread_ts=slack_request.thread_ts or None, + is_welcome_message=True, + ) return self.respond() if not organization_id: diff --git a/tests/sentry/integrations/slack/test_requests.py b/tests/sentry/integrations/slack/test_requests.py index 7f1f43b3e902..6891087c69e6 100644 --- a/tests/sentry/integrations/slack/test_requests.py +++ b/tests/sentry/integrations/slack/test_requests.py @@ -321,16 +321,12 @@ def setUp(self) -> None: self.slack_request.authorize() self.slack_request.validate_integration() - @patch( - "sentry.integrations.slack.requests.event.send_identity_link_prompt", - ) - def test_identity_not_linked(self, mock_prompt): + def test_identity_not_linked(self): with assume_test_silo_mode_of(self.identity): self.identity.delete() result = self.slack_request.resolve_seer_organization() assert result.organization_id is None assert result.error_reason == SeerSlackHaltReason.IDENTITY_NOT_LINKED - mock_prompt.assert_called_once() def test_no_organization_integrations(self): with assume_test_silo_mode_of(OrganizationIntegration): diff --git a/tests/sentry/integrations/slack/webhooks/events/test_app_mention.py b/tests/sentry/integrations/slack/webhooks/events/test_app_mention.py index c79df31c169c..f5c4f379f80d 100644 --- a/tests/sentry/integrations/slack/webhooks/events/test_app_mention.py +++ b/tests/sentry/integrations/slack/webhooks/events/test_app_mention.py @@ -100,7 +100,7 @@ def test_app_mention_no_explorer_access(self, mock_apply_async, mock_record): assert_halt_metric(mock_record, SeerSlackHaltReason.NO_VALID_ORGANIZATION) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") - @patch("sentry.integrations.slack.requests.event.send_identity_link_prompt") + @patch("sentry.integrations.slack.webhooks.event.send_identity_link_prompt") @patch("sentry.seer.entrypoints.slack.tasks.process_mention_for_slack.apply_async") def test_app_mention_no_identity_prompt_linkage( self, mock_apply_async, mock_send_link, mock_record @@ -113,7 +113,7 @@ def test_app_mention_no_identity_prompt_linkage( mock_apply_async.assert_not_called() mock_send_link.assert_called_once() assert mock_send_link.call_args[1]["slack_user_id"] == "U1234567890" - assert mock_send_link.call_args[1]["is_welcome_message"] is False + assert mock_send_link.call_args[1].get("is_welcome_message", False) is False assert_halt_metric(mock_record, SeerSlackHaltReason.IDENTITY_NOT_LINKED) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") diff --git a/tests/sentry/integrations/slack/webhooks/events/test_assistant_thread_started.py b/tests/sentry/integrations/slack/webhooks/events/test_assistant_thread_started.py index 6162343148fc..b6d3a2942789 100644 --- a/tests/sentry/integrations/slack/webhooks/events/test_assistant_thread_started.py +++ b/tests/sentry/integrations/slack/webhooks/events/test_assistant_thread_started.py @@ -54,7 +54,7 @@ def test_prompt_titles_and_messages(self, mock_set_prompts): assert prompt["message"] @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") - @patch("sentry.integrations.slack.requests.event.send_identity_link_prompt") + @patch("sentry.integrations.slack.webhooks.event.send_identity_link_prompt") @patch("sentry.integrations.slack.integration.SlackIntegration.set_suggested_prompts") def test_identity_not_linked(self, mock_set_prompts, mock_send_link, mock_record): self.unlink_identity() diff --git a/tests/sentry/integrations/slack/webhooks/events/test_direct_message.py b/tests/sentry/integrations/slack/webhooks/events/test_direct_message.py index a03c937fce93..4f4eff5e971a 100644 --- a/tests/sentry/integrations/slack/webhooks/events/test_direct_message.py +++ b/tests/sentry/integrations/slack/webhooks/events/test_direct_message.py @@ -73,7 +73,7 @@ def test_dm_threaded_dispatches_task(self, mock_apply_async): assert kwargs["thread_ts"] == THREADED_MESSAGE_DM_EVENT["thread_ts"] @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") - @patch("sentry.integrations.slack.requests.event.send_identity_link_prompt") + @patch("sentry.integrations.slack.webhooks.event.send_identity_link_prompt") @patch("sentry.seer.entrypoints.slack.tasks.process_mention_for_slack.apply_async") def test_dm_identity_not_linked(self, mock_apply_async, mock_send_link, mock_record): """When no identity is linked, send a link prompt and halt.""" diff --git a/tests/sentry/middleware/integrations/parsers/test_slack.py b/tests/sentry/middleware/integrations/parsers/test_slack.py index 82b8273c2dad..ea6a14e2631e 100644 --- a/tests/sentry/middleware/integrations/parsers/test_slack.py +++ b/tests/sentry/middleware/integrations/parsers/test_slack.py @@ -41,7 +41,9 @@ def setUp(self) -> None: self.integration = self.create_integration( organization=self.organization, external_id="TXXXXXXX1", provider="slack" ) - self.org_mapping = organization_mapping_service.get(organization_id=self.organization.id) + org_mapping = organization_mapping_service.get(organization_id=self.organization.id) + assert org_mapping is not None + self.org_mapping = org_mapping patcher = patch( "sentry.integrations.slack.requests.base.SlackRequest._check_signing_secret", return_value=True, From f8f9f2305d494b01a8c6e476f66bb3307b64d5e4 Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Thu, 16 Apr 2026 14:32:05 -0400 Subject: [PATCH 8/9] Revert "fix(slack): Validate request data before logging in SlackRequest" This reverts commit e628eb8acd55ee0e9dad3068a2c62a0eec3200b3. --- src/sentry/integrations/slack/requests/base.py | 4 ++-- tests/sentry/integrations/slack/test_requests.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sentry/integrations/slack/requests/base.py b/src/sentry/integrations/slack/requests/base.py index db1cda053a0b..e8267f76c0e6 100644 --- a/src/sentry/integrations/slack/requests/base.py +++ b/src/sentry/integrations/slack/requests/base.py @@ -86,10 +86,10 @@ def validate(self) -> None: """ Ensure everything is present to properly process this request """ - self._validate_data() self._log_request() self._get_context() self.authorize() + self._validate_data() self.validate_integration() def is_bot(self) -> bool: @@ -154,7 +154,7 @@ def data(self) -> Mapping[str, Any]: @property def logging_data(self) -> Mapping[str, str]: - _data = self.data + _data = self.request.data data = { "slack_team_id": _get_field_id_option(_data, "team"), "slack_channel_id": _get_field_id_option(_data, "channel"), diff --git a/tests/sentry/integrations/slack/test_requests.py b/tests/sentry/integrations/slack/test_requests.py index 6891087c69e6..9b508e940501 100644 --- a/tests/sentry/integrations/slack/test_requests.py +++ b/tests/sentry/integrations/slack/test_requests.py @@ -78,6 +78,7 @@ def test_disregards_None_logging_values(self) -> None: "slack_user_id": "2", } + @pytest.mark.xfail(strict=True, reason="crashes in _log_request before validation can occur") def test_returns_400_on_invalid_data(self) -> None: type(self.request).data = mock.PropertyMock(side_effect=ValueError()) From b2478abce1f73dcd3477bec8b323f45da243fe00 Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Thu, 16 Apr 2026 14:58:58 -0400 Subject: [PATCH 9/9] =?UTF-8?q?=F0=9F=94=A5=20rm=20unused=20var?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/sentry/integrations/slack/webhooks/event.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/sentry/integrations/slack/webhooks/event.py b/src/sentry/integrations/slack/webhooks/event.py index d651240ca765..1ce458902ab5 100644 --- a/src/sentry/integrations/slack/webhooks/event.py +++ b/src/sentry/integrations/slack/webhooks/event.py @@ -32,7 +32,6 @@ from sentry.integrations.slack.unfurl.handlers import link_handlers, match_link from sentry.integrations.slack.unfurl.types import LinkType, UnfurlableUrl from sentry.integrations.slack.views.link_identity import build_linking_url -from sentry.integrations.types import IntegrationProviderSlug from sentry.organizations.services.organization import organization_service from sentry.organizations.services.organization.model import RpcOrganization from sentry.seer.entrypoints.slack.messaging import send_identity_link_prompt @@ -71,7 +70,6 @@ "Hold on, I've seen this one before...", "It worked on my machine...", ] -SLACK_PROVIDERS = [IntegrationProviderSlug.SLACK, IntegrationProviderSlug.SLACK_STAGING] @all_silo_endpoint # Only challenge verification is handled at control