fix(slack): serialize Search bot removal with member setup, refuse a custom row on the shared app ID

Author: waleedlatif1

apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-empty-state/changelog-empty-state.tsx

@@ -24,10 +24,8 @@ interface Release {
 
 const LATEST: Release = { top: 10, notes: [118, 92] }
 
-const EARLIER: readonly Release[] = [
-  { top: 70, notes: [104, 126, 80] },
-  { top: 142, notes: [96, 112] },
-] as const
+/** Every release ends inside the 148px viewBox; the fade carries the timeline past it. */
+const EARLIER: readonly Release[] = [{ top: 70, notes: [104, 126, 80] }] as const
 
 const SPINE_X = 24
 const NOTE_PITCH = 12

apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts

@@ -44,9 +44,12 @@ import { SLACK_SEARCH_USER_SCOPES } from '@/lib/credential-groups/slack-managed-
 import { slackCredentialGroupProviderAdapter } from '@/lib/credential-groups/slack-provider'
 import { createOrganizationAccountsGroup } from '@/lib/credential-groups/workspace-accounts'
 import { encryptManagedOAuthTokenSet } from '@/lib/credentials/managed-oauth'
-import type { DbOrTx } from '@/lib/db/types'
+import type { DbOrTx, DbTransaction } from '@/lib/db/types'
 import { approveSearchIntegration } from '@/lib/knowledge/application/search-integrations'
-import { listSlackSearchInstallations } from '@/lib/knowledge/application/slack-search/installations'
+import {
+  listSlackSearchInstallations,
+  removeSlackSearchInstallation,
+} from '@/lib/knowledge/application/slack-search/installations'
 import { revokeSlackSearchAccess } from '@/lib/knowledge/application/slack-search/lifecycle'
 import { defaultLiveSearchPolicy } from '@/lib/sim-search/live/policy-schema'
 import { getSharedSlackSearchAppConfiguration } from '@/lib/slack-search/shared-app-env'
@@ -181,8 +184,9 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
       createdBy: ids.owner,
       encryptedServiceAccountKey: secret,
     })
+    const installationId = generateId()
     await executor.insert(slackSearchInstallation).values({
-      id: generateId(),
+      id: installationId,
       organizationId: ids.organization,
       credentialId: botId,
       appId,
@@ -193,6 +197,7 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
       credentialVersion: generateId(),
       revision: generateId(),
     })
+    return installationId
   }
 
   async function accountsGroup() {
@@ -203,6 +208,40 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
     return group
   }
 
