Skip to content

feat(slack): Implement process_mention_for_slack task for Explorer#109733

Merged
alexsohn1126 merged 307 commits intomasterfrom
alexsohn/iswf-2023-add-taskbroker-task-for-processing-slack-explorer-mentions
Mar 31, 2026
Merged

feat(slack): Implement process_mention_for_slack task for Explorer#109733
alexsohn1126 merged 307 commits intomasterfrom
alexsohn/iswf-2023-add-taskbroker-task-for-processing-slack-explorer-mentions

Conversation

@alexsohn1126
Copy link
Copy Markdown
Member

@alexsohn1126 alexsohn1126 commented Mar 2, 2026

Summary

  • Implement the full process_mention_for_slack task body (replacing the previous TODO stub): fetch org, check explorer access, construct SlackExplorerEntrypoint, resolve the Sentry user from Slack identity, extract the user prompt, build thread context, and trigger an Explorer run via SeerExplorerOperator
  • Add _resolve_user() helper that maps a Slack user ID to an RpcUser through the linked identity provider, _send_link_identity_prompt() that sends an ephemeral Slack message prompting the user to link their identity when no link exists, and _send_not_org_member_message() that sends an ephemeral error when the user is not a member of the organization
  • Extract bot_user_id from the Slack authorizations payload in on_app_mention so the task can strip the bot mention from the user's prompt via extract_prompt()
  • Set assistant thread status ("Thinking..." with loading messages) immediately on mention for user feedback before dispatching the async task; clear thread status on early exits (identity not linked, not an org member)
  • Fix thread_ts resolution in on_app_mention to correctly distinguish ts (message timestamp) from thread_ts (parent thread timestamp, None for top-level messages), and pass both to the task
  • Move return self.respond() inside the lifecycle context manager so halts/failures are properly recorded
  • Simplify SlackExplorerEntrypoint: remove message_ts parameter (caller now resolves thread_ts), make thread_ts required, raise EntrypointSetupError instead of ValueError, and store self.integration for use by identity-linking helpers
  • Add Block Kit text extraction utilities (_extract_block_text, _extract_text_from_blocks, _extract_rich_text_element_text) for building thread context from rich_text, section, context, header, and markdown blocks — with defensive isinstance checks for external Slack API data
  • Update build_thread_context to prefer block-based text extraction over plain text fallback, preserving URLs and mentions from rich text blocks
  • Fix SlackActionRequest.get_action_list to use .get("value", "") instead of ["value"] to avoid KeyError on actions without a value field
  • Rename MISSING_CHANNEL_OR_TEXT to MISSING_EVENT_DATA in AppMentionHaltReason to reflect broader validation
  • Add SlackEntrypointInteractionType.PROCESS_MENTION and ProcessMentionHaltReason/ProcessMentionFailureReason enums for structured observability
  • Change SeerExplorerOperator.execute to pass summary=None instead of a hardcoded fallback string when no assistant content is found in Explorer results
  • Add loading_messages parameter to SlackIntegration.set_thread_status for rotating status messages
  • Add comprehensive tests for the task (test_tasks.py) covering happy path, org-not-found, no-access, integration-not-found, identity-not-linked, user-not-org-member, and thread-context scenarios; add Block Kit extraction tests; update webhook and entrypoint tests accordingly

Refs ISWF-2023

@linear
Copy link
Copy Markdown

linear bot commented Mar 2, 2026

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 2, 2026
@alexsohn1126 alexsohn1126 force-pushed the alexsohn/iswf-2022-add-trigger-explorer-method-to-seeroperator branch from 881c751 to c96afad Compare March 3, 2026 18:02
@alexsohn1126 alexsohn1126 force-pushed the alexsohn/iswf-2023-add-taskbroker-task-for-processing-slack-explorer-mentions branch from 9727f67 to 5896d0f Compare March 3, 2026 18:02
@alexsohn1126 alexsohn1126 force-pushed the alexsohn/iswf-2022-add-trigger-explorer-method-to-seeroperator branch from c96afad to 740d420 Compare March 3, 2026 21:29
@alexsohn1126 alexsohn1126 force-pushed the alexsohn/iswf-2023-add-taskbroker-task-for-processing-slack-explorer-mentions branch from 5896d0f to c97eaa9 Compare March 3, 2026 21:29
try:
organization = Organization.objects.get_from_cache(id=organization_id)
except Organization.DoesNotExist:
lifecycle.record_halt(halt_reason="org_not_found")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We typically try to group these in an enum to contain all of the possible halt reasons. It's probably a good idea to do that here as it makes test assertions easier as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done:

