Skip to content

Commit f02249a

Browse files
committed
fix(knowledge): remove connections with durable background cleanup
1 parent e21130b commit f02249a

20 files changed

Lines changed: 943 additions & 160 deletions

File tree

‎apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.test.tsx‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const mocks = vi.hoisted(() => ({
1313
detail: vi.fn(),
1414
integrations: vi.fn(),
1515
push: vi.fn(),
16+
replace: vi.fn(),
1617
documents: vi.fn(),
1718
actions: vi.fn(),
1819
recovery: vi.fn(),
@@ -23,7 +24,7 @@ const mocks = vi.hoisted(() => ({
2324
save: vi.fn(),
2425
}))
2526
vi.mock('next/navigation', () => ({
26-
useRouter: () => ({ push: mocks.push }),
27+
useRouter: () => ({ push: mocks.push, replace: mocks.replace }),
2728
usePathname: () => '/o/org-one/settings/integrations/sources/source-one',
2829
}))
2930
vi.mock('@/app/o/[organizationId]/providers/organization-provider', () => ({
@@ -185,6 +186,17 @@ describe('organization source detail navigation', () => {
185186
expect(button, `Missing ${text}`).toBeTruthy()
186187
await act(async () => button!.click())
187188
}
189+
it.each(['documents', 'settings', 'history'])(
190+
'replaces the removed connection with Sources from the %s view',
191+
async (view) => {
192+
await render(`?view=${view}`)
193+
const options: ConnectorActionsOptions = mocks.actions.mock.lastCall![0]
194+
act(() => options.onRemoved?.())
195+
expect(mocks.replace).toHaveBeenCalledWith('/o/org-one/settings/integrations')
196+
expect(mocks.push).not.toHaveBeenCalled()
197+
}
198+
)
199+
188200
it('opens documents by default and uses the exact canonical search index', async () => {
189201
await render()
190202
expect(mocks.detail).toHaveBeenLastCalledWith('index-one', 'source-one')

‎apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ function SourceDetailContent({
197197
const description =
198198
[title === meta?.name ? undefined : meta?.name, status].filter(Boolean).join(' · ') || undefined
199199
const onBack = () => router.push(backHref)
200+
const onRemoved = () =>
201+
router.replace(organizationRoutes(organization.id).settingsSection('integrations'))
200202
const onViewChange = (value: string) => {
201203
const next = sourceViewParam.parser.parse(value)
202204
if (next) void setView(next)
@@ -254,6 +256,7 @@ function SourceDetailContent({
254256
queryError={integrationFeedback}
255257
backText={backText}
256258
onBack={onBack}
259+
onRemoved={onRemoved}
257260
onViewChange={onViewChange}
258261
/>
259262
)
@@ -264,7 +267,7 @@ function SourceDetailContent({
264267
title={title}
265268
description={description}
266269
docsLink={meta?.searchDocsUrl}
267-
onRemoved={onBack}
270+
onRemoved={onRemoved}
268271
>
269272
{integrationFeedback}
270273
<SourceNavigation view={view} onViewChange={onViewChange} />
@@ -367,6 +370,7 @@ interface SourceSettingsEditorProps {
367370
queryError?: ReactNode
368371
backText: string
369372
onBack: () => void
373+
onRemoved: () => void
370374
onViewChange: (view: string) => void
371375
}
372376

@@ -400,6 +404,7 @@ function SourceSettingsForm({
400404
queryError,
401405
backText,
402406
onBack,
407+
onRemoved,
403408
onViewChange,
404409
onSaved,
405410
onDiscard,
@@ -420,7 +425,7 @@ function SourceSettingsForm({
420425
description={description}
421426
docsLink={form.docsUrl}
422427
lifecycleDisabled={form.dirty || form.saving}
423-
onRemoved={onBack}
428+
onRemoved={onRemoved}
424429
actions={saveDiscardActions({
425430
dirty: form.dirty,
426431
saving: form.saving,

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.test.tsx‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ const {
3636
isFetching: false,
3737
},
3838
lifecycle: {
39+
removeOptions: { onSuccess: undefined as (() => void) | undefined },
3940
sync: { mutate: vi.fn(), reset: vi.fn(), error: null as Error | null, isPending: false },
4041
update: { mutate: vi.fn(), reset: vi.fn(), error: null as Error | null, isPending: false },
4142
remove: { mutate: vi.fn(), reset: vi.fn(), error: null as Error | null, isPending: false },
@@ -235,7 +236,10 @@ vi.mock('@/hooks/queries/kb/connectors', () => ({
235236
isPlaceholderData: lifecycle.detail.isPlaceholderData,
236237
refetch: lifecycle.detail.refetch,
237238
})),
238-
useDeleteConnector: () => lifecycle.remove,
239+
useDeleteConnector: (options: { onSuccess: () => void }) => {
240+
lifecycle.removeOptions = options
241+
return lifecycle.remove
242+
},
239243
useTriggerSync: () => lifecycle.sync,
240244
useUpdateConnector: () => lifecycle.update,
241245
}))
@@ -943,15 +947,12 @@ describe('shared connector lifecycle actions', () => {
943947
expect(dialog.textContent).not.toContain('remain unless')
944948
}
945949
act(() => findButton(dialog, 'Remove').click())
946-
expect(lifecycle.remove.mutate).toHaveBeenCalledWith(
947-
{
948-
knowledgeBaseId: 'knowledge-1',
949-
connectorId: 'connector-1',
950-
deleteDocuments: accessMode !== 'workspace',
951-
},
952-
expect.any(Object)
953-
)
954-
act(() => lifecycle.remove.mutate.mock.calls[0][1].onSuccess())
950+
expect(lifecycle.remove.mutate).toHaveBeenCalledWith({
951+
knowledgeBaseId: 'knowledge-1',
952+
connectorId: 'connector-1',
953+
deleteDocuments: accessMode !== 'workspace',
954+
})
955+
act(() => lifecycle.removeOptions.onSuccess?.())
955956
expect(onRemoved).toHaveBeenCalledOnce()
956957
expect(container.querySelector('[role="dialog"]')).toBeNull()
957958
}

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/use-connector-actions.ts‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,15 @@ export function useConnectorActions({
3131
}: ConnectorActionsOptions) {
3232
const sync = useTriggerSync()
3333
const update = useUpdateConnector()
34-
const remove = useDeleteConnector()
3534
const [confirmRemove, setConfirmRemove] = useState(false)
3635
const [deleteDocuments, setDeleteDocuments] = useState(false)
36+
const remove = useDeleteConnector({
37+
onSuccess: () => {
38+
setConfirmRemove(false)
39+
setDeleteDocuments(false)
40+
onRemoved?.()
41+
},
42+
})
3743
const requiresDocumentDeletion = connector.accessMode !== 'workspace'
3844
const state = getConnectorSyncState(connector)
3945
const actionsDisabled = disabled || sync.isPending || update.isPending || remove.isPending
@@ -117,20 +123,11 @@ export function useConnectorActions({
117123
error: remove.error,
118124
onConfirm: () => {
119125
if (!canEdit || actionsDisabled) return
120-
remove.mutate(
121-
{
122-
knowledgeBaseId,
123-
connectorId: connector.id,
124-
deleteDocuments: requiresDocumentDeletion || deleteDocuments,
125-
},
126-
{
127-
onSuccess: () => {
128-
setConfirmRemove(false)
129-
setDeleteDocuments(false)
130-
onRemoved?.()
131-
},
132-
}
133-
)
126+
remove.mutate({
127+
knowledgeBaseId,
128+
connectorId: connector.id,
129+
deleteDocuments: requiresDocumentDeletion || deleteDocuments,
130+
})
134131
},
135132
},
136133
}

‎apps/sim/hooks/queries/kb/connectors-cache.test.tsx‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,46 @@ describe('connector Search result cache reconciliation', () => {
388388
})
389389

390390
describe('Search source list reconciliation', () => {
391+
it('runs removal navigation before refetches and retains it after the caller unmounts', async () => {
392+
const client = createQueryClient()
393+
const request = Promise.withResolvers<object>()
394+
mocks.requestJson.mockReturnValueOnce(request.promise)
395+
const invalidated = vi.spyOn(client, 'invalidateQueries')
396+
const onSuccess = vi.fn(() => expect(invalidated).not.toHaveBeenCalled())
397+
const mutation = renderMutation(client, () => useDeleteConnector({ onSuccess }))
398+
let done!: Promise<void>
399+
await act(async () => {
400+
done = mutation().mutateAsync({
401+
knowledgeBaseId: KNOWLEDGE_BASE_ID,
402+
connectorId: CONNECTOR_ID,
403+
deleteDocuments: true,
404+
})
405+
})
406+
act(() => mountedRoots.pop()!.unmount())
407+
request.resolve({ success: true })
408+
await act(async () => {
409+
await done
410+
})
411+
expect(onSuccess).toHaveBeenCalledOnce()
412+
expect(invalidated).toHaveBeenCalledWith({
413+
queryKey: connectorKeys.detail(KNOWLEDGE_BASE_ID, CONNECTOR_ID),
414+
refetchType: 'none',
415+
})
416+
})
417+
418+
it('does not navigate when removal fails', async () => {
419+
const client = createQueryClient()
420+
const onSuccess = vi.fn()
421+
mocks.requestJson.mockRejectedValueOnce(new Error('Removal failed'))
422+
const mutation = renderMutation(client, () => useDeleteConnector({ onSuccess }))
423+
await act(async () => {
424+
await expect(
425+
mutation().mutateAsync({ knowledgeBaseId: KNOWLEDGE_BASE_ID, connectorId: CONNECTOR_ID })
426+
).rejects.toThrow('Removal failed')
427+
})
428+
expect(onSuccess).not.toHaveBeenCalled()
429+
})
430+
391431
it('refreshes summaries after editing source configuration or pausing sync', async () => {
392432
const queryClient = createQueryClient()
393433
const mutation = renderMutation(queryClient, useUpdateConnector)

‎apps/sim/hooks/queries/kb/connectors.ts‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -657,19 +657,37 @@ async function deleteConnector({
657657
})
658658
}
659659

660-
export function useDeleteConnector() {
660+
interface UseDeleteConnectorOptions {
661+
onSuccess?: () => void
662+
}
663+
664+
export function useDeleteConnector(options?: UseDeleteConnectorOptions) {
661665
const queryClient = useQueryClient()
662666

663667
return useMutation({
664668
mutationFn: deleteConnector,
669+
/** Run before invalidation can unmount the source page on a 404 response. */
670+
onSuccess: () => options?.onSuccess?.(),
665671
/**
666672
* Removing a connector can take its documents with it, so the document
667673
* lists and the base's own totals move — but nothing below them does.
668674
* Invalidating `knowledgeKeys.detail` as a prefix would also refetch every
669675
* cached document detail, chunk page, and chunk search in the base.
670676
*/
671-
onSettled: (_data, _error, { knowledgeBaseId, deleteDocuments }) => {
672-
queryClient.invalidateQueries({ queryKey: connectorKeys.all(knowledgeBaseId) })
677+
onSettled: (_data, error, { knowledgeBaseId, connectorId, deleteDocuments }) => {
678+
if (error) {
679+
queryClient.invalidateQueries({ queryKey: connectorKeys.all(knowledgeBaseId) })
680+
} else {
681+
queryClient.invalidateQueries({ queryKey: connectorKeys.lists(knowledgeBaseId) })
682+
/** Retire stale detail pages without fetching the just-deleted resource during navigation. */
683+
void queryClient.cancelQueries({
684+
queryKey: connectorKeys.detail(knowledgeBaseId, connectorId),
685+
})
686+
queryClient.invalidateQueries({
687+
queryKey: connectorKeys.detail(knowledgeBaseId, connectorId),
688+
refetchType: 'none',
689+
})
690+
}
673691
queryClient.invalidateQueries({ queryKey: searchSourceKeys.lists() })
674692
queryClient.invalidateQueries({ queryKey: searchIntegrationKeys.lists() })
675693
queryClient.invalidateQueries({ queryKey: knowledgeKeys.documentLists(knowledgeBaseId) })

0 commit comments

Comments
 (0)