From 385673566c718d76fab10819cb464eecea871437 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Wed, 9 Sep 2026 13:17:53 -0700 Subject: [PATCH 1/4] fix(slack): expose account setup removal in sources --- .../integrations/integrations.test.tsx | 1 - .../integrations/integrations.tsx | 7 +- .../integrations/slack-account-removal.tsx | 51 +++++++++ .../[connectorType]/provider-detail.test.tsx | 103 ++++++++++++++++++ .../[connectorType]/provider-detail.tsx | 29 ++++- .../[workspaceId]/search/search.test.tsx | 1 - .../workspace/[workspaceId]/search/search.tsx | 7 +- .../organization-account-people.tsx | 11 +- .../queries/kb/connectors-cache.test.tsx | 2 +- apps/sim/hooks/queries/kb/connectors.test.ts | 2 +- apps/sim/hooks/queries/kb/connectors.ts | 32 +----- apps/sim/hooks/queries/kb/knowledge.ts | 3 +- .../kb/organization-search-overview.test.tsx | 8 +- .../kb/search-source-progress.test.tsx | 9 +- .../queries/organization-accounts.test.tsx | 75 ++++++++++++- .../hooks/queries/organization-accounts.ts | 8 ++ apps/sim/hooks/queries/search-integrations.ts | 2 +- apps/sim/hooks/queries/slack-search.ts | 4 +- .../hooks/queries/utils/search-source-keys.ts | 32 ++++++ 19 files changed, 323 insertions(+), 64 deletions(-) create mode 100644 apps/sim/app/o/[organizationId]/settings/components/integrations/slack-account-removal.tsx create mode 100644 apps/sim/hooks/queries/utils/search-source-keys.ts diff --git a/apps/sim/app/o/[organizationId]/integrations/integrations.test.tsx b/apps/sim/app/o/[organizationId]/integrations/integrations.test.tsx index 8024deccfba..ecdfb02e5ba 100644 --- a/apps/sim/app/o/[organizationId]/integrations/integrations.test.tsx +++ b/apps/sim/app/o/[organizationId]/integrations/integrations.test.tsx @@ -54,7 +54,6 @@ vi.mock('@/app/workspace/[workspaceId]/integrations/components/integrations-show vi.mock('@/hooks/queries/kb/connectors', () => ({ useSearchSources: mocks.sources, useSearchSourceOverview: mocks.overview, - searchSourceKeys: { list: (scope: unknown) => ['sources', scope] }, })) vi.mock('@/hooks/use-member-enrollment', () => ({ CONNECTABLE_MEMBERSHIPS: new Set(['invited', 'not_enrolled', 'needs_reauth']), diff --git a/apps/sim/app/o/[organizationId]/integrations/integrations.tsx b/apps/sim/app/o/[organizationId]/integrations/integrations.tsx index f07c4e8b13a..23c0d287e5c 100644 --- a/apps/sim/app/o/[organizationId]/integrations/integrations.tsx +++ b/apps/sim/app/o/[organizationId]/integrations/integrations.tsx @@ -28,12 +28,9 @@ import { RESOURCE_LIST_STACK, SettingsResourceRow, } from '@/app/workspace/[workspaceId]/settings/components/settings-resource-row' -import { - searchSourceKeys, - useSearchSourceOverview, - useSearchSources, -} from '@/hooks/queries/kb/connectors' +import { useSearchSourceOverview, useSearchSources } from '@/hooks/queries/kb/connectors' import { useSearchIntegrations } from '@/hooks/queries/search-integrations' +import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys' import { useDebounce } from '@/hooks/use-debounce' import { useMemberEnrollment } from '@/hooks/use-member-enrollment' import { useDesktopOAuthConnectListener, useOAuthReturnRouter } from '@/hooks/use-oauth-return' diff --git a/apps/sim/app/o/[organizationId]/settings/components/integrations/slack-account-removal.tsx b/apps/sim/app/o/[organizationId]/settings/components/integrations/slack-account-removal.tsx new file mode 100644 index 00000000000..2dff4d17b5f --- /dev/null +++ b/apps/sim/app/o/[organizationId]/settings/components/integrations/slack-account-removal.tsx @@ -0,0 +1,51 @@ +'use client' + +import { ChipConfirmModal, ChipModalError } from '@sim/emcn' +import type { OrganizationAccountsSettings } from '@/lib/api/contracts/organization-accounts' +import { useUpdateOrganizationAccounts } from '@/hooks/queries/organization-accounts' + +interface OrganizationSlackAccountRemovalProps { + organizationId: string + group: NonNullable + onClose: () => void + onRemoved: () => void +} + +export function OrganizationSlackAccountRemoval({ + organizationId, + group, + onClose, + onRemoved, +}: OrganizationSlackAccountRemovalProps) { + const update = useUpdateOrganizationAccounts() + return ( + { + if (!open && !update.isPending) onClose() + }} + title='Remove Slack account setup?' + text='This disconnects your organization’s Slack accounts and clears their saved app configuration. Remove any sources using these accounts first.' + confirm={{ + label: 'Remove', + variant: 'destructive', + pending: update.isPending, + onClick: () => + update.mutate( + { + organizationId, + groupId: group.id, + update: { + options: group.options + .filter((option) => option.provider !== 'slack') + .map(({ id, provider, label, required }) => ({ id, provider, label, required })), + }, + }, + { onSuccess: onRemoved } + ), + }} + > + {update.error?.message} + + ) +} diff --git a/apps/sim/app/o/[organizationId]/settings/integrations/providers/[connectorType]/provider-detail.test.tsx b/apps/sim/app/o/[organizationId]/settings/integrations/providers/[connectorType]/provider-detail.test.tsx index e138a7201b8..dd9ad344035 100644 --- a/apps/sim/app/o/[organizationId]/settings/integrations/providers/[connectorType]/provider-detail.test.tsx +++ b/apps/sim/app/o/[organizationId]/settings/integrations/providers/[connectorType]/provider-detail.test.tsx @@ -21,6 +21,9 @@ const mocks = vi.hoisted(() => ({ approvalError: null as Error | null, availabilityError: null as Error | null, retryAvailability: vi.fn(), + removeAccounts: vi.fn(), + accountRemovalError: null as Error | null, + accountRemovalPending: false, })) vi.mock('next/navigation', () => ({ @@ -60,6 +63,11 @@ vi.mock('@/hooks/queries/kb/connectors', () => ({ })) vi.mock('@/hooks/queries/organization-accounts', () => ({ useOrganizationAccounts: mocks.accounts, + useUpdateOrganizationAccounts: () => ({ + mutate: mocks.removeAccounts, + error: mocks.accountRemovalError, + isPending: mocks.accountRemovalPending, + }), })) vi.mock('@/hooks/queries/search-integrations', () => ({ useUpdateSearchIntegration: () => ({ @@ -126,6 +134,8 @@ describe('organization provider management', () => { mocks.access = { admin: true, members: true } mocks.approvalError = null mocks.availabilityError = null + mocks.accountRemovalError = null + mocks.accountRemovalPending = false mocks.overview.mockReturnValue({ data: { providers: [provider] }, isPending: false }) mocks.sources.mockReturnValue({ data: [source], @@ -168,6 +178,96 @@ describe('organization provider management', () => { await act(async () => button!.click()) } + function withSlackAccounts(approved = true, status = 'active') { + mocks.overview.mockReturnValue({ + data: { providers: [{ ...provider, connectorType: 'slack', approved }] }, + }) + mocks.accounts.mockReturnValue({ + data: { + credentialGroup: { + ...credentialGroup, + options: [ + { ...credentialGroup.options[0], label: 'Google', required: true }, + { + id: 'slack-option', + provider: 'slack', + label: 'Slack', + required: false, + status, + configurationStatus: 'ready', + }, + ], + }, + }, + }) + } + + it.each(['active', 'disabled'])( + 'removes only Slack account setup after confirmation, including a %s option', + async (status) => { + withSlackAccounts(true, status) + await render('slack') + await click('Remove account setup') + expect(mocks.removeAccounts).not.toHaveBeenCalled() + expect(document.querySelector('[role="dialog"]')).toHaveTextContent('saved app configuration') + await click('Remove') + expect(mocks.removeAccounts).toHaveBeenCalledExactlyOnceWith( + { + organizationId: 'org-one', + groupId: 'accounts-one', + update: { + options: [{ id: 'google-option', provider: 'google', label: 'Google', required: true }], + }, + }, + { onSuccess: expect.any(Function) } + ) + await act(async () => mocks.removeAccounts.mock.calls[0][1].onSuccess()) + expect(document.querySelector('[role="dialog"]')).toBeNull() + } + ) + + it('offers removal when Slack is deactivated and allows cancelling without a mutation', async () => { + withSlackAccounts(false) + await render('slack') + await click('Remove account setup') + await click('Cancel') + expect(document.querySelector('[role="dialog"]')).toBeNull() + expect(mocks.removeAccounts).not.toHaveBeenCalled() + }) + + it('keeps connector-dependency errors visible in the removal dialog', async () => { + withSlackAccounts() + mocks.accountRemovalError = new Error('Remove the source using these accounts first.') + await render('slack') + await click('Remove account setup') + await click('Remove') + expect(document.querySelector('[role="dialog"] [role="alert"]')).toHaveTextContent( + 'Remove the source using these accounts first.' + ) + }) + + it('passes the removal action to the Slack Accounts tab header', async () => { + withSlackAccounts() + await render('slack', '?view=accounts') + const actions = mocks.people.mock.calls.at(-1)![0].panel.actions + expect(actions).toEqual([ + expect.objectContaining({ text: 'Remove account setup', onSelect: expect.any(Function) }), + ]) + await act(async () => actions[0].onSelect()) + expect(document.querySelector('[role="dialog"]')).toHaveTextContent( + 'Remove Slack account setup?' + ) + }) + + it('keeps Slack cleanup available even when personal source creation is unavailable', async () => { + withSlackAccounts() + mocks.personal = false + await render('slack') + expect(mocks.accounts).toHaveBeenCalledWith('org-one') + await click('Remove account setup') + expect(document.querySelector('[role="dialog"]')).not.toBeNull() + }) + it('uses named source links even when the admin has not reconnected their own account', async () => { await render() expect(mocks.sources).toHaveBeenCalledWith( @@ -450,6 +550,9 @@ describe('organization provider management', () => { expect(container.textContent).toContain('Set up the Slack app to connect accounts.') expect(container.querySelector('input[placeholder="Search people..."]')).toHaveValue('alex') await click('Set up Slack app') + await act(async () => { + await vi.waitFor(() => expect(mocks.updateUrl).toHaveBeenCalled()) + }) const query = new URLSearchParams(mocks.updateUrl.mock.calls.at(-1)![0].queryString) expect(query.get('connectedAccounts')).toBe('slack') expect(query.get('view')).toBe('accounts') diff --git a/apps/sim/app/o/[organizationId]/settings/integrations/providers/[connectorType]/provider-detail.tsx b/apps/sim/app/o/[organizationId]/settings/integrations/providers/[connectorType]/provider-detail.tsx index 8c6d079a0f8..8080d989f0a 100644 --- a/apps/sim/app/o/[organizationId]/settings/integrations/providers/[connectorType]/provider-detail.tsx +++ b/apps/sim/app/o/[organizationId]/settings/integrations/providers/[connectorType]/provider-detail.tsx @@ -19,6 +19,7 @@ import { connectedAccountsParam, organizationProviderTabParam, } from '@/app/o/[organizationId]/settings/components/integrations/search-params' +import { OrganizationSlackAccountRemoval } from '@/app/o/[organizationId]/settings/components/integrations/slack-account-removal' import { OrganizationSlackAccountSetup } from '@/app/o/[organizationId]/settings/components/integrations/slack-account-setup' import { SearchSourcePagination } from '@/app/workspace/[workspaceId]/search/components/search-source-pagination' import { SearchSourceSetup } from '@/app/workspace/[workspaceId]/search/components/search-source-setup' @@ -59,6 +60,7 @@ export function OrganizationProviderDetail({ connectorType }: OrganizationProvid const [peopleSearch, setPeopleSearch] = useOrganizationAccountPeopleSearch() const sourceSearch = useDebounce(search.trim(), SEARCH_DEBOUNCE_MS) const [deactivating, setDeactivating] = useState(false) + const [removingSlackAccounts, setRemovingSlackAccounts] = useState(false) const scope = { kind: 'organization', organizationId: organization.id } as const const meta = CONNECTOR_META_REGISTRY[connectorType] const personal = Boolean(meta && canConnectPersonally(meta) && searchAccess.memberScoped) @@ -72,7 +74,7 @@ export function OrganizationProviderDetail({ connectorType }: OrganizationProvid const availability = usePermissionConfig() const approval = useUpdateSearchIntegration() const accounts = useOrganizationAccounts( - viewer.isAdmin && personal && (showAccounts || connectorType === 'slack') + viewer.isAdmin && (connectorType === 'slack' || (personal && showAccounts)) ? organization.id : undefined ) @@ -123,6 +125,20 @@ export function OrganizationProviderDetail({ connectorType }: OrganizationProvid const option = accounts.data?.credentialGroup?.options.find( (item) => item.provider === credentialProvider && item.status === 'active' ) + const group = accounts.data?.credentialGroup + const removalActions: SettingsAction[] = + connectorType === 'slack' && + !accounts.isError && + group?.options.some((item) => item.provider === 'slack') + ? [ + { + text: 'Remove account setup', + textTone: 'error', + disabled: accounts.isFetching, + onSelect: () => setRemovingSlackAccounts(true), + }, + ] + : [] const needsSlackSetup = connectorType === 'slack' && (option?.provider !== 'slack' || option.configurationStatus !== 'ready') @@ -170,6 +186,7 @@ export function OrganizationProviderDetail({ connectorType }: OrganizationProvid onSelect: activate, }, ] + actions.push(...removalActions) if (overview.isError) return ( @@ -297,7 +314,7 @@ export function OrganizationProviderDetail({ connectorType }: OrganizationProvid ) : ( @@ -335,6 +352,14 @@ export function OrganizationProviderDetail({ connectorType }: OrganizationProvid mirroredAccessAvailable={searchAccess.sourceMirrored} /> + {removingSlackAccounts && group && ( + setRemovingSlackAccounts(false)} + onRemoved={() => setRemovingSlackAccounts(false)} + /> + )} { diff --git a/apps/sim/app/workspace/[workspaceId]/search/search.test.tsx b/apps/sim/app/workspace/[workspaceId]/search/search.test.tsx index ea529d415c1..2012833a8f7 100644 --- a/apps/sim/app/workspace/[workspaceId]/search/search.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/search/search.test.tsx @@ -40,7 +40,6 @@ vi.mock('@/hooks/queries/workspace', () => ({ useWorkspacePermissionsQuery: () => ({ data: { viewer: { isAdmin: mocks.canAdmin } } }), })) vi.mock('@/hooks/queries/kb/connectors', () => ({ - searchSourceKeys: { list: (id: string) => ['search-sources', id] }, useSearchSources: (id: string, options: { search: string }) => { mocks.sourceQuery(id, options) return { diff --git a/apps/sim/app/workspace/[workspaceId]/search/search.tsx b/apps/sim/app/workspace/[workspaceId]/search/search.tsx index 2284ac9d141..39e424a1bf5 100644 --- a/apps/sim/app/workspace/[workspaceId]/search/search.tsx +++ b/apps/sim/app/workspace/[workspaceId]/search/search.tsx @@ -25,11 +25,8 @@ import { SettingsEmptyState, SettingsQueryErrorState, } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' -import { - searchSourceKeys, - useSearchSources, - useWorkspaceMemberConnectors, -} from '@/hooks/queries/kb/connectors' +import { useSearchSources, useWorkspaceMemberConnectors } from '@/hooks/queries/kb/connectors' +import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys' import { useWorkspacePermissionsQuery } from '@/hooks/queries/workspace' import { useDebounce } from '@/hooks/use-debounce' import { useDebouncedSearchSetter } from '@/hooks/use-debounced-search-setter' diff --git a/apps/sim/ee/credential-groups/components/organization-account-people.tsx b/apps/sim/ee/credential-groups/components/organization-account-people.tsx index 06c9a95a9fa..8bac323aff8 100644 --- a/apps/sim/ee/credential-groups/components/organization-account-people.tsx +++ b/apps/sim/ee/credential-groups/components/organization-account-people.tsx @@ -3,7 +3,7 @@ import { type ReactNode, useState } from 'react' import { Chip, ChipConfirmModal, ChipModalError, toast } from '@sim/emcn' import { Plus } from '@sim/emcn/icons' -import type { SettingsBackAction } from '@/components/settings/settings-header' +import type { SettingsAction, SettingsBackAction } from '@/components/settings/settings-header' import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state' import { MemberAvatar } from '@/app/workspace/[workspaceId]/settings/components/member-list' import { RowActionsMenu } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu' @@ -29,7 +29,13 @@ import { useOrganizationAccountPeopleSearch } from '@/hooks/use-organization-acc interface OrganizationAccountPeopleProps { organizationId: string searchConnection?: { optionId: string; providerName: string } - panel?: { back: SettingsBackAction; title: string; description?: string; docsLink?: string } + panel?: { + back: SettingsBackAction + title: string + description?: string + docsLink?: string + actions?: SettingsAction[] + } enabled?: boolean setupFallback?: ReactNode } @@ -66,6 +72,7 @@ export function OrganizationAccountPeople({ disabled: pending || awaitingSetup, onSelect: () => setInviteOpen(true), }, + ...(panel?.actions ?? []), ]} > {awaitingSetup ? ( diff --git a/apps/sim/hooks/queries/kb/connectors-cache.test.tsx b/apps/sim/hooks/queries/kb/connectors-cache.test.tsx index c8d54f82d4d..7bce795ad62 100644 --- a/apps/sim/hooks/queries/kb/connectors-cache.test.tsx +++ b/apps/sim/hooks/queries/kb/connectors-cache.test.tsx @@ -6,13 +6,13 @@ import { act } from 'react' import { QueryClient, QueryClientProvider, type QueryKey } from '@tanstack/react-query' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys' const mocks = vi.hoisted(() => ({ requestJson: vi.fn() })) vi.mock('@/lib/api/client/request', () => ({ requestJson: mocks.requestJson })) import { - searchSourceKeys, useConnectSimSearchConnector, useCreateConnector, useDeleteConnector, diff --git a/apps/sim/hooks/queries/kb/connectors.test.ts b/apps/sim/hooks/queries/kb/connectors.test.ts index f374210a0ca..dac82a534f6 100644 --- a/apps/sim/hooks/queries/kb/connectors.test.ts +++ b/apps/sim/hooks/queries/kb/connectors.test.ts @@ -3,6 +3,7 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys' const mocks = vi.hoisted(() => ({ requestJson: vi.fn(), @@ -51,7 +52,6 @@ import { connectorKeys, isConnectorSyncingOrPending, memberConnectorKeys, - searchSourceKeys, useConnectorDetail, useConnectorDocuments, useConnectorList, diff --git a/apps/sim/hooks/queries/kb/connectors.ts b/apps/sim/hooks/queries/kb/connectors.ts index 1a3698f52b4..3a79d0be8e6 100644 --- a/apps/sim/hooks/queries/kb/connectors.ts +++ b/apps/sim/hooks/queries/kb/connectors.ts @@ -61,6 +61,7 @@ import { organizationAccountsKeys } from '@/hooks/queries/organization-accounts' import { credentialGroupKeys } from '@/hooks/queries/utils/credential-group-queries' import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys' import { searchIntegrationKeys } from '@/hooks/queries/utils/search-integration-keys' +import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys' export type { SearchSourceSummary, @@ -432,37 +433,6 @@ async function startConnectorMemberEnrollment({ return response.data } -export const searchSourceKeys = { - all: ['search-sources'] as const, - lists: () => [...searchSourceKeys.all, 'list'] as const, - progress: (scope: ResourceScope | undefined, connectorIds: string[]) => - [ - ...searchSourceKeys.all, - 'progress', - scope ? resourceScopeKey(scope) : '', - connectorIds, - ] as const, - pages: ( - scope: string | ResourceScope | undefined, - filters: { search: string; mine: boolean; connectorType?: string } - ) => [...searchSourceKeys.list(scope), 'pages', filters] as const, - overview: (scope?: string | ResourceScope) => - [...searchSourceKeys.list(scope), 'overview'] as const, - organizationOverview: (organizationId: string) => - [...searchSourceKeys.list({ kind: 'organization', organizationId }), 'admin-overview'] as const, - list: (scope?: string | ResourceScope) => - [ - ...searchSourceKeys.lists(), - typeof scope === 'string' - ? scope - : scope?.kind === 'workspace' - ? scope.workspaceId - : scope - ? resourceScopeKey(scope) - : '', - ] as const, -} - export const searchIndexKeys = { all: ['search-index'] as const, details: () => [...searchIndexKeys.all, 'detail'] as const, diff --git a/apps/sim/hooks/queries/kb/knowledge.ts b/apps/sim/hooks/queries/kb/knowledge.ts index b2d4ac8014b..5870972170e 100644 --- a/apps/sim/hooks/queries/kb/knowledge.ts +++ b/apps/sim/hooks/queries/kb/knowledge.ts @@ -62,13 +62,14 @@ import { resourceScopeKey, } from '@/lib/core/resource-scope' import type { DocumentSortField, SortOrder } from '@/lib/knowledge/documents/types' -import { connectorKeys, searchSourceKeys } from '@/hooks/queries/kb/connectors' +import { connectorKeys } from '@/hooks/queries/kb/connectors' import { folderKeys } from '@/hooks/queries/utils/folder-keys' import { KNOWLEDGE_BASE_LIST_STALE_TIME, type KnowledgeQueryScope, knowledgeKeys, } from '@/hooks/queries/utils/knowledge-keys' +import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys' const logger = createLogger('KnowledgeQueries') diff --git a/apps/sim/hooks/queries/kb/organization-search-overview.test.tsx b/apps/sim/hooks/queries/kb/organization-search-overview.test.tsx index e2dc9aed9bd..565ea648c67 100644 --- a/apps/sim/hooks/queries/kb/organization-search-overview.test.tsx +++ b/apps/sim/hooks/queries/kb/organization-search-overview.test.tsx @@ -1,17 +1,15 @@ /** @vitest-environment jsdom */ + import { act } from 'react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys' const mocks = vi.hoisted(() => ({ requestJson: vi.fn() })) vi.mock('@/lib/api/client/request', () => ({ requestJson: mocks.requestJson })) -import { - searchSourceKeys, - useOrganizationSearchOverview, - useSearchSources, -} from '@/hooks/queries/kb/connectors' +import { useOrganizationSearchOverview, useSearchSources } from '@/hooks/queries/kb/connectors' let root: Root let container: HTMLDivElement diff --git a/apps/sim/hooks/queries/kb/search-source-progress.test.tsx b/apps/sim/hooks/queries/kb/search-source-progress.test.tsx index 4fe230fd650..153b7e3907e 100644 --- a/apps/sim/hooks/queries/kb/search-source-progress.test.tsx +++ b/apps/sim/hooks/queries/kb/search-source-progress.test.tsx @@ -1,18 +1,15 @@ /** @vitest-environment jsdom */ + import { act } from 'react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys' const mocks = vi.hoisted(() => ({ requestJson: vi.fn() })) vi.mock('@/lib/api/client/request', () => ({ requestJson: mocks.requestJson })) -import { - connectorKeys, - searchSourceKeys, - useSearchSources, - useTriggerSync, -} from '@/hooks/queries/kb/connectors' +import { connectorKeys, useSearchSources, useTriggerSync } from '@/hooks/queries/kb/connectors' let root: Root let container: HTMLDivElement diff --git a/apps/sim/hooks/queries/organization-accounts.test.tsx b/apps/sim/hooks/queries/organization-accounts.test.tsx index f396c2b3e54..d26b2bb931d 100644 --- a/apps/sim/hooks/queries/organization-accounts.test.tsx +++ b/apps/sim/hooks/queries/organization-accounts.test.tsx @@ -9,11 +9,84 @@ const mocks = vi.hoisted(() => ({ request: vi.fn() })) vi.mock('@/lib/api/client/request', () => ({ requestJson: mocks.request })) import { ApiClientError } from '@/lib/api/client/errors' -import { listOrganizationAccountPeopleContract } from '@/lib/api/contracts/organization-accounts' +import { + listOrganizationAccountPeopleContract, + updateOrganizationAccountsContract, +} from '@/lib/api/contracts/organization-accounts' import { organizationAccountsKeys, useOrganizationAccountPeople, + useUpdateOrganizationAccounts, } from '@/hooks/queries/organization-accounts' +import { slackSearchKeys } from '@/hooks/queries/slack-search' +import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys' + +describe('organization account setup updates', () => { + it.each([true, false])( + 'refreshes only this organization’s setup after the caller unmounts on success=%s', + async (success) => { + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true) + mocks.request.mockReset() + const response = Promise.withResolvers() + mocks.request.mockReturnValue(response.promise) + const client = new QueryClient() + const container = document.createElement('div') + const root = createRoot(container) + let mutation: ReturnType + function Probe() { + mutation = useUpdateOrganizationAccounts() + return null + } + const current = slackSearchKeys.manifest('org-1', 'Sim Search') + const renamed = slackSearchKeys.manifest('org-1', 'Custom name') + const other = slackSearchKeys.manifest('org-2', 'Sim Search') + const overview = searchSourceKeys.organizationOverview('org-1') + const otherOverview = searchSourceKeys.organizationOverview('org-2') + for (const key of [current, renamed, other]) client.setQueryData(key, { existingApp: 'A1' }) + for (const key of [overview, otherOverview]) client.setQueryData(key, { providers: [] }) + try { + await act(async () => + root.render( + + + + ) + ) + const input = { organizationId: 'org-1', groupId: 'group-1', update: { options: [] } } + let update: Promise + await act(async () => { + update = mutation.mutateAsync(input) + }) + await act(async () => + root.render({null}) + ) + await act(async () => { + if (success) { + response.resolve({}) + await update + } else { + const rejection = expect(update).rejects.toThrow('Source still uses') + response.reject(new Error('Source still uses Slack accounts')) + await rejection + } + }) + expect(mocks.request).toHaveBeenCalledExactlyOnceWith(updateOrganizationAccountsContract, { + params: { id: 'org-1', groupId: 'group-1' }, + body: { options: [] }, + }) + expect(client.getQueryState(current)?.isInvalidated).toBe(success) + expect(client.getQueryState(renamed)?.isInvalidated).toBe(success) + expect(client.getQueryState(other)?.isInvalidated).toBe(false) + expect(client.getQueryState(overview)?.isInvalidated).toBe(success) + expect(client.getQueryState(otherOverview)?.isInvalidated).toBe(false) + } finally { + await act(async () => root.unmount()) + client.clear() + vi.unstubAllGlobals() + } + } + ) +}) describe('organization people search pagination', () => { let root: Root diff --git a/apps/sim/hooks/queries/organization-accounts.ts b/apps/sim/hooks/queries/organization-accounts.ts index 21f048e062b..fce40bb80ae 100644 --- a/apps/sim/hooks/queries/organization-accounts.ts +++ b/apps/sim/hooks/queries/organization-accounts.ts @@ -38,6 +38,8 @@ import { updateOrganizationAccountsContract, updateOrganizationAccountWorkspaceAccessContract, } from '@/lib/api/contracts/organization-accounts' +import { slackSearchKeys } from '@/hooks/queries/slack-search' +import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys' export const ORGANIZATION_ACCOUNTS_STALE_TIME = 30_000 @@ -143,6 +145,12 @@ export function useUpdateOrganizationAccounts() { }), queryClient.invalidateQueries({ queryKey: organizationAccountsKeys.workspaces() }), queryClient.invalidateQueries({ queryKey: organizationAccountsKeys.personal() }), + queryClient.invalidateQueries({ + queryKey: slackSearchKeys.organizationManifests(organizationId), + }), + queryClient.invalidateQueries({ + queryKey: searchSourceKeys.organizationOverview(organizationId), + }), ]), }) } diff --git a/apps/sim/hooks/queries/search-integrations.ts b/apps/sim/hooks/queries/search-integrations.ts index c1b7213617d..4fcf974086a 100644 --- a/apps/sim/hooks/queries/search-integrations.ts +++ b/apps/sim/hooks/queries/search-integrations.ts @@ -7,9 +7,9 @@ import { updateSearchIntegrationContract, } from '@/lib/api/contracts/knowledge/search-integrations' import { resourceScopeKey } from '@/lib/core/resource-scope' -import { searchSourceKeys } from '@/hooks/queries/kb/connectors' import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys' import { searchIntegrationKeys } from '@/hooks/queries/utils/search-integration-keys' +import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys' export const SEARCH_INTEGRATIONS_STALE_TIME = 30_000 diff --git a/apps/sim/hooks/queries/slack-search.ts b/apps/sim/hooks/queries/slack-search.ts index 7219ee8270a..a6766834257 100644 --- a/apps/sim/hooks/queries/slack-search.ts +++ b/apps/sim/hooks/queries/slack-search.ts @@ -22,8 +22,10 @@ export const slackSearchKeys = { lists: () => [...slackSearchKeys.all, 'list'] as const, list: (organizationId?: string) => [...slackSearchKeys.lists(), organizationId ?? ''] as const, manifests: () => [...slackSearchKeys.all, 'manifest'] as const, + organizationManifests: (organizationId: string) => + [...slackSearchKeys.manifests(), organizationId] as const, manifest: (organizationId: string, name: string) => - [...slackSearchKeys.manifests(), organizationId, name] as const, + [...slackSearchKeys.organizationManifests(organizationId), name] as const, } export function useSlackSearchManifest(organizationId: string, name = SLACK_SEARCH_DEFAULT_NAME) { diff --git a/apps/sim/hooks/queries/utils/search-source-keys.ts b/apps/sim/hooks/queries/utils/search-source-keys.ts new file mode 100644 index 00000000000..17f69eb5683 --- /dev/null +++ b/apps/sim/hooks/queries/utils/search-source-keys.ts @@ -0,0 +1,32 @@ +import { type ResourceScope, resourceScopeKey } from '@/lib/core/resource-scope' + +export const searchSourceKeys = { + all: ['search-sources'] as const, + lists: () => [...searchSourceKeys.all, 'list'] as const, + progress: (scope: ResourceScope | undefined, connectorIds: string[]) => + [ + ...searchSourceKeys.all, + 'progress', + scope ? resourceScopeKey(scope) : '', + connectorIds, + ] as const, + pages: ( + scope: string | ResourceScope | undefined, + filters: { search: string; mine: boolean; connectorType?: string } + ) => [...searchSourceKeys.list(scope), 'pages', filters] as const, + overview: (scope?: string | ResourceScope) => + [...searchSourceKeys.list(scope), 'overview'] as const, + organizationOverview: (organizationId: string) => + [...searchSourceKeys.list({ kind: 'organization', organizationId }), 'admin-overview'] as const, + list: (scope?: string | ResourceScope) => + [ + ...searchSourceKeys.lists(), + typeof scope === 'string' + ? scope + : scope?.kind === 'workspace' + ? scope.workspaceId + : scope + ? resourceScopeKey(scope) + : '', + ] as const, +} From bec75cfee66c3c93f61395ecf7ce180c518df23c Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Wed, 9 Sep 2026 13:34:34 -0700 Subject: [PATCH 2/4] fix(slack): render source citations inline with answers --- .../lib/slack-search/assistant-stream.test.ts | 252 +++++++++++++++++- apps/sim/lib/slack-search/assistant-stream.ts | 121 ++++++--- 2 files changed, 336 insertions(+), 37 deletions(-) diff --git a/apps/sim/lib/slack-search/assistant-stream.test.ts b/apps/sim/lib/slack-search/assistant-stream.test.ts index a250f70bf77..e96c3cdbf6f 100644 --- a/apps/sim/lib/slack-search/assistant-stream.test.ts +++ b/apps/sim/lib/slack-search/assistant-stream.test.ts @@ -1,7 +1,13 @@ /** @vitest-environment node */ import { beforeEach, describe, expect, it, vi } from 'vitest' -const api = vi.hoisted(() => ({ start: vi.fn(), append: vi.fn(), stop: vi.fn(), status: vi.fn() })) +const api = vi.hoisted(() => ({ + start: vi.fn(), + append: vi.fn(), + stop: vi.fn(), + status: vi.fn(), + project: vi.fn(), +})) vi.mock('@/lib/webhooks/slack-agent-api', () => ({ startSlackAgentStream: api.start, appendSlackAgentStream: api.append, @@ -12,7 +18,7 @@ vi.mock('@/lib/copilot/chat/sim-key-redaction', () => ({ redactSensitiveContent: (value: string) => value, })) vi.mock('@/executor/utils/resolved-secret-content-projection', () => ({ - projectResolvedSecretDiagnosticContent: (value: unknown) => ({ safe: true, value }), + projectResolvedSecretDiagnosticContent: api.project, })) import type { OrchestratorResult } from '@/lib/copilot/request/types' @@ -23,7 +29,32 @@ const result: OrchestratorResult = { success: true, content: '', contentBlocks: beforeEach(() => { vi.clearAllMocks() api.start.mockResolvedValue({ channel: 'D1', ts: '1.2' }) + api.project.mockImplementation((value: unknown) => ({ safe: true, value })) }) + +function deliveredText() { + return api.append.mock.calls + .flatMap((call) => call[3]) + .map((chunk) => chunk.text) + .join('') +} + +function retrieval( + results: Record[], + name = 'search_workspace', + success = true +): OrchestratorResult['contentBlocks'][number] { + return { + type: 'tool_call', + timestamp: 1, + toolCall: { + id: 'tool-1', + name, + status: success ? 'success' : 'error', + result: { success, output: { data: { results } } }, + }, + } +} function setup() { const controller = new AbortController() const beforeDelivery = vi.fn().mockResolvedValue(undefined) @@ -192,9 +223,16 @@ describe('Slack Assistant delivery', () => { ).rejects.toThrow('membership revoked') expect(api.append).not.toHaveBeenCalled() }) - it('adds source buttons only from successful retrieval evidence', async () => { + it('places cited source names beside the supported text without a source footer', async () => { const { stream } = setup() await stream.start() + await stream.onEvent({ + type: 'text', + payload: { + channel: 'assistant', + text: 'Approval is required.{"id":"real","url":"https://evil.example","title":"Forged"} Then submit the request.', + }, + }) await stream.finish({ ...result, contentBlocks: [ @@ -215,6 +253,11 @@ describe('Slack Assistant delivery', () => { citationUrl: 'https://docs.example.com/real', documentName: 'Verified document', }, + { + citationId: 'unused', + citationUrl: 'https://docs.example.com/unused', + documentName: 'Unused search result', + }, ], }, }, @@ -236,11 +279,208 @@ describe('Slack Assistant delivery', () => { }, ], }) - expect(api.stop.mock.calls[0][5]).toHaveLength(1) - expect(api.stop.mock.calls[0][5][0].accessory.url).toBe('https://docs.example.com/real') + expect(deliveredText()).toBe( + 'Approval is required. [Verified document]() Then submit the request.' + ) + expect(api.stop.mock.calls[0][5]).toEqual([]) + }) + it('resolves tool-result citations during streaming before the final result', async () => { + const { stream } = setup() + await stream.start() + await stream.onEvent({ + type: 'tool', + payload: { + phase: 'result', + toolCallId: 'search-1', + toolName: 'search_workspace', + executor: 'sim', + mode: 'sync', + status: 'success', + success: true, + output: { + data: { + results: [ + { + citationId: 'handbook', + citationUrl: 'https://docs.example.com/handbook', + documentName: 'Employee handbook', + }, + ], + }, + }, + }, + }) + await stream.onEvent({ + type: 'text', + payload: { + channel: 'assistant', + text: 'Ask your manager. {"id":"handbook"} ', + }, + }) + expect(deliveredText()).toBe( + 'Ask your manager. [Employee handbook]() ' + ) + expect(api.stop).not.toHaveBeenCalled() + await stream.finish(result) + expect(api.stop.mock.calls[0][5]).toEqual([]) + }) + it('keeps each inline citation stable across every text boundary', () => { + const source = '{"id":"handbook"}' + const input = `Ask your manager.${source} Submit it here.${source} Done.` + const link = '[Employee handbook]()' + const sources = new Map([['handbook', link]]) + const expected = `Ask your manager. ${link} Submit it here. ${link} Done.` + let previous = '' + for (let end = 0; end <= input.length; end++) { + const current = publicSlackAnswer(input.slice(0, end), false, sources) + expect(current.startsWith(previous)).toBe(true) + expect(expected.startsWith(current)).toBe(true) + previous = current + } + expect(publicSlackAnswer(input, true, sources)).toBe(expected) + }) + it('withholds text after an unresolved citation until evidence is available', () => { + const input = 'Answer. {"id":"late"} More text. ' + expect(publicSlackAnswer(input, false)).toBe('Answer. ') + expect( + publicSlackAnswer(input, false, new Map([['late', '[Policy]()']])) + ).toBe('Answer. [Policy]() More text. ') + expect(publicSlackAnswer(input, true)).toBe('Answer. More text. ') + }) + it.each([ + ['failed retrieval', 'search_workspace', false, 'https://example.com/document'], + ['unrelated tool', 'web_search', true, 'https://example.com/document'], + ['non-web URL', 'read_document', true, 'javascript:alert(1)'], + ['embedded credentials', 'read_document', true, 'https://user:password@example.com/document'], + ])('does not link %s', async (_name, tool, success, url) => { + const { stream } = setup() + await stream.start() + await stream.onEvent({ + type: 'text', + payload: { + channel: 'assistant', + text: 'Answer. {"id":"invalid"} End.', + }, + }) + await stream.finish({ + ...result, + contentBlocks: [ + retrieval( + [{ citationId: 'invalid', citationUrl: url, documentName: 'Unsafe source' }], + tool, + success + ), + ], + }) + expect(deliveredText()).toBe('Answer. End.') + expect(api.stop.mock.calls[0][5]).toEqual([]) + }) + it('omits source metadata that fails secret projection', async () => { + const { stream } = setup() + api.project.mockImplementation((value: unknown) => + typeof value === 'string' ? { safe: true, value } : { safe: false } + ) + await stream.start() + await stream.onEvent({ + type: 'text', + payload: { + channel: 'assistant', + text: 'Answer. {"id":"private"} End.', + }, + }) + await stream.finish({ + ...result, + contentBlocks: [ + retrieval([ + { + citationId: 'private', + citationUrl: 'https://example.com/private', + documentName: 'Secret', + }, + ]), + ], + }) + expect(deliveredText()).toBe('Answer. End.') + }) + it('escapes source labels and bounds long titles without changing their destinations', async () => { + const { stream } = setup() + await stream.start() + await stream.onEvent({ + type: 'text', + payload: { + channel: 'assistant', + text: 'Answer. {"id":"source"}', + }, + }) + await stream.finish({ + ...result, + contentBlocks: [ + retrieval([ + { + citationId: 'source', + citationUrl: 'https://example.com/a_(b)?a=1&b=2', + documentName: `[Policy] & <@everyone>\n${'a'.repeat(100)}`, + }, + ]), + ], + }) + expect(deliveredText()).toContain('[\\[Policy\\] & <@everyone> ') + expect(deliveredText()).toContain(']()') + expect(deliveredText()).not.toContain('a'.repeat(60)) + }) + it('keeps an inline link intact when it crosses the append size boundary', async () => { + const { stream } = setup() + const prefix = `${'a'.repeat(3970)} ` + await stream.start() + await stream.onEvent({ + type: 'tool', + payload: { + phase: 'result', + toolCallId: 'search-1', + toolName: 'search_workspace', + executor: 'sim', + mode: 'sync', + success: true, + output: { + data: { + results: [ + { + citationId: 'policy', + citationUrl: 'https://example.com/policy', + documentName: 'Employee policy', + }, + ], + }, + }, + }, + }) + await stream.onEvent({ + type: 'text', + payload: { + channel: 'assistant', + text: `${prefix}{"id":"policy"} Done.`, + }, + }) + await stream.finish({ + ...result, + contentBlocks: [ + retrieval([ + { + citationId: 'policy', + citationUrl: 'https://example.com/policy', + documentName: 'Employee policy', + }, + ]), + ], + }) + const link = '[Employee policy]()' + expect(deliveredText()).toBe(`${prefix}${link} Done.`) + const chunks = api.append.mock.calls.flatMap((call) => call[3]) + expect(chunks.some((chunk) => chunk.text.includes(link))).toBe(true) + expect(chunks.every((chunk) => chunk.text.length <= 4000)).toBe(true) }) it.each([ - ['Answer {"id":"x","url":"https://evil.test"} done ', 'Answer done '], + ['Answer {"id":"x","url":"https://evil.test"} done ', 'Answer '], [ 'Read [untrusted](https://evil.test) and https://evil.test/x now ', 'Read untrusted and now ', diff --git a/apps/sim/lib/slack-search/assistant-stream.ts b/apps/sim/lib/slack-search/assistant-stream.ts index ae5159e789a..79485fea134 100644 --- a/apps/sim/lib/slack-search/assistant-stream.ts +++ b/apps/sim/lib/slack-search/assistant-stream.ts @@ -1,6 +1,10 @@ import { toError } from '@sim/utils/errors' import { truncate } from '@sim/utils/string' -import { collectRetrievalCitationEvidence } from '@/lib/copilot/chat/citation-evidence' +import { + collectRetrievalCitationEvidence, + parseCitationRecord, + type RetrievalCitationBlock, +} from '@/lib/copilot/chat/citation-evidence' import { redactSensitiveContent } from '@/lib/copilot/chat/sim-key-redaction' import type { StreamEvent } from '@/lib/copilot/request/session/contract' import type { OrchestratorResult } from '@/lib/copilot/request/types' @@ -14,10 +18,14 @@ import { import { projectResolvedSecretDiagnosticContent } from '@/executor/utils/resolved-secret-content-projection' import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' -/** Withholds incomplete inline markup so split citation tags and URLs never leak into a stream. */ -export function publicSlackAnswer(text: string, complete: boolean): string { +/** Resolves inline citations while withholding incomplete tags and unverified destinations. */ +export function publicSlackAnswer( + text: string, + complete: boolean, + sources: ReadonlyMap = new Map() +): string { let value = text.replace( - /<(source|options|question|thinking|usage_upgrade|credential|workspace_resource)>[\s\S]*?(?:<\/\1>|$)/g, + /<(options|question|thinking|usage_upgrade|credential|workspace_resource)>[\s\S]*?(?:<\/\1>|$)/g, '' ) if (!complete) { @@ -28,6 +36,22 @@ export function publicSlackAnswer(text: string, complete: boolean): string { if (linkStart > value.lastIndexOf(')')) end = Math.min(end, linkStart) value = value.slice(0, end) } + let answer = '' + let offset = 0 + for (const match of value.matchAll(/([\s\S]*?)(<\/source>|$)/g)) { + answer += publicSlackText(value.slice(offset, match.index)) + const source = match[2] ? parseCitationRecord(match[1]) : null + const id = typeof source?.id === 'string' ? source.id : undefined + /** A result may arrive after its citation; keep subsequent text pending until it resolves. */ + if (!complete && (!match[2] || (id !== undefined && !sources.has(id)))) return answer + const link = id === undefined ? undefined : sources.get(id) + if (link) answer += `${answer && !/\s$/.test(answer) ? ' ' : ''}${link}` + offset = match.index + match[0].length + } + return answer + publicSlackText(value.slice(offset)) +} + +function publicSlackText(value: string): string { return value .replace(/\[([^\]]*)\]\([^)]*\)/g, '$1') .replace(/<[^>]*>/g, '') @@ -37,6 +61,25 @@ export function publicSlackAnswer(text: string, complete: boolean): string { .replaceAll('>', '>') } +function sourceLink(source: Record): string { + if (typeof source.url !== 'string' || source.url.length > 3000) return '' + const url = new URL(source.url) + if ( + !['http:', 'https:'].includes(url.protocol) || + url.username || + url.password || + url.href.length > 3000 + ) + return '' + const title = typeof source.title === 'string' ? source.title.replace(/\s+/g, ' ').trim() : '' + const label = truncate(title || 'Source', 60) + .replace(/[\\`*_[\]~!]/g, '\\$&') + .replaceAll('&', '&') + .replaceAll('<', '<') + .replaceAll('>', '>') + return `[${label}](<${url.href.replaceAll('>', '%3E')}>)` +} + interface AssistantStreamOptions { token: string channel: string @@ -62,6 +105,7 @@ export class SlackSearchAssistantStream { private closed = false private closeAttempted = false private separateNextText = false + private evidence = new Map>() constructor(private readonly options: AssistantStreamOptions) {} private async deliver(action: () => Promise) { @@ -98,6 +142,18 @@ export class SlackSearchAssistantStream { async onEvent(event: StreamEvent) { if (this.failure) throw this.failure + if (event.type === 'tool' && 'phase' in event.payload && event.payload.phase === 'result') { + const { toolName, success, status, output } = event.payload + this.collectSources([ + { + toolCall: { + name: toolName, + status: status ?? (success ? 'success' : 'error'), + result: { success, output }, + }, + }, + ]) + } if (event.type === 'tool' && !event.scope) this.separateNextText = true if (event.type !== 'text' || event.payload.channel !== 'assistant' || event.scope) return if (this.separateNextText && this.text) this.text += '\n\n' @@ -107,6 +163,12 @@ export class SlackSearchAssistantStream { if (Date.now() - this.lastSentAt >= 750) await this.flush(false) } + private collectSources(blocks: readonly RetrievalCitationBlock[]) { + for (const [id, source] of collectRetrievalCitationEvidence(blocks)) { + if (!this.evidence.has(id)) this.evidence.set(id, source) + } + } + private async flush(complete: boolean) { const { registry, token, controller } = this.options if (!registry.isComplete()) throw new Error('Answer secret provenance is unavailable') @@ -115,11 +177,31 @@ export class SlackSearchAssistantStream { const projection = projectResolvedSecretDiagnosticContent(this.text, registry, 512_000) if (!projection.safe || typeof projection.value !== 'string') throw new Error('Answer could not be safely projected') - const text = publicSlackAnswer(redactSensitiveContent(projection.value), complete) + const sources = new Map() + for (const [id, source] of this.evidence) { + const projected = projectResolvedSecretDiagnosticContent(source, registry) + sources.set( + id, + projected.safe && JSON.stringify(projected.value) === JSON.stringify(source) + ? sourceLink(source) + : '' + ) + } + const text = publicSlackAnswer(redactSensitiveContent(projection.value), complete, sources) if (!text.startsWith(this.sent)) throw new Error('The safe answer changed after delivery') let pending = text.slice(this.sent.length) while (pending.length) { - const chunk = pending.slice(0, 4000) + let end = Math.min(4000, pending.length) + /** Keep each verified link in one append so Slack never briefly displays a partial URL. */ + for (const match of pending.matchAll(/\[(?:\\.|[^[\]\\])*\]\(<[^>]*>\)/g)) { + if (match.index >= end) break + if (match.index + match[0].length > end) { + end = match.index + break + } + } + if (end === 0) throw new Error('Slack citation exceeds the supported chunk size') + const chunk = pending.slice(0, end) await this.deliver(async () => { if (!this.stream || this.closed) throw new Error('Slack stream is not active') await appendSlackAgentStream( @@ -138,32 +220,9 @@ export class SlackSearchAssistantStream { async finish(result: OrchestratorResult) { if (this.failure) throw this.failure + this.collectSources(result.contentBlocks) await this.flush(true) - const evidence = collectRetrievalCitationEvidence(result.contentBlocks) - const blocks: Record[] = [] - const seen = new Set() - for (const source of evidence.values()) { - if (typeof source.url !== 'string' || seen.has(source.url) || source.url.length > 3000) - continue - const projection = projectResolvedSecretDiagnosticContent(source, this.options.registry) - if (!projection.safe || JSON.stringify(projection.value) !== JSON.stringify(source)) continue - seen.add(source.url) - blocks.push({ - type: 'section', - text: { - type: 'plain_text', - text: truncate(typeof source.title === 'string' ? source.title : 'Source', 150), - }, - accessory: { - type: 'button', - text: { type: 'plain_text', text: 'Open source' }, - url: source.url, - action_id: `slack_search_source_${blocks.length}`, - }, - }) - if (blocks.length === 5) break - } - await this.close(blocks) + await this.close([]) } /** A confirmed Assistant failure closes the established stream without exposing backend errors. */ From 451258ce648492b683c9390e87eabfefa87dc497 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Wed, 9 Sep 2026 13:51:13 -0700 Subject: [PATCH 3/4] feat(slack): add personalized sources to app Home --- .../authorized-knowledge-use-case.ts | 9 +- .../knowledge/application/operations.test.ts | 13 +- .../lib/knowledge/application/operations.ts | 50 +++- .../application/search-sources.test.ts | 67 +++++ .../knowledge/application/search-sources.ts | 8 +- .../application/slack-search/assistant.ts | 26 +- .../application/slack-search/home.test.ts | 281 ++++++++++++++++++ .../application/slack-search/home.ts | 188 ++++++++++++ .../slack-search/member-principal.ts | 21 ++ apps/sim/lib/slack-search/dispatcher.test.ts | 57 ++++ apps/sim/lib/slack-search/dispatcher.ts | 18 ++ apps/sim/lib/slack-search/home.test.ts | 175 +++++++++++ apps/sim/lib/slack-search/home.ts | 165 ++++++++++ apps/sim/lib/slack-search/manifest.test.ts | 1 + apps/sim/lib/slack-search/manifest.ts | 2 +- 15 files changed, 1036 insertions(+), 45 deletions(-) create mode 100644 apps/sim/lib/knowledge/application/slack-search/home.test.ts create mode 100644 apps/sim/lib/knowledge/application/slack-search/home.ts create mode 100644 apps/sim/lib/knowledge/application/slack-search/member-principal.ts create mode 100644 apps/sim/lib/slack-search/dispatcher.test.ts create mode 100644 apps/sim/lib/slack-search/home.test.ts create mode 100644 apps/sim/lib/slack-search/home.ts diff --git a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts index e2b2cba69f3..e11ea2ab337 100644 --- a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts +++ b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts @@ -21,11 +21,10 @@ import type { ScopedKnowledgeOperation } from '@/lib/knowledge/application/opera type KnowledgePrincipalForOperation = | PrincipalForOperation - | ('copilot' extends NonNullable[number] - ? O['minimumRole'] extends 'read' - ? OrganizationDelegatedPrincipal - : never - : never) + | Extract< + OrganizationDelegatedPrincipal, + { serviceId: NonNullable[number] } + > function requireKnowledgePrincipal( principal: Principal, diff --git a/apps/sim/lib/knowledge/application/operations.test.ts b/apps/sim/lib/knowledge/application/operations.test.ts index d5732c64c53..b7867fb6b03 100644 --- a/apps/sim/lib/knowledge/application/operations.test.ts +++ b/apps/sim/lib/knowledge/application/operations.test.ts @@ -7,11 +7,15 @@ import { describe, expect, it } from 'vitest' import { knowledgeOperations } from '@/lib/knowledge/application/operations' describe('knowledge operation registry', () => { - it('limits Slack member delegation to the existing search operation', () => { + it('limits Slack member delegation to search and the personalized source list', () => { const allowed = Object.values(knowledgeOperations).filter((operation) => operation.organizationOperation.delegatedServices?.includes('slack-search') ) - expect(allowed).toEqual([knowledgeOperations.search]) + expect(allowed).toEqual([knowledgeOperations.search, knowledgeOperations.listSearchSources]) + expect(knowledgeOperations.listSearchSources.organizationOperation.delegatedServices).toEqual([ + 'slack-search', + ]) + expect(knowledgeOperations.listSearchSources.principalKinds).toEqual(['session']) }) it('defines unique stable semantic operation IDs', () => { const ids = Object.values(knowledgeOperations).map((operation) => operation.id) @@ -144,12 +148,13 @@ describe('knowledge operation registry', () => { } }) - it('permits organization delegation only for Copilot reads', () => { + it('permits organization delegation only for explicitly delegated reads', () => { for (const operation of Object.values(knowledgeOperations)) { if (!operation.organizationOperation.principalKinds.includes('organization_delegated')) continue expect(operation.minimumRole).toBe('read') - expect(operation.delegatedServices).toContain('copilot') + if (operation !== knowledgeOperations.listSearchSources) + expect(operation.delegatedServices).toContain('copilot') expect(operation.organizationOperation.delegationAudience).toBe('sim:knowledge') } expect(knowledgeOperations.search.organizationOperation.principalKinds).toContain( diff --git a/apps/sim/lib/knowledge/application/operations.ts b/apps/sim/lib/knowledge/application/operations.ts index b72dadcf115..4423bbedcdf 100644 --- a/apps/sim/lib/knowledge/application/operations.ts +++ b/apps/sim/lib/knowledge/application/operations.ts @@ -1,3 +1,4 @@ +import type { OrganizationDelegatedPrincipal } from '@sim/auth/principal' import { type ApplicationOperation, assertOperationCapability, @@ -15,19 +16,47 @@ export type ScopedKnowledgeOperation { organizationDelegation?: 'deny' + organizationDelegatedServices?: Services +} + +type DelegatingKnowledgeOperation< + O extends WorkspaceOperation, + Services extends readonly OrganizationDelegatedPrincipal['serviceId'][], +> = ScopedKnowledgeOperation & { + readonly organizationOperation: { + readonly delegatedServices?: readonly ( + | Services[number] + | ('copilot' extends NonNullable[number] + ? O['minimumRole'] extends 'read' + ? OrganizationDelegatedPrincipal['serviceId'] + : never + : never) + )[] + } } /** Binds organization policy to the same semantic operation declared for workspace access. */ -function defineKnowledgeOperation( +function defineKnowledgeOperation< + const O extends WorkspaceOperation, + const Services extends readonly OrganizationDelegatedPrincipal['serviceId'][] = readonly [], +>( operation: O, - options?: KnowledgeOperationOptions -): ScopedKnowledgeOperation { + options?: KnowledgeOperationOptions +): DelegatingKnowledgeOperation { + if ( + options?.organizationDelegatedServices?.length && + (operation.minimumRole !== 'read' || options.organizationDelegation === 'deny') + ) + throw new Error(`Operation ${operation.id} cannot delegate organization writes`) const supportsOrganizationDelegation = options?.organizationDelegation !== 'deny' && operation.minimumRole === 'read' && - operation.delegatedServices?.includes('copilot') + (operation.delegatedServices?.includes('copilot') || + Boolean(options?.organizationDelegatedServices?.length)) const organizationOperation = defineOrganizationOperation({ id: operation.id, capability: operation.capability, @@ -44,11 +73,15 @@ function defineKnowledgeOperation( ], delegationAudience: 'sim:knowledge', delegatedServices: - operation.id === 'knowledge.search' ? ['copilot', 'slack-search'] : ['copilot'], + options?.organizationDelegatedServices ?? + (operation.id === 'knowledge.search' ? ['copilot', 'slack-search'] : ['copilot']), } as const) : ({ principalKinds: ['session', 'personal_api_key', 'oauth_access_token'] } as const)), }) - return Object.freeze({ ...operation, organizationOperation }) + return Object.freeze({ ...operation, organizationOperation }) as DelegatingKnowledgeOperation< + O, + Services + > } const ALL_PRINCIPAL_POLICY = { @@ -681,7 +714,8 @@ export const knowledgeOperations = { workspaceApiKey: 'deny', capability: 'knowledge.use', principalKinds: ['session'], - }) + }), + { organizationDelegatedServices: ['slack-search'] } ), readSearchSourceOverview: defineKnowledgeOperation( defineWorkspaceOperation({ diff --git a/apps/sim/lib/knowledge/application/search-sources.test.ts b/apps/sim/lib/knowledge/application/search-sources.test.ts index 036aed5ce73..f7b0933f525 100644 --- a/apps/sim/lib/knowledge/application/search-sources.test.ts +++ b/apps/sim/lib/knowledge/application/search-sources.test.ts @@ -68,6 +68,7 @@ import { import { readSearchSourceOverview } from '@/lib/knowledge/application/search-source-overview' import { readSearchSourceProgress } from '@/lib/knowledge/application/search-source-progress' import { listSearchSources } from '@/lib/knowledge/application/search-sources' +import { slackSearchMemberPrincipal } from '@/lib/knowledge/application/slack-search/member-principal' const principal = { kind: 'session' as const, userId: 'reader', sessionId: 'session' } const input = { workspaceId: 'workspace' } @@ -371,6 +372,72 @@ describe('Search source summaries', () => { }) describe('organization Search source summaries', () => { + function slackPrincipal() { + return slackSearchMemberPrincipal( + { installationId: 'i1', message: { eventId: 'Ev1' } }, + 'org-1', + 'reader' + ) + } + it('uses the Slack member’s ACL and includes their expired connection for Home', async () => { + const delegated = slackPrincipal() + mocks.context.mockResolvedValue({ organizationId: 'org-1' }) + queueTableRows(member, [{ role: 'member' }]) + mocks.memberships.mockResolvedValue(new Map([['drive', 'needs_reauth']])) + seed([source('drive', 'google_drive', 'members')]) + const result = await listSearchSources.execute({ + principal: delegated, + input: { organizationId: 'org-1' }, + }) + expect(result.sources[0]).toMatchObject({ + viewerMembership: 'needs_reauth', + connectionRequired: true, + }) + expect(mocks.access).toHaveBeenCalledWith(delegated, { organizationId: 'org-1' }) + expect(mocks.memberships).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'reader', organizationId: 'org-1' }) + ) + }) + it('refuses Slack delegation into another organization', async () => { + mocks.context.mockResolvedValue({ organizationId: 'org-2' }) + await expect( + listSearchSources.execute({ principal: slackPrincipal(), input: { organizationId: 'org-2' } }) + ).rejects.toThrow('delegation') + expect(mocks.memberships).not.toHaveBeenCalled() + }) + it('rechecks Slack membership and delegation expiry before listing sources', async () => { + mocks.context.mockResolvedValue({ organizationId: 'org-1' }) + queueTableRows(member, []) + await expect( + listSearchSources.execute({ principal: slackPrincipal(), input: { organizationId: 'org-1' } }) + ).rejects.toThrow('Organization not found') + await expect( + listSearchSources.execute({ + principal: { ...slackPrincipal(), expiresAt: new Date(0) }, + input: { organizationId: 'org-1' }, + }) + ).rejects.toThrow('delegation') + expect(mocks.memberships).not.toHaveBeenCalled() + }) + it('does not make the source list available to Copilot or Slack installation authority', async () => { + await expect( + listSearchSources.execute({ + principal: { + kind: 'organization_delegated', + serviceId: 'copilot', + organizationId: 'org-1', + subjectUserId: 'reader', + delegationId: 'd1', + audience: 'sim:knowledge', + issuedAt: new Date(), + expiresAt: new Date(Date.now() + 60_000), + resourceScope: { chatId: 'chat1' }, + }, + input: { organizationId: 'org-1' }, + }) + ).rejects.toThrow('delegation') + expect(mocks.context).not.toHaveBeenCalled() + }) it.each(['member', 'admin'])( 'returns only the current %s viewer ACL counts without a workspace membership', async (role) => { diff --git a/apps/sim/lib/knowledge/application/search-sources.ts b/apps/sim/lib/knowledge/application/search-sources.ts index 8e1599057be..b19f3cc335c 100644 --- a/apps/sim/lib/knowledge/application/search-sources.ts +++ b/apps/sim/lib/knowledge/application/search-sources.ts @@ -1,3 +1,4 @@ +import { requirePrincipalSubjectUserId } from '@sim/auth/principal' import { db } from '@sim/db' import { document, embedding, knowledgeBase, knowledgeConnector, user } from '@sim/db/schema' import { and, desc, eq, exists, inArray, isNull, lt, or, sql } from 'drizzle-orm' @@ -37,12 +38,13 @@ export const listSearchSources = defineAuthorizedKnowledgeUseCase({ resolveContext: ({ input }: { input: ListSearchSourcesInput }) => resolveKnowledgeOwnerContext(input), async execute({ principal, input, context }) { + const userId = requirePrincipalSubjectUserId(principal) const search = input.search?.trim().toLowerCase() ?? '' const connectorType = input.connectorType?.trim() const cursorScope = cursorScopeKey(cursorRoute(listSearchSourcesContract), { workspaceId: context.workspaceId, organizationId: context.organizationId, - userId: principal.userId, + userId, search, connectorType: connectorType ?? '', mine: input.mine === true, @@ -109,7 +111,7 @@ export const listSearchSources = defineAuthorizedKnowledgeUseCase({ const [availability, memberships, viewers, access, approvals] = await Promise.all([ resolveKnowledgeAccessAvailability(context), resolveViewerConnectorMemberships({ - userId: principal.userId, + userId, workspaceId: context.workspaceId, organizationId: context.organizationId, connectors: scanned, @@ -117,7 +119,7 @@ export const listSearchSources = defineAuthorizedKnowledgeUseCase({ db .select({ emailVerified: user.emailVerified }) .from(user) - .where(eq(user.id, principal.userId)) + .where(eq(user.id, userId)) .limit(1), createKnowledgeAccessProvider(principal, context).get(), context.organizationId ? listOrganizationSearchApprovals(context.organizationId) : null, diff --git a/apps/sim/lib/knowledge/application/slack-search/assistant.ts b/apps/sim/lib/knowledge/application/slack-search/assistant.ts index 8628b03be93..8ead8880bf1 100644 --- a/apps/sim/lib/knowledge/application/slack-search/assistant.ts +++ b/apps/sim/lib/knowledge/application/slack-search/assistant.ts @@ -1,7 +1,4 @@ -import type { - OrganizationDelegatedPrincipal, - SlackInstallationPrincipal, -} from '@sim/auth/principal' +import type { SlackInstallationPrincipal } from '@sim/auth/principal' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' @@ -41,6 +38,7 @@ import { resolveSlackSearchMember, SlackSearchIdentityError, } from '@/lib/knowledge/application/slack-search/identity' +import { slackSearchMemberPrincipal } from '@/lib/knowledge/application/slack-search/member-principal' import { sendSlackSearchOnboarding } from '@/lib/knowledge/application/slack-search/onboarding' import { recordSlackSearchOutcome } from '@/lib/knowledge/application/slack-search/repository' import { getSlackSearchSourceStatus } from '@/lib/knowledge/application/slack-search/source-status' @@ -60,26 +58,6 @@ import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secr const logger = createLogger('SlackSearchAssistant') -/** Creates narrowly scoped, short-lived authority for the current verified Slack sender. */ -export function slackSearchMemberPrincipal( - job: SlackSearchJob, - organizationId: string, - userId: string -): OrganizationDelegatedPrincipal { - const issuedAt = new Date() - return { - kind: 'organization_delegated', - serviceId: 'slack-search', - organizationId, - subjectUserId: userId, - delegationId: `${job.installationId}:${job.message.eventId}`, - audience: 'sim:knowledge', - issuedAt, - expiresAt: new Date(issuedAt.getTime() + 60_000), - resourceScope: { installationId: job.installationId, eventId: job.message.eventId }, - } -} - /** Runs the product's organization Assistant with its ordinary tools, chat lock, billing, and persistence. */ export async function runSlackSearchAssistant( principal: SlackInstallationPrincipal, diff --git a/apps/sim/lib/knowledge/application/slack-search/home.test.ts b/apps/sim/lib/knowledge/application/slack-search/home.test.ts new file mode 100644 index 00000000000..a8f24fba210 --- /dev/null +++ b/apps/sim/lib/knowledge/application/slack-search/home.test.ts @@ -0,0 +1,281 @@ +/** @vitest-environment node */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + installation: vi.fn(), + sender: vi.fn(), + member: vi.fn(), + list: vi.fn(), + authorize: vi.fn(), + publish: vi.fn(), + enqueue: vi.fn(), +})) +vi.mock('@/lib/core/async-jobs', () => ({ + getInlineJobQueue: async () => ({ enqueue: mocks.enqueue }), +})) +vi.mock('@/lib/core/utils/urls', () => ({ getBaseUrl: () => 'https://sim.test' })) +vi.mock('@/lib/internal/slack/client', () => ({ requestSlackApi: mocks.publish })) +vi.mock('@/lib/internal/slack/search-client', () => ({ getSlackSearchSender: mocks.sender })) +vi.mock('@/lib/knowledge/application/search-sources', () => ({ + listSearchSources: { execute: mocks.list, authorize: mocks.authorize }, +})) +vi.mock('@/lib/knowledge/application/slack-search/repository', () => ({ + findSlackSearchInstallation: mocks.installation, + loadSlackSearchCredential: async () => ({ version: 'v1', botToken: 'private-bot-token' }), +})) +vi.mock('@/lib/knowledge/access/availability', () => ({ + requireOrganizationSearchAvailable: async () => undefined, +})) +vi.mock('@/lib/knowledge/application/slack-search/identity', () => ({ + resolveSlackSearchMember: mocks.member, + SlackSearchIdentityError: class extends Error {}, +})) +vi.mock('@/connectors/registry', () => ({ getConnectorMeta: () => ({ name: 'Google Drive' }) })) + +import { + publishSlackSearchHome, + receiveSlackSearchHome, +} from '@/lib/knowledge/application/slack-search/home' +import { SlackSearchIdentityError } from '@/lib/knowledge/application/slack-search/identity' + +const installation = { + id: 'i1', + revision: 'r1', + appId: 'A1', + teamId: 'T1', + credentialId: 'c1', + credentialVersion: 'v1', + organizationId: 'org1', + botUserId: 'UBOT', + enabled: true, +} +const event = { appId: 'A1', teamId: 'T1', eventId: 'Ev1', userId: 'U1', viewHash: 'h1' } +function principal() { + return { + kind: 'slack_installation' as const, + credentialId: 'c1', + credentialVersion: 'v1', + appId: 'A1', + teamId: 'T1', + eventId: 'Ev1', + receivedAt: new Date(), + } +} +function publish(signal = new AbortController().signal) { + return publishSlackSearchHome.execute({ + principal: principal(), + input: { + job: { + installationId: 'i1', + revision: 'r1', + credentialId: 'c1', + credentialVersion: 'v1', + receivedAt: Date.now(), + event, + }, + signal, + }, + }) +} +function source(id = 'source1') { + return { + connectorId: id, + connectorType: 'google_drive', + sourceDescription: 'Folder', + enabled: true, + approved: true, + availability: 'available', + connectionRequired: true, + viewerMembership: 'connected', + isSyncing: false, + hasSyncError: false, + } +} +beforeEach(() => { + vi.clearAllMocks() + mocks.installation.mockResolvedValue(installation) + mocks.sender.mockResolvedValue({ email: 'viewer@example.test' }) + mocks.member.mockResolvedValue('member1') + mocks.list.mockResolvedValue({ sources: [source()], nextCursor: null }) + mocks.authorize.mockResolvedValue(undefined) + mocks.publish.mockResolvedValue({ status: 200, data: { ok: true } }) +}) + +describe('Slack Home intake', () => { + it('queues a deduplicated app-process update with no token or Sim user in the payload', async () => { + await receiveSlackSearchHome.execute({ principal: principal(), input: event }) + expect(mocks.enqueue).toHaveBeenCalledWith( + 'slack-search', + expect.objectContaining({ event, revision: 'r1' }), + expect.objectContaining({ + jobId: 'slack-search-home:i1:Ev1', + maxAttempts: 1, + maxDurationSeconds: 30, + concurrencyLimit: 2, + }) + ) + expect(JSON.stringify(mocks.enqueue.mock.calls[0][1])).not.toMatch( + /private-bot-token|member1|viewer@/ + ) + expect(mocks.sender).not.toHaveBeenCalled() + const [, payload, options] = mocks.enqueue.mock.calls[0] + await options.runner(payload, new AbortController().signal) + expect(mocks.publish).toHaveBeenCalledOnce() + }) + it.each(['appId', 'teamId', 'eventId'] as const)( + 'rejects a mismatched %s before loading the installation', + async (field) => { + await expect( + receiveSlackSearchHome.execute({ + principal: principal(), + input: { ...event, [field]: 'other' }, + }) + ).rejects.toThrow('authority') + expect(mocks.installation).not.toHaveBeenCalled() + expect(mocks.enqueue).not.toHaveBeenCalled() + } + ) + it('rejects a non-Slack principal and expired intake', async () => { + await expect( + receiveSlackSearchHome.execute({ + principal: { kind: 'session', userId: 'member1', sessionId: 's1' }, + input: event, + }) + ).rejects.toThrow('installation authority') + await expect( + receiveSlackSearchHome.execute({ + principal: { ...principal(), receivedAt: new Date(Date.now() - 301_000) }, + input: event, + }) + ).rejects.toThrow('authority') + expect(mocks.enqueue).not.toHaveBeenCalled() + }) + it('does not queue disabled installations or bot users', async () => { + mocks.installation.mockResolvedValueOnce({ ...installation, enabled: false }) + await receiveSlackSearchHome.execute({ principal: principal(), input: event }) + await receiveSlackSearchHome.execute({ + principal: principal(), + input: { ...event, userId: 'UBOT' }, + }) + expect(mocks.enqueue).not.toHaveBeenCalled() + }) +}) + +describe('personalized Home delivery', () => { + it('uses the installation’s organization, the current member, and the same source use case', async () => { + await publish() + expect(mocks.sender).toHaveBeenCalledWith( + 'private-bot-token', + 'U1', + 'T1', + expect.any(AbortSignal) + ) + expect(mocks.list).toHaveBeenCalledWith({ + principal: expect.objectContaining({ + kind: 'organization_delegated', + serviceId: 'slack-search', + organizationId: 'org1', + subjectUserId: 'member1', + resourceScope: { installationId: 'i1', eventId: 'Ev1' }, + }), + input: { organizationId: 'org1', cursor: undefined }, + }) + expect(mocks.member).toHaveBeenCalledTimes(2) + expect(mocks.authorize).toHaveBeenCalledOnce() + expect(mocks.publish).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'views.publish', + body: expect.objectContaining({ + user_id: 'U1', + hash: 'h1', + view: expect.objectContaining({ type: 'home' }), + }), + }) + ) + expect(JSON.stringify(mocks.publish.mock.calls[0][0].body)).toContain( + 'https://sim.test/o/org1/integrations' + ) + }) + it.each([ + 'account_required', + 'verify_email', + 'membership_required', + 'identity_conflict', + ] as const)('shows no protected source details for %s', async (reason) => { + mocks.member.mockRejectedValue(new SlackSearchIdentityError(reason)) + await publish() + expect(mocks.list).not.toHaveBeenCalled() + expect(JSON.stringify(mocks.publish.mock.calls[0][0].body)).toContain('Sign in to Sim') + expect(JSON.stringify(mocks.publish.mock.calls[0][0].body)).not.toContain('Google Drive') + }) + it('propagates infrastructure failures instead of inventing an empty source list', async () => { + mocks.list.mockRejectedValueOnce(new Error('database unavailable')) + await expect(publish()).rejects.toThrow('database unavailable') + expect(mocks.publish).not.toHaveBeenCalled() + }) + it('rejects Slack users with no valid workspace email', async () => { + mocks.sender.mockResolvedValueOnce(null) + await expect(publish()).rejects.toThrow() + expect(mocks.list).not.toHaveBeenCalled() + expect(mocks.publish).not.toHaveBeenCalled() + }) + it.each([ + { enabled: false }, + { revision: 'r2' }, + { credentialVersion: 'v2' }, + { appId: 'A2' }, + { teamId: 'T2' }, + ])('invalidates queued work when its binding changes: %j', async (change) => { + mocks.installation.mockResolvedValue({ ...installation, ...change }) + if ('enabled' in change) await publish() + else await expect(publish()).rejects.toThrow('binding') + expect(mocks.publish).not.toHaveBeenCalled() + }) + it('rechecks disablement immediately before publishing', async () => { + mocks.installation + .mockResolvedValueOnce(installation) + .mockResolvedValueOnce({ ...installation, enabled: false }) + await publish() + expect(mocks.publish).not.toHaveBeenCalled() + }) + it('rejects lost member authorization before delivery', async () => { + mocks.authorize.mockRejectedValueOnce(new Error('member access removed')) + await expect(publish()).rejects.toThrow('member access removed') + expect(mocks.publish).not.toHaveBeenCalled() + }) + it('caps sparse pagination and avoids claiming there are no connections', async () => { + mocks.list.mockResolvedValue({ sources: [], nextCursor: 'cursor' }) + await publish() + expect(mocks.list).toHaveBeenCalledTimes(4) + expect(JSON.stringify(mocks.publish.mock.calls[0][0].body)).not.toContain( + 'No sources connected yet' + ) + }) + it('caps rendered sources without fetching more pages', async () => { + mocks.list.mockResolvedValue({ + sources: Array.from({ length: 25 }, (_, i) => source(`source${i}`)), + nextCursor: 'cursor', + }) + await publish() + expect(mocks.list).toHaveBeenCalledOnce() + expect(mocks.publish.mock.calls[0][0].body.view.blocks).toHaveLength(25) + }) + it('stops work on cancellation', async () => { + await expect(publish(AbortSignal.abort())).rejects.toThrow() + expect(mocks.list).not.toHaveBeenCalled() + expect(mocks.publish).not.toHaveBeenCalled() + }) + it.each([ + { status: 500, data: { ok: false } }, + { status: 200, data: { ok: false, error: 'hash_conflict' } }, + ])('records failed publishing without a fallback or replay: %j', async (response) => { + mocks.publish.mockResolvedValueOnce(response) + await expect(publish()).rejects.toThrow('Could not publish') + expect(mocks.publish).toHaveBeenCalledOnce() + }) + it('does not retry an ambiguous send', async () => { + mocks.publish.mockRejectedValueOnce(new Error('response lost')) + await expect(publish()).rejects.toThrow('response lost') + expect(mocks.publish).toHaveBeenCalledOnce() + }) +}) diff --git a/apps/sim/lib/knowledge/application/slack-search/home.ts b/apps/sim/lib/knowledge/application/slack-search/home.ts new file mode 100644 index 00000000000..9a1efcefeea --- /dev/null +++ b/apps/sim/lib/knowledge/application/slack-search/home.ts @@ -0,0 +1,188 @@ +import type { SlackInstallationPrincipal } from '@sim/auth/principal' +import type { OperationUseCase } from '@/lib/core/application/operation' +import { getInlineJobQueue } from '@/lib/core/async-jobs' +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { getBaseUrl } from '@/lib/core/utils/urls' +import { requestSlackApi } from '@/lib/internal/slack/client' +import { getSlackSearchSender } from '@/lib/internal/slack/search-client' +import { listSearchSources } from '@/lib/knowledge/application/search-sources' +import { + authorizeSlackSearchInstallation, + requireSlackInstallationPrincipal, +} from '@/lib/knowledge/application/slack-search/authorization' +import { + resolveSlackSearchMember, + SlackSearchIdentityError, +} from '@/lib/knowledge/application/slack-search/identity' +import { slackSearchMemberPrincipal } from '@/lib/knowledge/application/slack-search/member-principal' +import { organizationRoutes } from '@/lib/navigation/paths' +import { + renderSlackSearchHome, + SLACK_SEARCH_HOME_MAX_AGE_MS, + SLACK_SEARCH_HOME_MAX_SOURCES, + type SlackSearchHomeEvent, + type SlackSearchHomeJob, + type SlackSearchHomeSource, + slackSearchHomeJobSchema, + slackSearchHomeSource, +} from '@/lib/slack-search/home' + +const operation = Object.freeze({ + id: 'knowledge.slack.home.publish', + capability: 'knowledge.use', + principalKinds: ['slack_installation'] as const, +}) + +function requireHomeBinding(principal: SlackInstallationPrincipal, event: SlackSearchHomeEvent) { + if ( + principal.appId !== event.appId || + principal.teamId !== event.teamId || + principal.eventId !== event.eventId || + principal.receivedAt.getTime() < Date.now() - SLACK_SEARCH_HOME_MAX_AGE_MS + ) + throw new OrchestrationError('forbidden', 'Slack Home event authority is no longer valid') +} + +/** Persists a deduplicated, bounded app-process job so Slack can acknowledge Home opens promptly. */ +export const receiveSlackSearchHome: OperationUseCase< + typeof operation, + SlackSearchHomeEvent, + void +> = { + operation, + async execute({ principal, input }) { + requireSlackInstallationPrincipal(principal) + requireHomeBinding(principal, input) + const context = await authorizeSlackSearchInstallation(principal) + if (!context || input.userId === context.installation.botUserId) return + const job: SlackSearchHomeJob = { + installationId: context.installation.id, + revision: context.installation.revision, + credentialId: principal.credentialId, + credentialVersion: principal.credentialVersion, + receivedAt: principal.receivedAt.getTime(), + event: input, + } + await (await getInlineJobQueue()).enqueue('slack-search', job, { + jobId: `slack-search-home:${job.installationId}:${input.eventId}`, + maxAttempts: 1, + maxDurationSeconds: 30, + concurrencyKey: `slack-search-home:${job.installationId}`, + concurrencyLimit: 2, + async runner(payload, signal) { + const queued = slackSearchHomeJobSchema.parse(payload) + await publishSlackSearchHome.execute({ + principal: { + kind: 'slack_installation', + credentialId: queued.credentialId, + credentialVersion: queued.credentialVersion, + appId: queued.event.appId, + teamId: queued.event.teamId, + eventId: queued.event.eventId, + receivedAt: new Date(queued.receivedAt), + }, + input: { job: queued, signal: AbortSignal.any([signal, AbortSignal.timeout(30_000)]) }, + }) + }, + }) + }, +} + +/** Resolves the event's member, reads the product's authorized source list, and publishes only to that Slack user. */ +export const publishSlackSearchHome: OperationUseCase< + typeof operation, + { job: SlackSearchHomeJob; signal: AbortSignal }, + void +> = { + operation, + async execute({ principal, input: { job, signal } }) { + requireSlackInstallationPrincipal(principal) + requireHomeBinding(principal, job.event) + signal.throwIfAborted() + const context = await authorizeSlackSearchInstallation(principal, job) + if (!context) return + const { installation, secret } = context + const sender = await getSlackSearchSender( + secret.botToken, + job.event.userId, + installation.teamId, + signal + ) + if (!sender) throw new SlackSearchIdentityError() + const sources: SlackSearchHomeSource[] = [] + let userId: string | undefined + try { + userId = await resolveSlackSearchMember( + installation.organizationId, + installation.teamId, + job.event.userId, + sender.email + ) + } catch (error) { + if (!(error instanceof SlackSearchIdentityError)) throw error + } + let hasMore = false + let memberPrincipal: ReturnType | undefined + if (userId) { + memberPrincipal = slackSearchMemberPrincipal( + { installationId: installation.id, message: { eventId: job.event.eventId } }, + installation.organizationId, + userId + ) + let cursor: string | undefined + /** Bound sparse pages as well as visible rows; the existing Sources page handles the remainder. */ + for (let page = 0; page < 4; page++) { + signal.throwIfAborted() + const result = await listSearchSources.execute({ + principal: memberPrincipal, + input: { organizationId: installation.organizationId, cursor }, + }) + const visible = result.sources.flatMap((source) => { + const row = slackSearchHomeSource(source) + return row ? [row] : [] + }) + const remaining = SLACK_SEARCH_HOME_MAX_SOURCES - sources.length + sources.push(...visible.slice(0, remaining)) + hasMore = Boolean(result.nextCursor) || visible.length > remaining + if (!result.nextCursor || sources.length === SLACK_SEARCH_HOME_MAX_SOURCES) break + cursor = result.nextCursor + } + } + const current = await authorizeSlackSearchInstallation(principal, job) + if (!current) return + if (memberPrincipal) { + const currentUserId = await resolveSlackSearchMember( + installation.organizationId, + installation.teamId, + job.event.userId, + sender.email + ) + if (currentUserId !== userId) throw new SlackSearchIdentityError('identity_conflict') + await listSearchSources.authorize({ + principal: memberPrincipal, + input: { organizationId: installation.organizationId }, + }) + } + signal.throwIfAborted() + const response = await requestSlackApi({ + accessToken: current.secret.botToken, + method: 'views.publish', + body: { + user_id: job.event.userId, + ...(job.event.viewHash ? { hash: job.event.viewHash } : {}), + view: renderSlackSearchHome({ + sourcesUrl: new URL( + organizationRoutes(installation.organizationId).integrations, + getBaseUrl() + ).href, + sources, + hasMore, + accountRequired: !userId, + }), + }, + signal, + }) + if (response.status !== 200 || response.data.ok !== true) + throw new Error('Could not publish the Slack Search Home tab') + }, +} diff --git a/apps/sim/lib/knowledge/application/slack-search/member-principal.ts b/apps/sim/lib/knowledge/application/slack-search/member-principal.ts new file mode 100644 index 00000000000..c79bd5f5de3 --- /dev/null +++ b/apps/sim/lib/knowledge/application/slack-search/member-principal.ts @@ -0,0 +1,21 @@ +import type { OrganizationDelegatedPrincipal } from '@sim/auth/principal' + +/** Creates narrowly scoped, short-lived authority for a current verified Slack sender. */ +export function slackSearchMemberPrincipal( + event: { installationId: string; message: { eventId: string } }, + organizationId: string, + userId: string +): OrganizationDelegatedPrincipal { + const issuedAt = new Date() + return { + kind: 'organization_delegated', + serviceId: 'slack-search', + organizationId, + subjectUserId: userId, + delegationId: `${event.installationId}:${event.message.eventId}`, + audience: 'sim:knowledge', + issuedAt, + expiresAt: new Date(issuedAt.getTime() + 60_000), + resourceScope: { installationId: event.installationId, eventId: event.message.eventId }, + } +} diff --git a/apps/sim/lib/slack-search/dispatcher.test.ts b/apps/sim/lib/slack-search/dispatcher.test.ts new file mode 100644 index 00000000000..2dba021f9f8 --- /dev/null +++ b/apps/sim/lib/slack-search/dispatcher.test.ts @@ -0,0 +1,57 @@ +/** @vitest-environment node */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ home: vi.fn(), message: vi.fn(), stop: vi.fn() })) +vi.mock('@/lib/knowledge/application/slack-search/home', () => ({ + receiveSlackSearchHome: { execute: mocks.home }, +})) +vi.mock('@/lib/knowledge/application/slack-search/process-message', () => ({ + receiveSlackSearchMessage: { execute: mocks.message }, +})) +vi.mock('@/lib/knowledge/application/slack-search/stop', () => ({ + slackSearchStopSchema: { safeParse: () => ({ success: false }) }, + stopSlackSearchThread: { execute: mocks.stop }, +})) + +import { dispatchSlackSearch } from '@/lib/slack-search/dispatcher' + +beforeEach(() => vi.clearAllMocks()) +describe('authenticated Slack Home dispatch', () => { + function dispatch(tab: string) { + return dispatchSlackSearch({ + credentialId: 'c1', + credentialVersion: 'v1', + receivedAt: Date.now(), + body: { + type: 'event_callback', + api_app_id: 'A1', + team_id: 'T1', + event_id: 'Ev1', + event_time: Math.floor(Date.now() / 1000), + event: { type: 'app_home_opened', user: 'U1', tab }, + }, + }) + } + it('routes only Home opens to the Home application use case', async () => { + await dispatch('home') + expect(mocks.home).toHaveBeenCalledWith({ + principal: { + kind: 'slack_installation', + credentialId: 'c1', + credentialVersion: 'v1', + appId: 'A1', + teamId: 'T1', + eventId: 'Ev1', + receivedAt: expect.any(Date), + }, + input: { appId: 'A1', teamId: 'T1', eventId: 'Ev1', userId: 'U1' }, + }) + expect(mocks.message).not.toHaveBeenCalled() + expect(mocks.stop).not.toHaveBeenCalled() + }) + it('acknowledges opening Messages without sending a chat response or replacing Home', async () => { + await dispatch('messages') + expect(mocks.home).not.toHaveBeenCalled() + expect(mocks.message).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/slack-search/dispatcher.ts b/apps/sim/lib/slack-search/dispatcher.ts index 2ab03d0f900..de48c5ec2b6 100644 --- a/apps/sim/lib/slack-search/dispatcher.ts +++ b/apps/sim/lib/slack-search/dispatcher.ts @@ -1,8 +1,10 @@ +import { receiveSlackSearchHome } from '@/lib/knowledge/application/slack-search/home' import { receiveSlackSearchMessage } from '@/lib/knowledge/application/slack-search/process-message' import { slackSearchStopSchema, stopSlackSearchThread, } from '@/lib/knowledge/application/slack-search/stop' +import { parseSlackSearchHomeEvent } from '@/lib/slack-search/home' import { parseSlackSearchMessage } from '@/lib/slack-search/types' /** Routes authenticated Slack payloads to named application handlers. Unsupported interactions acknowledge immediately. */ @@ -12,6 +14,22 @@ export async function dispatchSlackSearch(input: { body: unknown receivedAt: number }) { + const home = parseSlackSearchHomeEvent(input.body) + if (home) { + await receiveSlackSearchHome.execute({ + principal: { + kind: 'slack_installation', + credentialId: input.credentialId, + credentialVersion: input.credentialVersion, + appId: home.appId, + teamId: home.teamId, + eventId: home.eventId, + receivedAt: new Date(input.receivedAt), + }, + input: home, + }) + return + } const stopped = slackSearchStopSchema.safeParse(input.body) if (stopped.success) { await stopSlackSearchThread.execute({ diff --git a/apps/sim/lib/slack-search/home.test.ts b/apps/sim/lib/slack-search/home.test.ts new file mode 100644 index 00000000000..0a60be38db4 --- /dev/null +++ b/apps/sim/lib/slack-search/home.test.ts @@ -0,0 +1,175 @@ +/** @vitest-environment node */ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@/connectors/registry', () => ({ + getConnectorMeta: (id: string) => (id === 'google_drive' ? { name: 'Google Drive' } : undefined), +})) + +import { + parseSlackSearchHomeEvent, + renderSlackSearchHome, + slackSearchHomeSource, +} from '@/lib/slack-search/home' + +const now = 1_800_000_000_000 +const event = { + type: 'event_callback', + api_app_id: 'A1', + team_id: 'T1', + event_id: 'Ev1', + event_time: now / 1000, + event: { type: 'app_home_opened', tab: 'home', user: 'U1' }, +} +const source = { + knowledgeBaseId: 'kb1', + connectorId: 'source1', + connectorType: 'google_drive', + sourceDescription: '1 folder selected', + accessMode: 'members' as const, + availability: 'available' as const, + enabled: true, + approved: true, + isSyncing: false, + lastSyncAt: null, + hasSyncError: false, + viewerDocumentCount: 1, + viewerFailedDocumentCount: 0, + viewerEmailVerified: true, + connectionRequired: true as const, + viewerMembership: 'connected' as const, +} + +describe('Slack Home events', () => { + it('retains only the routing identity and concurrency hash', () => { + expect( + parseSlackSearchHomeEvent( + { + ...event, + event: { + ...event.event, + view: { type: 'home', hash: 'h1', blocks: ['old private data'] }, + }, + }, + now + ) + ).toEqual({ + appId: 'A1', + teamId: 'T1', + eventId: 'Ev1', + userId: 'U1', + viewHash: 'h1', + }) + }) + it.each(['messages', 'about', undefined])('ignores the %s tab', (tab) => { + expect(parseSlackSearchHomeEvent({ ...event, event: { ...event.event, tab } }, now)).toBeNull() + }) + it.each([now - 301_000, now + 61_000])('ignores stale or future events: %s', (timestamp) => { + expect(parseSlackSearchHomeEvent({ ...event, event_time: timestamp / 1000 }, now)).toBeNull() + }) + it('rejects incomplete identity and unrelated events', () => { + expect(parseSlackSearchHomeEvent({ ...event, team_id: '' }, now)).toBeNull() + expect( + parseSlackSearchHomeEvent({ ...event, event: { type: 'message', user: 'U1' } }, now) + ).toBeNull() + }) +}) + +describe('personalized source statuses', () => { + it('shows connected and syncing sources, with reconnect taking precedence', () => { + expect(slackSearchHomeSource(source)?.status).toBe('Connected') + expect(slackSearchHomeSource({ ...source, isSyncing: true })?.status).toBe('Syncing') + expect( + slackSearchHomeSource({ ...source, viewerMembership: 'needs_reauth', isSyncing: true }) + ?.status + ).toBe('Reconnect needed') + }) + it.each(['invited', 'not_enrolled', 'revoked', 'unverified_email', null] as const)( + 'does not list someone else’s connection for %s', + (viewerMembership) => { + expect(slackSearchHomeSource({ ...source, viewerMembership })).toBeNull() + } + ) + it('includes available shared sources that do not require individual connections', () => { + expect( + slackSearchHomeSource({ + ...source, + accessMode: 'admin', + connectionRequired: false, + viewerMembership: null, + })?.status + ).toBe('Connected') + }) + it('omits disabled, unavailable, and unapproved sources', () => { + expect(slackSearchHomeSource({ ...source, enabled: false })).toBeNull() + expect(slackSearchHomeSource({ ...source, approved: false })).toBeNull() + expect(slackSearchHomeSource({ ...source, availability: 'unavailable' })).toBeNull() + }) + it('does not mislabel a failed sync as expired authorization', () => { + expect(slackSearchHomeSource({ ...source, hasSyncError: true })).toMatchObject({ + status: 'Connected', + syncError: true, + }) + }) +}) + +describe('Slack Home presentation', () => { + const sourcesUrl = 'https://sim.test/o/org1/integrations' + it('renders a single URL button and compact source statuses as plain text', () => { + const view = renderSlackSearchHome({ + sourcesUrl, + sources: [slackSearchHomeSource(source)!], + hasMore: false, + }) + expect(view).toMatchObject({ type: 'home' }) + expect(JSON.stringify(view)).toContain('Google Drive — Connected') + const blocks = view.blocks as Record[] + expect(blocks.filter((block) => block.type === 'actions')).toEqual([ + { + type: 'actions', + elements: [ + { + type: 'button', + action_id: 'sim_search.connect_sources', + text: { type: 'plain_text', text: 'Connect sources' }, + style: 'primary', + url: sourcesUrl, + }, + ], + }, + ]) + expect(JSON.stringify(view)).not.toContain('mrkdwn') + }) + it('never renders source details for an unmatched account', () => { + const view = renderSlackSearchHome({ + sourcesUrl, + sources: [slackSearchHomeSource(source)!], + hasMore: false, + accountRequired: true, + }) + expect(JSON.stringify(view)).toContain('Sign in to Sim') + expect(JSON.stringify(view)).not.toContain('Google Drive') + }) + it('caps provider content and source rows, including hostile labels', () => { + const view = renderSlackSearchHome({ + sourcesUrl, + sources: Array.from({ length: 200 }, () => ({ + name: '', + description: 'x'.repeat(4000), + status: 'Connected', + syncError: false, + })), + hasMore: true, + }) + expect((view.blocks as unknown[]).length).toBe(25) + expect(JSON.stringify(view)).not.toContain('x'.repeat(201)) + expect(JSON.stringify(view)).toContain('More sources are available') + }) + it('explains empty connections and bounded sparse lists honestly', () => { + expect( + JSON.stringify(renderSlackSearchHome({ sourcesUrl, sources: [], hasMore: false })) + ).toContain('No sources connected yet') + expect( + JSON.stringify(renderSlackSearchHome({ sourcesUrl, sources: [], hasMore: true })) + ).not.toContain('No sources connected yet') + }) +}) diff --git a/apps/sim/lib/slack-search/home.ts b/apps/sim/lib/slack-search/home.ts new file mode 100644 index 00000000000..868c70ef7b0 --- /dev/null +++ b/apps/sim/lib/slack-search/home.ts @@ -0,0 +1,165 @@ +import { truncate } from '@sim/utils/string' +import { z } from 'zod' +import type { SlackJsonObject } from '@/lib/internal/slack/client' +import type { listSearchSources } from '@/lib/knowledge/application/search-sources' +import { getConnectorMeta } from '@/connectors/registry' + +const id = z.string().min(1).max(200) +export const SLACK_SEARCH_HOME_MAX_SOURCES = 20 +export const SLACK_SEARCH_HOME_MAX_AGE_MS = 5 * 60_000 + +const homeEventSchema = z.object({ + type: z.literal('event_callback'), + api_app_id: id, + team_id: id, + event_id: id, + event_time: z.number().int(), + event: z.object({ + type: z.literal('app_home_opened'), + tab: z.literal('home'), + user: id, + view: z.object({ type: z.literal('home'), hash: id }).optional(), + }), +}) + +const slackSearchHomeEventSchema = z.object({ + appId: id, + teamId: id, + eventId: id, + userId: id, + viewHash: id.optional(), +}) +export type SlackSearchHomeEvent = z.infer + +/** Queue-only identity; no tokens, source data, or preauthorized Sim user IDs are persisted. */ +export const slackSearchHomeJobSchema = z.object({ + installationId: id, + revision: id, + credentialId: id, + credentialVersion: id, + receivedAt: z.number().int().positive(), + event: slackSearchHomeEventSchema, +}) +export type SlackSearchHomeJob = z.infer + +/** The Messages tab emits the same event; it must never trigger a Home publish or an answer. */ +export function parseSlackSearchHomeEvent( + body: unknown, + now = Date.now() +): SlackSearchHomeEvent | null { + const parsed = homeEventSchema.safeParse(body) + if (!parsed.success) return null + const { event, ...envelope } = parsed.data + if ( + envelope.event_time * 1000 < now - SLACK_SEARCH_HOME_MAX_AGE_MS || + envelope.event_time * 1000 > now + 60_000 + ) + return null + return { + appId: envelope.api_app_id, + teamId: envelope.team_id, + eventId: envelope.event_id, + userId: event.user, + ...(event.view ? { viewHash: event.view.hash } : {}), + } +} + +type SearchSource = Awaited>['sources'][number] + +export interface SlackSearchHomeSource { + name: string + description: string + status: 'Connected' | 'Syncing' | 'Reconnect needed' + syncError: boolean +} + +/** Uses viewer membership, never another member's grant or a generic sync failure, for connection state. */ +export function slackSearchHomeSource(source: SearchSource): SlackSearchHomeSource | null { + if (!source.enabled || source.approved === false || source.availability !== 'available') + return null + if ( + source.connectionRequired && + source.viewerMembership !== 'connected' && + source.viewerMembership !== 'needs_reauth' + ) + return null + return { + name: getConnectorMeta(source.connectorType)?.name ?? source.connectorType, + description: source.sourceDescription, + status: + source.viewerMembership === 'needs_reauth' + ? 'Reconnect needed' + : source.isSyncing + ? 'Syncing' + : 'Connected', + syncError: source.hasSyncError, + } +} + +/** Plain text source rows cannot turn provider labels into Slack mentions or attacker links. */ +export function renderSlackSearchHome(input: { + sourcesUrl: string + sources: SlackSearchHomeSource[] + hasMore: boolean + accountRequired?: boolean +}): SlackJsonObject { + const blocks: SlackJsonObject[] = [ + { type: 'header', text: { type: 'plain_text', text: 'Your sources' } }, + { + type: 'section', + text: { + type: 'plain_text', + text: input.accountRequired + ? 'Sign in to Sim with your Slack email and join this organization to see your sources.' + : 'Connect your tools to search them with Sim. Manage connections and sync details in Sim.', + }, + }, + { + type: 'actions', + elements: [ + { + type: 'button', + action_id: 'sim_search.connect_sources', + text: { type: 'plain_text', text: 'Connect sources' }, + style: 'primary', + url: input.sourcesUrl, + }, + ], + }, + { type: 'divider' }, + ] + if (!input.accountRequired) { + for (const source of input.sources.slice(0, SLACK_SEARCH_HOME_MAX_SOURCES)) { + blocks.push({ + type: 'section', + text: { + type: 'plain_text', + text: `${truncate(source.name, 100)} — ${source.status}${source.description ? `\n${truncate(source.description, 200)}` : ''}${source.syncError && source.status === 'Connected' ? '\nSync needs attention. Open Sim for details.' : ''}`, + }, + }) + } + if (input.sources.length === 0) { + blocks.push({ + type: 'section', + text: { + type: 'plain_text', + text: input.hasMore + ? 'Open Sim to view your sources and connect more tools.' + : 'No sources connected yet. Connect a source to get started.', + }, + }) + } + blocks.push({ + type: 'context', + elements: [ + { + type: 'plain_text', + text: input.hasMore + ? 'More sources are available in Sim. Statuses refresh when you open this tab.' + : 'Statuses refresh when you open this tab.', + }, + ], + }) + } + return { type: 'home', blocks } +} diff --git a/apps/sim/lib/slack-search/manifest.test.ts b/apps/sim/lib/slack-search/manifest.test.ts index 6008d09d0e2..d8b0b6f46ec 100644 --- a/apps/sim/lib/slack-search/manifest.test.ts +++ b/apps/sim/lib/slack-search/manifest.test.ts @@ -38,6 +38,7 @@ describe('Search app manifest', () => { ) expect(manifest.settings.event_subscriptions.bot_events).not.toContain('message.channels') expect(manifest.features.app_home.messages_tab_read_only_enabled).toBe(false) + expect(manifest.features.app_home.home_tab_enabled).toBe(true) expect(manifest.oauth_config.redirect_urls).toHaveLength(3) expect( manifest.oauth_config.redirect_urls.every((url) => new URL(url).origin === 'https://sim.test') diff --git a/apps/sim/lib/slack-search/manifest.ts b/apps/sim/lib/slack-search/manifest.ts index 2407534840c..a1b73ca8a55 100644 --- a/apps/sim/lib/slack-search/manifest.ts +++ b/apps/sim/lib/slack-search/manifest.ts @@ -32,7 +32,7 @@ export function createSlackSearchManifest( features: { bot_user: { display_name: name, always_online: false }, app_home: { - home_tab_enabled: false, + home_tab_enabled: true, messages_tab_enabled: true, messages_tab_read_only_enabled: false, }, From 51ec31b827bf63fb25b5dddb3774c9a4f52d02a9 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Wed, 9 Sep 2026 14:08:46 -0700 Subject: [PATCH 4/4] improvement(slack): simplify Home to a persistent connect link --- .../authorized-knowledge-use-case.ts | 9 +- .../knowledge/application/operations.test.ts | 13 +- .../lib/knowledge/application/operations.ts | 50 +---- .../application/search-sources.test.ts | 67 ------- .../knowledge/application/search-sources.ts | 8 +- .../application/slack-search/assistant.ts | 26 ++- .../application/slack-search/home.test.ts | 160 +++++----------- .../application/slack-search/home.ts | 98 ++-------- .../slack-search/member-principal.ts | 21 --- apps/sim/lib/slack-search/home.test.ts | 178 ++++++------------ apps/sim/lib/slack-search/home.ts | 100 +++------- 11 files changed, 181 insertions(+), 549 deletions(-) delete mode 100644 apps/sim/lib/knowledge/application/slack-search/member-principal.ts diff --git a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts index e11ea2ab337..e2b2cba69f3 100644 --- a/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts +++ b/apps/sim/lib/knowledge/application/authorized-knowledge-use-case.ts @@ -21,10 +21,11 @@ import type { ScopedKnowledgeOperation } from '@/lib/knowledge/application/opera type KnowledgePrincipalForOperation = | PrincipalForOperation - | Extract< - OrganizationDelegatedPrincipal, - { serviceId: NonNullable[number] } - > + | ('copilot' extends NonNullable[number] + ? O['minimumRole'] extends 'read' + ? OrganizationDelegatedPrincipal + : never + : never) function requireKnowledgePrincipal( principal: Principal, diff --git a/apps/sim/lib/knowledge/application/operations.test.ts b/apps/sim/lib/knowledge/application/operations.test.ts index b7867fb6b03..d5732c64c53 100644 --- a/apps/sim/lib/knowledge/application/operations.test.ts +++ b/apps/sim/lib/knowledge/application/operations.test.ts @@ -7,15 +7,11 @@ import { describe, expect, it } from 'vitest' import { knowledgeOperations } from '@/lib/knowledge/application/operations' describe('knowledge operation registry', () => { - it('limits Slack member delegation to search and the personalized source list', () => { + it('limits Slack member delegation to the existing search operation', () => { const allowed = Object.values(knowledgeOperations).filter((operation) => operation.organizationOperation.delegatedServices?.includes('slack-search') ) - expect(allowed).toEqual([knowledgeOperations.search, knowledgeOperations.listSearchSources]) - expect(knowledgeOperations.listSearchSources.organizationOperation.delegatedServices).toEqual([ - 'slack-search', - ]) - expect(knowledgeOperations.listSearchSources.principalKinds).toEqual(['session']) + expect(allowed).toEqual([knowledgeOperations.search]) }) it('defines unique stable semantic operation IDs', () => { const ids = Object.values(knowledgeOperations).map((operation) => operation.id) @@ -148,13 +144,12 @@ describe('knowledge operation registry', () => { } }) - it('permits organization delegation only for explicitly delegated reads', () => { + it('permits organization delegation only for Copilot reads', () => { for (const operation of Object.values(knowledgeOperations)) { if (!operation.organizationOperation.principalKinds.includes('organization_delegated')) continue expect(operation.minimumRole).toBe('read') - if (operation !== knowledgeOperations.listSearchSources) - expect(operation.delegatedServices).toContain('copilot') + expect(operation.delegatedServices).toContain('copilot') expect(operation.organizationOperation.delegationAudience).toBe('sim:knowledge') } expect(knowledgeOperations.search.organizationOperation.principalKinds).toContain( diff --git a/apps/sim/lib/knowledge/application/operations.ts b/apps/sim/lib/knowledge/application/operations.ts index 4423bbedcdf..b72dadcf115 100644 --- a/apps/sim/lib/knowledge/application/operations.ts +++ b/apps/sim/lib/knowledge/application/operations.ts @@ -1,4 +1,3 @@ -import type { OrganizationDelegatedPrincipal } from '@sim/auth/principal' import { type ApplicationOperation, assertOperationCapability, @@ -16,47 +15,19 @@ export type ScopedKnowledgeOperation { +interface KnowledgeOperationOptions { organizationDelegation?: 'deny' - organizationDelegatedServices?: Services -} - -type DelegatingKnowledgeOperation< - O extends WorkspaceOperation, - Services extends readonly OrganizationDelegatedPrincipal['serviceId'][], -> = ScopedKnowledgeOperation & { - readonly organizationOperation: { - readonly delegatedServices?: readonly ( - | Services[number] - | ('copilot' extends NonNullable[number] - ? O['minimumRole'] extends 'read' - ? OrganizationDelegatedPrincipal['serviceId'] - : never - : never) - )[] - } } /** Binds organization policy to the same semantic operation declared for workspace access. */ -function defineKnowledgeOperation< - const O extends WorkspaceOperation, - const Services extends readonly OrganizationDelegatedPrincipal['serviceId'][] = readonly [], ->( +function defineKnowledgeOperation( operation: O, - options?: KnowledgeOperationOptions -): DelegatingKnowledgeOperation { - if ( - options?.organizationDelegatedServices?.length && - (operation.minimumRole !== 'read' || options.organizationDelegation === 'deny') - ) - throw new Error(`Operation ${operation.id} cannot delegate organization writes`) + options?: KnowledgeOperationOptions +): ScopedKnowledgeOperation { const supportsOrganizationDelegation = options?.organizationDelegation !== 'deny' && operation.minimumRole === 'read' && - (operation.delegatedServices?.includes('copilot') || - Boolean(options?.organizationDelegatedServices?.length)) + operation.delegatedServices?.includes('copilot') const organizationOperation = defineOrganizationOperation({ id: operation.id, capability: operation.capability, @@ -73,15 +44,11 @@ function defineKnowledgeOperation< ], delegationAudience: 'sim:knowledge', delegatedServices: - options?.organizationDelegatedServices ?? - (operation.id === 'knowledge.search' ? ['copilot', 'slack-search'] : ['copilot']), + operation.id === 'knowledge.search' ? ['copilot', 'slack-search'] : ['copilot'], } as const) : ({ principalKinds: ['session', 'personal_api_key', 'oauth_access_token'] } as const)), }) - return Object.freeze({ ...operation, organizationOperation }) as DelegatingKnowledgeOperation< - O, - Services - > + return Object.freeze({ ...operation, organizationOperation }) } const ALL_PRINCIPAL_POLICY = { @@ -714,8 +681,7 @@ export const knowledgeOperations = { workspaceApiKey: 'deny', capability: 'knowledge.use', principalKinds: ['session'], - }), - { organizationDelegatedServices: ['slack-search'] } + }) ), readSearchSourceOverview: defineKnowledgeOperation( defineWorkspaceOperation({ diff --git a/apps/sim/lib/knowledge/application/search-sources.test.ts b/apps/sim/lib/knowledge/application/search-sources.test.ts index f7b0933f525..036aed5ce73 100644 --- a/apps/sim/lib/knowledge/application/search-sources.test.ts +++ b/apps/sim/lib/knowledge/application/search-sources.test.ts @@ -68,7 +68,6 @@ import { import { readSearchSourceOverview } from '@/lib/knowledge/application/search-source-overview' import { readSearchSourceProgress } from '@/lib/knowledge/application/search-source-progress' import { listSearchSources } from '@/lib/knowledge/application/search-sources' -import { slackSearchMemberPrincipal } from '@/lib/knowledge/application/slack-search/member-principal' const principal = { kind: 'session' as const, userId: 'reader', sessionId: 'session' } const input = { workspaceId: 'workspace' } @@ -372,72 +371,6 @@ describe('Search source summaries', () => { }) describe('organization Search source summaries', () => { - function slackPrincipal() { - return slackSearchMemberPrincipal( - { installationId: 'i1', message: { eventId: 'Ev1' } }, - 'org-1', - 'reader' - ) - } - it('uses the Slack member’s ACL and includes their expired connection for Home', async () => { - const delegated = slackPrincipal() - mocks.context.mockResolvedValue({ organizationId: 'org-1' }) - queueTableRows(member, [{ role: 'member' }]) - mocks.memberships.mockResolvedValue(new Map([['drive', 'needs_reauth']])) - seed([source('drive', 'google_drive', 'members')]) - const result = await listSearchSources.execute({ - principal: delegated, - input: { organizationId: 'org-1' }, - }) - expect(result.sources[0]).toMatchObject({ - viewerMembership: 'needs_reauth', - connectionRequired: true, - }) - expect(mocks.access).toHaveBeenCalledWith(delegated, { organizationId: 'org-1' }) - expect(mocks.memberships).toHaveBeenCalledWith( - expect.objectContaining({ userId: 'reader', organizationId: 'org-1' }) - ) - }) - it('refuses Slack delegation into another organization', async () => { - mocks.context.mockResolvedValue({ organizationId: 'org-2' }) - await expect( - listSearchSources.execute({ principal: slackPrincipal(), input: { organizationId: 'org-2' } }) - ).rejects.toThrow('delegation') - expect(mocks.memberships).not.toHaveBeenCalled() - }) - it('rechecks Slack membership and delegation expiry before listing sources', async () => { - mocks.context.mockResolvedValue({ organizationId: 'org-1' }) - queueTableRows(member, []) - await expect( - listSearchSources.execute({ principal: slackPrincipal(), input: { organizationId: 'org-1' } }) - ).rejects.toThrow('Organization not found') - await expect( - listSearchSources.execute({ - principal: { ...slackPrincipal(), expiresAt: new Date(0) }, - input: { organizationId: 'org-1' }, - }) - ).rejects.toThrow('delegation') - expect(mocks.memberships).not.toHaveBeenCalled() - }) - it('does not make the source list available to Copilot or Slack installation authority', async () => { - await expect( - listSearchSources.execute({ - principal: { - kind: 'organization_delegated', - serviceId: 'copilot', - organizationId: 'org-1', - subjectUserId: 'reader', - delegationId: 'd1', - audience: 'sim:knowledge', - issuedAt: new Date(), - expiresAt: new Date(Date.now() + 60_000), - resourceScope: { chatId: 'chat1' }, - }, - input: { organizationId: 'org-1' }, - }) - ).rejects.toThrow('delegation') - expect(mocks.context).not.toHaveBeenCalled() - }) it.each(['member', 'admin'])( 'returns only the current %s viewer ACL counts without a workspace membership', async (role) => { diff --git a/apps/sim/lib/knowledge/application/search-sources.ts b/apps/sim/lib/knowledge/application/search-sources.ts index b19f3cc335c..8e1599057be 100644 --- a/apps/sim/lib/knowledge/application/search-sources.ts +++ b/apps/sim/lib/knowledge/application/search-sources.ts @@ -1,4 +1,3 @@ -import { requirePrincipalSubjectUserId } from '@sim/auth/principal' import { db } from '@sim/db' import { document, embedding, knowledgeBase, knowledgeConnector, user } from '@sim/db/schema' import { and, desc, eq, exists, inArray, isNull, lt, or, sql } from 'drizzle-orm' @@ -38,13 +37,12 @@ export const listSearchSources = defineAuthorizedKnowledgeUseCase({ resolveContext: ({ input }: { input: ListSearchSourcesInput }) => resolveKnowledgeOwnerContext(input), async execute({ principal, input, context }) { - const userId = requirePrincipalSubjectUserId(principal) const search = input.search?.trim().toLowerCase() ?? '' const connectorType = input.connectorType?.trim() const cursorScope = cursorScopeKey(cursorRoute(listSearchSourcesContract), { workspaceId: context.workspaceId, organizationId: context.organizationId, - userId, + userId: principal.userId, search, connectorType: connectorType ?? '', mine: input.mine === true, @@ -111,7 +109,7 @@ export const listSearchSources = defineAuthorizedKnowledgeUseCase({ const [availability, memberships, viewers, access, approvals] = await Promise.all([ resolveKnowledgeAccessAvailability(context), resolveViewerConnectorMemberships({ - userId, + userId: principal.userId, workspaceId: context.workspaceId, organizationId: context.organizationId, connectors: scanned, @@ -119,7 +117,7 @@ export const listSearchSources = defineAuthorizedKnowledgeUseCase({ db .select({ emailVerified: user.emailVerified }) .from(user) - .where(eq(user.id, userId)) + .where(eq(user.id, principal.userId)) .limit(1), createKnowledgeAccessProvider(principal, context).get(), context.organizationId ? listOrganizationSearchApprovals(context.organizationId) : null, diff --git a/apps/sim/lib/knowledge/application/slack-search/assistant.ts b/apps/sim/lib/knowledge/application/slack-search/assistant.ts index 8ead8880bf1..8628b03be93 100644 --- a/apps/sim/lib/knowledge/application/slack-search/assistant.ts +++ b/apps/sim/lib/knowledge/application/slack-search/assistant.ts @@ -1,4 +1,7 @@ -import type { SlackInstallationPrincipal } from '@sim/auth/principal' +import type { + OrganizationDelegatedPrincipal, + SlackInstallationPrincipal, +} from '@sim/auth/principal' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' @@ -38,7 +41,6 @@ import { resolveSlackSearchMember, SlackSearchIdentityError, } from '@/lib/knowledge/application/slack-search/identity' -import { slackSearchMemberPrincipal } from '@/lib/knowledge/application/slack-search/member-principal' import { sendSlackSearchOnboarding } from '@/lib/knowledge/application/slack-search/onboarding' import { recordSlackSearchOutcome } from '@/lib/knowledge/application/slack-search/repository' import { getSlackSearchSourceStatus } from '@/lib/knowledge/application/slack-search/source-status' @@ -58,6 +60,26 @@ import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secr const logger = createLogger('SlackSearchAssistant') +/** Creates narrowly scoped, short-lived authority for the current verified Slack sender. */ +export function slackSearchMemberPrincipal( + job: SlackSearchJob, + organizationId: string, + userId: string +): OrganizationDelegatedPrincipal { + const issuedAt = new Date() + return { + kind: 'organization_delegated', + serviceId: 'slack-search', + organizationId, + subjectUserId: userId, + delegationId: `${job.installationId}:${job.message.eventId}`, + audience: 'sim:knowledge', + issuedAt, + expiresAt: new Date(issuedAt.getTime() + 60_000), + resourceScope: { installationId: job.installationId, eventId: job.message.eventId }, + } +} + /** Runs the product's organization Assistant with its ordinary tools, chat lock, billing, and persistence. */ export async function runSlackSearchAssistant( principal: SlackInstallationPrincipal, diff --git a/apps/sim/lib/knowledge/application/slack-search/home.test.ts b/apps/sim/lib/knowledge/application/slack-search/home.test.ts index a8f24fba210..5d6b9ffe769 100644 --- a/apps/sim/lib/knowledge/application/slack-search/home.test.ts +++ b/apps/sim/lib/knowledge/application/slack-search/home.test.ts @@ -1,24 +1,12 @@ /** @vitest-environment node */ import { beforeEach, describe, expect, it, vi } from 'vitest' -const mocks = vi.hoisted(() => ({ - installation: vi.fn(), - sender: vi.fn(), - member: vi.fn(), - list: vi.fn(), - authorize: vi.fn(), - publish: vi.fn(), - enqueue: vi.fn(), -})) +const mocks = vi.hoisted(() => ({ installation: vi.fn(), publish: vi.fn(), enqueue: vi.fn() })) vi.mock('@/lib/core/async-jobs', () => ({ getInlineJobQueue: async () => ({ enqueue: mocks.enqueue }), })) vi.mock('@/lib/core/utils/urls', () => ({ getBaseUrl: () => 'https://sim.test' })) vi.mock('@/lib/internal/slack/client', () => ({ requestSlackApi: mocks.publish })) -vi.mock('@/lib/internal/slack/search-client', () => ({ getSlackSearchSender: mocks.sender })) -vi.mock('@/lib/knowledge/application/search-sources', () => ({ - listSearchSources: { execute: mocks.list, authorize: mocks.authorize }, -})) vi.mock('@/lib/knowledge/application/slack-search/repository', () => ({ findSlackSearchInstallation: mocks.installation, loadSlackSearchCredential: async () => ({ version: 'v1', botToken: 'private-bot-token' }), @@ -26,17 +14,12 @@ vi.mock('@/lib/knowledge/application/slack-search/repository', () => ({ vi.mock('@/lib/knowledge/access/availability', () => ({ requireOrganizationSearchAvailable: async () => undefined, })) -vi.mock('@/lib/knowledge/application/slack-search/identity', () => ({ - resolveSlackSearchMember: mocks.member, - SlackSearchIdentityError: class extends Error {}, -})) -vi.mock('@/connectors/registry', () => ({ getConnectorMeta: () => ({ name: 'Google Drive' }) })) import { publishSlackSearchHome, receiveSlackSearchHome, } from '@/lib/knowledge/application/slack-search/home' -import { SlackSearchIdentityError } from '@/lib/knowledge/application/slack-search/identity' +import { slackSearchHomeViewKey } from '@/lib/slack-search/home' const installation = { id: 'i1', @@ -77,32 +60,14 @@ function publish(signal = new AbortController().signal) { }, }) } -function source(id = 'source1') { - return { - connectorId: id, - connectorType: 'google_drive', - sourceDescription: 'Folder', - enabled: true, - approved: true, - availability: 'available', - connectionRequired: true, - viewerMembership: 'connected', - isSyncing: false, - hasSyncError: false, - } -} beforeEach(() => { vi.clearAllMocks() mocks.installation.mockResolvedValue(installation) - mocks.sender.mockResolvedValue({ email: 'viewer@example.test' }) - mocks.member.mockResolvedValue('member1') - mocks.list.mockResolvedValue({ sources: [source()], nextCursor: null }) - mocks.authorize.mockResolvedValue(undefined) mocks.publish.mockResolvedValue({ status: 200, data: { ok: true } }) }) -describe('Slack Home intake', () => { - it('queues a deduplicated app-process update with no token or Sim user in the payload', async () => { +describe('static Slack Home intake', () => { + it('queues one deduplicated app-process publication for a first or legacy visit', async () => { await receiveSlackSearchHome.execute({ principal: principal(), input: event }) expect(mocks.enqueue).toHaveBeenCalledWith( 'slack-search', @@ -114,14 +79,29 @@ describe('Slack Home intake', () => { concurrencyLimit: 2, }) ) - expect(JSON.stringify(mocks.enqueue.mock.calls[0][1])).not.toMatch( - /private-bot-token|member1|viewer@/ - ) - expect(mocks.sender).not.toHaveBeenCalled() + expect(JSON.stringify(mocks.enqueue.mock.calls[0][1])).not.toContain('private-bot-token') const [, payload, options] = mocks.enqueue.mock.calls[0] await options.runner(payload, new AbortController().signal) expect(mocks.publish).toHaveBeenCalledOnce() }) + it('acknowledges a current view without Home lookups, queue writes, or Slack API calls', async () => { + await receiveSlackSearchHome.execute({ + principal: principal(), + input: { ...event, viewKey: slackSearchHomeViewKey('c1', 'v1', 'https://sim.test') }, + }) + expect(mocks.installation).not.toHaveBeenCalled() + expect(mocks.enqueue).not.toHaveBeenCalled() + expect(mocks.publish).not.toHaveBeenCalled() + }) + it.each([ + slackSearchHomeViewKey('c1', 'old-version', 'https://sim.test'), + slackSearchHomeViewKey('c1', 'v1', 'https://old.test'), + slackSearchHomeViewKey('another-credential', 'v1', 'https://sim.test'), + 'old-layout', + ])('replaces a stale or differently bound view: %s', async (viewKey) => { + await receiveSlackSearchHome.execute({ principal: principal(), input: { ...event, viewKey } }) + expect(mocks.enqueue).toHaveBeenCalledOnce() + }) it.each(['appId', 'teamId', 'eventId'] as const)( 'rejects a mismatched %s before loading the installation', async (field) => { @@ -159,66 +139,40 @@ describe('Slack Home intake', () => { }) expect(mocks.enqueue).not.toHaveBeenCalled() }) + it('propagates queue failures so Slack can retry intake', async () => { + mocks.enqueue.mockRejectedValueOnce(new Error('queue unavailable')) + await expect( + receiveSlackSearchHome.execute({ principal: principal(), input: event }) + ).rejects.toThrow('queue unavailable') + }) }) -describe('personalized Home delivery', () => { - it('uses the installation’s organization, the current member, and the same source use case', async () => { +describe('static Home delivery', () => { + it('publishes only the static organization link and marker to the Slack viewer', async () => { await publish() - expect(mocks.sender).toHaveBeenCalledWith( - 'private-bot-token', - 'U1', - 'T1', - expect.any(AbortSignal) - ) - expect(mocks.list).toHaveBeenCalledWith({ - principal: expect.objectContaining({ - kind: 'organization_delegated', - serviceId: 'slack-search', - organizationId: 'org1', - subjectUserId: 'member1', - resourceScope: { installationId: 'i1', eventId: 'Ev1' }, - }), - input: { organizationId: 'org1', cursor: undefined }, - }) - expect(mocks.member).toHaveBeenCalledTimes(2) - expect(mocks.authorize).toHaveBeenCalledOnce() + expect(mocks.publish).toHaveBeenCalledOnce() expect(mocks.publish).toHaveBeenCalledWith( expect.objectContaining({ method: 'views.publish', body: expect.objectContaining({ user_id: 'U1', hash: 'h1', - view: expect.objectContaining({ type: 'home' }), + view: expect.objectContaining({ + type: 'home', + callback_id: slackSearchHomeViewKey('c1', 'v1', 'https://sim.test'), + }), }), }) ) - expect(JSON.stringify(mocks.publish.mock.calls[0][0].body)).toContain( - 'https://sim.test/o/org1/integrations' - ) - }) - it.each([ - 'account_required', - 'verify_email', - 'membership_required', - 'identity_conflict', - ] as const)('shows no protected source details for %s', async (reason) => { - mocks.member.mockRejectedValue(new SlackSearchIdentityError(reason)) - await publish() - expect(mocks.list).not.toHaveBeenCalled() - expect(JSON.stringify(mocks.publish.mock.calls[0][0].body)).toContain('Sign in to Sim') - expect(JSON.stringify(mocks.publish.mock.calls[0][0].body)).not.toContain('Google Drive') + const { view } = mocks.publish.mock.calls[0][0].body + expect(view.blocks).toHaveLength(3) + expect(view.blocks[2].elements[0].url).toBe('https://sim.test/o/org1/integrations') }) - it('propagates infrastructure failures instead of inventing an empty source list', async () => { - mocks.list.mockRejectedValueOnce(new Error('database unavailable')) + it('propagates infrastructure failures instead of sending a fallback link', async () => { + mocks.installation.mockRejectedValueOnce(new Error('database unavailable')) await expect(publish()).rejects.toThrow('database unavailable') expect(mocks.publish).not.toHaveBeenCalled() }) - it('rejects Slack users with no valid workspace email', async () => { - mocks.sender.mockResolvedValueOnce(null) - await expect(publish()).rejects.toThrow() - expect(mocks.list).not.toHaveBeenCalled() - expect(mocks.publish).not.toHaveBeenCalled() - }) it.each([ { enabled: false }, { revision: 'r2' }, @@ -231,44 +185,14 @@ describe('personalized Home delivery', () => { else await expect(publish()).rejects.toThrow('binding') expect(mocks.publish).not.toHaveBeenCalled() }) - it('rechecks disablement immediately before publishing', async () => { - mocks.installation - .mockResolvedValueOnce(installation) - .mockResolvedValueOnce({ ...installation, enabled: false }) - await publish() - expect(mocks.publish).not.toHaveBeenCalled() - }) - it('rejects lost member authorization before delivery', async () => { - mocks.authorize.mockRejectedValueOnce(new Error('member access removed')) - await expect(publish()).rejects.toThrow('member access removed') - expect(mocks.publish).not.toHaveBeenCalled() - }) - it('caps sparse pagination and avoids claiming there are no connections', async () => { - mocks.list.mockResolvedValue({ sources: [], nextCursor: 'cursor' }) - await publish() - expect(mocks.list).toHaveBeenCalledTimes(4) - expect(JSON.stringify(mocks.publish.mock.calls[0][0].body)).not.toContain( - 'No sources connected yet' - ) - }) - it('caps rendered sources without fetching more pages', async () => { - mocks.list.mockResolvedValue({ - sources: Array.from({ length: 25 }, (_, i) => source(`source${i}`)), - nextCursor: 'cursor', - }) - await publish() - expect(mocks.list).toHaveBeenCalledOnce() - expect(mocks.publish.mock.calls[0][0].body.view.blocks).toHaveLength(25) - }) it('stops work on cancellation', async () => { await expect(publish(AbortSignal.abort())).rejects.toThrow() - expect(mocks.list).not.toHaveBeenCalled() expect(mocks.publish).not.toHaveBeenCalled() }) it.each([ { status: 500, data: { ok: false } }, { status: 200, data: { ok: false, error: 'hash_conflict' } }, - ])('records failed publishing without a fallback or replay: %j', async (response) => { + ])('records failed publishing without a replay: %j', async (response) => { mocks.publish.mockResolvedValueOnce(response) await expect(publish()).rejects.toThrow('Could not publish') expect(mocks.publish).toHaveBeenCalledOnce() diff --git a/apps/sim/lib/knowledge/application/slack-search/home.ts b/apps/sim/lib/knowledge/application/slack-search/home.ts index 9a1efcefeea..fdc56d80678 100644 --- a/apps/sim/lib/knowledge/application/slack-search/home.ts +++ b/apps/sim/lib/knowledge/application/slack-search/home.ts @@ -4,27 +4,18 @@ import { getInlineJobQueue } from '@/lib/core/async-jobs' import { OrchestrationError } from '@/lib/core/orchestration/types' import { getBaseUrl } from '@/lib/core/utils/urls' import { requestSlackApi } from '@/lib/internal/slack/client' -import { getSlackSearchSender } from '@/lib/internal/slack/search-client' -import { listSearchSources } from '@/lib/knowledge/application/search-sources' import { authorizeSlackSearchInstallation, requireSlackInstallationPrincipal, } from '@/lib/knowledge/application/slack-search/authorization' -import { - resolveSlackSearchMember, - SlackSearchIdentityError, -} from '@/lib/knowledge/application/slack-search/identity' -import { slackSearchMemberPrincipal } from '@/lib/knowledge/application/slack-search/member-principal' import { organizationRoutes } from '@/lib/navigation/paths' import { renderSlackSearchHome, SLACK_SEARCH_HOME_MAX_AGE_MS, - SLACK_SEARCH_HOME_MAX_SOURCES, type SlackSearchHomeEvent, type SlackSearchHomeJob, - type SlackSearchHomeSource, slackSearchHomeJobSchema, - slackSearchHomeSource, + slackSearchHomeViewKey, } from '@/lib/slack-search/home' const operation = Object.freeze({ @@ -43,7 +34,7 @@ function requireHomeBinding(principal: SlackInstallationPrincipal, event: SlackS throw new OrchestrationError('forbidden', 'Slack Home event authority is no longer valid') } -/** Persists a deduplicated, bounded app-process job so Slack can acknowledge Home opens promptly. */ +/** Current views acknowledge immediately; first visits and obsolete layouts queue one static publication. */ export const receiveSlackSearchHome: OperationUseCase< typeof operation, SlackSearchHomeEvent, @@ -53,6 +44,11 @@ export const receiveSlackSearchHome: OperationUseCase< async execute({ principal, input }) { requireSlackInstallationPrincipal(principal) requireHomeBinding(principal, input) + if ( + input.viewKey === + slackSearchHomeViewKey(principal.credentialId, principal.credentialVersion, getBaseUrl()) + ) + return const context = await authorizeSlackSearchInstallation(principal) if (!context || input.userId === context.installation.botUserId) return const job: SlackSearchHomeJob = { @@ -88,7 +84,7 @@ export const receiveSlackSearchHome: OperationUseCase< }, } -/** Resolves the event's member, reads the product's authorized source list, and publishes only to that Slack user. */ +/** Publishes a static organization link with no member, account, or source lookup. Sim authorizes the click. */ export const publishSlackSearchHome: OperationUseCase< typeof operation, { job: SlackSearchHomeJob; signal: AbortSignal }, @@ -102,82 +98,22 @@ export const publishSlackSearchHome: OperationUseCase< const context = await authorizeSlackSearchInstallation(principal, job) if (!context) return const { installation, secret } = context - const sender = await getSlackSearchSender( - secret.botToken, - job.event.userId, - installation.teamId, - signal - ) - if (!sender) throw new SlackSearchIdentityError() - const sources: SlackSearchHomeSource[] = [] - let userId: string | undefined - try { - userId = await resolveSlackSearchMember( - installation.organizationId, - installation.teamId, - job.event.userId, - sender.email - ) - } catch (error) { - if (!(error instanceof SlackSearchIdentityError)) throw error - } - let hasMore = false - let memberPrincipal: ReturnType | undefined - if (userId) { - memberPrincipal = slackSearchMemberPrincipal( - { installationId: installation.id, message: { eventId: job.event.eventId } }, - installation.organizationId, - userId - ) - let cursor: string | undefined - /** Bound sparse pages as well as visible rows; the existing Sources page handles the remainder. */ - for (let page = 0; page < 4; page++) { - signal.throwIfAborted() - const result = await listSearchSources.execute({ - principal: memberPrincipal, - input: { organizationId: installation.organizationId, cursor }, - }) - const visible = result.sources.flatMap((source) => { - const row = slackSearchHomeSource(source) - return row ? [row] : [] - }) - const remaining = SLACK_SEARCH_HOME_MAX_SOURCES - sources.length - sources.push(...visible.slice(0, remaining)) - hasMore = Boolean(result.nextCursor) || visible.length > remaining - if (!result.nextCursor || sources.length === SLACK_SEARCH_HOME_MAX_SOURCES) break - cursor = result.nextCursor - } - } - const current = await authorizeSlackSearchInstallation(principal, job) - if (!current) return - if (memberPrincipal) { - const currentUserId = await resolveSlackSearchMember( - installation.organizationId, - installation.teamId, - job.event.userId, - sender.email - ) - if (currentUserId !== userId) throw new SlackSearchIdentityError('identity_conflict') - await listSearchSources.authorize({ - principal: memberPrincipal, - input: { organizationId: installation.organizationId }, - }) - } + const baseUrl = getBaseUrl() signal.throwIfAborted() const response = await requestSlackApi({ - accessToken: current.secret.botToken, + accessToken: secret.botToken, method: 'views.publish', body: { user_id: job.event.userId, ...(job.event.viewHash ? { hash: job.event.viewHash } : {}), view: renderSlackSearchHome({ - sourcesUrl: new URL( - organizationRoutes(installation.organizationId).integrations, - getBaseUrl() - ).href, - sources, - hasMore, - accountRequired: !userId, + sourcesUrl: new URL(organizationRoutes(installation.organizationId).integrations, baseUrl) + .href, + viewKey: slackSearchHomeViewKey( + principal.credentialId, + principal.credentialVersion, + baseUrl + ), }), }, signal, diff --git a/apps/sim/lib/knowledge/application/slack-search/member-principal.ts b/apps/sim/lib/knowledge/application/slack-search/member-principal.ts deleted file mode 100644 index c79bd5f5de3..00000000000 --- a/apps/sim/lib/knowledge/application/slack-search/member-principal.ts +++ /dev/null @@ -1,21 +0,0 @@ -import type { OrganizationDelegatedPrincipal } from '@sim/auth/principal' - -/** Creates narrowly scoped, short-lived authority for a current verified Slack sender. */ -export function slackSearchMemberPrincipal( - event: { installationId: string; message: { eventId: string } }, - organizationId: string, - userId: string -): OrganizationDelegatedPrincipal { - const issuedAt = new Date() - return { - kind: 'organization_delegated', - serviceId: 'slack-search', - organizationId, - subjectUserId: userId, - delegationId: `${event.installationId}:${event.message.eventId}`, - audience: 'sim:knowledge', - issuedAt, - expiresAt: new Date(issuedAt.getTime() + 60_000), - resourceScope: { installationId: event.installationId, eventId: event.message.eventId }, - } -} diff --git a/apps/sim/lib/slack-search/home.test.ts b/apps/sim/lib/slack-search/home.test.ts index 0a60be38db4..6465184d83b 100644 --- a/apps/sim/lib/slack-search/home.test.ts +++ b/apps/sim/lib/slack-search/home.test.ts @@ -1,14 +1,9 @@ /** @vitest-environment node */ -import { describe, expect, it, vi } from 'vitest' - -vi.mock('@/connectors/registry', () => ({ - getConnectorMeta: (id: string) => (id === 'google_drive' ? { name: 'Google Drive' } : undefined), -})) - +import { describe, expect, it } from 'vitest' import { parseSlackSearchHomeEvent, renderSlackSearchHome, - slackSearchHomeSource, + slackSearchHomeViewKey, } from '@/lib/slack-search/home' const now = 1_800_000_000_000 @@ -20,34 +15,21 @@ const event = { event_time: now / 1000, event: { type: 'app_home_opened', tab: 'home', user: 'U1' }, } -const source = { - knowledgeBaseId: 'kb1', - connectorId: 'source1', - connectorType: 'google_drive', - sourceDescription: '1 folder selected', - accessMode: 'members' as const, - availability: 'available' as const, - enabled: true, - approved: true, - isSyncing: false, - lastSyncAt: null, - hasSyncError: false, - viewerDocumentCount: 1, - viewerFailedDocumentCount: 0, - viewerEmailVerified: true, - connectionRequired: true as const, - viewerMembership: 'connected' as const, -} describe('Slack Home events', () => { - it('retains only the routing identity and concurrency hash', () => { + it('retains routing identity, the published marker, and concurrency hash without the old content', () => { expect( parseSlackSearchHomeEvent( { ...event, event: { ...event.event, - view: { type: 'home', hash: 'h1', blocks: ['old private data'] }, + view: { + type: 'home', + hash: 'h1', + callback_id: 'view-key', + blocks: ['old private data'], + }, }, }, now @@ -58,8 +40,18 @@ describe('Slack Home events', () => { eventId: 'Ev1', userId: 'U1', viewHash: 'h1', + viewKey: 'view-key', }) }) + it('accepts first visits and legacy views without a marker', () => { + expect(parseSlackSearchHomeEvent(event, now)?.viewKey).toBeUndefined() + expect( + parseSlackSearchHomeEvent( + { ...event, event: { ...event.event, view: { type: 'home', hash: 'h1' } } }, + now + ) + ).toMatchObject({ userId: 'U1', viewHash: 'h1' }) + }) it.each(['messages', 'about', undefined])('ignores the %s tab', (tab) => { expect(parseSlackSearchHomeEvent({ ...event, event: { ...event.event, tab } }, now)).toBeNull() }) @@ -74,102 +66,46 @@ describe('Slack Home events', () => { }) }) -describe('personalized source statuses', () => { - it('shows connected and syncing sources, with reconnect taking precedence', () => { - expect(slackSearchHomeSource(source)?.status).toBe('Connected') - expect(slackSearchHomeSource({ ...source, isSyncing: true })?.status).toBe('Syncing') - expect( - slackSearchHomeSource({ ...source, viewerMembership: 'needs_reauth', isSyncing: true }) - ?.status - ).toBe('Reconnect needed') - }) - it.each(['invited', 'not_enrolled', 'revoked', 'unverified_email', null] as const)( - 'does not list someone else’s connection for %s', - (viewerMembership) => { - expect(slackSearchHomeSource({ ...source, viewerMembership })).toBeNull() - } - ) - it('includes available shared sources that do not require individual connections', () => { - expect( - slackSearchHomeSource({ - ...source, - accessMode: 'admin', - connectionRequired: false, - viewerMembership: null, - })?.status - ).toBe('Connected') - }) - it('omits disabled, unavailable, and unapproved sources', () => { - expect(slackSearchHomeSource({ ...source, enabled: false })).toBeNull() - expect(slackSearchHomeSource({ ...source, approved: false })).toBeNull() - expect(slackSearchHomeSource({ ...source, availability: 'unavailable' })).toBeNull() - }) - it('does not mislabel a failed sync as expired authorization', () => { - expect(slackSearchHomeSource({ ...source, hasSyncError: true })).toMatchObject({ - status: 'Connected', - syncError: true, - }) - }) -}) - -describe('Slack Home presentation', () => { - const sourcesUrl = 'https://sim.test/o/org1/integrations' - it('renders a single URL button and compact source statuses as plain text', () => { - const view = renderSlackSearchHome({ - sourcesUrl, - sources: [slackSearchHomeSource(source)!], - hasMore: false, - }) - expect(view).toMatchObject({ type: 'home' }) - expect(JSON.stringify(view)).toContain('Google Drive — Connected') - const blocks = view.blocks as Record[] - expect(blocks.filter((block) => block.type === 'actions')).toEqual([ - { - type: 'actions', - elements: [ - { - type: 'button', - action_id: 'sim_search.connect_sources', - text: { type: 'plain_text', text: 'Connect sources' }, - style: 'primary', - url: sourcesUrl, - }, - ], - }, +describe('persistent Home view', () => { + it('uses a stable opaque marker and invalidates it when the binding or origin changes', () => { + const key = slackSearchHomeViewKey('c1', 'v1', 'https://sim.test') + expect(key).toBe(slackSearchHomeViewKey('c1', 'v1', 'https://sim.test')) + for (const changed of [ + slackSearchHomeViewKey('c2', 'v1', 'https://sim.test'), + slackSearchHomeViewKey('c1', 'v2', 'https://sim.test'), + slackSearchHomeViewKey('c1', 'v1', 'https://other.test'), ]) - expect(JSON.stringify(view)).not.toContain('mrkdwn') + expect(changed).not.toBe(key) + expect(key).toMatch(/^sim_search\.connect_sources\.v1:[a-f0-9]{64}$/) }) - it('never renders source details for an unmatched account', () => { - const view = renderSlackSearchHome({ - sourcesUrl, - sources: [slackSearchHomeSource(source)!], - hasMore: false, - accountRequired: true, - }) - expect(JSON.stringify(view)).toContain('Sign in to Sim') - expect(JSON.stringify(view)).not.toContain('Google Drive') - }) - it('caps provider content and source rows, including hostile labels', () => { - const view = renderSlackSearchHome({ - sourcesUrl, - sources: Array.from({ length: 200 }, () => ({ - name: '', - description: 'x'.repeat(4000), - status: 'Connected', - syncError: false, - })), - hasMore: true, + it('publishes static copy and a stable URL button with no invitation or status data', () => { + const sourcesUrl = 'https://sim.test/o/org1/integrations' + const viewKey = slackSearchHomeViewKey('c1', 'v1', 'https://sim.test') + expect(renderSlackSearchHome({ sourcesUrl, viewKey })).toEqual({ + type: 'home', + callback_id: viewKey, + blocks: [ + { type: 'header', text: { type: 'plain_text', text: 'Connect your sources' } }, + { + type: 'section', + text: { + type: 'plain_text', + text: 'Connect more accounts in Sim to expand what you can search.', + }, + }, + { + type: 'actions', + elements: [ + { + type: 'button', + action_id: 'sim_search.connect_sources', + text: { type: 'plain_text', text: 'Connect sources' }, + style: 'primary', + url: sourcesUrl, + }, + ], + }, + ], }) - expect((view.blocks as unknown[]).length).toBe(25) - expect(JSON.stringify(view)).not.toContain('x'.repeat(201)) - expect(JSON.stringify(view)).toContain('More sources are available') - }) - it('explains empty connections and bounded sparse lists honestly', () => { - expect( - JSON.stringify(renderSlackSearchHome({ sourcesUrl, sources: [], hasMore: false })) - ).toContain('No sources connected yet') - expect( - JSON.stringify(renderSlackSearchHome({ sourcesUrl, sources: [], hasMore: true })) - ).not.toContain('No sources connected yet') }) }) diff --git a/apps/sim/lib/slack-search/home.ts b/apps/sim/lib/slack-search/home.ts index 868c70ef7b0..cdcd0427144 100644 --- a/apps/sim/lib/slack-search/home.ts +++ b/apps/sim/lib/slack-search/home.ts @@ -1,11 +1,8 @@ -import { truncate } from '@sim/utils/string' +import { createHash } from 'node:crypto' import { z } from 'zod' import type { SlackJsonObject } from '@/lib/internal/slack/client' -import type { listSearchSources } from '@/lib/knowledge/application/search-sources' -import { getConnectorMeta } from '@/connectors/registry' const id = z.string().min(1).max(200) -export const SLACK_SEARCH_HOME_MAX_SOURCES = 20 export const SLACK_SEARCH_HOME_MAX_AGE_MS = 5 * 60_000 const homeEventSchema = z.object({ @@ -18,7 +15,9 @@ const homeEventSchema = z.object({ type: z.literal('app_home_opened'), tab: z.literal('home'), user: id, - view: z.object({ type: z.literal('home'), hash: id }).optional(), + view: z + .object({ type: z.literal('home'), hash: id, callback_id: z.string().max(255).optional() }) + .optional(), }), }) @@ -28,6 +27,7 @@ const slackSearchHomeEventSchema = z.object({ eventId: id, userId: id, viewHash: id.optional(), + viewKey: z.string().max(255).optional(), }) export type SlackSearchHomeEvent = z.infer @@ -60,58 +60,34 @@ export function parseSlackSearchHomeEvent( teamId: envelope.team_id, eventId: envelope.event_id, userId: event.user, - ...(event.view ? { viewHash: event.view.hash } : {}), + ...(event.view ? { viewHash: event.view.hash, viewKey: event.view.callback_id } : {}), } } -type SearchSource = Awaited>['sources'][number] - -export interface SlackSearchHomeSource { - name: string - description: string - status: 'Connected' | 'Syncing' | 'Reconnect needed' - syncError: boolean -} - -/** Uses viewer membership, never another member's grant or a generic sync failure, for connection state. */ -export function slackSearchHomeSource(source: SearchSource): SlackSearchHomeSource | null { - if (!source.enabled || source.approved === false || source.availability !== 'available') - return null - if ( - source.connectionRequired && - source.viewerMembership !== 'connected' && - source.viewerMembership !== 'needs_reauth' - ) - return null - return { - name: getConnectorMeta(source.connectorType)?.name ?? source.connectorType, - description: source.sourceDescription, - status: - source.viewerMembership === 'needs_reauth' - ? 'Reconnect needed' - : source.isSyncing - ? 'Syncing' - : 'Connected', - syncError: source.hasSyncError, - } +/** Slack retains this marker with the view; reconnecting or changing the app origin invalidates it. */ +export function slackSearchHomeViewKey( + credentialId: string, + credentialVersion: string, + baseUrl: string +): string { + const binding = createHash('sha256') + .update(JSON.stringify([credentialId, credentialVersion, baseUrl])) + .digest('hex') + return `sim_search.connect_sources.v1:${binding}` } -/** Plain text source rows cannot turn provider labels into Slack mentions or attacker links. */ +/** A persistent link to Sim; it carries no invitation, account details, or source status. */ export function renderSlackSearchHome(input: { sourcesUrl: string - sources: SlackSearchHomeSource[] - hasMore: boolean - accountRequired?: boolean + viewKey: string }): SlackJsonObject { const blocks: SlackJsonObject[] = [ - { type: 'header', text: { type: 'plain_text', text: 'Your sources' } }, + { type: 'header', text: { type: 'plain_text', text: 'Connect your sources' } }, { type: 'section', text: { type: 'plain_text', - text: input.accountRequired - ? 'Sign in to Sim with your Slack email and join this organization to see your sources.' - : 'Connect your tools to search them with Sim. Manage connections and sync details in Sim.', + text: 'Connect more accounts in Sim to expand what you can search.', }, }, { @@ -126,40 +102,6 @@ export function renderSlackSearchHome(input: { }, ], }, - { type: 'divider' }, ] - if (!input.accountRequired) { - for (const source of input.sources.slice(0, SLACK_SEARCH_HOME_MAX_SOURCES)) { - blocks.push({ - type: 'section', - text: { - type: 'plain_text', - text: `${truncate(source.name, 100)} — ${source.status}${source.description ? `\n${truncate(source.description, 200)}` : ''}${source.syncError && source.status === 'Connected' ? '\nSync needs attention. Open Sim for details.' : ''}`, - }, - }) - } - if (input.sources.length === 0) { - blocks.push({ - type: 'section', - text: { - type: 'plain_text', - text: input.hasMore - ? 'Open Sim to view your sources and connect more tools.' - : 'No sources connected yet. Connect a source to get started.', - }, - }) - } - blocks.push({ - type: 'context', - elements: [ - { - type: 'plain_text', - text: input.hasMore - ? 'More sources are available in Sim. Statuses refresh when you open this tab.' - : 'Statuses refresh when you open this tab.', - }, - ], - }) - } - return { type: 'home', blocks } + return { type: 'home', callback_id: input.viewKey, blocks } }