Skip to content

Commit e4d3573

Browse files
waleedlatif1claude
waleedlatif1
and
claude
authored
fix(knowledge): give users choice to keep or delete documents when removing connector (#3825)
* fix(knowledge): give users choice to keep or delete documents when removing connector * refactor(knowledge): clean up connector delete and extract shared extension validator - Extract `isAlphanumericExtension` helper to deduplicate regex across parser-extension.ts and validation.ts - Extract `closeDeleteModal` callback to eliminate 4x scattered state resets - Add archivedAt/deletedAt filters to UPDATE query in keep-docs delete path - Parallelize storage file cleanup and tag definition cleanup with Promise.all - Deduplicate URL construction in delete connector hook Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(knowledge): remove duplicate extension list from parser-extension Use SUPPORTED_DOCUMENT_EXTENSIONS and isSupportedExtension from validation.ts instead of maintaining a separate identical list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(db): change document.connectorId FK from cascade to set null The cascade behavior meant deleting a connector would always delete its documents, contradicting the "keep documents" option. With set null, the database automatically nullifies connectorId when a connector is removed, and we only need explicit deletion when the user opts in. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore(db): add migration metadata for connectorId FK change Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(knowledge): fix connector delete test and use URL-safe searchParams Use `new URL(request.url).searchParams` instead of `request.nextUrl.searchParams` for compatibility with test mocks. Add missing `connectorType` to test fixture. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * spacing --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b4064c5 commit e4d3573

File tree

12 files changed

+14532
-66
lines changed

12 files changed

+14532
-66
lines changed

‎apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ describe('Knowledge Connector By ID API Route', () => {
237237
.mockReturnValueOnce(mockDbChain)
238238
.mockResolvedValueOnce([{ id: 'doc-1', fileUrl: '/api/uploads/test.txt' }])
239239
.mockReturnValueOnce(mockDbChain)
240-
mockDbChain.limit.mockResolvedValueOnce([{ id: 'conn-456' }])
240+
mockDbChain.limit.mockResolvedValueOnce([{ id: 'conn-456', connectorType: 'jira' }])
241241
mockDbChain.returning.mockResolvedValueOnce([{ id: 'conn-456' }])
242242

243243
const req = createMockRequest('DELETE')

‎apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,10 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
292292
return NextResponse.json({ error: 'Connector not found' }, { status: 404 })
293293
}
294294

295-
const connectorDocuments = await db.transaction(async (tx) => {
295+
const { searchParams } = new URL(request.url)
296+
const deleteDocuments = searchParams.get('deleteDocuments') === 'true'
297+
298+
const { deletedDocs, docCount } = await db.transaction(async (tx) => {
296299
await tx.execute(sql`SELECT 1 FROM knowledge_connector WHERE id = ${connectorId} FOR UPDATE`)
297300

298301
const docs = await tx
@@ -306,10 +309,12 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
306309
)
307310
)
308311

309-
const documentIds = docs.map((doc) => doc.id)
310-
if (documentIds.length > 0) {
311-
await tx.delete(embedding).where(inArray(embedding.documentId, documentIds))
312-
await tx.delete(document).where(inArray(document.id, documentIds))
312+
if (deleteDocuments) {
313+
const documentIds = docs.map((doc) => doc.id)
314+
if (documentIds.length > 0) {
315+
await tx.delete(embedding).where(inArray(embedding.documentId, documentIds))
316+
await tx.delete(document).where(inArray(document.id, documentIds))
317+
}
313318
}
314319

315320
const deletedConnectors = await tx
@@ -328,16 +333,23 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
328333
throw new Error('Connector not found')
329334
}
330335

331-
return docs
336+
return { deletedDocs: deleteDocuments ? docs : [], docCount: docs.length }
332337
})
333338

334-
await deleteDocumentStorageFiles(connectorDocuments, requestId)
335-
336-
await cleanupUnusedTagDefinitions(knowledgeBaseId, requestId).catch((error) => {
337-
logger.warn(`[${requestId}] Failed to cleanup tag definitions`, error)
338-
})
339+
if (deleteDocuments) {
340+
await Promise.all([
341+
deletedDocs.length > 0
342+
? deleteDocumentStorageFiles(deletedDocs, requestId)
343+
: Promise.resolve(),
344+
cleanupUnusedTagDefinitions(knowledgeBaseId, requestId).catch((error) => {
345+
logger.warn(`[${requestId}] Failed to cleanup tag definitions`, error)
346+
}),
347+
])
348+
}
339349

340-
logger.info(`[${requestId}] Hard-deleted connector ${connectorId} and its documents`)
350+
logger.info(
351+
`[${requestId}] Deleted connector ${connectorId}${deleteDocuments ? ` and ${docCount} documents` : `, kept ${docCount} documents`}`
352+
)
341353

342354
recordAudit({
343355
workspaceId: writeCheck.knowledgeBase.workspaceId,
@@ -349,7 +361,11 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
349361
resourceId: connectorId,
350362
resourceName: existingConnector[0].connectorType,
351363
description: `Deleted connector from knowledge base "${writeCheck.knowledgeBase.name}"`,
352-
metadata: { knowledgeBaseId, documentsDeleted: connectorDocuments.length },
364+
metadata: {
365+
knowledgeBaseId,
366+
documentsDeleted: deleteDocuments ? docCount : 0,
367+
documentsKept: deleteDocuments ? 0 : docCount,
368+
},
353369
request,
354370
})
355371

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ export function AddConnectorModal({ open, onOpenChange, knowledgeBaseId }: AddCo
373373
<div className='flex flex-col gap-3'>
374374
{/* Auth: API key input or OAuth credential selection */}
375375
{isApiKeyMode ? (
376-
<div className='flex flex-col gap-1'>
376+
<div className='flex flex-col gap-2'>
377377
<Label>
378378
{connectorConfig.auth.mode === 'apiKey' && connectorConfig.auth.label
379379
? connectorConfig.auth.label
@@ -394,7 +394,7 @@ export function AddConnectorModal({ open, onOpenChange, knowledgeBaseId }: AddCo
394394
/>
395395
</div>
396396
) : (
397-
<div className='flex flex-col gap-1'>
397+
<div className='flex flex-col gap-2'>
398398
<Label>Account</Label>
399399
{credentialsLoading ? (
400400
<div className='flex items-center gap-2 text-[var(--text-muted)] text-small'>
@@ -442,7 +442,7 @@ export function AddConnectorModal({ open, onOpenChange, knowledgeBaseId }: AddCo
442442
canonicalId && (canonicalGroups.get(canonicalId)?.length ?? 0) === 2
443443

444444
return (
445-
<div key={field.id} className='flex flex-col gap-1'>
445+
<div key={field.id} className='flex flex-col gap-2'>
446446
<div className='flex items-center justify-between'>
447447
<Label>
448448
{field.title}
@@ -507,7 +507,7 @@ export function AddConnectorModal({ open, onOpenChange, knowledgeBaseId }: AddCo
507507

508508
{/* Tag definitions (opt-out) */}
509509
{connectorConfig.tagDefinitions && connectorConfig.tagDefinitions.length > 0 && (
510-
<div className='flex flex-col gap-1'>
510+
<div className='flex flex-col gap-2'>
511511
<Label>Metadata Tags</Label>
512512
{connectorConfig.tagDefinitions.map((tagDef) => (
513513
<div
@@ -550,7 +550,7 @@ export function AddConnectorModal({ open, onOpenChange, knowledgeBaseId }: AddCo
550550
)}
551551

552552
{/* Sync interval */}
553-
<div className='flex flex-col gap-1'>
553+
<div className='flex flex-col gap-2'>
554554
<Label>Sync Frequency</Label>
555555
<ButtonGroup
556556
value={String(syncInterval)}

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

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
import {
1919
Badge,
2020
Button,
21+
Checkbox,
2122
Modal,
2223
ModalBody,
2324
ModalContent,
@@ -77,6 +78,12 @@ export function ConnectorsSection({
7778
const { mutate: updateConnector } = useUpdateConnector()
7879
const { mutate: deleteConnector, isPending: isDeleting } = useDeleteConnector()
7980
const [deleteTarget, setDeleteTarget] = useState<string | null>(null)
81+
const [deleteDocuments, setDeleteDocuments] = useState(false)
82+
83+
const closeDeleteModal = useCallback(() => {
84+
setDeleteTarget(null)
85+
setDeleteDocuments(false)
86+
}, [])
8087
const [editingConnector, setEditingConnector] = useState<ConnectorData | null>(null)
8188
const [error, setError] = useState<string | null>(null)
8289
const [syncingIds, setSyncingIds] = useState<Set<string>>(() => new Set())
@@ -224,22 +231,30 @@ export function ConnectorsSection({
224231
/>
225232
)}
226233

227-
<Modal open={deleteTarget !== null} onOpenChange={() => setDeleteTarget(null)}>
234+
<Modal open={deleteTarget !== null} onOpenChange={closeDeleteModal}>
228235
<ModalContent size='sm'>
229-
<ModalHeader>Delete Connector</ModalHeader>
236+
<ModalHeader>Remove Connector</ModalHeader>
230237
<ModalBody>
231238
<p className='text-[var(--text-secondary)] text-sm'>
232-
Are you sure you want to remove this connected source?{' '}
233-
<span className='text-[var(--text-error)]'>
234-
This will stop future syncs from this source.
235-
</span>{' '}
236-
<span className='text-[var(--text-tertiary)]'>
237-
Documents already synced will remain in the knowledge base.
238-
</span>
239+
This will disconnect the source and stop future syncs. Documents already synced will
240+
remain in the knowledge base unless you choose to delete them.
239241
</p>
242+
<div className='mt-3 flex items-center gap-2'>
243+
<Checkbox
244+
id='delete-docs'
245+
checked={deleteDocuments}
246+
onCheckedChange={(checked) => setDeleteDocuments(checked === true)}
247+
/>
248+
<label
249+
htmlFor='delete-docs'
250+
className='cursor-pointer text-[var(--text-secondary)] text-sm'
251+
>
252+
Also delete all synced documents
253+
</label>
254+
</div>
240255
</ModalBody>
241256
<ModalFooter>
242-
<Button variant='default' onClick={() => setDeleteTarget(null)} disabled={isDeleting}>
257+
<Button variant='default' onClick={closeDeleteModal} disabled={isDeleting}>
243258
Cancel
244259
</Button>
245260
<Button
@@ -248,23 +263,23 @@ export function ConnectorsSection({
248263
onClick={() => {
249264
if (deleteTarget) {
250265
deleteConnector(
251-
{ knowledgeBaseId, connectorId: deleteTarget },
266+
{ knowledgeBaseId, connectorId: deleteTarget, deleteDocuments },
252267
{
253268
onSuccess: () => {
254269
setError(null)
255-
setDeleteTarget(null)
270+
closeDeleteModal()
256271
},
257272
onError: (err) => {
258273
logger.error('Delete connector failed', { error: err.message })
259274
setError(err.message)
260-
setDeleteTarget(null)
275+
closeDeleteModal()
261276
},
262277
}
263278
)
264279
}
265280
}}
266281
>
267-
{isDeleting ? 'Deleting...' : 'Delete'}
282+
{isDeleting ? 'Removing...' : 'Remove'}
268283
</Button>
269284
</ModalFooter>
270285
</ModalContent>

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/edit-connector-modal.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ function SettingsTab({
198198
return (
199199
<div className='flex flex-col gap-3'>
200200
{connectorConfig?.configFields.map((field) => (
201-
<div key={field.id} className='flex flex-col gap-1'>
201+
<div key={field.id} className='flex flex-col gap-2'>
202202
<Label>
203203
{field.title}
204204
{field.required && <span className='ml-0.5 text-[var(--text-error)]'>*</span>}
@@ -227,7 +227,7 @@ function SettingsTab({
227227
</div>
228228
))}
229229

230-
<div className='flex flex-col gap-1'>
230+
<div className='flex flex-col gap-2'>
231231
<Label>Sync Frequency</Label>
232232
<ButtonGroup
233233
value={String(syncInterval)}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,13 +224,16 @@ export function useUpdateConnector() {
224224
export interface DeleteConnectorParams {
225225
knowledgeBaseId: string
226226
connectorId: string
227+
deleteDocuments?: boolean
227228
}
228229

229230
async function deleteConnector({
230231
knowledgeBaseId,
231232
connectorId,
233+
deleteDocuments,
232234
}: DeleteConnectorParams): Promise<void> {
233-
const response = await fetch(`/api/knowledge/${knowledgeBaseId}/connectors/${connectorId}`, {
235+
const base = `/api/knowledge/${knowledgeBaseId}/connectors/${connectorId}`
236+
const response = await fetch(deleteDocuments ? `${base}?deleteDocuments=true` : base, {
234237
method: 'DELETE',
235238
})
236239

‎apps/sim/lib/knowledge/documents/parser-extension.ts‎

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,26 @@
11
import { getExtensionFromMimeType } from '@/lib/uploads/utils/file-utils'
2+
import {
3+
isAlphanumericExtension,
4+
isSupportedExtension,
5+
SUPPORTED_DOCUMENT_EXTENSIONS,
6+
} from '@/lib/uploads/utils/validation'
27

3-
const SUPPORTED_FILE_TYPES = [
4-
'pdf',
5-
'csv',
6-
'docx',
7-
'doc',
8-
'txt',
9-
'md',
10-
'xlsx',
11-
'xls',
12-
'pptx',
13-
'ppt',
14-
'html',
15-
'htm',
16-
'json',
17-
'yaml',
18-
'yml',
19-
] as const
20-
21-
const SUPPORTED_FILE_TYPES_TEXT = SUPPORTED_FILE_TYPES.join(', ')
22-
23-
function isSupportedParserExtension(extension: string): boolean {
24-
return SUPPORTED_FILE_TYPES.includes(extension as (typeof SUPPORTED_FILE_TYPES)[number])
25-
}
8+
const SUPPORTED_EXTENSIONS_TEXT = SUPPORTED_DOCUMENT_EXTENSIONS.join(', ')
269

2710
export function resolveParserExtension(
2811
filename: string,
2912
mimeType?: string,
3013
fallback?: string
3114
): string {
3215
const raw = filename.includes('.') ? filename.split('.').pop()?.toLowerCase() : undefined
33-
const filenameExtension = raw && /^[a-z0-9]+$/.test(raw) ? raw : undefined
16+
const filenameExtension = raw && isAlphanumericExtension(raw) ? raw : undefined
3417

35-
if (filenameExtension && isSupportedParserExtension(filenameExtension)) {
18+
if (filenameExtension && isSupportedExtension(filenameExtension)) {
3619
return filenameExtension
3720
}
3821

3922
const mimeExtension = mimeType ? getExtensionFromMimeType(mimeType) : undefined
40-
if (mimeExtension && isSupportedParserExtension(mimeExtension)) {
23+
if (mimeExtension && isSupportedExtension(mimeExtension)) {
4124
return mimeExtension
4225
}
4326

@@ -47,7 +30,7 @@ export function resolveParserExtension(
4730

4831
if (filenameExtension) {
4932
throw new Error(
50-
`Unsupported file type: ${filenameExtension}. Supported types are: ${SUPPORTED_FILE_TYPES_TEXT}`
33+
`Unsupported file type: ${filenameExtension}. Supported types are: ${SUPPORTED_EXTENSIONS_TEXT}`
5134
)
5235
}
5336

‎apps/sim/lib/uploads/utils/validation.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
import path from 'path'
22

3+
/**
4+
* Checks whether a string is a valid file extension (lowercase alphanumeric only).
5+
* Rejects extensions containing spaces, punctuation, or other non-alphanumeric characters
6+
* that arise from non-filename document names (e.g. "Sim.ai <> RVTech").
7+
*/
8+
export function isAlphanumericExtension(ext: string): boolean {
9+
return /^[a-z0-9]+$/.test(ext)
10+
}
11+
312
export const MAX_FILE_SIZE = 100 * 1024 * 1024 // 100MB
413

514
export const SUPPORTED_DOCUMENT_EXTENSIONS = [
@@ -138,7 +147,7 @@ export interface FileValidationError {
138147
*/
139148
export function validateFileType(fileName: string, mimeType: string): FileValidationError | null {
140149
const raw = path.extname(fileName).toLowerCase().substring(1)
141-
const extension = (/^[a-z0-9]+$/.test(raw) ? raw : '') as SupportedDocumentExtension
150+
const extension = (isAlphanumericExtension(raw) ? raw : '') as SupportedDocumentExtension
142151

143152
if (!SUPPORTED_DOCUMENT_EXTENSIONS.includes(extension)) {
144153
return {
@@ -223,7 +232,7 @@ export function validateMediaFileType(
223232
mimeType: string
224233
): FileValidationError | null {
225234
const raw = path.extname(fileName).toLowerCase().substring(1)
226-
const extension = /^[a-z0-9]+$/.test(raw) ? raw : ''
235+
const extension = isAlphanumericExtension(raw) ? raw : ''
227236

228237
const isAudio = SUPPORTED_AUDIO_EXTENSIONS.includes(extension as SupportedAudioExtension)
229238
const isVideo = SUPPORTED_VIDEO_EXTENSIONS.includes(extension as SupportedVideoExtension)

‎packages/db/migrations/0182_luxuriant_quasar.sql‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
ALTER TABLE "document" DROP CONSTRAINT "document_connector_id_knowledge_connector_id_fk";
2+
--> statement-breakpoint
3+
ALTER TABLE "document" ADD CONSTRAINT "document_connector_id_knowledge_connector_id_fk" FOREIGN KEY ("connector_id") REFERENCES "public"."knowledge_connector"("id") ON DELETE set null ON UPDATE no action;

0 commit comments

Comments
 (0)