fix(slack-search): acknowledge missing sources in the thread

Author: TheodoreSpeaks

apps/sim/lib/knowledge/application/slack-search/onboarding.test.ts

@@ -72,6 +72,7 @@ import {
   slackSearchConversation,
   slackSearchConversationKey,
 } from '@/lib/slack-search/conversation'
+import type { SlackSearchMessage } from '@/lib/slack-search/types'
 
 const principal = { kind: 'session', userId: 'user1', sessionId: 'session1' } as const
 const job = {
@@ -275,14 +276,18 @@ describe('Slack onboarding control delivery', () => {
     eventId: 'Ev1',
     receivedAt: new Date(),
   } as const
-  const send = (reason: 'account' | 'sources' = 'account') =>
+  const send = (
+    reason: 'account' | 'sources' = 'account',
+    message: SlackSearchMessage = job.message,
+    signal = new AbortController().signal
+  ) =>
     sendSlackSearchOnboarding(slackPrincipal, {
-      job,
+      job: { ...job, message },
       turnId: 'turn1',
       leaseId: 'lease1',
       email: state.email,
       reason,
-      signal: new AbortController().signal,
+      signal,
     })
   it('posts a thread-scoped signup link without bot secrets or email in its URL', async () => {
     const result = await send()
@@ -324,9 +329,27 @@ describe('Slack onboarding control delivery', () => {
       },
       signal: expect.any(AbortSignal),
     })
-    expect(m.post).not.toHaveBeenCalled()
     expect(m.outcome).toHaveBeenCalledWith(context.installation, 'sources_required')
   })
+  it.each([job.message.threadTs, undefined])(
+    'acknowledges missing sources in the question thread after private setup delivery: %s',
+    async (threadTs) => {
+      await send('sources', { ...job.message, threadTs })
+      expect(m.post).toHaveBeenCalledExactlyOnceWith(
+        'bot-secret',
+        {
+          channel: 'D1',
+          thread_ts: threadTs ?? job.message.messageTs,
+          text: 'I don’t have any sources I can search for you yet. Check the “Connect sources” message in our DM to get set up, then retry this question.',
+          unfurl_links: false,
+          unfurl_media: false,
+        },
+        expect.any(AbortSignal)
+      )
+      expect(m.api.mock.invocationCallOrder[1]).toBeLessThan(m.post.mock.invocationCallOrder[0])
+      expect(m.post.mock.invocationCallOrder[0]).toBeLessThan(m.outcome.mock.invocationCallOrder[0])
+    }
+  )
   it.each(['rejected', 'ambiguous'] as const)(
     'does not replace a %s ephemeral delivery with a persistent message',
     async (outcome) => {
@@ -360,6 +383,49 @@ describe('Slack onboarding control delivery', () => {
       expect(m.lease).toHaveBeenCalledWith('turn1', 'lease1')
     }
   )
+  it.each(['binding', 'lease', 'cancellation'] as const)(
+    'stops before the thread notice if %s changes after the ephemeral prompt',
+    async (change) => {
+      const controller = new AbortController()
+      m.api.mockResolvedValueOnce({
+        status: 200,
+        data: { ok: true, permalink: state.slackUrl },
+      })
+      m.api.mockImplementationOnce(async () => {
+        if (change === 'binding') m.authorize.mockResolvedValueOnce(null)
+        if (change === 'lease') m.lease.mockRejectedValueOnce(new Error('lease lost'))
+        if (change === 'cancellation') controller.abort(new Error('cancelled'))
+        return { status: 200, data: { ok: true } }
+      })
+      await expect(send('sources', job.message, controller.signal)).rejects.toThrow(
+        change === 'binding' ? 'disabled' : change === 'lease' ? 'lease lost' : 'cancelled'
+      )
+      expect(m.api).toHaveBeenCalledTimes(2)
+      expect(m.post).not.toHaveBeenCalled()
+      expect(m.outcome).not.toHaveBeenCalled()
+    }
+  )
+  it.each(['rejected', 'ambiguous'] as const)(
+    'fails without replaying either message when the sources notice delivery is %s',
+    async (outcome) => {
+      if (outcome === 'rejected') {
+        m.post.mockResolvedValueOnce({
+          status: 200,
+          data: { ok: false, error: 'channel_not_found' },
+        })
+      } else {
+        m.post.mockRejectedValueOnce(new Error('connection lost after send'))
+      }
+      await expect(send('sources')).rejects.toThrow(
+        outcome === 'rejected'
+          ? 'Could not deliver the Slack sources notice'
+          : 'connection lost after send'
+      )
+      expect(m.api).toHaveBeenCalledTimes(2)
+      expect(m.post).toHaveBeenCalledOnce()
+      expect(m.outcome).not.toHaveBeenCalled()
+    }
+  )
   it('does not retry an ambiguous post or fall back to another transport', async () => {
     m.post.mockRejectedValueOnce(new Error('connection lost after send'))
     await expect(send()).rejects.toThrow('connection lost after send')

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

@@ -155,6 +155,25 @@ export async function sendSlackSearchOnboarding(
         )
   if (response.status !== 200 || response.data.ok !== true)
     throw new Error('Could not deliver Slack onboarding')
+  if (reason === 'sources') {
+    await requireSlackSearchTurnLease(turnId, leaseId)
+    if (!(await authorizeSlackSearchInstallation(principal, job)))
+      throw new OrchestrationError('forbidden', 'Slack Search is disabled')
+    signal.throwIfAborted()
+    const reply = await postSlackMessage(
+      context.secret.botToken,
+      {
+        channel: job.message.channelId,
+        thread_ts: job.message.threadTs ?? job.message.messageTs,
+        text: 'I don’t have any sources I can search for you yet. Check the “Connect sources” message in our DM to get set up, then retry this question.',
+        unfurl_links: false,
+        unfurl_media: false,
+      },
+      signal
+    )
+    if (reply.status !== 200 || reply.data.ok !== true)
+      throw new Error('Could not deliver the Slack sources notice')
+  }
   await recordSlackSearchOutcome(
     context.installation,
     reason === 'account' ? 'account_required' : 'sources_required'