Repository navigation
Add self-serve Slack Connect channels for Pro/Max orgs - #809
devin-ai-integration[bot] wants to merge 5 commits into
Conversation
Co-Authored-By: Mohamed <mo@digger.dev>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
👀 4 findings need your review
Devin fixed 7 of 11 findings on f4a473d. Click a finding below to jump to its comment.
For your review (4)
- Shared channels omit support team members
- Failed cache writes orphan Slack channels
- Concurrent invites split one org across channels
- Channel names expose customer organization names
Fixed by Devin (7)
- Accepted members get a failing invite button
- Team changes skip existing shared channels
- Failed team invites strand Slack channels
- Long prefixes make channel collisions permanent
- Delivered invites appear to fail on storage errors
- Removed members can request private channel access
- Invitation requests lack a rate limit
View all findings in Devin Review
…ad of one shared channel Co-Authored-By: Mohamed <mo@digger.dev>
devin-ai-integration
Bot
changed the title
…es, mirror already-invited, tolerate KV write failure Co-Authored-By: Mohamed <mo@digger.dev>
…ship, refetch status after invite errors Co-Authored-By: Mohamed <mo@digger.dev>
| try { | ||
| await slack<InviteResponse>(env, "conversations.invite", { | ||
| channel: channel.id, | ||
| users: users.join(","), | ||
| }); |
There was a problem hiding this comment.
🔴 Shared channels omit support team members
When conversations.invite rejects a batch containing an existing member, inviteTeam treats the entire batch as successful. teamInvited then prevents retries, leaving the other support members outside the channel.
Learn more
A channel's support team is supplied as a comma-separated list of Slack user IDs. If Slack rejects the batch because a user is already in the channel, ignoring that error does not establish that the other users joined. ensureOrgChannel subsequently stores teamInvited: true, so later customer invites skip the team step entirely. The same success assumption also ignores the errors array in a successful conversations.invite response.
Example: With U1 already in channel C and configured users U1,U2, Slack rejects the batch as already_in_channel. C is marked complete although U2 never joined; the customer's Slack Connect invite still goes out.
Recommended fix: Invite missing users individually, or inspect channel membership and reconcile against every configured team ID before setting teamInvited: true. Handle partial errors in a successful invite response as incomplete membership.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Partially addressed in f4a473d: the channel record now stores the list of team IDs actually invited (team), and each call re-invites only the IDs missing from that list, so a failed batch is retried and team changes are backfilled. Remaining gap: if Slack returns already_in_channel for a mixed batch we still record the whole batch. Per Slack's docs the other users in the batch are still added in that case, and on a freshly created channel only the bot is a member, so this is mostly theoretical — left as-is rather than moving to per-user invites.
| channel = JSON.parse(cached) as OrgChannel; | ||
| } else { | ||
| channel = { ...(await createOrgChannel(env, orgID)), team: [] }; | ||
| await saveOrgChannel(env, orgID, channel); |
There was a problem hiding this comment.
🟡 Failed cache writes orphan Slack channels
If saveOrgChannel fails after creation, the Slack channel exists without its ID in KV. A retry sees name_taken and creates another channel, leaving the first abandoned.
Learn more
A successful Slack channel creation is an external side effect. Its ID is only stored in KV after Slack returns; a failed KV write loses the only reference to the channel. createOrgChannel treats the occupied name as a collision, not as a possible previous creation, so a retry creates another channel. When KV remains unavailable, slackFailure also fails to store the cooldown.
Example: Slack creates oc-acme-org1, but KV rejects the first write. After KV recovers, a retry sees name_taken and creates oc-acme-org1-1; the first private channel remains unused.
Recommended fix: Use a durable, recoverable channel record or reconcile name_taken against Slack channel metadata/membership before creating a suffixed channel. Treat failures to persist the channel separately from Slack invitation failures.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged, not fixing here: this needs a KV write to fail between conversations.create and saveOrgChannel, and the fallout is one unused private channel plus a -1 suffixed replacement. Making name_taken resolve to the existing channel would need conversations.list over all private channels (more scopes + pagination); a D1 row would be the proper fix if this shows up in practice. Called out under Known limitations in the description.
| const cached = await env.SESSIONS_KV.get(channelKey(orgID)); | ||
| let channel: OrgChannel; | ||
| if (cached) { | ||
| channel = JSON.parse(cached) as OrgChannel; | ||
| } else { | ||
| channel = { ...(await createOrgChannel(env, orgID)), team: [] }; | ||
| await saveOrgChannel(env, orgID, channel); |
There was a problem hiding this comment.
🔴 Concurrent invites split one org across channels
When two users invite concurrently, ensureOrgChannel can create a channel for each before either KV write becomes visible. The last write wins, leaving one user invited to a channel other members cannot find.
Learn more
The org channel key is shared by every member. Concurrent requests can both see no key, then each create a channel using createOrgChannel. Slack's name_taken retry makes the second request create a different valid channel, and both writes succeed in KV. Subsequent org members use whichever channel was written last, while the first user has an invitation to the other channel. Cloudflare KV also has eventual read consistency, so a recent write alone cannot serialize requests.
Example: Alice creates oc-acme-org1 while Bob receives name_taken and creates oc-acme-org1-1. Alice receives an invite to the first; Bob and later members receive invites to the second.
Recommended fix: Serialize org channel provisioning with a Durable Object or another atomic per-org coordinator; persist the canonical channel once and route all member invites through it. Ensure a concurrent name_taken checks for the canonical channel instead of immediately allocating a suffix.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged, not fixing here: requires two members of the same org to click within the KV propagation window (~1s) on the very first invite, and the outcome is two support channels with the team in both, not lost access. A DO/D1 lock is out of proportion for a support-onboarding button; documented under Known limitations and easy to add later if it happens.
| const slug = | ||
| name | ||
| .toLowerCase() | ||
| .normalize("NFKD") | ||
| .replace(/[^a-z0-9]+/g, "-") | ||
| .replace(/^-+|-+$/g, "") | ||
| .slice(0, 40) || "org"; | ||
| const idPart = orgID | ||
| .replace(/[^a-z0-9]/gi, "") | ||
| .toLowerCase() | ||
| .slice(-6); | ||
| const tail = `-${idPart}${attempt > 0 ? `-${attempt}` : ""}`; | ||
| const head = `${prefix}${slug}`.slice( | ||
| 0, | ||
| SLACK_CHANNEL_NAME_MAX - tail.length, | ||
| ); | ||
| return `${head}${tail}`; |
There was a problem hiding this comment.
🟨 Channel names expose customer organization names
slackChannelName places each customer name directly in a Slack channel name. Workspace members who can browse channel names can identify customers without accessing the private channel.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
By design — the user asked for a fresh channel per org with the team in it, and oc-<org-slug>-<id> is what makes those channels findable for the support team. Private channel names are only visible to workspace members (our team), not to the customer's workspace. SLACK_CONNECT_CHANNEL_PREFIX is configurable and the slug can be dropped if we'd rather use opaque names; leaving the decision to the reviewer.
Co-Authored-By: Mohamed <mo@digger.dev>
This branch has not been deployed
1 participant
Summary
Pro/Max users can set up a dedicated Slack Connect channel between their organization and the OpenComputer team from Settings, and invite themselves to it. The browser never picks a recipient — the server looks up the caller's own email in D1 and invites that.
api-edge (
src/slack_connect.ts, routed fromdashboard.ts):hasBYOKPlanAccess(legacy D1 plan or active Autumnpro/maxsubscription); ineligible → 403slack_connect_plan_required. The invite additionally requires a currentorg_membershipsrow (session JWTs outlive removal) → 403.ensureOrgChannel):conversations.createa private channel named{prefix}{org-slug}-{orgID tail}(retries with-1,-2… onname_taken; suffix always fits within Slack's 80-char limit){id,name,team:[]}inSESSIONS_KVatslack_connect_channel:{orgID}immediately, so a failed step 3 never orphans the channelconversations.inviteevery ID inSLACK_CONNECT_TEAM_USER_IDSnot yet inteam, then record them. This also runs on later calls, so failed invites are retried and newly configured team members get added to existing org channels.Subsequent users in the same org reuse that channel.
conversations.inviteShared{ channel, emails: [callerEmail], external_limited: true }; success recorded atslack_connect_invite:{orgID}:{userID}for 30 days so repeat POSTs returnalreadyInvited: truewithout hitting Slack. If the KV write fails after Slack succeeded, the request still returns success (logged).already_in_channel,invite_already_sent,user_already_team_member) → 409 and the invite record is (re)written so Settings stops offering the button; the panel refetches status on any error.slack_connect_rate_limitedwithout reaching Slack.available: false, panel hidden, invite → 503):SLACK_CONNECT_BOT_TOKENsecret — scopesgroups:write,conversations.connect:writeSLACK_CONNECT_TEAM_USER_IDS— comma-separated Slack user IDs auto-added to every org channelSLACK_CONNECT_CHANNEL_PREFIX— defaultoc-web:
SlackConnectPanelin Settings — hidden when unavailable, "Upgrade plan" link when ineligible, "Create shared channel" / "Join the channel" (if the org channel already exists) → "Invitation sent". Preview-mode mocks added.Tests:
src/slack_connect.test.ts(16: config, plan gating, membership gating, channel create + team invite + share, per-org reuse, team-change backfill,name_takenretry, long-prefix naming, stranded-channel recovery, cooldown, KV write failure, dedupe, error mapping).Known limitations (left as-is, flag if they matter):
conversations.createorphans that channel (next attempt getsname_takenand uses-1). A D1 row would fix both if needed.oc-acme-corp-…), visible to anyone in our workspace who can browse private channel names they're not in — setSLACK_CONNECT_CHANNEL_PREFIX/ drop the slug if that's a concern.Deploy note:
wrangler secret put SLACK_CONNECT_BOT_TOKEN, setSLACK_CONNECT_TEAM_USER_IDSon api-edge.Link to Devin session: https://app.devin.ai/sessions/86d4969a1e984a40913f71e44ebabe7c
Open in Devin Desktop: https://app.devin.ai/desktop/session/86d4969a1e984a40913f71e44ebabe7c?variant=devin
Requested by: @motatoes
Polylane reviews this pull request when you ask: