fix(slack): address review — set credentialId, scope OAuth path to workspace, drop event narrowing

Review round 1 (Greptile 4/5 + Cursor Bugbot):

- deploy.ts: the native Sim-app (OAuth) branch now sets providerConfig.credentialId
  (runtime token resolution in the slack provider and credential-disconnect cleanup
  both key slack_app rows on it — without it, file downloads / reaction text fail and
  disconnect leaves the webhook active).
- deploy.ts: resolve the OAuth credential through resolveTriggerCredentialId
  (workspace- and oauth-scoped) so a pasted foreign/other-tenant credential id can't
  bind to the workflow; use the resolved canonical id for token owner lookup + routing.
- deploy.ts: a deleted/secretless custom bot credential (getSlackBotCredential → null
  but a service_account row exists) now returns the "reconnect the bot" error instead
  of the misleading "connected Slack account" message.
- oauth.ts: drop the credential-based event-option narrowing. The shared dropdown
  framework doesn't revalidate a stored value when its dependency changes, so switching
  a custom bot → Sim account left an orphaned event that failed deploy with a 400. The
  event picker now offers all events; the deploy path is the authoritative gate.
- tests: add broken-bot and workspace-not-resolvable cases; assert credentialId is set.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018asmKsWQ5Vi7T7wD9uHofz

Author: TheodoreSpeaks

apps/sim/lib/webhooks/deploy.test.ts

@@ -278,8 +278,37 @@ describe('resolveWebhookConfigForBlock — slack_oauth routing', () => {
     expect(result?.error?.message).toContain('not available in this workspace')
   })
 
