From 0bf4b53af3b13a64554047cb22f8d22e0c00c0b3 Mon Sep 17 00:00:00 2001 From: Abdul Rahman <81605929+abdulrahmancodes@users.noreply.github.com> Date: Thu, 6 Aug 2026 15:20:53 +0530 Subject: [PATCH] feat(slack): cache the Slack bot user id at connect time (#23726) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Caches the Slack bot user id in workspace kv so the channel welcome stops calling `auth.test` on every channel-join event. ## Why Slack fires `member_joined_channel` for **every** person joining **any** channel the bot sits in, not just for the bot itself. The welcome path had to answer "was that our bot?", and did it by resolving the Slack connection and calling `auth.test` — a connection lookup plus an external API call, on every event, to conclude "no, that was a human, do nothing". The bot user id never changes for a given connection, so asking Slack repeatedly is the wrong shape. ## How `registerSlackConnection` already calls `auth.test` in the `onConnect` hook and had `user_id` in hand, so it now writes it to workspace kv. `resolveSlackBotUserId` reads it, falling back to `auth.test` (and backfilling) for connections created before this change. Reconnecting is the only thing that can change the bot user id, and reconnecting re-runs that hook — so the cache is self-correcting and needs no TTL. Because resolving the id no longer needs a Slack client, the bot check moved ahead of the connection lookup: | per join event | before | after | |---|---|---| | Twenty round trips | 1 | 1 | | Slack API calls | 1 | 0 | A secondary win: previously `getSlackClient()` ran before the bot check and threw on failure, so a revoked Slack connection made **every unrelated human join** fail its job and retry. Now a human join answers from kv and returns cleanly; the connection is only touched when there is genuinely something to post. ## Claim ordering Moving the client lookup after the claim opened a window where the claim is held but nothing was posted, so that path now releases the claim before throwing. The invariant the file follows is unchanged: release on any failure that produced no message, keep it once a message is out (a retry must not repost the channel message). ## Renames `claimSlackTeam` → `registerSlackConnection`, and the logic function `slack-team-claim` → `slack-register-connection`, since it now does more than claim the team and connect-time work will keep landing there. **The universal identifier value is unchanged** (`a29ae15d-…`) — it is the app's stable identity and what the connection provider binds `onConnectLogicFunction` to. Only the constant's name moved. Worth a second pair of eyes in review, since that is exactly the kind of thing a rename sweep regenerates by reflex. Note this changes `name` and `sourceHandlerPath`/`builtHandlerPath` in the manifest. Both are updates keyed on the unchanged identifier, not a delete-and-recreate, so installed apps re-sync cleanly — but the app needs rebuilding so the bundle path matches. Review in cubic ## Cache correctness A wrong cached id fails silently — the bot's own join reads as someone else's and the welcome never fires — so the entry is bounded and self-healing in three ways: - **Failed write drops the key.** Leaving the previous id in place would keep a superseded value authoritative. An absent cache is rebuilt from `auth.test`; a wrong one is believed. - **Entries expire after 7 days.** `registerSlackConnection` rewrites on every connect, so the expiry only matters when that write never lands. - **A kv outage falls through to `auth.test`** rather than throwing, which keeps the human joins that make up nearly all these events from failing their job. --- .../connection-providers/slack-connection.ts | 4 +- .../src/constants/universal-identifiers.ts | 2 +- .../constants/slack-bot-user-id-kv-key.ts | 1 + .../constants/slack-bot-user-id-ttl-ms.ts | 1 + .../slack-register-connection-handler.ts | 9 ++ .../handlers/slack-team-claim-handler.ts | 7 - .../slack-register-connection.ts | 13 ++ .../src/logic-functions/slack-team-claim.ts | 13 -- .../slack-bot-user-id-cache-entry.type.ts | 4 + ...slack-register-connection-payload.type.ts} | 2 +- .../post-slack-channel-welcome.test.ts | 132 ++++++++++++++++++ ...resolve-slack-bot-user-id-or-throw.test.ts | 127 +++++++++++++++++ .../utils/cache-slack-bot-user-id.ts | 16 +++ .../utils/claim-slack-channel-welcome.ts | 8 +- .../utils/has-kv-entry-expired.ts | 4 + .../utils/is-slack-thread-active.ts | 7 +- .../utils/post-slack-channel-welcome.ts | 23 +-- ...k-team.ts => register-slack-connection.ts} | 20 ++- .../resolve-slack-bot-user-id-or-throw.ts | 48 +++++++ 19 files changed, 391 insertions(+), 50 deletions(-) create mode 100644 packages/twenty-apps/public/slack/src/logic-functions/constants/slack-bot-user-id-kv-key.ts create mode 100644 packages/twenty-apps/public/slack/src/logic-functions/constants/slack-bot-user-id-ttl-ms.ts create mode 100644 packages/twenty-apps/public/slack/src/logic-functions/handlers/slack-register-connection-handler.ts delete mode 100644 packages/twenty-apps/public/slack/src/logic-functions/handlers/slack-team-claim-handler.ts create mode 100644 packages/twenty-apps/public/slack/src/logic-functions/slack-register-connection.ts delete mode 100644 packages/twenty-apps/public/slack/src/logic-functions/slack-team-claim.ts create mode 100644 packages/twenty-apps/public/slack/src/logic-functions/types/slack-bot-user-id-cache-entry.type.ts rename packages/twenty-apps/public/slack/src/logic-functions/types/{slack-team-claim-payload.type.ts => slack-register-connection-payload.type.ts} (67%) create mode 100644 packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/post-slack-channel-welcome.test.ts create mode 100644 packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/resolve-slack-bot-user-id-or-throw.test.ts create mode 100644 packages/twenty-apps/public/slack/src/logic-functions/utils/cache-slack-bot-user-id.ts create mode 100644 packages/twenty-apps/public/slack/src/logic-functions/utils/has-kv-entry-expired.ts rename packages/twenty-apps/public/slack/src/logic-functions/utils/{claim-slack-team.ts => register-slack-connection.ts} (57%) create mode 100644 packages/twenty-apps/public/slack/src/logic-functions/utils/resolve-slack-bot-user-id-or-throw.ts diff --git a/packages/twenty-apps/public/slack/src/connection-providers/slack-connection.ts b/packages/twenty-apps/public/slack/src/connection-providers/slack-connection.ts index 99d666f331..5feb306634 100644 --- a/packages/twenty-apps/public/slack/src/connection-providers/slack-connection.ts +++ b/packages/twenty-apps/public/slack/src/connection-providers/slack-connection.ts @@ -2,7 +2,7 @@ import { defineConnectionProvider } from 'twenty-sdk/define'; import { SLACK_CONNECTION_PROVIDER_UNIVERSAL_IDENTIFIER, - SLACK_TEAM_CLAIM_UNIVERSAL_IDENTIFIER, + SLACK_REGISTER_CONNECTION_UNIVERSAL_IDENTIFIER, } from 'src/constants/universal-identifiers'; export default defineConnectionProvider({ @@ -11,7 +11,7 @@ export default defineConnectionProvider({ displayName: 'Slack', type: 'oauth', onConnectLogicFunction: { - universalIdentifier: SLACK_TEAM_CLAIM_UNIVERSAL_IDENTIFIER, + universalIdentifier: SLACK_REGISTER_CONNECTION_UNIVERSAL_IDENTIFIER, }, oauth: { authorizationEndpoint: 'https://slack.com/oauth/v2/authorize', diff --git a/packages/twenty-apps/public/slack/src/constants/universal-identifiers.ts b/packages/twenty-apps/public/slack/src/constants/universal-identifiers.ts index b05c4559fb..72857a6624 100644 --- a/packages/twenty-apps/public/slack/src/constants/universal-identifiers.ts +++ b/packages/twenty-apps/public/slack/src/constants/universal-identifiers.ts @@ -55,7 +55,7 @@ export const SLACK_CHANNEL_WELCOME_UNIVERSAL_IDENTIFIER = export const SLACK_ASSISTANT_WORKER_UNIVERSAL_IDENTIFIER = '4b92a49f-d674-46ea-a3d9-e8d658ae3a17'; -export const SLACK_TEAM_CLAIM_UNIVERSAL_IDENTIFIER = +export const SLACK_REGISTER_CONNECTION_UNIVERSAL_IDENTIFIER = 'a29ae15d-dd16-4b99-bb6c-079842da55ab'; export const SLACK_ASSISTANT_REQUEST_OBJECT_UNIVERSAL_IDENTIFIER = diff --git a/packages/twenty-apps/public/slack/src/logic-functions/constants/slack-bot-user-id-kv-key.ts b/packages/twenty-apps/public/slack/src/logic-functions/constants/slack-bot-user-id-kv-key.ts new file mode 100644 index 0000000000..5aef54e180 --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/constants/slack-bot-user-id-kv-key.ts @@ -0,0 +1 @@ +export const SLACK_BOT_USER_ID_KV_KEY = 'slack-bot-user-id'; diff --git a/packages/twenty-apps/public/slack/src/logic-functions/constants/slack-bot-user-id-ttl-ms.ts b/packages/twenty-apps/public/slack/src/logic-functions/constants/slack-bot-user-id-ttl-ms.ts new file mode 100644 index 0000000000..a42b3b7127 --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/constants/slack-bot-user-id-ttl-ms.ts @@ -0,0 +1 @@ +export const SLACK_BOT_USER_ID_TTL_MS = 7 * 24 * 60 * 60 * 1000; diff --git a/packages/twenty-apps/public/slack/src/logic-functions/handlers/slack-register-connection-handler.ts b/packages/twenty-apps/public/slack/src/logic-functions/handlers/slack-register-connection-handler.ts new file mode 100644 index 0000000000..0455d9ceaa --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/handlers/slack-register-connection-handler.ts @@ -0,0 +1,9 @@ +import { type SlackRegisterConnectionPayload } from 'src/logic-functions/types/slack-register-connection-payload.type'; +import { registerSlackConnection } from 'src/logic-functions/utils/register-slack-connection'; + +export const slackRegisterConnectionHandler = ( + payload: SlackRegisterConnectionPayload, +) => + registerSlackConnection({ + connectedAccountId: payload.connectedAccountId, + }); diff --git a/packages/twenty-apps/public/slack/src/logic-functions/handlers/slack-team-claim-handler.ts b/packages/twenty-apps/public/slack/src/logic-functions/handlers/slack-team-claim-handler.ts deleted file mode 100644 index 6d5d6d90bb..0000000000 --- a/packages/twenty-apps/public/slack/src/logic-functions/handlers/slack-team-claim-handler.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { type SlackTeamClaimPayload } from 'src/logic-functions/types/slack-team-claim-payload.type'; -import { claimSlackTeam } from 'src/logic-functions/utils/claim-slack-team'; - -export const slackTeamClaimHandler = (payload: SlackTeamClaimPayload) => - claimSlackTeam({ - connectedAccountId: payload.connectedAccountId, - }); diff --git a/packages/twenty-apps/public/slack/src/logic-functions/slack-register-connection.ts b/packages/twenty-apps/public/slack/src/logic-functions/slack-register-connection.ts new file mode 100644 index 0000000000..a48db58de6 --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/slack-register-connection.ts @@ -0,0 +1,13 @@ +import { defineLogicFunction } from 'twenty-sdk/define'; + +import { SLACK_REGISTER_CONNECTION_UNIVERSAL_IDENTIFIER } from 'src/constants/universal-identifiers'; +import { slackRegisterConnectionHandler } from 'src/logic-functions/handlers/slack-register-connection-handler'; + +export default defineLogicFunction({ + universalIdentifier: SLACK_REGISTER_CONNECTION_UNIVERSAL_IDENTIFIER, + name: 'slack-register-connection', + description: + 'Runs when a Slack connection is established (via the connection provider onConnect hook). Resolves the Slack team_id for the just-created connection via auth.test and stores this workspace id under the server-scoped slack-team: key so inbound Slack events route here. Caches the bot user id from the same auth.test under slack-bot-user-id so the channel welcome can recognise the bot without calling Slack on every join event.', + timeoutSeconds: 30, + handler: slackRegisterConnectionHandler, +}); diff --git a/packages/twenty-apps/public/slack/src/logic-functions/slack-team-claim.ts b/packages/twenty-apps/public/slack/src/logic-functions/slack-team-claim.ts deleted file mode 100644 index d2eb4c9249..0000000000 --- a/packages/twenty-apps/public/slack/src/logic-functions/slack-team-claim.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { defineLogicFunction } from 'twenty-sdk/define'; - -import { SLACK_TEAM_CLAIM_UNIVERSAL_IDENTIFIER } from 'src/constants/universal-identifiers'; -import { slackTeamClaimHandler } from 'src/logic-functions/handlers/slack-team-claim-handler'; - -export default defineLogicFunction({ - universalIdentifier: SLACK_TEAM_CLAIM_UNIVERSAL_IDENTIFIER, - name: 'slack-team-claim', - description: - 'Runs when a Slack connection is established (via the connection provider onConnect hook). Resolves the Slack team_id for the just-created connection via auth.test and stores this workspace id under the server-scoped slack-team: key so inbound Slack events route here.', - timeoutSeconds: 30, - handler: slackTeamClaimHandler, -}); diff --git a/packages/twenty-apps/public/slack/src/logic-functions/types/slack-bot-user-id-cache-entry.type.ts b/packages/twenty-apps/public/slack/src/logic-functions/types/slack-bot-user-id-cache-entry.type.ts new file mode 100644 index 0000000000..2d5906bfc3 --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/types/slack-bot-user-id-cache-entry.type.ts @@ -0,0 +1,4 @@ +export type SlackBotUserIdCacheEntry = { + botUserId: string; + expiresAt: number; +}; diff --git a/packages/twenty-apps/public/slack/src/logic-functions/types/slack-team-claim-payload.type.ts b/packages/twenty-apps/public/slack/src/logic-functions/types/slack-register-connection-payload.type.ts similarity index 67% rename from packages/twenty-apps/public/slack/src/logic-functions/types/slack-team-claim-payload.type.ts rename to packages/twenty-apps/public/slack/src/logic-functions/types/slack-register-connection-payload.type.ts index b51266bc71..1b19fe4c51 100644 --- a/packages/twenty-apps/public/slack/src/logic-functions/types/slack-team-claim-payload.type.ts +++ b/packages/twenty-apps/public/slack/src/logic-functions/types/slack-register-connection-payload.type.ts @@ -1,4 +1,4 @@ -export type SlackTeamClaimPayload = { +export type SlackRegisterConnectionPayload = { connectionProviderId: string; connectionProviderName: string; connectedAccountId: string; diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/post-slack-channel-welcome.test.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/post-slack-channel-welcome.test.ts new file mode 100644 index 0000000000..44d4e0402c --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/post-slack-channel-welcome.test.ts @@ -0,0 +1,132 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { type SlackEventsRequestBody } from 'src/logic-functions/types/slack-events-request-body.type'; +import { postSlackChannelWelcome } from 'src/logic-functions/utils/post-slack-channel-welcome'; + +const { + claimSlackChannelWelcomeMock, + getSlackClientMock, + postSlackMessageMock, + releaseSlackChannelWelcomeMock, + resolveSlackBotUserIdOrThrowMock, +} = vi.hoisted(() => ({ + claimSlackChannelWelcomeMock: vi.fn(), + getSlackClientMock: vi.fn(), + postSlackMessageMock: vi.fn(), + releaseSlackChannelWelcomeMock: vi.fn(), + resolveSlackBotUserIdOrThrowMock: vi.fn(), +})); + +vi.mock('src/logic-functions/utils/claim-slack-channel-welcome', () => ({ + claimSlackChannelWelcome: claimSlackChannelWelcomeMock, +})); + +vi.mock('src/logic-functions/utils/get-slack-client', () => ({ + getSlackClient: getSlackClientMock, +})); + +vi.mock('src/logic-functions/utils/post-slack-message', () => ({ + postSlackMessage: postSlackMessageMock, +})); + +vi.mock('src/logic-functions/utils/release-slack-channel-welcome', () => ({ + releaseSlackChannelWelcome: releaseSlackChannelWelcomeMock, +})); + +vi.mock('src/logic-functions/utils/resolve-slack-bot-user-id-or-throw', () => ({ + resolveSlackBotUserIdOrThrow: resolveSlackBotUserIdOrThrowMock, +})); + +const BOT_USER_ID = 'UBOT'; +const CHANNEL_ID = 'C123'; + +const buildJoinEvent = (slackUserId: string): SlackEventsRequestBody => ({ + type: 'event_callback', + team_id: 'T123', + event: { + type: 'member_joined_channel', + channel: CHANNEL_ID, + user: slackUserId, + }, +}); + +describe('postSlackChannelWelcome', () => { + beforeEach(() => { + vi.clearAllMocks(); + resolveSlackBotUserIdOrThrowMock.mockResolvedValue(BOT_USER_ID); + claimSlackChannelWelcomeMock.mockResolvedValue(true); + releaseSlackChannelWelcomeMock.mockResolvedValue(undefined); + getSlackClientMock.mockResolvedValue({ success: true, client: {} }); + postSlackMessageMock.mockResolvedValue({ success: true, slackTs: '1.1' }); + }); + + it('should skip without touching the Slack connection when someone else joined', async () => { + const result = await postSlackChannelWelcome(buildJoinEvent('UHUMAN')); + + expect(result).toEqual({ + ok: true, + skipped: 'Someone other than the bot joined', + }); + expect(getSlackClientMock).not.toHaveBeenCalled(); + expect(claimSlackChannelWelcomeMock).not.toHaveBeenCalled(); + }); + + it('should skip when the channel was already welcomed', async () => { + claimSlackChannelWelcomeMock.mockResolvedValue(false); + + const result = await postSlackChannelWelcome(buildJoinEvent(BOT_USER_ID)); + + expect(result).toEqual({ + ok: true, + skipped: 'Channel was already welcomed', + }); + expect(getSlackClientMock).not.toHaveBeenCalled(); + }); + + it('should post the channel message and its thread reply on the bot join', async () => { + const result = await postSlackChannelWelcome(buildJoinEvent(BOT_USER_ID)); + + expect(result).toEqual({ ok: true }); + expect(postSlackMessageMock).toHaveBeenCalledTimes(2); + expect(postSlackMessageMock.mock.calls[1][1]).toMatchObject({ + slackChannelId: CHANNEL_ID, + parentMessageTimestamp: '1.1', + }); + expect(releaseSlackChannelWelcomeMock).not.toHaveBeenCalled(); + }); + + it('should release the claim when the Slack client cannot be built', async () => { + getSlackClientMock.mockResolvedValue({ + success: false, + error: 'Slack is not connected.', + }); + + await expect( + postSlackChannelWelcome(buildJoinEvent(BOT_USER_ID)), + ).rejects.toThrow('Slack is not connected.'); + expect(releaseSlackChannelWelcomeMock).toHaveBeenCalledWith(CHANNEL_ID); + }); + + it('should release the claim when the channel message fails to post', async () => { + postSlackMessageMock.mockResolvedValue({ + success: false, + error: 'channel_not_found', + }); + + await expect( + postSlackChannelWelcome(buildJoinEvent(BOT_USER_ID)), + ).rejects.toThrow('channel_not_found'); + expect(releaseSlackChannelWelcomeMock).toHaveBeenCalledWith(CHANNEL_ID); + }); + + it('should keep the claim when only the thread reply fails, so a retry cannot repost the channel message', async () => { + postSlackMessageMock + .mockResolvedValueOnce({ success: true, slackTs: '1.1' }) + .mockResolvedValueOnce({ success: false, error: 'thread_not_found' }); + + await expect( + postSlackChannelWelcome(buildJoinEvent(BOT_USER_ID)), + ).rejects.toThrow('thread_not_found'); + expect(releaseSlackChannelWelcomeMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/resolve-slack-bot-user-id-or-throw.test.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/resolve-slack-bot-user-id-or-throw.test.ts new file mode 100644 index 0000000000..78ac078bee --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/resolve-slack-bot-user-id-or-throw.test.ts @@ -0,0 +1,127 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { SLACK_BOT_USER_ID_KV_KEY } from 'src/logic-functions/constants/slack-bot-user-id-kv-key'; +import { SLACK_BOT_USER_ID_TTL_MS } from 'src/logic-functions/constants/slack-bot-user-id-ttl-ms'; +import { resolveSlackBotUserIdOrThrow } from 'src/logic-functions/utils/resolve-slack-bot-user-id-or-throw'; + +const { authTestMock, getSlackClientMock, kvGetMock, kvSetMock, kvDeleteMock } = + vi.hoisted(() => ({ + authTestMock: vi.fn(), + getSlackClientMock: vi.fn(), + kvGetMock: vi.fn(), + kvSetMock: vi.fn(), + kvDeleteMock: vi.fn(), + })); + +vi.mock('twenty-sdk/logic-function', () => ({ + kv: { get: kvGetMock, set: kvSetMock, delete: kvDeleteMock }, +})); + +vi.mock('src/logic-functions/utils/get-slack-client', () => ({ + getSlackClient: getSlackClientMock, +})); + +const BOT_USER_ID = 'UBOT'; + +const freshCacheEntry = () => ({ + botUserId: BOT_USER_ID, + expiresAt: Date.now() + SLACK_BOT_USER_ID_TTL_MS, +}); + +describe('resolveSlackBotUserIdOrThrow', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers(); + kvGetMock.mockResolvedValue(null); + kvSetMock.mockResolvedValue(undefined); + kvDeleteMock.mockResolvedValue(true); + authTestMock.mockResolvedValue({ user_id: BOT_USER_ID }); + getSlackClientMock.mockResolvedValue({ + success: true, + client: { auth: { test: authTestMock } }, + }); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('should return the cached id without calling Slack', async () => { + kvGetMock.mockResolvedValue(freshCacheEntry()); + + const botUserId = await resolveSlackBotUserIdOrThrow(); + + expect(botUserId).toBe(BOT_USER_ID); + expect(getSlackClientMock).not.toHaveBeenCalled(); + expect(authTestMock).not.toHaveBeenCalled(); + }); + + it('should fall back to auth.test when nothing is cached', async () => { + const botUserId = await resolveSlackBotUserIdOrThrow(); + + expect(botUserId).toBe(BOT_USER_ID); + expect(authTestMock).toHaveBeenCalled(); + }); + + it('should refetch rather than trust an expired entry', async () => { + kvGetMock.mockResolvedValue({ + botUserId: 'USTALE', + expiresAt: Date.now() - 1, + }); + + const botUserId = await resolveSlackBotUserIdOrThrow(); + + expect(botUserId).toBe(BOT_USER_ID); + expect(authTestMock).toHaveBeenCalled(); + }); + + it('should refetch when the cached entry has no expiry', async () => { + kvGetMock.mockResolvedValue({ botUserId: 'USTALE' }); + + await expect(resolveSlackBotUserIdOrThrow()).resolves.toBe(BOT_USER_ID); + }); + + it('should fall back to auth.test when the cache read fails', async () => { + kvGetMock.mockRejectedValue(new Error('kv unavailable')); + + const botUserId = await resolveSlackBotUserIdOrThrow(); + + expect(botUserId).toBe(BOT_USER_ID); + expect(authTestMock).toHaveBeenCalled(); + }); + + it('should backfill the cache with an expiry after falling back', async () => { + await resolveSlackBotUserIdOrThrow(); + + expect(kvSetMock).toHaveBeenCalledWith(SLACK_BOT_USER_ID_KV_KEY, { + botUserId: BOT_USER_ID, + expiresAt: Date.now() + SLACK_BOT_USER_ID_TTL_MS, + }); + }); + + it('should drop the key when the cache write fails, rather than leave a stale id', async () => { + kvSetMock.mockRejectedValue(new Error('kv unavailable')); + + await expect(resolveSlackBotUserIdOrThrow()).resolves.toBe(BOT_USER_ID); + expect(kvDeleteMock).toHaveBeenCalledWith(SLACK_BOT_USER_ID_KV_KEY); + }); + + it('should throw when Slack is not connected and nothing is cached', async () => { + getSlackClientMock.mockResolvedValue({ + success: false, + error: 'Slack is not connected.', + }); + + await expect(resolveSlackBotUserIdOrThrow()).rejects.toThrow( + 'Slack is not connected.', + ); + }); + + it('should throw when auth.test returns no user id', async () => { + authTestMock.mockResolvedValue({}); + + await expect(resolveSlackBotUserIdOrThrow()).rejects.toThrow( + 'Slack auth.test returned no user_id for the bot', + ); + }); +}); diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/cache-slack-bot-user-id.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/cache-slack-bot-user-id.ts new file mode 100644 index 0000000000..a23e5179fd --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/cache-slack-bot-user-id.ts @@ -0,0 +1,16 @@ +import { kv } from 'twenty-sdk/logic-function'; + +import { SLACK_BOT_USER_ID_KV_KEY } from 'src/logic-functions/constants/slack-bot-user-id-kv-key'; +import { SLACK_BOT_USER_ID_TTL_MS } from 'src/logic-functions/constants/slack-bot-user-id-ttl-ms'; +import { type SlackBotUserIdCacheEntry } from 'src/logic-functions/types/slack-bot-user-id-cache-entry.type'; + +export const cacheSlackBotUserId = async (botUserId: string): Promise => { + await kv + .set(SLACK_BOT_USER_ID_KV_KEY, { + botUserId, + expiresAt: Date.now() + SLACK_BOT_USER_ID_TTL_MS, + } satisfies SlackBotUserIdCacheEntry) + .catch(async () => { + await kv.delete(SLACK_BOT_USER_ID_KV_KEY).catch(() => undefined); + }); +}; diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/claim-slack-channel-welcome.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/claim-slack-channel-welcome.ts index b841264114..b6cdace39f 100644 --- a/packages/twenty-apps/public/slack/src/logic-functions/utils/claim-slack-channel-welcome.ts +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/claim-slack-channel-welcome.ts @@ -1,8 +1,8 @@ -import { isNumber } from '@sniptt/guards'; import { kv } from 'twenty-sdk/logic-function'; import { type SlackChannelWelcome } from 'src/logic-functions/types/slack-channel-welcome.type'; import { getSlackChannelWelcomeKvKey } from 'src/logic-functions/utils/get-slack-channel-welcome-kv-key'; +import { hasKvEntryExpired } from 'src/logic-functions/utils/has-kv-entry-expired'; const SLACK_CHANNEL_WELCOME_TTL_MS = 30 * 24 * 60 * 60 * 1000; @@ -12,11 +12,7 @@ export const claimSlackChannelWelcome = async ( const key = getSlackChannelWelcomeKvKey(channelId); const existingWelcome = await kv.get(key); - if ( - existingWelcome !== null && - isNumber(existingWelcome.expiresAt) && - existingWelcome.expiresAt > Date.now() - ) { + if (existingWelcome !== null && !hasKvEntryExpired(existingWelcome)) { return false; } diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/has-kv-entry-expired.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/has-kv-entry-expired.ts new file mode 100644 index 0000000000..845f37ca78 --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/has-kv-entry-expired.ts @@ -0,0 +1,4 @@ +import { isNumber } from '@sniptt/guards'; + +export const hasKvEntryExpired = (entry: { expiresAt?: number }): boolean => + !isNumber(entry.expiresAt) || entry.expiresAt <= Date.now(); diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/is-slack-thread-active.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/is-slack-thread-active.ts index 148e65a51e..185c61dfae 100644 --- a/packages/twenty-apps/public/slack/src/logic-functions/utils/is-slack-thread-active.ts +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/is-slack-thread-active.ts @@ -1,9 +1,10 @@ -import { isNonEmptyString, isNumber } from '@sniptt/guards'; +import { isNonEmptyString } from '@sniptt/guards'; import { kv } from 'twenty-sdk/logic-function'; import { type SlackThreadReference } from 'src/logic-functions/types/slack-thread-reference.type'; import { type SlackThreadSubscription } from 'src/logic-functions/types/slack-thread-subscription.type'; import { getSlackThreadKvKey } from 'src/logic-functions/utils/get-slack-thread-kv-key'; +import { hasKvEntryExpired } from 'src/logic-functions/utils/has-kv-entry-expired'; export const isSlackThreadActive = async ({ channelId, @@ -16,11 +17,11 @@ export const isSlackThreadActive = async ({ const key = getSlackThreadKvKey({ channelId, threadTimestamp }); const subscription = await kv.get(key); - if (subscription === null || !isNumber(subscription.expiresAt)) { + if (subscription === null) { return false; } - if (subscription.expiresAt <= Date.now()) { + if (hasKvEntryExpired(subscription)) { await kv.delete(key); return false; diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/post-slack-channel-welcome.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/post-slack-channel-welcome.ts index 41035dd17e..3a15b33ad3 100644 --- a/packages/twenty-apps/public/slack/src/logic-functions/utils/post-slack-channel-welcome.ts +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/post-slack-channel-welcome.ts @@ -8,6 +8,7 @@ import { getSlackClient } from 'src/logic-functions/utils/get-slack-client'; import { parseSlackChannelWelcomeEvent } from 'src/logic-functions/utils/parse-slack-channel-welcome-event'; import { postSlackMessage } from 'src/logic-functions/utils/post-slack-message'; import { releaseSlackChannelWelcome } from 'src/logic-functions/utils/release-slack-channel-welcome'; +import { resolveSlackBotUserIdOrThrow } from 'src/logic-functions/utils/resolve-slack-bot-user-id-or-throw'; type SlackChannelWelcomeResult = { ok: boolean; skipped?: string }; @@ -22,17 +23,9 @@ export const postSlackChannelWelcome = async ( const { slackChannelId, slackUserId } = parsed.channelJoin; - const slackClientResult = await getSlackClient(); + const botUserId = await resolveSlackBotUserIdOrThrow(); - if (!slackClientResult.success) { - throw new Error(slackClientResult.error); - } - - const { client } = slackClientResult; - - const authResult = await client.auth.test(); - - if (authResult.user_id !== slackUserId) { + if (botUserId !== slackUserId) { return { ok: true, skipped: 'Someone other than the bot joined' }; } @@ -42,6 +35,16 @@ export const postSlackChannelWelcome = async ( return { ok: true, skipped: 'Channel was already welcomed' }; } + const slackClientResult = await getSlackClient(); + + if (!slackClientResult.success) { + await releaseSlackChannelWelcome(slackChannelId); + + throw new Error(slackClientResult.error); + } + + const { client } = slackClientResult; + const channelMessageResult = await postSlackMessage(client, { slackChannelId, messageText: SLACK_CHANNEL_WELCOME_TEXT, diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/claim-slack-team.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/register-slack-connection.ts similarity index 57% rename from packages/twenty-apps/public/slack/src/logic-functions/utils/claim-slack-team.ts rename to packages/twenty-apps/public/slack/src/logic-functions/utils/register-slack-connection.ts index e534c01b3b..193706d139 100644 --- a/packages/twenty-apps/public/slack/src/logic-functions/utils/claim-slack-team.ts +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/register-slack-connection.ts @@ -2,37 +2,43 @@ import { WebClient } from '@slack/web-api'; import { isNonEmptyString } from '@sniptt/guards'; import { getConnection, kv } from 'twenty-sdk/logic-function'; +import { cacheSlackBotUserId } from 'src/logic-functions/utils/cache-slack-bot-user-id'; import { getSlackTeamKvKey } from 'src/logic-functions/utils/get-slack-team-kv-key'; -type ClaimSlackTeamArgs = { +type RegisterSlackConnectionArgs = { connectedAccountId: string; }; -type ClaimSlackTeamResult = { +type RegisterSlackConnectionResult = { ok: true; teamId: string; }; -export const claimSlackTeam = async ({ +export const registerSlackConnection = async ({ connectedAccountId, -}: ClaimSlackTeamArgs): Promise => { +}: RegisterSlackConnectionArgs): Promise => { if (!isNonEmptyString(connectedAccountId)) { throw new Error( - 'Slack team claim failed: onConnect payload is missing connectedAccountId', + 'Slack connection registration failed: onConnect payload is missing connectedAccountId', ); } const connection = await getConnection(connectedAccountId); const client = new WebClient(connection.accessToken); - const authResult = await client.auth.test(); - const teamId = authResult.team_id; + const { team_id: teamId, user_id: botUserId } = await client.auth.test(); if (!isNonEmptyString(teamId)) { throw new Error('Slack auth.test returned no team_id to claim'); } + if (!isNonEmptyString(botUserId)) { + throw new Error('Slack auth.test returned no user_id for the bot'); + } + // TODO: release the claim on disconnect once connection providers expose an onDisconnect hook. await kv.set(getSlackTeamKvKey(teamId), null, { scope: 'SERVER' }); + await cacheSlackBotUserId(botUserId); + return { ok: true, teamId }; }; diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/resolve-slack-bot-user-id-or-throw.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/resolve-slack-bot-user-id-or-throw.ts new file mode 100644 index 0000000000..3872abe923 --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/resolve-slack-bot-user-id-or-throw.ts @@ -0,0 +1,48 @@ +import { isNonEmptyString } from '@sniptt/guards'; +import { kv } from 'twenty-sdk/logic-function'; + +import { SLACK_BOT_USER_ID_KV_KEY } from 'src/logic-functions/constants/slack-bot-user-id-kv-key'; +import { type SlackBotUserIdCacheEntry } from 'src/logic-functions/types/slack-bot-user-id-cache-entry.type'; +import { cacheSlackBotUserId } from 'src/logic-functions/utils/cache-slack-bot-user-id'; +import { getSlackClient } from 'src/logic-functions/utils/get-slack-client'; +import { hasKvEntryExpired } from 'src/logic-functions/utils/has-kv-entry-expired'; + +const readCachedBotUserId = async (): Promise => { + const cacheEntry = await kv + .get(SLACK_BOT_USER_ID_KV_KEY) + .catch(() => null); + + if ( + cacheEntry === null || + !isNonEmptyString(cacheEntry.botUserId) || + hasKvEntryExpired(cacheEntry) + ) { + return undefined; + } + + return cacheEntry.botUserId; +}; + +export const resolveSlackBotUserIdOrThrow = async (): Promise => { + const cachedBotUserId = await readCachedBotUserId(); + + if (isNonEmptyString(cachedBotUserId)) { + return cachedBotUserId; + } + + const slackClientResult = await getSlackClient(); + + if (!slackClientResult.success) { + throw new Error(slackClientResult.error); + } + + const authResult = await slackClientResult.client.auth.test(); + + if (!isNonEmptyString(authResult.user_id)) { + throw new Error('Slack auth.test returned no user_id for the bot'); + } + + await cacheSlackBotUserId(authResult.user_id); + + return authResult.user_id; +};