fix(slack): stop requesting the unapproved scopes, gate them behind an opt-in flag - #5977
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR restores Slack OAuth connectivity while preserving an opt-in path for scopes awaiting approval.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/oauth/oauth.ts | Gates the three unapproved Slack scopes behind the new opt-in flag while leaving the approved default grant intact. |
| apps/sim/lib/core/config/env-flags.ts | Adds the server/client-aware Slack extended-scopes feature flag using the repository’s established environment-twin pattern. |
| apps/sim/lib/core/config/env.ts | Declares and exposes the server and public client variables required by the new Slack flag. |
| apps/sim/blocks/blocks/slack.ts | Removes the obsolete v1/v2 advertising split so credential requirements follow the configured canonical Slack scopes. |
| packages/testing/src/mocks/env-flags.mock.ts | Adds the new Slack flag to the shared mutable environment-flags test mock with its intended disabled default. |
Reviews (3): Last reviewed commit: "feat(slack): gate the approval-pending s..." | Re-trigger Greptile
|
@greptile review |
|
@cursor review |
PR SummaryMedium Risk Overview The Slack provider scope list now appends those three scopes only when Slack block cleanup: removes the v1/v2 advertised-scope split ( Tradeoff with flag off: native Sim app trigger events that need the extended scopes (mentions, assistant threads, DMs) won’t work until review passes or self-hosted operators enable the flag. Reviewed by Cursor Bugbot for commit c06da43. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c06da43. Configure here.
Summary
Unapproved permissions requested: assistant:write, app_mentions:read, im:history. Slack rejects the entire authorization when it requests a scope the app isn't approved for, so every native Slack connect is broken on prod.git diff 84e7aad633^against both files is empty. Same regression fix(slack): stop requesting unapproved OAuth scopes that break connect #5631 fixed in July.SLACK_V2_ONLY_SCOPES/SLACK_V1_ADVERTISED_SCOPESadvertising split improvement(slack): request the approved mention/assistant/DM scopes, advertised on v2 only #5898 added. With the three scopes out of the base list that filter removes nothing, and havingslack_v2advertise them would flag every Slack credential as missing scopes with no reconnect that clears it.SLACK_EXTENDED_SCOPES(+NEXT_PUBLIC_SLACK_EXTENDED_SCOPESclient twin) so the three scopes can be turned back on without another code change. Defaults to false —isTruthy(undefined)is false, so unset means the 17 approved scopes. Flip it the day Slack approves, or on a self-hosted deployment pointing at its own (unlisted) Slack app that already carries them.Gated via
env-flags.tsrather than AppConfig becauseOAUTH_PROVIDERSis imported by client components forrequiredScopes— an async server-only lookup can't reach it. Server var decides the grant; the client twin only decides what the credential pickers advertise. Set both together.Cost until the flag goes on is the same as pre-#5898:
app_mention, assistant-thread, and DM events don't deliver on the native Sim app trigger. Custom bot is unaffected and still covers those.Type of Change
Testing
Tested manually. Verified the flag toggles the grant correctly — off yields 17 scopes with none of the gated three, on yields 20 with exactly
assistant:write,app_mentions:read,im:history.bun run type-checkclean (19/19),bun run lintclean, and the full audit suite passes (check:boundaries,check:api-validation:strict,check:utils,check:zustand-v5,check:react-query,check:client-boundary,check:bare-icons,check:icon-paths,check:realtime-prune,skills:check,agent-stream-docs:check). 3,361 tests green acrosslib/oauth,lib/core,lib/auth,lib/credentials,blocks/,triggers/,tools/slack,lib/webhooks,lib/copilot,lib/permissions,lib/integrations— broad sweep because the new flag needed a field in the repo-wide sharedenv-flagstest mock.Checklist