Skip to content

feat(slack): Request extended OAuth scopes on production install#114091

Merged
alexsohn1126 merged 6 commits intomasterfrom
alexsohn1126/feat/slack-extended-oauth-scopes
Apr 28, 2026
Merged

feat(slack): Request extended OAuth scopes on production install#114091
alexsohn1126 merged 6 commits intomasterfrom
alexsohn1126/feat/slack-extended-oauth-scopes

Conversation

@alexsohn1126
Copy link
Copy Markdown
Member

@alexsohn1126 alexsohn1126 commented Apr 27, 2026

we have the approval from slack to publish the updated version of our slack app with upgraded permissions.

when we launch, we will need to request upgraded permissions with the production slack app. we now include the new permissions that we'll be approved for in the list of permissions we request.

we will keep the extended_oauth_scopes and the logic behind the staging app, so in the future, if we want to test out a new scope, we can use the staging app.

DO NOT MERGE UNTIL APP UPDATE HAS BEEN PUBLISHED

completes iswf-2551

The production SlackIntegrationProvider only requested identity_oauth_scopes,
omitting app_mentions:read, channels:history, groups:history, and
assistant:write. Without app_mentions:read Slack does not deliver app_mention
events to the bot, so the @mention-driven Seer Explorer entrypoint never
fires for production installs.

Union extended_oauth_scopes into the production scope set, matching what
SlackStagingIntegrationProvider already does, so newly installed and
reauthorized apps grant the scopes needed by Seer Explorer.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 27, 2026
Collapse the separate extended_oauth_scopes frozenset into the canonical
identity_oauth_scopes so the production install requests a single, flat
scope list. This replaces the prior union-based approach with one source
of truth and removes the implication that the additional scopes are
conditional.

extended_oauth_scopes is left as an empty frozenset because
SlackStagingIntegrationProvider still references it; its override now ORs
identity_oauth_scopes with an empty set, so behavior is unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Explain why the empty frozenset still exists: it is the place to stage
new scopes for testing via SlackStagingIntegrationProvider before
promoting them to identity_oauth_scopes. Without this comment, the empty
collection reads as dead code.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@linear-code
Copy link
Copy Markdown

linear-code Bot commented Apr 27, 2026

@alexsohn1126
Copy link
Copy Markdown
Member Author

========== DO NOT MERGE ===============

DO NOT MERGE UNTIL APP UPDATE HAS BEEN PUBLISHED

========== DO NOT MERGE ===============

@alexsohn1126 alexsohn1126 marked this pull request as ready for review April 27, 2026 22:31
@alexsohn1126 alexsohn1126 requested review from a team as code owners April 27, 2026 22:31
The four scopes promoted into identity_oauth_scopes already have
SlackScope enum constants defined; reference those instead of the raw
strings to keep a single source of truth and stay consistent with how
they were declared previously.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

good call!

Comment on lines +371 to +374
# Stage new scopes here to test them via SlackStagingIntegrationProvider
# (which unions these into its install scopes) before promoting to
# identity_oauth_scopes. Empty in steady state.
extended_oauth_scopes: frozenset[str] = frozenset()
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.

clever! I like this pattern, do we want to rename it something like staging_oauth_scopes to be explicit about its usage?

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.

Makes sense, changed in 8a5e022

SlackScope.CHANNELS_HISTORY,
"groups:read",
SlackScope.GROUPS_HISTORY,
"users:read",
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.

TBH I don't know how we guard against this fully, but we should probably make it explicit for future integration devs that these new scopes must always be checked before using them.

Since its possible that some organizations never reinstall, We don't want to break their flows.

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.

thinking about it more, it might actually be easier to enforce a pattern of checking even the existing assumed scopes rather than delineate between our re-publishing of the slack app 🤷 all of that is not within scope but a comment could be helpful in the interim

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.

that's a good idea, also coding agents may also pick up that pattern if we do it for existing scopes. let's add a comment for now as you mentioned: e047126

@alexsohn1126 alexsohn1126 merged commit d37a007 into master Apr 28, 2026
62 of 63 checks passed
@alexsohn1126 alexsohn1126 deleted the alexsohn1126/feat/slack-extended-oauth-scopes branch April 28, 2026 15:46
cleptric pushed a commit that referenced this pull request May 5, 2026
…4091)

we have the approval from slack to publish the updated version of our
slack app with upgraded permissions.

when we launch, we will need to request upgraded permissions with the
production slack app. we now include the new permissions that we'll be
approved for in the list of permissions we request.

we will keep the `extended_oauth_scopes` and the logic behind the
staging app, so in the future, if we want to test out a new scope, we
can use the staging app.

DO NOT MERGE UNTIL APP UPDATE HAS BEEN PUBLISHED

completes iswf-2551

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants