feat(seer-slack): Check channel type for correct history scope before API call#112371
Merged
alexsohn1126 merged 38 commits intomasterfrom Apr 14, 2026
Merged
Conversation
github-actions
bot
added
the
Scope: Backend
Contributor
Backend Test FailuresFailures on
|
alexsohn1126
commented
Apr 7, 2026
Comment on lines
+227
to
+228
| if all(s in installed_scope_set for s in history_scopes): | ||
| return True |
Member
Author
There was a problem hiding this comment.
If we have both channels:history and groups:history, we are covered for both cases whether the mention is in a private or a public channel.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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: Missing early return causes unnecessary API call
- Added early return when neither history scope is present to avoid unnecessary Slack API call before checking channel type.
Or push these changes by commenting:
@cursor push 0cbe4af674
Preview (0cbe4af674)
diff --git a/src/sentry/integrations/slack/integration.py b/src/sentry/integrations/slack/integration.py
--- a/src/sentry/integrations/slack/integration.py
+++ b/src/sentry/integrations/slack/integration.py
@@ -227,6 +227,9 @@
if all(s in installed_scope_set for s in history_scopes):
return True
+ if not any(s in installed_scope_set for s in history_scopes):
+ return False
+
conversation_data = self.get_conversations_info(channel_id=channel_id)
channel_info = conversation_data.get("channel", {})This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
Add the assistant:write scope to the Slack integration to enable the bot to act as a Slack Agent, supporting DM-based agent interfaces. Refs ISWF-2388 Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
Extract shared org-resolution logic into _resolve_seer_organization helper and merge on_app_mention/on_dm into a single _handle_seer_mention method. Replace three identical halt reason enums with unified SeerSlackHaltReason. Extract duplicated loading messages list into a module-level constant. Refs ISWF-2388 Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
…nges Update tests to match the refactored _resolve_seer_organization which now iterates org integrations and uses SlackExplorerEntrypoint.has_access instead of checking a single feature flag. Align halt reasons with the consolidated enum values (NO_VALID_INTEGRATION, NO_VALID_ORGANIZATION) and update the has_access test for the seer-slack-explorer flag rename. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…scope footer Check the correct OAuth scope (channels:history vs groups:history) based on channel type before calling conversations.replies, and append a reinstall footer to Seer Explorer responses when the integration lacks the required history scope. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ions_info Remove debug _logger.info/warning calls from has_history_scope that were left from local testing. Update all tests that go through has_history_scope to mock the conversations_info API call, and remove DM tests with incorrect expectations.
…onse Use cache.add() to atomically track whether the missing-scope footer has already been shown in a given thread. Subsequent Seer Explorer responses in the same thread no longer repeat the warning.
alexsohn1126
force-pushed
the
alexsohn/iswf-2353-ensure-history-scopes-before-api-call
branch
from
6f6d707 to
78f49b6
Compare
alexsohn1126
changed the base branch from
master
to
leanderrodrigues/iswf-2388-explore-slack-dms-agent-interface-for-seer-explorer
…helper If Organization.objects.get raised DoesNotExist, the cache key was already consumed by cache.add, preventing the footer on any future call for the thread. More critically, the unhandled exception propagated into on_explorer_update and prevented schedule_all_thread_updates from executing, blocking delivery of the Seer Explorer response entirely. Move the org lookup before cache.add so the key is never consumed on failure, and wrap it in try/except to return None gracefully — consistent with the integration lookup above. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
leeandher
reviewed
Apr 13, 2026
Member
leeandher
left a comment
leeandher
left a comment
There was a problem hiding this comment.
Looks good! Some small suggestions but logically sound 👏
Comment on lines
+272
to
+273
| f"_Thread context is unavailable because optional scopes are disabled. " | ||
| f"<{settings_url}|Reinstall Sentry's Slack app> to enable this feature._" |
Member
There was a problem hiding this comment.
I think it should be a bit more personal and less technical IMO, what do you think of:
I'm working off mentions only, I can't read the whole thread. <{settings_url}|Reinstall me> to change that.
Member
Author
There was a problem hiding this comment.
True, thanks!
| """ | ||
| Returns whether this integration is allowed to access the history in the channel. | ||
| """ | ||
| history_scopes = [SlackScope.CHANNELS_HISTORY, SlackScope.GROUPS_HISTORY] |
Member
There was a problem hiding this comment.
Yeah the bots got a point, I think you need im:history here, and it will make that not any(... check redundant since all installs have im:history
Member
There was a problem hiding this comment.
Oh it's handled below, then maybe just slot this all under the is_im check 👍
Member
Author
There was a problem hiding this comment.
The bots were right, even if someone doesn't have neither SlackScope.CHANNELS_HISTORY and SlackScope.GROUPS_HISTORY, they should still be able to DM the bot.
Removed the early exit if neither of those history scopes are available
Comment on lines
+268
to
+296
| history_scopes = [SlackScope.CHANNELS_HISTORY, SlackScope.GROUPS_HISTORY] | ||
| installed_scope_set = frozenset(self.model.metadata.get("scopes", [])) | ||
|
|
||
| installed_history_scopes = [s in installed_scope_set for s in history_scopes] | ||
|
|
||
| if all(installed_history_scopes): | ||
| return True | ||
|
|
||
| if not any(installed_history_scopes): | ||
| return False | ||
|
|
||
| conversation_data = self.get_conversations_info(channel_id=channel_id) | ||
|
|
||
| channel_info = conversation_data.get("channel", {}) | ||
| is_channel = channel_info.get("is_channel", False) | ||
| is_private = channel_info.get("is_private", False) | ||
| is_im = channel_info.get("is_im", False) | ||
|
|
||
| # DMs and assistant threads: the bot is a participant and can | ||
| # always read its own conversation history. | ||
| if is_im: | ||
| return True | ||
|
|
||
| if is_channel and is_private: | ||
| return SlackScope.GROUPS_HISTORY in installed_scope_set | ||
| if is_channel: | ||
| return SlackScope.CHANNELS_HISTORY in installed_scope_set | ||
|
|
||
| # Shouldn't reach here unless channel_info is empty (most likely |
Member
There was a problem hiding this comment.
I think it's worthwhile to add a logger.warning or something similar just in case! we might catch users trying multi person DMs, and missing the mpim:history scope for example
Member
Author
There was a problem hiding this comment.
Added a logger warning:
|
|
||
| def has_history_scope(self, channel_id: str) -> bool: | ||
| """ | ||
| Returns whether this integration is allowed to access the history in the channel. |
Member
There was a problem hiding this comment.
Worth adding the detail here that the check is different depending on the type of channel. And that type of channel will be determined by an API call in the function.
Member
Author
There was a problem hiding this comment.
Makes sense, added more detail in:
| if not any(installed_history_scopes): | ||
| return False | ||
|
|
||
| conversation_data = self.get_conversations_info(channel_id=channel_id) |
Member
There was a problem hiding this comment.
I'd try/catch this in case this ever gets used somewhere else where users control what channel_id we provide. If that's the case it's possible the channel doesn't exist 🤷
Member
Author
There was a problem hiding this comment.
It already is try/catched in get_conversations_info method:
sentry/src/sentry/integrations/slack/integration.py
Lines 300 to 323 in f9d824d
leeandher
approved these changes
Apr 13, 2026
Member
leeandher
left a comment
leeandher
left a comment
There was a problem hiding this comment.
meant to approve, small fixes shouldn't need re-review
…y for old slack groups
…tory_scope Log a warning when has_history_scope falls through to the default return without matching any known channel type, making it easier to diagnose unexpected conversation types from the Slack API.
Clarify which scope applies to which channel type and that conversations.info is used to determine the channel type.
Contributor
Backend Test FailuresFailures on
|
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 48ab446. Configure here.
Update assertion to match new missing-scope footer copy and add missing has_history_scope mock to test_on_explorer_update so it does not hit the Slack API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
A transient Slack conversations.info failure returned an empty dict, causing has_history_scope to incorrectly report missing scopes and show a misleading "reinstall" footer. Return True on the fall-through path so API errors don't trigger false prompts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit aedb432.
alexsohn1126
deleted the
alexsohn/iswf-2353-ensure-history-scopes-before-api-call
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Right now, we require every Sentry Slack app integration to require every scope that we request. However, with Seer Explorer requiring
channels:historyandgroups:history(public and private channels history read scopes), some organization may not want to allow us to have access to those kinds of data.Therefore we must check whether the bot mention happened in a public or a private channel, and whether we have the appropriate scopes to read the history from those channels.
Details
Use Slack's
conversations.infoAPI to determine channel type (public vs private) and check the appropriate OAuth scope (channels:historyvsgroups:history) before callingconversations.replies. Previously we only checkedchannels:historyregardless of channel type, which meant private channel threads were always blocked.Also adds a missing-scope footer to the first Seer Explorer response in a thread when the integration lacks the required history scope. Subsequent responses in the same thread do not repeat the warning.
If an installation does not have the correct permissions and is not able to fetch the whole thread context and pass it onto Seer, this footer will appear on the first response:
If the installation has the correct scopes and is able to grab the thread context, it won't show the footer:
Refs ISWF-2353