+  /** Holds the Slack organization lock until `contend` is observed waiting on it, then releases. */
+  async function whileOrganizationSlackAppLocked<T>(
+    contend: () => Promise<T>,
+    holdWith: (tx: DbTransaction) => Promise<unknown> = async () => undefined
+  ): Promise<T> {
+    const locked = createDeferred<number>()
+    const release = createDeferred<void>()
+    const holder = db.transaction(async (tx) => {
+      await lockOrganizationSlackApp(tx, ids.organization)
+      await holdWith(tx)
+      const [connection] = await tx.execute<{ pid: number }>(sql`SELECT pg_backend_pid() AS pid`)
+      locked.resolve(connection.pid)
+      await release.promise
+    })
+    const holderPid = await locked.promise
+    const contender = contend()
+    contender.catch(() => undefined)
+    try {
+      await vi.waitFor(
+        async () => {
+          const [waiting] = await db.execute<{ pid: number }>(
+            sql`SELECT pid FROM pg_stat_activity WHERE ${holderPid} = ANY(pg_blocking_pids(pid))`
+          )
+          expect(waiting).toBeDefined()
+        },
+        { timeout: 5_000 }
+      )
+    } finally {
+      release.resolve()
+      await holder
+    }
+    return contender
+  }
+
   function provideSlackConsent(teamId: string) {
     restoreSlackHttp?.()
     const spy = vi.spyOn(globalThis, 'fetch').mockImplementation(async (input) => {
@@ -345,34 +384,50 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
     const pending = new URL((await startSetup(groupId)).authorizationUrl)
     const before = await accountsGroup()
     provideSlackConsent(MEMBER_TEAM)
-    const locked = createDeferred<number>()
-    const release = createDeferred<void>()
-    const install = db.transaction(async (tx) => {
-      await lockOrganizationSlackApp(tx, ids.organization)
-      await installBot(CUSTOM_APP_ID, OTHER_TEAM, tx)
-      const [connection] = await tx.execute<{ pid: number }>(sql`SELECT pg_backend_pid() AS pid`)
-      locked.resolve(connection.pid)
-      await release.promise
-    })
-    const installPid = await locked.promise
-    const completion = completeSetup(pending.searchParams.get('state') ?? '')
-    completion.catch(() => undefined)
-    try {
-      await vi.waitFor(
-        async () => {
-          const [waiting] = await db.execute<{ pid: number }>(
-            sql`SELECT pid FROM pg_stat_activity WHERE ${installPid} = ANY(pg_blocking_pids(pid))`
-          )
-          expect(waiting).toBeDefined()
-        },
-        { timeout: 5_000 }
+    await expect(
+      whileOrganizationSlackAppLocked(
+        () => completeSetup(pending.searchParams.get('state') ?? ''),
+        (tx) => installBot(CUSTOM_APP_ID, OTHER_TEAM, tx)
       )
-    } finally {
-      release.resolve()
-      await install
-    }
-    await expect(completion).rejects.toThrow('installations changed during authorization')
+    ).rejects.toThrow('installations changed during authorization')
+    expect(await accountsGroup()).toEqual(before)
+  })
+
+  it('waits on the Slack organization lock before removing a Search bot', async () => {
+    await approveSlack()
+    const installationId = await installBot(SHARED_APP_ID, MEMBER_TEAM)
+    await expect(
+      whileOrganizationSlackAppLocked(() =>
+        removeSlackSearchInstallation.execute({
+          principal: principal(),
+          input: { organizationId: ids.organization, installationId },
+        })
+      )
+    ).resolves.toEqual({ id: installationId })
+  })
+
+  it('explains a conflict when a custom app row holds the shared app ID', async () => {
+    const groupId = (await approveSlack()).memberAccounts?.groupId
+    if (!groupId) throw new Error('Approval did not provision organization accounts')
+    const secret = (await encryptSecret('fixture-custom')).encrypted
+    await db.insert(slackApp).values({
+      id: SHARED_APP_ID,
+      kind: 'custom',
+      organizationId: ids.organization,
+      clientId: 'fixture-custom-client',
+      encryptedClientSecret: secret,
+      encryptedSigningSecret: secret,
+      revision: generateId(),
+    })
+    const pending = new URL((await startSetup(groupId)).authorizationUrl)
+    const before = await accountsGroup()
+    provideSlackConsent(MEMBER_TEAM)
+    await expect(completeSetup(pending.searchParams.get('state') ?? '')).rejects.toThrow(
+      'belongs to another installation owner'
+    )
     expect(await accountsGroup()).toEqual(before)
+    const [row] = await db.select().from(slackApp).where(eq(slackApp.id, SHARED_APP_ID))
+    expect(row).toMatchObject({ kind: 'custom', organizationId: ids.organization })
   })
 
   it('reports a custom-app member setup and keeps it off the shared-app path', async () => {

apps/sim/lib/credential-groups/organization-slack-app.ts

@@ -10,8 +10,11 @@ import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks'
 import type { DbOrTx, DbTransaction } from '@/lib/db/types'
 
 /**
- * Serializes an organization's Slack member setup with its Search bot installs, so each sees the
- * other's committed app and workspace. Taken before any Slack app or workspace lock.
+ * Serializes an organization's Slack member setup with Slack Search's own installation writes
+ * (install, rebind, enable, disable, removal), so neither commits against an app or workspace the
+ * other changed. Taken before any Slack app or workspace lock. Slack revocations, which must never
+ * wait on setup, and the cascade from deleting a bot credential skip it: they only withdraw a bot,
+ * and member setup stays valid without one.
  */
 export async function lockOrganizationSlackApp(tx: DbTransaction, organizationId: string) {
   await acquireAdvisoryXactLock(tx, 'slack_org_app', `slack-org-app:${organizationId}`)

apps/sim/lib/credential-groups/slack-managed-users.ts

@@ -16,6 +16,7 @@ import { isRecordLike } from '@sim/utils/object'
 import { compareStrings } from '@sim/utils/string'
 import { and, eq, inArray, isNull, or } from 'drizzle-orm'
 import { getRedisClient } from '@/lib/core/config/redis'
+import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { resourceScopeFields, resourceScopeFromOwner } from '@/lib/core/resource-scope'
 import { resourceScopeCondition } from '@/lib/core/resource-scope.server'
 import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
@@ -496,7 +497,8 @@ export function getSlackManagedUsersRedirectUri(): string {
 
 /**
  * Names an organization's Search installation set. Installing, rebinding, enabling, disabling,
- * and removing a bot each rotate or drop a row's revision.
+ * and removing a bot each rotate or drop a row's revision. Writes that can add a bot all hold the
+ * Slack organization lock, so a match under that lock means none landed during authorization.
  */
 async function organizationInstallationsRevision(executor: DbOrTx, organizationId: string) {
   const rows = await executor
@@ -899,6 +901,12 @@ export async function exchangeAndConfigureSlackManagedUsers(params: {
         )
         .limit(1)
         .for('update')
+      /** A custom row holding the shared app's ID belongs to another owner; never adopt it. */
+      if (sharedApp && app?.kind !== 'shared')
+        throw new OrchestrationError(
+          'conflict',
+          'This Slack app belongs to another installation owner'
+        )
       if (app?.kind === 'shared')
         await requireSlackSearchAppAvailable(app.id, params.attempt.organizationId)
       const resolved = app ? await resolveSlackAppCredentials(app) : null

apps/sim/lib/knowledge/application/slack-search/installations.ts

@@ -271,15 +271,19 @@ export const removeSlackSearchInstallation = defineAuthorizedKnowledgeUseCase({
   operation: knowledgeOperations.removeSlackInstallation,
   resolveContext: ({ input }: { input: RemoveInput }) => resolveKnowledgeOrganizationContext(input),
   async execute({ input, context }) {
-    const [removed] = await db
-      .delete(slackSearchInstallation)
-      .where(
-        and(
-          eq(slackSearchInstallation.id, input.installationId),
-          eq(slackSearchInstallation.organizationId, context.organizationId)
+    const removed = await db.transaction(async (tx) => {
+      await lockOrganizationSlackApp(tx, context.organizationId)
+      const [row] = await tx
+        .delete(slackSearchInstallation)
+        .where(
+          and(
+            eq(slackSearchInstallation.id, input.installationId),
+            eq(slackSearchInstallation.organizationId, context.organizationId)
+          )
         )
-      )
-      .returning({ id: slackSearchInstallation.id })
+        .returning({ id: slackSearchInstallation.id })
+      return row
+    })
     if (!removed) throw new OrchestrationError('not_found', 'Slack Search installation not found')
     return removed
   },