class ProcessMentionHaltReason(StrEnum):
ORG_NOT_FOUND = "org_not_found"
NO_EXPLORER_ACCESS = "no_explorer_access"
INTEGRATION_NOT_FOUND = "integration_not_found"

slack_user_id=slack_user_id,
)
except ValueError:
lifecycle.record_halt(halt_reason="integration_not_found")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we be raising a more explicit exception from the Entrypoint code? ValueError on the surface doesn't necessarily imply to me that the integration is missing, so this grouping seems funny.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

if not integration:
raise EntrypointSetupError(f"Slack integration {integration_id} not found")

Comment on lines +85 to +89
entrypoint.install.add_reaction(
channel_id=channel_id,
message_ts=message_ts,
emoji="thinking_face",
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this is being processed in region (which it appears to be), then this won't be immediate. If you want to keep latency on this reaction to a minimum, you likely need to move this reaction logic to Control to occur before the processing here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in e72be98

operator = SeerOperator(entrypoint=entrypoint)
operator.trigger_explorer(
organization=organization,
user=None,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is user here a sentry user? If so, do we have plans to fill this in later?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hm, good point, it seems like Seer explorer may use user's ID for context building. It also seems like we are working towards making the user info required.

class SeerViewerContext(TypedDict, total=False):
organization_id: int
# TODO(jeremy.stanley): user_id is int | None as a temporary state while
# consolidating viewer context across call sites. Some pass request.user.id
# (which can be None for anonymous users), others omit the key entirely.
# Once all call sites are wired up, tighten this to int and ensure callers
# only set user_id when an authenticated user is present.
user_id: int | None

Fixed in 48161fa48161fa

self._run_task(organization_id=999999999)

mock_has_access.assert_not_called()
mock_from_mention.assert_not_called()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You may want to assert on halts here to ensure the code you're testing is actually being invoked. It also ensures we have the appropriate metrics logged when this occurs. See our testing helpers here:

def assert_halt_metric(mock_record, error_msg):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in 48161fa

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Backend Test Failures

Failures on 0b25cab in this run:

tests/sentry/seer/entrypoints/slack/test_tasks.py::ProcessMentionForSlackTest::test_happy_pathlog
tests/sentry/seer/entrypoints/slack/test_tasks.py:40: in test_happy_path
    mock_entrypoint.install.add_reaction.assert_called_once_with(
/opt/hostedtoolcache/Python/3.13.1/x64/lib/python3.13/unittest/mock.py:988: in assert_called_once_with
    raise AssertionError(msg)
E   AssertionError: Expected 'add_reaction' to be called once. Called 0 times.
tests/sentry/notifications/platform/slack/renderers/test_seer.py::SeerSlackRendererExplorerTest::test_render_explorer_response_with_summarylog
tests/sentry/notifications/platform/slack/renderers/test_seer.py:206: in test_render_explorer_response_with_summary
    assert len(blocks) == 2
E   AssertionError: assert 1 == 2
E    +  where 1 = len([<slack_sdk.SectionBlock: {'text': {'text': 'Found a spike in 500 errors from the auth service.\n\n<https://sentry.sentry.io/explore/seer/123/|View in Sentry>', 'type': 'mrkdwn'}, 'type': 'section'}>])
tests/sentry/notifications/platform/slack/renderers/test_seer.py::SeerSlackRendererExplorerTest::test_render_explorer_response_without_summarylog
tests/sentry/notifications/platform/slack/renderers/test_seer.py:223: in test_render_explorer_response_without_summary
    assert len(blocks) == 2
E   assert 1 == 2
E    +  where 1 = len([<slack_sdk.SectionBlock: {'text': {'text': "I've finished analyzing your question.\n\n<https://sentry.sentry.io/explore/seer/123/|View in Sentry>", 'type': 'mrkdwn'}, 'type': 'section'}>])

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Backend Test Failures

Failures on 50c0bf3 in this run:

tests/sentry/seer/entrypoints/slack/test_tasks.py::ProcessMentionForSlackTest::test_happy_pathlog
tests/sentry/seer/entrypoints/slack/test_tasks.py:40: in test_happy_path
    mock_entrypoint.install.add_reaction.assert_called_once_with(
/opt/hostedtoolcache/Python/3.13.1/x64/lib/python3.13/unittest/mock.py:988: in assert_called_once_with
    raise AssertionError(msg)
E   AssertionError: Expected 'add_reaction' to be called once. Called 0 times.
tests/sentry/notifications/platform/slack/renderers/test_seer.py::SeerSlackRendererExplorerTest::test_render_explorer_response_with_summarylog
tests/sentry/notifications/platform/slack/renderers/test_seer.py:206: in test_render_explorer_response_with_summary
    assert len(blocks) == 2
E   AssertionError: assert 1 == 2
E    +  where 1 = len([<slack_sdk.SectionBlock: {'text': {'text': 'Found a spike in 500 errors from the auth service.\n\n<https://sentry.sentry.io/explore/seer/123/|View in Sentry>', 'type': 'mrkdwn'}, 'type': 'section'}>])
tests/sentry/notifications/platform/slack/renderers/test_seer.py::SeerSlackRendererExplorerTest::test_render_explorer_response_without_summarylog
tests/sentry/notifications/platform/slack/renderers/test_seer.py:223: in test_render_explorer_response_without_summary
    assert len(blocks) == 2
E   assert 1 == 2
E    +  where 1 = len([<slack_sdk.SectionBlock: {'text': {'text': "I've finished analyzing your question.\n\n<https://sentry.sentry.io/explore/seer/123/|View in Sentry>", 'type': 'mrkdwn'}, 'type': 'section'}>])

Comment thread src/sentry/seer/entrypoints/slack/tasks.py
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 10, 2026

Backend Test Failures

Failures on 0ed202a in this run:

tests/sentry/notifications/platform/slack/renderers/test_seer.py::SeerSlackRendererExplorerTest::test_render_explorer_response_with_summarylog
tests/sentry/notifications/platform/slack/renderers/test_seer.py:206: in test_render_explorer_response_with_summary
    assert len(blocks) == 2
E   AssertionError: assert 1 == 2
E    +  where 1 = len([<slack_sdk.SectionBlock: {'text': {'text': 'Found a spike in 500 errors from the auth service.\n\n<https://sentry.sentry.io/explore/seer/123/|View in Sentry>', 'type': 'mrkdwn'}, 'type': 'section'}>])
tests/sentry/profiles/test_task.py::DeobfuscationViaSymbolicator::test_basic_resolvinglog
tests/sentry/profiles/test_task.py:627: in test_basic_resolving
    assert android_profile["profile"]["methods"] == [
E   AssertionError: assert [{'class_name...oolean', ...}] == [{'class_name...oolean', ...}]
E     
E     At index 0 diff: {'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager', 'name': 'getClassContext', 'signature': '()', 'source_file': 'Util.java', 'source_line': 67, 'data': {'deobfuscation_status': 'deobfuscated'}} != {'data': {'deobfuscation_status': 'deobfuscated'}, 'name': 'getClassContext', 'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager', 'signature': '()', 'source_file': 'Something.java', 'source_line': 67}
E     
E     Full diff:
E       [
E           {
E               'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'name': 'getClassContext',
E               'signature': '()',
E     -         'source_file': 'Something.java',
E     ?                         ^^^^ - ^^
E     +         'source_file': 'Util.java',
E     ?                         ^  ^
E               'source_line': 67,
E           },
E           {
E               'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'name': 'getExtraClassContext',
E               'signature': '(): boolean',
E     -         'source_file': 'Else.java',
E     ?                         ^ --
E     +         'source_file': 'Util.java',
E     ?                         ^^^
E               'source_line': 69,
E           },
E       ]
tests/sentry/seer/entrypoints/slack/test_tasks.py::ProcessMentionForSlackTest::test_happy_pathlog
tests/sentry/seer/entrypoints/slack/test_tasks.py:40: in test_happy_path
    mock_entrypoint.install.add_reaction.assert_called_once_with(
/opt/hostedtoolcache/Python/3.13.1/x64/lib/python3.13/unittest/mock.py:988: in assert_called_once_with
    raise AssertionError(msg)
E   AssertionError: Expected 'add_reaction' to be called once. Called 0 times.
tests/sentry/profiles/test_task.py::DeobfuscationViaSymbolicator::test_inline_resolvinglog
tests/sentry/profiles/test_task.py:683: in test_inline_resolving
    assert android_profile["profile"]["methods"] == [
E   AssertionError: assert [{'class_name...andler', ...}] == [{'class_name...andler', ...}]
E     
E     At index 0 diff: {'class_name': 'io.sentry.sample.-$$Lambda$r3Avcbztes2hicEObh02jjhQqd4', 'name': 'onClick', 'signature': '()', 'source_file': '-.java', 'source_line': 2, 'data': {'deobfuscation_status': 'deobfuscated'}} != {'class_name': 'io.sentry.sample.-$$Lambda$r3Avcbztes2hicEObh02jjhQqd4', 'data': {'deobfuscation_status': 'deobfuscated'}, 'name': 'onClick', 'signature': '()', 'source_file': None, 'source_line': 2}
E     
E     Full diff:
E       [
E           {
E               'class_name': 'io.sentry.sample.-$$Lambda$r3Avcbztes2hicEObh02jjhQqd4',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'name': 'onClick',
E               'signature': '()',
E     -         'source_file': None,
E     ?                        ^^^^
E     +         'source_file': '-.java',
E     ?                        ^^^^^^^^
E               'source_line': 2,
E           },
E           {
E               'class_name': 'io.sentry.sample.MainActivity',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'inline_frames': [
E                   {
E                       'class_name': 'io.sentry.sample.MainActivity',
E                       'data': {
E                           'deobfuscation_status': 'deobfuscated',
E                       },
E                       'name': 'onClickHandler',
E                       'signature': '()',
E                       'source_file': 'MainActivity.java',
E                       'source_line': 40,
E                   },
E                   {
E                       'class_name': 'io.sentry.sample.MainActivity',
E                       'data': {
E                           'deobfuscation_status': 'deobfuscated',
E                       },
E                       'name': 'foo',
E                       'signature': '()',
E                       'source_file': 'MainActivity.java',
E                       'source_line': 44,
E                   },
E                   {
E                       'class_name': 'io.sentry.sample.MainActivity',
E                       'data': {
... (14 more lines)
tests/sentry/notifications/platform/slack/renderers/test_seer.py::SeerSlackRendererExplorerTest::test_render_explorer_response_without_summarylog
tests/sentry/notifications/platform/slack/renderers/test_seer.py:223: in test_render_explorer_response_without_summary
    assert len(blocks) == 2
E   assert 1 == 2
E    +  where 1 = len([<slack_sdk.SectionBlock: {'text': {'text': "I've finished analyzing your question.\n\n<https://sentry.sentry.io/explore/seer/123/|View in Sentry>", 'type': 'mrkdwn'}, 'type': 'section'}>])

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 10, 2026

Backend Test Failures

Failures on 9054348 in this run:

tests/sentry/seer/entrypoints/slack/test_tasks.py::ProcessMentionForSlackTest::test_happy_pathlog
tests/sentry/seer/entrypoints/slack/test_tasks.py:40: in test_happy_path
    mock_entrypoint.install.add_reaction.assert_called_once_with(
/opt/hostedtoolcache/Python/3.13.1/x64/lib/python3.13/unittest/mock.py:988: in assert_called_once_with
    raise AssertionError(msg)
E   AssertionError: Expected 'add_reaction' to be called once. Called 0 times.
tests/sentry/notifications/platform/slack/renderers/test_seer.py::SeerSlackRendererExplorerTest::test_render_explorer_response_without_summarylog
tests/sentry/notifications/platform/slack/renderers/test_seer.py:223: in test_render_explorer_response_without_summary
    assert len(blocks) == 2
E   assert 1 == 2
E    +  where 1 = len([<slack_sdk.SectionBlock: {'text': {'text': "I've finished analyzing your question.\n\n<https://sentry.sentry.io/explore/seer/123/|View in Sentry>", 'type': 'mrkdwn'}, 'type': 'section'}>])
tests/sentry/notifications/platform/slack/renderers/test_seer.py::SeerSlackRendererExplorerTest::test_render_explorer_response_with_summarylog
tests/sentry/notifications/platform/slack/renderers/test_seer.py:206: in test_render_explorer_response_with_summary
    assert len(blocks) == 2
E   AssertionError: assert 1 == 2
E    +  where 1 = len([<slack_sdk.SectionBlock: {'text': {'text': 'Found a spike in 500 errors from the auth service.\n\n<https://sentry.sentry.io/explore/seer/123/|View in Sentry>', 'type': 'mrkdwn'}, 'type': 'section'}>])
tests/sentry/profiles/test_task.py::DeobfuscationViaSymbolicator::test_basic_resolvinglog
tests/sentry/profiles/test_task.py:627: in test_basic_resolving
    assert android_profile["profile"]["methods"] == [
E   AssertionError: assert [{'class_name...oolean', ...}] == [{'class_name...oolean', ...}]
E     
E     At index 0 diff: {'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager', 'name': 'getClassContext', 'signature': '()', 'source_file': 'Util.java', 'source_line': 67, 'data': {'deobfuscation_status': 'deobfuscated'}} != {'data': {'deobfuscation_status': 'deobfuscated'}, 'name': 'getClassContext', 'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager', 'signature': '()', 'source_file': 'Something.java', 'source_line': 67}
E     
E     Full diff:
E       [
E           {
E               'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'name': 'getClassContext',
E               'signature': '()',
E     -         'source_file': 'Something.java',
E     ?                         ^^^^ - ^^
E     +         'source_file': 'Util.java',
E     ?                         ^  ^
E               'source_line': 67,
E           },
E           {
E               'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'name': 'getExtraClassContext',
E               'signature': '(): boolean',
E     -         'source_file': 'Else.java',
E     ?                         ^ --
E     +         'source_file': 'Util.java',
E     ?                         ^^^
E               'source_line': 69,
E           },
E       ]
tests/sentry/profiles/test_task.py::DeobfuscationViaSymbolicator::test_inline_resolvinglog
tests/sentry/profiles/test_task.py:683: in test_inline_resolving
    assert android_profile["profile"]["methods"] == [
E   AssertionError: assert [{'class_name...andler', ...}] == [{'class_name...andler', ...}]
E     
E     At index 0 diff: {'class_name': 'io.sentry.sample.-$$Lambda$r3Avcbztes2hicEObh02jjhQqd4', 'name': 'onClick', 'signature': '()', 'source_file': '-.java', 'source_line': 2, 'data': {'deobfuscation_status': 'deobfuscated'}} != {'class_name': 'io.sentry.sample.-$$Lambda$r3Avcbztes2hicEObh02jjhQqd4', 'data': {'deobfuscation_status': 'deobfuscated'}, 'name': 'onClick', 'signature': '()', 'source_file': None, 'source_line': 2}
E     
E     Full diff:
E       [
E           {
E               'class_name': 'io.sentry.sample.-$$Lambda$r3Avcbztes2hicEObh02jjhQqd4',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'name': 'onClick',
E               'signature': '()',
E     -         'source_file': None,
E     ?                        ^^^^
E     +         'source_file': '-.java',
E     ?                        ^^^^^^^^
E               'source_line': 2,
E           },
E           {
E               'class_name': 'io.sentry.sample.MainActivity',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'inline_frames': [
E                   {
E                       'class_name': 'io.sentry.sample.MainActivity',
E                       'data': {
E                           'deobfuscation_status': 'deobfuscated',
E                       },
E                       'name': 'onClickHandler',
E                       'signature': '()',
E                       'source_file': 'MainActivity.java',
E                       'source_line': 40,
E                   },
E                   {
E                       'class_name': 'io.sentry.sample.MainActivity',
E                       'data': {
E                           'deobfuscation_status': 'deobfuscated',
E                       },
E                       'name': 'foo',
E                       'signature': '()',
E                       'source_file': 'MainActivity.java',
E                       'source_line': 44,
E                   },
E                   {
E                       'class_name': 'io.sentry.sample.MainActivity',
E                       'data': {
... (14 more lines)

@github-actions
Copy link
Copy Markdown
Contributor

Backend Test Failures

Failures on 0530fb7 in this run:

tests/sentry/seer/entrypoints/slack/test_tasks.py::ProcessMentionForSlackTest::test_happy_pathlog
tests/sentry/seer/entrypoints/slack/test_tasks.py:40: in test_happy_path
    mock_entrypoint.install.add_reaction.assert_called_once_with(
/opt/hostedtoolcache/Python/3.13.1/x64/lib/python3.13/unittest/mock.py:988: in assert_called_once_with
    raise AssertionError(msg)
E   AssertionError: Expected 'add_reaction' to be called once. Called 0 times.

@alexsohn1126 alexsohn1126 force-pushed the alexsohn/iswf-2023-add-taskbroker-task-for-processing-slack-explorer-mentions branch from 0236280 to 9840cc4 Compare March 11, 2026 15:12
@alexsohn1126 alexsohn1126 force-pushed the alexsohn/iswf-2022-add-trigger-explorer-method-to-seeroperator branch from 5138896 to 5cc7997 Compare March 11, 2026 15:12
The custom message parameter is no longer needed since the link
identity prompt now uses SlackRenderable directly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

Backend Test Failures

Failures on cb2d12a in this run:

tests/sentry/seer/entrypoints/slack/test_tasks.py::ProcessMentionForSlackTest::test_identity_not_linkedlog
tests/sentry/seer/entrypoints/slack/test_tasks.py:111: in test_identity_not_linked
    mock_send_link.assert_called_once_with(entrypoint=mock_entrypoint)
/opt/hostedtoolcache/Python/3.13.1/x64/lib/python3.13/unittest/mock.py:989: in assert_called_once_with
    return self.assert_called_with(*args, **kwargs)
/opt/hostedtoolcache/Python/3.13.1/x64/lib/python3.13/unittest/mock.py:977: in assert_called_with
    raise AssertionError(_error_message()) from cause
E   AssertionError: expected call not found.
E   Expected: _send_link_identity_prompt(entrypoint=<MagicMock name='SlackExplorerEntrypoint()' id='139971776247328'>)
E     Actual: _send_link_identity_prompt(entrypoint=<MagicMock name='SlackExplorerEntrypoint()' id='139971776247328'>, thread_ts='1234567890.123456')

Move the ephemeral link-identity prompt to fire before clearing the
thread status indicator. Remove the thread_ts parameter from
_send_link_identity_prompt so the prompt is sent as a top-level
ephemeral message rather than threaded.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread src/sentry/seer/entrypoints/slack/tasks.py Outdated
Copy link
Copy Markdown
Member

@leeandher leeandher left a comment

Choose a reason for hiding this comment

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

minor comments about lifecycles and comments, looks good!

Comment thread src/sentry/integrations/messaging/metrics.py
Comment thread src/sentry/integrations/slack/webhooks/event.py
slack_user_id=slack_user_id,
)
if not user:
lifecycle.record_failure(failure_reason=ProcessMentionFailureReason.IDENTITY_NOT_LINKED)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could probably be a halt, since it's expected behavior. We don't want to trigger sentry errors if users haven't linked their accounts yet for example.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks! Changed:

lifecycle.record_halt(ProcessMentionHaltReason.IDENTITY_NOT_LINKED)

Comment thread src/sentry/seer/entrypoints/slack/tasks.py Outdated
… context

Instead of relying solely on the plain `text` field, thread context
parsing now prefers Block Kit `blocks` and extracts links with URLs,
user mentions, channel refs, emojis, and broadcasts. Falls back to
`text` when blocks are absent or empty.
…mention context

Pass ts and thread_ts separately from the webhook handler so the task
knows whether the mention was top-level or in a thread. The ephemeral
identity-linking prompt now appears in the thread when mentioned in a
thread, or at the channel level for top-level mentions.
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Unlinked identity uses record_failure creating noisy Sentry issues
    • Changed record_failure to record_halt for unlinked identity case, which defaults to create_issue=False and prevents noisy Sentry alerts for expected behavior.

Create PR

Or push these changes by commenting:

@cursor push 1f17feeb59
Preview (1f17feeb59)
diff --git a/src/sentry/seer/entrypoints/slack/tasks.py b/src/sentry/seer/entrypoints/slack/tasks.py
--- a/src/sentry/seer/entrypoints/slack/tasks.py
+++ b/src/sentry/seer/entrypoints/slack/tasks.py
@@ -99,7 +99,7 @@
             slack_user_id=slack_user_id,
         )
         if not user:
-            lifecycle.record_failure(failure_reason=ProcessMentionFailureReason.IDENTITY_NOT_LINKED)
+            lifecycle.record_halt(halt_reason=ProcessMentionFailureReason.IDENTITY_NOT_LINKED)
             # In a thread, show the prompt in the thread; top-level, show in the channel.
             _send_link_identity_prompt(
                 entrypoint=entrypoint,

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Comment thread src/sentry/seer/entrypoints/slack/tasks.py Outdated
Comment thread tests/sentry/seer/entrypoints/slack/test_tasks.py Outdated
alexsohn1126 and others added 3 commits March 30, 2026 13:14
The test was incorrectly using assert_failure_metric with
ProcessMentionFailureReason.IDENTITY_NOT_LINKED, but the production
code calls lifecycle.record_halt() with ProcessMentionHaltReason.
…n-cleanup

- Use SlackIntegration for org lookup in app_mention handler
- Add Sentry Voice loading messages for thinking state
- Set completion summary to None instead of error text on fetch failure
- Pass loading_messages through set_thread_status
Comment thread src/sentry/integrations/slack/webhooks/event.py
Comment thread src/sentry/seer/entrypoints/slack/tasks.py
Comment thread tests/sentry/seer/entrypoints/slack/test_slack.py
@github-actions
Copy link
Copy Markdown
Contributor

Backend Test Failures

Failures on 3d78b63 in this run:

tests/sentry/seer/entrypoints/slack/test_slack.py::SlackExplorerEntrypointTest::test_init_defaults_thread_ts_to_message_ts_when_nonelog
[gw0] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/seer/entrypoints/slack/test_slack.py:523: in test_init_defaults_thread_ts_to_message_ts_when_none
    message_ts=self.message_ts,
E   AttributeError: 'SlackExplorerEntrypointTest' object has no attribute 'message_ts'

alexsohn1126 and others added 2 commits March 30, 2026 17:52
Remove test_init_defaults_thread_ts_to_message_ts_when_none since
SlackExplorerEntrypoint does not use message_ts — the thread_ts
fallback is handled at the call site in tasks.py instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The process_mention_for_slack task resolved a Sentry user from a Slack
identity but never verified the user is a member of the target
organization. This allowed a user who linked their Sentry identity to
a Slack workspace to continue using Seer Explorer for an organization
after losing membership. Add an organization.has_access(user) check
after user resolution, consistent with other Slack action handlers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment thread src/sentry/seer/entrypoints/slack/tasks.py
alexsohn1126 and others added 2 commits March 31, 2026 10:41
The USER_NOT_ORG_MEMBER early return was missing the
set_thread_status(status="") call to clear the "Thinking..." indicator,
leaving it stuck for up to 2 minutes. Match the IDENTITY_NOT_LINKED
branch which already clears it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show an ephemeral message including the org name so the user knows
which Sentry organization they need to join.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread src/sentry/seer/entrypoints/slack/mention.py
Add isinstance check before calling .get() on rich_text container
elements parsed from Slack API data. This prevents AttributeError
if the API returns malformed data with non-dict values, consistent
with the defensive pattern already used in the context block handler.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines +81 to +83
_extract_rich_text_element_text(el) for el in container.get("elements", [])
]
parts.append("".join(container_parts))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The _extract_block_text function lacks an isinstance check for inner rich_text elements, risking an AttributeError if the Slack API returns malformed data.
Severity: MEDIUM

Suggested Fix

Add an isinstance(el, dict) check inside the list comprehension that processes the inner elements of a rich_text container. This will filter out any non-dictionary elements, preventing the AttributeError and aligning the code with the defensive pattern used elsewhere in the file.

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

Location: src/sentry/seer/entrypoints/slack/mention.py#L81-L83

Potential issue: In the `_extract_block_text` function, when processing a `rich_text`
block, the code iterates through nested elements. While it checks if the outer
`container` is a dictionary, it fails to perform the same check for the inner elements
(`el`) before passing them to `_extract_rich_text_element_text`. This helper function
immediately calls `.get()` on its argument. If the Slack API returns a malformed payload
where an inner element is not a dictionary (e.g., `None` or a string), the call will
raise an `AttributeError`. This would crash the task and prevent Explorer runs from
being triggered.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this assumes that slack's api returns a malformed response. blockkits are pretty well-defined and i don't think we should assume it'll fail

if not identity:
return None

return user_service.get_user(identity.user_id)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Future thought: Depending on the scale of requests we get, we may want to combine this with the identity RPC call to reduce latency here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants