diff --git a/apps/sim/app/o/[organizationId]/settings/components/organization-search-slack.test.tsx b/apps/sim/app/o/[organizationId]/settings/components/organization-search-slack.test.tsx index 3bd128195e7..4e4b229507e 100644 --- a/apps/sim/app/o/[organizationId]/settings/components/organization-search-slack.test.tsx +++ b/apps/sim/app/o/[organizationId]/settings/components/organization-search-slack.test.tsx @@ -12,6 +12,7 @@ const mocks = vi.hoisted(() => ({ remove: vi.fn(), install: vi.fn(), refetch: vi.fn(), + copy: vi.fn(), removeError: null as Error | null, })) vi.mock('nuqs', () => ({ useQueryState: () => [null, vi.fn()] })) @@ -53,6 +54,8 @@ let container: HTMLDivElement beforeEach(() => { vi.clearAllMocks() vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true) + vi.stubGlobal('navigator', { clipboard: { writeText: mocks.copy } }) + mocks.copy.mockReset().mockResolvedValue(undefined) mocks.context.mockReturnValue({ organization: { id: 'org-1' }, viewer: { isAdmin: true } }) mocks.list.mockReturnValue({ data: { installations: [], bots: [] } }) mocks.manifest.mockReturnValue({ @@ -104,11 +107,14 @@ async function action(label: string) { } describe('Slack Search settings and shared wizard', () => { - it('starts with one setup action and the prefilled manifest, with no name or token form', async () => { + it('starts with one setup action and a Slack app link, with no manifest preview or form', async () => { await render() expect(container.querySelectorAll('button')).toHaveLength(1) await click('Set up') - expect(document.querySelector('[role="dialog"]')).toHaveTextContent('App manifest') + expect(document.querySelector('[role="dialog"]')).not.toHaveTextContent('App manifest') + expect(document.querySelector('a[href="https://api.slack.com/apps"]')).toHaveTextContent( + 'Create app in Slack' + ) expect(document.querySelectorAll('input')).toHaveLength(0) expect(mocks.manifest).toHaveBeenCalledWith('org-1', 'Sim Search') expect(mocks.install).not.toHaveBeenCalled() @@ -135,7 +141,11 @@ describe('Slack Search settings and shared wizard', () => { expect(container.textContent).toContain('Enabled') await action('Reconnect') expect(document.querySelector('[role="dialog"]')).toHaveTextContent('Reconnect Slack Search') + await click('Copy app configuration') + expect(mocks.copy).toHaveBeenCalledExactlyOnceWith('{}') expect(document.querySelector('a[href="https://api.slack.com/apps/A1"]')).not.toBeNull() + expect(document.querySelector('[role="dialog"]')).toHaveTextContent('Configuration copied') + expect(document.querySelector('pre')).toBeNull() await click('Continue') expect(document.querySelector('[role="dialog"]')).toHaveTextContent('Leave fields blank') await click('Continue') @@ -151,6 +161,40 @@ describe('Slack Search settings and shared wizard', () => { expect(mocks.install.mock.calls[0][0]).not.toHaveProperty('clientSecret') }) + it('keeps the update action available when clipboard access fails', async () => { + mocks.copy.mockRejectedValueOnce(new Error('Clipboard access denied')) + await render(true) + await action('Reconnect') + await click('Copy app configuration') + expect(document.querySelector('[role="alert"]')).toHaveTextContent('Allow clipboard access') + expect(document.querySelector('a[href="https://api.slack.com/apps/A1"]')).toBeNull() + expect(button('Copy app configuration')).toBeDefined() + await click('Copy app configuration') + expect(document.querySelector('[role="alert"]')).toBeNull() + expect(document.querySelector('a[href="https://api.slack.com/apps/A1"]')).not.toBeNull() + }) + + it('offers the same configuration update for an app shared with Slack sources', async () => { + mocks.manifest.mockReturnValue({ + data: { + manifest: '{"display_information":{"name":"Shared Slack app"}}', + existingApp: { appId: 'A2' }, + createAppUrl: 'https://api.slack.com/apps', + }, + isPending: false, + refetch: mocks.refetch, + }) + await render() + await click('Set up') + expect(document.querySelector('[role="dialog"]')).toHaveTextContent('Update your Slack app') + await click('Copy app configuration') + expect(mocks.copy).toHaveBeenCalledExactlyOnceWith( + '{"display_information":{"name":"Shared Slack app"}}' + ) + expect(document.querySelector('a[href="https://api.slack.com/apps/A2"]')).not.toBeNull() + expect(document.querySelector('pre')).toBeNull() + }) + it('disables the selected connection from the actions menu', async () => { await render(true) await action('Disable') diff --git a/apps/sim/components/integrations/slack-search-setup-wizard.tsx b/apps/sim/components/integrations/slack-search-setup-wizard.tsx index 1d345840fed..c584cb7d30d 100644 --- a/apps/sim/components/integrations/slack-search-setup-wizard.tsx +++ b/apps/sim/components/integrations/slack-search-setup-wizard.tsx @@ -10,9 +10,9 @@ import { ChipModalField, ChipModalFooter, ChipModalHeader, + writeTextToClipboard, } from '@sim/emcn' import { SlackIcon } from '@/components/icons' -import { SlackAppManifest } from '@/components/integrations/slack-app-manifest' import { SLACK_SEARCH_DEFAULT_DESCRIPTION, SLACK_SEARCH_DEFAULT_NAME, @@ -43,11 +43,26 @@ export function SlackSearchSetupWizard({ const [clientId, setClientId] = useState('') const [clientSecret, setClientSecret] = useState('') const [signingSecret, setSigningSecret] = useState('') - const error = prepare.error ?? oauth.error + const [configurationCopied, setConfigurationCopied] = useState(false) + const [copyError, setCopyError] = useState(null) + const error = prepare.error ?? oauth.error ?? copyError const busy = oauth.isPending const stepNumber = step === 'manifest' ? 1 : step === 'credentials' ? 2 : 3 const configuredAppId = appId ?? prepare.data?.existingApp?.appId + async function copyConfiguration() { + if (!prepare.data) throw new Error('Slack app configuration is not ready') + setCopyError(null) + try { + await writeTextToClipboard(prepare.data.manifest) + setConfigurationCopied(true) + } catch { + setCopyError( + new Error('Could not copy the app configuration. Allow clipboard access and try again.') + ) + } + } + function advance() { if (step === 'manifest') { setStep('credentials') @@ -101,11 +116,20 @@ export function SlackSearchSetupWizard({

)} {step === 'manifest' && prepare.data && ( - <> - + + {configuredAppId && !configurationCopied ? ( + void copyConfiguration()}>Copy app configuration + ) : ( {configuredAppId ? 'Open Slack app settings' : 'Create app in Slack'} -

- {configuredAppId - ? 'Open App Manifest in your existing app and apply the updated configuration.' - : 'Choose your Slack workspace, review the prepared manifest, then create the app.'} -

- - - - - + )} + )} {step === 'credentials' && ( <> diff --git a/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx b/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx index f9171915ed7..2314c7d445c 100644 --- a/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx +++ b/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx @@ -1,5 +1,6 @@ /** @vitest-environment jsdom */ import { act } from 'react' +import { toast } from '@sim/emcn' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -44,6 +45,8 @@ describe('Slack member access selection', () => { let root: Root let container: HTMLDivElement let client: QueryClient + let channels: Array<{ onmessage: ((event: MessageEvent) => void) | null }> + let popup: { location: { href: string }; closed: boolean; close: ReturnType } const bot: WorkspaceCredential = { id: '11111111-1111-4111-8111-111111111111', workspaceId: 'workspace-1', @@ -62,6 +65,8 @@ describe('Slack member access selection', () => { beforeEach(() => { vi.clearAllMocks() + vi.spyOn(toast, 'error').mockReturnValue('toast') + vi.spyOn(toast, 'success').mockReturnValue('toast') vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true) mocks.create.mockResolvedValue(undefined) mocks.apps.mockReturnValue({ @@ -81,17 +86,23 @@ describe('Slack member access selection', () => { state: 'state', authorizationUrl: 'https://slack.com/oauth/v2/authorize', }) + channels = [] vi.stubGlobal( 'BroadcastChannel', class { + onmessage: ((event: MessageEvent) => void) | null = null + constructor() { + channels.push(this) + } close() {} } ) - vi.spyOn(window, 'open').mockReturnValue({ + popup = { location: { href: '' }, closed: false, close: vi.fn(), - } as unknown as Window) + } + vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window) container = document.createElement('div') document.body.appendChild(container) root = createRoot(container) @@ -103,6 +114,7 @@ describe('Slack member access selection', () => { client.clear() vi.restoreAllMocks() vi.unstubAllGlobals() + vi.useRealTimers() }) async function render( @@ -166,6 +178,145 @@ describe('Slack member access selection', () => { ) } + async function completeAuthorization(state = 'state') { + await act(async () => { + for (const channel of channels) { + channel.onmessage?.( + new MessageEvent('message', { + data: { + type: 'slack-managed-users', + ok: true, + state, + credentialGroupId: 'group-1', + slackBotCredentialId: bot.id, + }, + }) + ) + } + }) + } + + it('accepts authorization after browser isolation reports a live popup as closed', async () => { + vi.useFakeTimers() + await render() + await submit() + popup.closed = true + await act(async () => vi.advanceTimersByTimeAsync(1_000)) + + expect(toast.error).not.toHaveBeenCalled() + expect(document.body.textContent).toContain('Waiting for Slack...') + await completeAuthorization('unrelated-state') + expect(toast.success).not.toHaveBeenCalled() + await completeAuthorization() + expect(toast.success).toHaveBeenCalledWith('Slack configured') + expect(mocks.onOpenChange).toHaveBeenCalledWith(false) + await act(async () => vi.advanceTimersByTimeAsync(10 * 60 * 1_000)) + expect(toast.error).not.toHaveBeenCalled() + }) + + it('expires only after the authorization deadline and ignores a late callback', async () => { + vi.useFakeTimers() + await render() + await submit() + await act(async () => vi.advanceTimersByTimeAsync(10 * 60 * 1_000 - 1)) + expect(toast.error).not.toHaveBeenCalled() + await act(async () => vi.advanceTimersByTimeAsync(1)) + expect(toast.error).toHaveBeenCalledExactlyOnceWith( + 'Slack authorization expired. Please try again.' + ) + expect(popup.close).toHaveBeenCalledOnce() + await completeAuthorization() + expect(toast.success).not.toHaveBeenCalled() + }) + + it('lets the user cancel an abandoned popup without reporting expiry', async () => { + vi.useFakeTimers() + await render() + await submit() + await clickButton('Cancel') + expect(popup.close).toHaveBeenCalledOnce() + expect(mocks.onOpenChange).toHaveBeenCalledWith(false) + await completeAuthorization() + await act(async () => vi.advanceTimersByTimeAsync(10 * 60 * 1_000)) + expect(toast.error).not.toHaveBeenCalled() + expect(toast.success).not.toHaveBeenCalled() + }) + + it('keeps a new authorization intact if an old deadline callback runs', async () => { + vi.useFakeTimers() + const timeouts = vi.spyOn(window, 'setTimeout') + await render() + await submit() + const oldDeadline = timeouts.mock.calls.find(([, delay]) => delay === 10 * 60 * 1_000)?.[0] + if (typeof oldDeadline !== 'function') throw new Error('Authorization deadline was not set') + await clickButton('Cancel') + + const nextPopup = { location: { href: '' }, closed: false, close: vi.fn() } + vi.mocked(window.open).mockReturnValueOnce(nextPopup as unknown as Window) + mocks.start.mockResolvedValueOnce({ + state: 'new-state', + authorizationUrl: 'https://slack.com/oauth/v2/authorize', + }) + await submit() + await act(async () => oldDeadline()) + + expect(toast.error).not.toHaveBeenCalled() + expect(nextPopup.close).not.toHaveBeenCalled() + expect(document.body.textContent).toContain('Waiting for Slack...') + await completeAuthorization('new-state') + expect(toast.success).toHaveBeenCalledExactlyOnceWith('Slack configured') + await act(async () => vi.advanceTimersByTimeAsync(10 * 60 * 1_000)) + expect(toast.error).not.toHaveBeenCalled() + }) + + it('does not navigate or start a timeout when authorization startup finishes after cancel', async () => { + vi.useFakeTimers() + let finishStartup!: (value: { state: string; authorizationUrl: string }) => void + mocks.start.mockReturnValueOnce( + new Promise((resolve) => { + finishStartup = resolve + }) + ) + await render() + await submit() + await clickButton('Cancel') + await act(async () => { + finishStartup({ state: 'state', authorizationUrl: 'https://slack.com/oauth/v2/authorize' }) + }) + expect(popup.location.href).toBe('') + await act(async () => vi.advanceTimersByTimeAsync(10 * 60 * 1_000)) + expect(toast.error).not.toHaveBeenCalled() + }) + + it.each(['resolve', 'reject'] as const)( + 'ignores authorization startup that completes with %s after unmount', + async (outcome) => { + vi.useFakeTimers() + let finishStartup!: () => void + mocks.start.mockReturnValueOnce( + new Promise((resolve, reject) => { + finishStartup = () => + outcome === 'resolve' + ? resolve({ + state: 'state', + authorizationUrl: 'https://slack.com/oauth/v2/authorize', + }) + : reject(new Error('Authorization startup failed')) + }) + ) + await render() + await submit() + await act(async () => root.render(null)) + await act(async () => finishStartup()) + + expect(popup.close).toHaveBeenCalledOnce() + expect(popup.location.href).toBe('') + await act(async () => vi.advanceTimersByTimeAsync(10 * 60 * 1_000)) + expect(toast.error).not.toHaveBeenCalled() + expect(toast.success).not.toHaveBeenCalled() + } + ) + it('opens Slack app setup inline and returns to member setup when canceled', async () => { await render(undefined, []) expect(document.querySelector('a')).toBeNull() @@ -199,7 +350,8 @@ describe('Slack member access selection', () => { const dialog = appSetupDialog(true) expect(dialog).toBeDefined() expect(dialog?.textContent).toContain('Step 1 of 3') - expect(dialog?.textContent).toContain('App manifest') + expect(dialog?.textContent).not.toContain('App manifest') + expect(dialog?.textContent).toContain('Create app in Slack') expect(mocks.manifest).toHaveBeenCalledWith('org-1', 'Sim Search') expect(mocks.start).not.toHaveBeenCalled() expect(mocks.create).not.toHaveBeenCalled() diff --git a/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx b/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx index ae88f11bf09..aed7460ef51 100644 --- a/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx +++ b/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx @@ -108,7 +108,7 @@ export function SlackManagedUsersModal({ const expectedState = useRef(null) const expectedCredentialId = useRef(null) const popup = useRef(null) - const popupWatcher = useRef(null) + const authorizationTimeout = useRef(null) const defaultCredentialId = initialCredentialId ? bots.some((bot) => bot.id === initialCredentialId) @@ -141,8 +141,8 @@ export function SlackManagedUsersModal({ const reset = () => { popup.current?.close() popup.current = null - if (popupWatcher.current !== null) window.clearInterval(popupWatcher.current) - popupWatcher.current = null + if (authorizationTimeout.current !== null) window.clearTimeout(authorizationTimeout.current) + authorizationTimeout.current = null expectedState.current = null expectedCredentialId.current = null setAppSetupOpen(false) @@ -160,8 +160,8 @@ export function SlackManagedUsersModal({ const verifiedCredentialId = expectedCredentialId.current expectedState.current = null expectedCredentialId.current = null - if (popupWatcher.current !== null) window.clearInterval(popupWatcher.current) - popupWatcher.current = null + if (authorizationTimeout.current !== null) window.clearTimeout(authorizationTimeout.current) + authorizationTimeout.current = null popup.current?.close() popup.current = null setPending(false) @@ -222,14 +222,17 @@ export function SlackManagedUsersModal({ useEffect( () => () => { - if (popupWatcher.current !== null) window.clearInterval(popupWatcher.current) + if (authorizationTimeout.current !== null) window.clearTimeout(authorizationTimeout.current) popup.current?.close() + popup.current = null + authorizationTimeout.current = null + expectedState.current = null + expectedCredentialId.current = null }, [] ) const handleOpenChange = (nextOpen: boolean) => { - if (pending && !nextOpen) return onOpenChange(nextOpen) if (!nextOpen) reset() } @@ -273,22 +276,23 @@ export function SlackManagedUsersModal({ requiredScopes, }, }) + if (popup.current !== opened) return expectedState.current = result.state expectedCredentialId.current = selectedBot?.id ?? null opened.location.href = result.authorizationUrl - const startedAt = Date.now() - popupWatcher.current = window.setInterval(() => { - if (!opened.closed && Date.now() - startedAt < AUTHORIZATION_TIMEOUT_MS) return - window.clearInterval(popupWatcher.current ?? undefined) - popupWatcher.current = null + /** COOP can report a live OAuth popup as closed; only the deadline expires its state. */ + authorizationTimeout.current = window.setTimeout(() => { + if (popup.current !== opened) return + authorizationTimeout.current = null opened.close() popup.current = null expectedState.current = null expectedCredentialId.current = null setPending(false) toast.error('Slack authorization expired. Please try again.') - }, 500) + }, AUTHORIZATION_TIMEOUT_MS) } catch (authorizationError) { + if (popup.current !== opened) return opened.close() popup.current = null setPending(false) @@ -318,15 +322,10 @@ export function SlackManagedUsersModal({ - handleOpenChange(false)} - closeDisabled={pending} - > + handleOpenChange(false)}> {title} @@ -467,7 +466,6 @@ export function SlackManagedUsersModal({ handleOpenChange(false)} - cancelDisabled={pending} {...(needsApp ? { primaryAction: { diff --git a/apps/sim/lib/knowledge/application/slack-search/onboarding.test.ts b/apps/sim/lib/knowledge/application/slack-search/onboarding.test.ts index 3c46da18d63..7f85d50b285 100644 --- a/apps/sim/lib/knowledge/application/slack-search/onboarding.test.ts +++ b/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 = () => + 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: 'account', - signal: new AbortController().signal, + reason, + signal, }) it('posts a thread-scoped signup link without bot secrets or email in its URL', async () => { const result = await send() @@ -301,12 +306,126 @@ describe('Slack onboarding control delivery', () => { expect.objectContaining({ turnId: 'turn1', email: state.email }) ) }) - it('rechecks the binding and lease immediately before delivery', async () => { - m.authorize.mockResolvedValueOnce(context).mockResolvedValueOnce(null) - await expect(send()).rejects.toThrow('disabled') - expect(m.post).not.toHaveBeenCalled() - expect(m.lease).toHaveBeenCalledWith('turn1', 'lease1') + it('sends Connect sources only to the requesting Slack user without requiring an active thread', async () => { + await send('sources') + expect(m.api).toHaveBeenLastCalledWith({ + accessToken: 'bot-secret', + method: 'chat.postEphemeral', + body: { + channel: 'D1', + user: job.message.userId, + text: 'Connect your sources', + blocks: expect.arrayContaining([ + expect.objectContaining({ + type: 'actions', + elements: [ + expect.objectContaining({ + text: { type: 'plain_text', text: 'Connect sources' }, + url: 'https://sim.test/slack-search/connect/opaque-token', + }), + ], + }), + ]), + }, + signal: expect.any(AbortSignal), + }) + 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) => { + m.api.mockResolvedValueOnce({ + status: 200, + data: { ok: true, permalink: state.slackUrl }, + }) + if (outcome === 'rejected') { + m.api.mockResolvedValueOnce({ + status: 200, + data: { ok: false, error: 'user_not_in_channel' }, + }) + } else { + m.api.mockRejectedValueOnce(new Error('connection lost after send')) + } + await expect(send('sources')).rejects.toThrow( + outcome === 'rejected' ? 'Could not deliver Slack onboarding' : 'connection lost after send' + ) + expect(m.api).toHaveBeenCalledTimes(2) + expect(m.post).not.toHaveBeenCalled() + expect(m.outcome).not.toHaveBeenCalled() + } + ) + it.each(['account', 'sources'] as const)( + 'rechecks the binding and lease immediately before %s delivery', + async (reason) => { + m.authorize.mockResolvedValueOnce(context).mockResolvedValueOnce(null) + await expect(send(reason)).rejects.toThrow('disabled') + expect(m.post).not.toHaveBeenCalled() + expect(m.api).toHaveBeenCalledTimes(1) + 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') diff --git a/apps/sim/lib/knowledge/application/slack-search/onboarding.ts b/apps/sim/lib/knowledge/application/slack-search/onboarding.ts index b6301d7f216..4cd4c7d79b1 100644 --- a/apps/sim/lib/knowledge/application/slack-search/onboarding.ts +++ b/apps/sim/lib/knowledge/application/slack-search/onboarding.ts @@ -113,36 +113,67 @@ export async function sendSlackSearchOnboarding( await requireSlackSearchTurnLease(turnId, leaseId) if (!(await authorizeSlackSearchInstallation(principal, job))) throw new OrchestrationError('forbidden', 'Slack Search is disabled') - const response = await postSlackMessage( - context.secret.botToken, - { - channel: job.message.channelId, - thread_ts: job.message.threadTs ?? job.message.messageTs, - text, - unfurl_links: false, - unfurl_media: false, - blocks: [ - { type: 'section', text: { type: 'plain_text', text } }, - { - type: 'actions', - elements: [ - { - type: 'button', - text: { - type: 'plain_text', - text: reason === 'account' ? 'Get started with Sim' : 'Connect sources', - }, - url, - action_id: 'slack_search_onboarding', + const message = { + channel: job.message.channelId, + text, + blocks: [ + { type: 'section', text: { type: 'plain_text', text } }, + { + type: 'actions', + elements: [ + { + type: 'button', + text: { + type: 'plain_text', + text: reason === 'account' ? 'Get started with Sim' : 'Connect sources', }, - ], - }, - ], - }, - signal - ) + url, + action_id: 'slack_search_onboarding', + }, + ], + }, + ], + } + /** Channel-level ephemeral prompts also display before the first persistent thread reply exists. */ + const response = + reason === 'sources' + ? await requestSlackApi({ + accessToken: context.secret.botToken, + method: 'chat.postEphemeral', + body: { ...message, user: job.message.userId }, + signal, + }) + : await postSlackMessage( + context.secret.botToken, + { + ...message, + thread_ts: job.message.threadTs ?? job.message.messageTs, + unfurl_links: false, + unfurl_media: false, + }, + signal + ) 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' diff --git a/apps/sim/lib/slack-search/README.md b/apps/sim/lib/slack-search/README.md deleted file mode 100644 index eba002f4b5e..00000000000 --- a/apps/sim/lib/slack-search/README.md +++ /dev/null @@ -1,278 +0,0 @@ -# Enterprise Search in Slack - -Organization admins configure this under **Settings → Sim Search in Slack**. -Members DM the bot to use the existing organization Enterprise Search Assistant, -including its search/read tools, model policy, source permissions, and billing. -The orchestration is TypeScript application code; no saved workflow graph or -separate agent framework is involved. - -## Setup - -1. The wizard generates the **Sim Search** app manifest with the default - description, “Ask questions about your organization’s knowledge and get - answers with sources.” Create the app in your Slack workspace using the - prepared link or copyable JSON preview. -2. Copy **Client ID**, **Client Secret**, and **Signing Secret** from Slack’s - **Basic Information** page into the wizard. -3. Choose **Install in Slack** and complete Slack OAuth. The bot token comes from - OAuth; it is never entered manually. -4. The callback rechecks the initiating admin’s current access, validates the - app/workspace/bot identity and granted scopes, and atomically enables Search. - -Slack source setup and Settings → Sim Search in Slack open the same -`components/integrations/slack-search-setup-wizard.tsx`. Source setup prompts -**Install Sim Search first** when the organization has no registered app. Once -installed, source setup only verifies member authorization against that app; -it never asks for a second client ID or client secret. Members then connect -their individual accounts through the existing source connection flow. - -Every installation uses one code-defined app configuration. The wizard has no -feature switches and stores no per-app capabilities. The bot grants -`assistant:write`, `chat:write`, `im:history`, `im:write`, `app_mentions:read`, -`users:read`, and `users:read.email`. Agent View is enabled, with `message.im`, -`app_mention`, and `agent_session_stopped` subscriptions. - -The same app supplies separate member OAuth grants for channel and DM indexing: -`users:read`, `users:read.email`, and the read/history scopes for channels, -private channels, DMs, and group DMs. Installing the bot does not authorize a -member's personal history. Members connect through the existing source flow; -connector settings choose which conversations to index. DM indexing is opt-in. -Existing member scopes are preserved when regenerating the app manifest. -Enterprise Grid deployment and token rotation remain disabled. - -Organization credential groups reference the registered `slack_app` instead of -storing another copy of its client credentials. Setting up Search adopts an -existing member app only when its app and workspace identities match. Reconnect -reuses saved app credentials unless the admin supplies replacements. - -`NEXT_PUBLIC_APP_URL` must be the public HTTPS origin used consistently by both: - -- Events and interactivity: `/api/webhooks/slack` -- Bot OAuth callback: `/api/knowledge/slack/oauth/callback` -- Member app validation: `/api/credential-groups/slack-managed-users/callback` -- Member enrollment: `/api/credential-groups/oauth/slack/callback` - -Redis is required for ten-minute, single-use OAuth state. The encrypted setup -attempt is bound to the initiating user, browser session, organization, and -expected installation revision. A different session cannot consume it. App -secrets are never included in setup read responses. - -## Storage and deployment - -Apply `0332_slack_search` after the existing staging migration history. Local development can use `bun run -db:push` from `packages/db`. Current staging schema requires PostgreSQL 15 or -newer; PostgreSQL 17 with pgvector is suitable. - -The credential-to-app foreign key uses `NOT VALID` for the expand deployment, -following the repository's migration-safety playbook. It enforces new writes; -the added nullable column has no pre-existing non-null values. -**contract-pending(after #7644 is fully deployed):** validate -`credential_slack_app_id_slack_app_id_fk` in a separate migration. - -| Table | Purpose | -| --- | --- | -| `slack_app` | Slack app ID, custom/shared ownership, client ID, encrypted client and signing secrets, and configuration revision. | -| `credential` | Organization-owned bot token with `workspace_id = NULL` and `slack_app_id` referencing the app configuration. | -| `slack_search_installation` | Verified app/workspace/bot binding to an organization and credential, enabled state, and revisions. | -| `copilot_chats` | Ordinary private Mothership chat with a unique external conversation key and Slack sender/root/Stop metadata. | -| `slack_search_turn` | External conversation key, deduplicated event, FIFO ordinal, execution lease, status, and outcome. | -| `outbox_event` | Durable wake-up delivery for pending Slack turns. | - -Slack threads use the existing `copilot_chats.id` UUID in history URLs: -`/o/[organizationId]/chat/[chatId]`. `external_conversation_key` is a namespaced -JSON tuple of installation ID, DM channel ID, and root Slack timestamp, with a -unique database index. It is an external identity, not an authorization token. -`external_conversation_metadata` stores the stable Slack user ID and Stop -watermark; `user_id` owns the private chat. Email is resolved and checked against -current verified membership, never used as the conversation key. No separate -Slack thread table is needed. Forking a chat does not copy its external binding. - -The first durable event binds the Slack sender even before they have a Sim -account; a chat is created only after member authorization. A changed Sim -identity or deleted chat cannot silently rebind the same conversation. Event -IDs are unique per installation, independently of the conversation key. -Installation locks and database uniqueness prevent concurrent deliveries from -creating duplicate chats or executing one turn twice. - -Slack chat titles use the same `requestChatTitle` backend and organization -billing protocol as normal chats, based on the thread's first question, with -` (Slack)` appended. Naming runs alongside the response, including source-setup -replies, with a fifteen-second request deadline. Existing `Slack Search` -placeholders are eligible on the next Slack message; generated and manually -chosen titles are preserved. Saving rechecks current access and compares the -original title so an in-flight request cannot overwrite a manual rename. -Naming failures are logged separately from answer delivery and do not create a -substitute title. - -The turn table remains execution bookkeeping, not a second transcript. Generic -`async_jobs` currently does not provide the required per-conversation FIFO, -installation concurrency cap, and durable execution lease. All chat messages -continue to use the existing Mothership persistence and history paths. - -Only one enabled Search installation can own a Slack workspace. Reconnecting -preserves the installation and credential IDs and requires the same Slack app -and workspace. Disable, removal, and credential/configuration rotation invalidate -pending authority. Removing Search leaves the reusable credential intact. - -**Register the existing platform Slack app before deploying unified ingress.** -Use its verified app ID and existing deployment credentials: - -```sh -cd apps/sim -bun --env-file=.env scripts/register-platform-slack-app.ts A_VERIFIED_APP_ID -``` - -The script reads `SLACK_CLIENT_ID`, `SLACK_CLIENT_SECRET`, and -`SLACK_SIGNING_SECRET`, encrypts the secrets, and registers the app as shared. -It refuses to overwrite a custom app. Unknown app IDs fail authentication; -ingress does not silently substitute environment signing secrets. Resolve any -existing conflicting active workspace bindings before applying the unique index. - -## Request lifecycle - -1. Unified ingress uses the untrusted `api_app_id` only to select a candidate - signing key. It verifies the raw-body Slack signature before resolving the - app/workspace installation through an authorized application operation. - URL verification is bounded and challenge-only; it cannot dispatch work. -2. `dispatcher.ts` accepts human DMs, channel mentions, and native Stop events. Existing platform - workflow dispatch remains in place. The old - `/api/webhooks/slack/custom/[credentialId]` URLs are compatibility adapters - using the same Search dispatcher and provider primitives. -3. Intake atomically persists a deduplicated turn and outbox event. PostgreSQL - installation locks coordinate workers: two active threads per installation, - one active turn per thread, FIFO follow-ups, and at most twenty pending turns - per thread. The three-minute execution deadline starts after a claim. - Expired executions are marked failed and never replayed after an ambiguous - external send. The shared outbox processor repairs missed wake-ups. -4. `knowledge/application/slack-search/assistant.ts` resolves the sender’s email - through that installation’s bot credential. It requires exactly one current, - verified Sim member of the bound organization. Email never selects the - organization. Existing managed identity conflicts are rejected. -5. A short-lived member delegation authorizes the ordinary private organization - Assistant conversation. The original DM timestamp becomes the thread root; - replies reuse that conversation. Execution holds the existing chat stream - lock and uses `buildCopilotRequestPayload` and - `runHeadlessCopilotLifecycle` with the normal Assistant tools, environment - projection, model configuration, permissions, and organization billing. -6. Installation revision, credential version, current member authorization, - execution lease, and chat lock are rechecked during execution and before - delivery. Sim history uses the existing message persistence/finalization path - and remains private to the acting member. - -A bot mention in a public or private channel starts a private DM thread for the -sender. Each mention starts its own thread; subsequent DM replies use the same -FIFO queue and private Sim conversation. The original channel, thread, and -message timestamps are retained in the chat's Slack origin metadata. The bot -posts no search results in the source channel. A failed or ambiguous DM creation -is terminal and does not replay the Assistant run. - -Questions longer than 2,000 characters receive a private length notice without -starting an Assistant run. DMs receive the notice in their original thread; -channel mentions receive it as the root of the new private DM thread. - -## Member onboarding - -A sender without a matching verified Sim organization membership receives a -**Get started with Sim** button in the original DM thread. The destination -offers the existing signup and login flows and preserves the return path through -authentication. Creating an account does not grant organization membership: -existing invitations, SSO, email verification, and permission policies still -apply. A wrong account or conflicting identity cannot see the original question -or organization details on the return page. - -For an authorized member with no accessible indexed documents, the bot sends a -**Connect sources** button without starting an Assistant run. The return page -opens the organization's existing Integrations page directly, using the same -source list and connection dialogs as normal visits. Slack adds only indexing, -retry, and thread-return actions to its header. Shared documents that the member can already search do not -require an additional personal connection. Readiness uses the normal document -access predicate, including source approval and current source ACL evidence. - -Once indexing makes documents searchable, **Retry question in Slack** explicitly -queues the original question in the same Slack thread. Opening the page never -runs a question. Repeated clicks reuse one durable retry event; the transaction -binds the current Sim user to the thread before a worker starts. Execution and -delivery recheck current membership, installation revision, and credential -version through the ordinary Assistant lifecycle. Successful member setup -notices are also saved in that member's private Sim conversation. - -`slack-search/onboarding-state.ts` stores opaque navigation context in Redis for -24 hours, keyed by a hash of a random token. It references the original durable -turn, sender email, and Slack-provided thread permalink. The Slack button's link carries no -email, bot credential, or organization ID and grants no authority. Every read -and retry requires a session and re-resolves the current Slack identity. Expired -links, disabled installations, identity changes, and ambiguous failed deliveries -cannot replay the original run. No additional database tables are required. - -The application behavior lives in -`knowledge/application/slack-search/onboarding.ts` and `source-status.ts`. -`/slack-search/connect/[token]` handles account setup and redirects authorized -members to `/o/[organizationId]/integrations?slack=[token]`; -`/api/knowledge/slack/onboarding` and its `/retry` endpoint use the shared route -builders and contracts. - -## Streaming and Stop - -`assistant-stream.ts` calls the same `chat.startStream`, `chat.appendStream`, -and `chat.stopStream` provider primitives as Slack blocks. It batches public -Assistant text, excludes reasoning and tool-scoped text, and projects secrets -before delivery. Sim-only UI payloads such as suggested follow-up options are -withheld even when their tags span multiple deltas. Text before and after a tool -call is separated into paragraphs. Answers with active secret literals are held until complete -so a secret split across deltas cannot leak. - -The shared citation evidence parser accepts successful retrieval results. -Up to five source buttons use verified retrieval URLs; model-generated URLs and -incomplete citation markup are not sent. Stream failures abort execution and -record a failed outcome without switching delivery methods or replaying the run. -A failed turn closes an established stream once with a generic error and saves -that error in private Sim history. Cleanup has its own five-second signal and -rechecks installation, membership, lease, and chat ownership after execution -aborts. It never retries an ambiguous stop or guesses a stream ID after a failed -start. Cleanup failures remain part of the recorded turn failure. Only a persisted -native Stop event marks the response as stopped by the user. - -Slack’s native Stop cancels the active turn and pending follow-ups for the -authenticated sender’s thread. A persisted timestamp watermark also rejects -late deliveries from before Stop; a replayed Stop cannot cancel later messages. -[Slack stops active streaming messages when the user clicks Stop](https://docs.slack.dev/ai/agent-sessions/#stopping-a-session). -The handler cancels Sim work and clears Slack’s processing state; failure cleanup -does not send an additional stop or error message for this native cancellation. - -## Verification - -Focused Vitest suites cover OAuth identity/replay/authorization, signature -rejection and routing, reconnects, public streaming and citations, cancellation, -and delivery failures. `knowledge/__integration__/slack-search-turns.integration.ts` -exercises real PostgreSQL deduplication, competing claims, FIFO, concurrency and -queue bounds, expired leases, and private conversation ownership. It creates and -cleans only its own fixture rows; normal integration runs use the repository’s -disposable-database harness. - -Live validation requires a reachable Assistant backend implementing -`SIM_AGENT_API_URL` + `/api/mothership`, an HTTPS tunnel to this checkout, and an -installed Slack app. Verify real streamed answers and source links, contextual -and queued follow-ups, native Stop, matching private Sim history, and -disable/reconnect behavior before declaring the live flow complete. - -### Manual validation - -Use a dedicated local server and one public HTTPS origin for all Slack callbacks. -Install the generated app manifest through the shared wizard, then verify: - -- A bot DM streams an answer with source links and appears in the sender's - private Sim history; a thread follow-up keeps the same conversation. -- Channel mentions start private DM threads without posting knowledge in the - source channel. -- Queued follow-ups run FIFO, duplicate events do not execute twice, and Slack's - native Stop control cancels the active turn and queued follow-ups. -- Account onboarding and source setup return to the original Slack question. -- Member OAuth uses the installed app; opt-in DM indexing respects each - connected member's access. -- Disable, reconnect, removed membership, and ambiguous delivery failures stop - stale work without replaying a response. - -Live validation of the expanded member OAuth, mentions, DM indexing, and native -Stop UI is still pending. Automated coverage does not replace those checks. -Shared-app installation UX, channel-visible answers, attachment ingestion, -interactive pagination/modals, and workflow branching remain future work. diff --git a/apps/sim/lib/slack-search/manifest.test.ts b/apps/sim/lib/slack-search/manifest.test.ts index c89305e5e65..6008d09d0e2 100644 --- a/apps/sim/lib/slack-search/manifest.test.ts +++ b/apps/sim/lib/slack-search/manifest.test.ts @@ -29,7 +29,12 @@ describe('Search app manifest', () => { ]) ) expect(manifest.settings.event_subscriptions.bot_events).toEqual( - expect.arrayContaining(['message.im', 'app_mention', 'agent_session_stopped']) + expect.arrayContaining([ + 'app_home_opened', + 'message.im', + 'app_mention', + 'agent_session_stopped', + ]) ) expect(manifest.settings.event_subscriptions.bot_events).not.toContain('message.channels') expect(manifest.features.app_home.messages_tab_read_only_enabled).toBe(false) @@ -63,6 +68,7 @@ describe('Search app manifest', () => { 'https://search-test.ngrok.app/api/webhooks/slack' ) expect(manifest.settings.event_subscriptions.bot_events).toEqual([ + 'app_home_opened', 'message.im', 'app_mention', 'agent_session_stopped', diff --git a/apps/sim/lib/slack-search/manifest.ts b/apps/sim/lib/slack-search/manifest.ts index 7d021bd33eb..2407534840c 100644 --- a/apps/sim/lib/slack-search/manifest.ts +++ b/apps/sim/lib/slack-search/manifest.ts @@ -52,7 +52,7 @@ export function createSlackSearchManifest( settings: { event_subscriptions: { request_url: webhookUrl, - bot_events: ['message.im', 'app_mention', 'agent_session_stopped'], + bot_events: ['app_home_opened', 'message.im', 'app_mention', 'agent_session_stopped'], }, interactivity: { is_enabled: true, request_url: webhookUrl }, org_deploy_enabled: false, diff --git a/apps/sim/lib/slack-search/queue.test.ts b/apps/sim/lib/slack-search/queue.test.ts index 7d674b360d6..124b3c5ac52 100644 --- a/apps/sim/lib/slack-search/queue.test.ts +++ b/apps/sim/lib/slack-search/queue.test.ts @@ -1,8 +1,11 @@ /** @vitest-environment node */ import { beforeEach, describe, expect, it, vi } from 'vitest' -const mocks = vi.hoisted(() => ({ enqueue: vi.fn(), run: vi.fn() })) -vi.mock('@/lib/core/async-jobs', () => ({ getJobQueue: async () => ({ enqueue: mocks.enqueue }) })) +const mocks = vi.hoisted(() => ({ enqueue: vi.fn(), run: vi.fn(), externalEnqueue: vi.fn() })) +vi.mock('@/lib/core/async-jobs', () => ({ + getInlineJobQueue: async () => ({ enqueue: mocks.enqueue }), + getJobQueue: async () => ({ enqueue: mocks.externalEnqueue }), +})) vi.mock('@/lib/slack-search/handlers/search-message', () => ({ handleSlackSearchMessage: mocks.run, })) @@ -32,11 +35,12 @@ describe('Slack Search queue', () => { ]) } }) - it('supplies the same handler to the database runner', async () => { + it('runs in the app process even when the default queue has an external worker', async () => { const signal = new AbortController().signal await enqueueSlackSearch(job) await mocks.enqueue.mock.calls[0][2].runner(job, signal) expect(mocks.run).toHaveBeenCalledWith(job, signal) + expect(mocks.externalEnqueue).not.toHaveBeenCalled() }) it('propagates enqueue failures so ingress can request a retry', async () => { mocks.enqueue.mockRejectedValueOnce(new Error('unavailable')) diff --git a/apps/sim/lib/slack-search/queue.ts b/apps/sim/lib/slack-search/queue.ts index cafb95d408d..d50e42b1f0b 100644 --- a/apps/sim/lib/slack-search/queue.ts +++ b/apps/sim/lib/slack-search/queue.ts @@ -1,13 +1,13 @@ import { generateId } from '@sim/utils/id' -import { getJobQueue } from '@/lib/core/async-jobs' +import { getInlineJobQueue } from '@/lib/core/async-jobs' import { SLACK_SEARCH_CONCURRENCY, SLACK_SEARCH_MAX_DURATION_SECONDS, } from '@/lib/slack-search/constants' -/** Wakes a durable turn; a repeated wake cannot repeat a claimed execution. */ +/** Runs durable turns in the app process; repeated wakes cannot repeat a claimed execution. */ export async function enqueueSlackSearch(input: { turnId: string; installationId: string }) { - return (await getJobQueue()).enqueue( + return (await getInlineJobQueue()).enqueue( 'slack-search', { turnId: input.turnId }, { diff --git a/apps/sim/lib/slack-search/types.test.ts b/apps/sim/lib/slack-search/types.test.ts index 019bd541144..d222a32165a 100644 --- a/apps/sim/lib/slack-search/types.test.ts +++ b/apps/sim/lib/slack-search/types.test.ts @@ -67,6 +67,7 @@ describe('Slack Search message dispatch', () => { }) }) it.each([ + { type: 'app_home_opened', tab: 'messages' }, { channel_type: 'channel' }, { channel_type: 'mpim' }, { bot_id: 'B1' },