+  it('rejects a deleted or secretless custom bot credential as an invalid bot', async () => {
+    mockGetSlackBotCredential.mockResolvedValue(null)
+    mockResolveOAuthAccountId.mockResolvedValue({ credentialType: 'service_account' })
+
+    const result = await resolveSlack({ eventType: 'message', customBotCredential: 'cred_bot_x' })
+
+    expect(result?.success).toBe(false)
+    if (result?.success) throw new Error('expected failure')
+    expect(result?.error?.status).toBe(400)
+    expect(result?.error?.message).toContain('bot credential is missing or invalid')
+    expect(mockRefreshAccessTokenIfNeeded).not.toHaveBeenCalled()
+  })
+
+  it('rejects an OAuth credential not resolvable in the workflow workspace', async () => {
+    mockGetSlackBotCredential.mockResolvedValue(null)
+    mockResolveOAuthAccountId.mockResolvedValue({ accountId: 'acct-1' })
+    // No credential row queued → resolveTriggerCredentialId returns null.
+
+    const result = await resolveSlack({ eventType: 'message', customBotCredential: 'cred_foreign' })
+
+    expect(result?.success).toBe(false)
+    if (result?.success) throw new Error('expected failure')
+    expect(result?.error?.status).toBe(400)
+    expect(result?.error?.message).toContain('not available in this workspace')
+    expect(mockRefreshAccessTokenIfNeeded).not.toHaveBeenCalled()
+  })
+
   it('rejects a non-simSubscribed event on the native Sim app (OAuth account)', async () => {
     mockGetSlackBotCredential.mockResolvedValue(null)
+    mockResolveOAuthAccountId.mockResolvedValue({ accountId: 'acct-1' })
+    queueTableRows(credential, [{ id: 'cred_oauth_1' }])
 
     const result = await resolveSlack({
       eventType: 'file_shared',
@@ -296,6 +325,7 @@ describe('resolveWebhookConfigForBlock — slack_oauth routing', () => {
   it('routes an OAuth account by team_id on the slack_app provider', async () => {
     mockGetSlackBotCredential.mockResolvedValue(null)
     mockResolveOAuthAccountId.mockResolvedValue({ accountId: 'acct-1' })
+    queueTableRows(credential, [{ id: 'cred_oauth_1' }])
     queueTableRows(account, [{ userId: 'owner-1' }])
     mockRefreshAccessTokenIfNeeded.mockResolvedValue('xoxb-token')
     mockFetchSlackTeamId.mockResolvedValue({ teamId: 'T123', userId: 'UBOT' })
@@ -308,13 +338,16 @@ describe('resolveWebhookConfigForBlock — slack_oauth routing', () => {
     expect(result.config.routingKey).toBe('T123')
     expect(result.config.triggerPath).toBeNull()
     expect(result.config.providerConfig.bot_user_id).toBe('UBOT')
+    // Runtime token resolution + disconnect cleanup key slack_app rows on this.
+    expect(result.config.providerConfig.credentialId).toBe('cred_oauth_1')
     // Owner's token, not the deploying actor's.
     expect(mockRefreshAccessTokenIfNeeded).toHaveBeenCalledWith('cred_oauth_1', 'owner-1', 'req-1')
   })
 
   it('fails when the connected Slack account token cannot be resolved', async () => {
     mockGetSlackBotCredential.mockResolvedValue(null)
     mockResolveOAuthAccountId.mockResolvedValue({ accountId: '' })
+    queueTableRows(credential, [{ id: 'cred_oauth_1' }])
     mockRefreshAccessTokenIfNeeded.mockResolvedValue(null)
 
     const result = await resolveSlack({ eventType: 'message', customBotCredential: 'cred_oauth_1' })

apps/sim/lib/webhooks/deploy.ts

@@ -469,8 +469,40 @@ export async function resolveWebhookConfigForBlock(input: {
       providerConfig.credentialId = slackCredentialId
       if (botCredential.botUserId) providerConfig.bot_user_id = botCredential.botUserId
     } else {
-      // Native Sim app: an OAuth-connected account. The app only subscribes to a
-      // fixed event set, so reject anything outside it before deriving routing.
+      // getSlackBotCredential also returns null for a custom bot credential that
+      // was deleted or lost its stored secrets. Name that case so the error
+      // directs the user to reconnect the bot rather than mislabeling it an OAuth
+      // account below.
+      const resolvedKind = await resolveOAuthAccountId(slackCredentialId)
+      if (resolvedKind?.credentialType === 'service_account') {
+        return {
+          success: false,
+          error: {
+            message: 'The selected Slack bot credential is missing or invalid. Reconnect it.',
+            status: 400,
+          },
+        }
+      }
+      // Native Sim app: a workspace OAuth Slack credential. Resolve it through the
+      // same workspace/provider-scoped lookup the generic credential path uses, so
+      // a pasted foreign or other-tenant credential id can't bind here and the
+      // canonical id is what routing and runtime token resolution key on.
+      const workflowWorkspace =
+        typeof input.workflow.workspaceId === 'string' ? input.workflow.workspaceId : undefined
+      const resolvedCredentialId = workflowWorkspace
+        ? await resolveTriggerCredentialId(slackCredentialId, workflowWorkspace, 'slack')
+        : null
+      if (!resolvedCredentialId) {
+        return {
+          success: false,
+          error: {
+            message: 'The selected Slack credential is not available in this workspace.',
+            status: 400,
+          },
+        }
+      }
+      // The shared app only subscribes to a fixed event set; reject anything
+      // outside it before deriving routing.
       const eventType =
         typeof providerConfig.eventType === 'string' ? providerConfig.eventType : null
       if (!eventType || !SIM_SUBSCRIBED_EVENTS.includes(eventType)) {
@@ -487,7 +519,7 @@ export async function resolveWebhookConfigForBlock(input: {
       // shared workspace a teammate can deploy a trigger wired to someone else's
       // Slack account.
       let tokenOwnerUserId = input.userId
-      const resolvedAccount = await resolveOAuthAccountId(slackCredentialId)
+      const resolvedAccount = await resolveOAuthAccountId(resolvedCredentialId)
       if (resolvedAccount?.accountId) {
         const [owner] = await db
           .select({ userId: account.userId })
@@ -497,7 +529,7 @@ export async function resolveWebhookConfigForBlock(input: {
         if (owner?.userId) tokenOwnerUserId = owner.userId
       }
       const botToken = await refreshAccessTokenIfNeeded(
-        slackCredentialId,
+        resolvedCredentialId,
         tokenOwnerUserId,
         input.requestId
       )
@@ -529,6 +561,9 @@ export async function resolveWebhookConfigForBlock(input: {
       }
       effectiveProvider = 'slack_app'
       effectivePath = null
+      // Runtime token resolution and credential-disconnect cleanup key native
+      // (`slack_app`) rows on providerConfig.credentialId.
+      providerConfig.credentialId = resolvedCredentialId
     }
   }
 

apps/sim/triggers/slack/oauth.ts

@@ -1,53 +1,14 @@
 import { SlackIcon } from '@/components/icons'
-import { getProviderIdFromServiceId, getScopesForService } from '@/lib/oauth/utils'
-import { getQueryClient } from '@/app/_shell/providers/get-query-client'
-import {
-  fetchOAuthCredentials,
-  OAUTH_CREDENTIAL_LIST_STALE_TIME,
-  oauthCredentialKeys,
-} from '@/hooks/queries/oauth/oauth-credentials'
-import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
-import { useSubBlockStore } from '@/stores/workflows/subblock/store'
+import { getScopesForService } from '@/lib/oauth/utils'
 import {
   SLACK_ALL_EVENT_OPTIONS,
-  SLACK_SIM_EVENT_OPTIONS,
   SLACK_SOURCE_OPTIONS,
   SLACK_THREAD_OPTIONS,
   SLACK_TRIGGER_OUTPUTS,
   slackEventsSupportingFilter,
 } from '@/triggers/slack/shared'
 import type { TriggerConfig } from '@/triggers/types'
 
-const SLACK_PROVIDER_ID = getProviderIdFromServiceId('slack')
-
-/**
- * Event options for the picker, narrowed to what the selected credential can
- * actually receive: a native Sim-app OAuth account is limited to the events the
- * shared app subscribes to, while a reusable custom bot generates its own
- * manifest and can use any event. Resolved from the credential list the picker
- * already warmed, so this reads the cache without a refetch in the common case.
- */
-async function fetchSlackEventOptions(blockId: string) {
-  const credentialId = useSubBlockStore.getState().getValue(blockId, 'customBotCredential')
-  if (typeof credentialId !== 'string' || !credentialId) return [...SLACK_ALL_EVENT_OPTIONS]
-
-  const registry = useWorkflowRegistry.getState()
-  const workspaceId = registry.hydration.workspaceId ?? undefined
-  const workflowId = registry.activeWorkflowId ?? undefined
-
-  const credentials = await getQueryClient().fetchQuery({
-    queryKey: oauthCredentialKeys.list(SLACK_PROVIDER_ID, workspaceId, workflowId),
-    queryFn: ({ signal }) =>
-      fetchOAuthCredentials({ providerId: SLACK_PROVIDER_ID, workspaceId, workflowId }, signal),
-    staleTime: OAUTH_CREDENTIAL_LIST_STALE_TIME,
-  })
-
-  const selected = credentials.find((cred) => cred.id === credentialId)
-  return selected && selected.type !== 'service_account'
-    ? [...SLACK_SIM_EVENT_OPTIONS]
-    : [...SLACK_ALL_EVENT_OPTIONS]
-}
-
 // Filter sub-block gating is derived from the catalog's `filters` field so the
 // UI conditions and the ingest route share one source of truth.
 const SOURCE_FILTER_EVENTS = slackEventsSupportingFilter('source')
@@ -69,7 +30,10 @@ const OWN_MESSAGE_EVENTS = ['message', 'app_mention', 'reaction_added', 'reactio
  *   credentialId`) to one shared ingest URL verified with the bot's own signing
  *   secret, so many triggers on the same bot share a single Request URL.
  * - Native Sim app: events route by Slack `team_id` on the official shared app
- *   (derived at deploy time via `auth.test`, no path or app setup).
+ *   (derived at deploy time via `auth.test`, no path or app setup). Only the
+ *   events the shared app subscribes to are usable; the deploy path enforces
+ *   that (`SIM_SUBSCRIBED_EVENTS`), so the event picker offers every event
+ *   rather than mutating its option set with the selected credential.
  *
  * The trigger is only reachable through the preview-gated `slack_v2` block, so
  * the native Sim-app mode inherits that gate — no separate env flag.
@@ -93,8 +57,6 @@ export const slackOAuthTrigger: TriggerConfig = {
         'The single Slack event this trigger fires on. Add another trigger block for another event.',
       required: true,
       mode: 'trigger',
-      dependsOn: ['customBotCredential'],
-      fetchOptions: fetchSlackEventOptions,
     },
     {
       id: 'customBotCredential',

apps/sim/triggers/slack/shared.ts

@@ -271,17 +271,16 @@ export const slackEventById = new Map<string, SlackEventCatalogEntry>(
   SLACK_EVENT_CATALOG.map((entry) => [entry.id, entry])
 )
 
-/** Event ids the official shared Sim app subscribes to (Sim-app gating SOT). */
+/**
+ * Event ids the official shared Sim app subscribes to. Single source of truth
+ * for the deploy-time gate that rejects a native Sim-app trigger configured with
+ * an event the shared app can't deliver.
+ */
 export const SIM_SUBSCRIBED_EVENTS: readonly string[] = SLACK_EVENT_CATALOG.filter(
   (entry) => entry.simSubscribed
 ).map((entry) => entry.id)
 
-/** Dropdown options for the native Sim app — only officially-subscribed events. */
-export const SLACK_SIM_EVENT_OPTIONS = SLACK_EVENT_CATALOG.filter(
-  (entry) => entry.simSubscribed
-).map((entry) => ({ label: entry.label, id: entry.id }))
-
-/** Dropdown options for a custom bot — every event (manifest generated on demand). */
+/** Dropdown options for the event picker — every selectable event. */
 export const SLACK_ALL_EVENT_OPTIONS = SLACK_EVENT_CATALOG.map((entry) => ({
   label: entry.label,
   id: entry.id,