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>
2026-10-31 03:47:12 +08:00 · 2026-03-28 12:04:26 -07:00
parent b4064c57fb
commit e4d35735b1
12 changed files with 14532 additions and 66 deletions
@@ -237,7 +237,7 @@ describe('Knowledge Connector By ID API Route', () => {
        .mockReturnValueOnce(mockDbChain)
        .mockResolvedValueOnce([{ id: 'doc-1', fileUrl: '/api/uploads/test.txt' }])
        .mockReturnValueOnce(mockDbChain)
-      mockDbChain.limit.mockResolvedValueOnce([{ id: 'conn-456' }])
+      mockDbChain.limit.mockResolvedValueOnce([{ id: 'conn-456', connectorType: 'jira' }])
      mockDbChain.returning.mockResolvedValueOnce([{ id: 'conn-456' }])

      const req = createMockRequest('DELETE')
@@ -292,7 +292,10 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
      return NextResponse.json({ error: 'Connector not found' }, { status: 404 })
    }

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

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

-      const documentIds = docs.map((doc) => doc.id)
-      if (documentIds.length > 0) {
-        await tx.delete(embedding).where(inArray(embedding.documentId, documentIds))
-        await tx.delete(document).where(inArray(document.id, documentIds))
+      if (deleteDocuments) {
+        const documentIds = docs.map((doc) => doc.id)
+        if (documentIds.length > 0) {
+          await tx.delete(embedding).where(inArray(embedding.documentId, documentIds))
+          await tx.delete(document).where(inArray(document.id, documentIds))
+        }
      }

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

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

-    await deleteDocumentStorageFiles(connectorDocuments, requestId)
+    if (deleteDocuments) {
+      await Promise.all([
+        deletedDocs.length > 0
+          ? deleteDocumentStorageFiles(deletedDocs, requestId)
+          : Promise.resolve(),
+        cleanupUnusedTagDefinitions(knowledgeBaseId, requestId).catch((error) => {
+          logger.warn(`[${requestId}] Failed to cleanup tag definitions`, error)
+        }),
+      ])
+    }

-    await cleanupUnusedTagDefinitions(knowledgeBaseId, requestId).catch((error) => {
-      logger.warn(`[${requestId}] Failed to cleanup tag definitions`, error)
-    })
-
-    logger.info(`[${requestId}] Hard-deleted connector ${connectorId} and its documents`)
+    logger.info(
+      `[${requestId}] Deleted connector ${connectorId}${deleteDocuments ? ` and ${docCount} documents` : `, kept ${docCount} documents`}`
+    )

    recordAudit({
      workspaceId: writeCheck.knowledgeBase.workspaceId,
@@ -349,7 +361,11 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
      resourceId: connectorId,
      resourceName: existingConnector[0].connectorType,
      description: `Deleted connector from knowledge base "${writeCheck.knowledgeBase.name}"`,
-      metadata: { knowledgeBaseId, documentsDeleted: connectorDocuments.length },
+      metadata: {
+        knowledgeBaseId,
+        documentsDeleted: deleteDocuments ? docCount : 0,
+        documentsKept: deleteDocuments ? 0 : docCount,
+      },
      request,
    })

@@ -373,7 +373,7 @@ export function AddConnectorModal({ open, onOpenChange, knowledgeBaseId }: AddCo
              <div className='flex flex-col gap-3'>
                {/* Auth: API key input or OAuth credential selection */}
                {isApiKeyMode ? (
-                  <div className='flex flex-col gap-1'>
+                  <div className='flex flex-col gap-2'>
                    <Label>
                      {connectorConfig.auth.mode === 'apiKey' && connectorConfig.auth.label
                        ? connectorConfig.auth.label
@@ -394,7 +394,7 @@ export function AddConnectorModal({ open, onOpenChange, knowledgeBaseId }: AddCo
                    />
                  </div>
                ) : (
-                  <div className='flex flex-col gap-1'>
+                  <div className='flex flex-col gap-2'>
                    <Label>Account</Label>
                    {credentialsLoading ? (
                      <div className='flex items-center gap-2 text-[var(--text-muted)] text-small'>
@@ -442,7 +442,7 @@ export function AddConnectorModal({ open, onOpenChange, knowledgeBaseId }: AddCo
                    canonicalId && (canonicalGroups.get(canonicalId)?.length ?? 0) === 2

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

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

                {/* Sync interval */}
-                <div className='flex flex-col gap-1'>
+                <div className='flex flex-col gap-2'>
                  <Label>Sync Frequency</Label>
                  <ButtonGroup
                    value={String(syncInterval)}
@@ -18,6 +18,7 @@ import {
 import {
  Badge,
  Button,
+  Checkbox,
  Modal,
  ModalBody,
  ModalContent,
@@ -77,6 +78,12 @@ export function ConnectorsSection({
  const { mutate: updateConnector } = useUpdateConnector()
  const { mutate: deleteConnector, isPending: isDeleting } = useDeleteConnector()
  const [deleteTarget, setDeleteTarget] = useState<string | null>(null)
+  const [deleteDocuments, setDeleteDocuments] = useState(false)
+
+  const closeDeleteModal = useCallback(() => {
+    setDeleteTarget(null)
+    setDeleteDocuments(false)
+  }, [])
  const [editingConnector, setEditingConnector] = useState<ConnectorData | null>(null)
  const [error, setError] = useState<string | null>(null)
  const [syncingIds, setSyncingIds] = useState<Set<string>>(() => new Set())
@@ -224,22 +231,30 @@ export function ConnectorsSection({
        />
      )}

-      <Modal open={deleteTarget !== null} onOpenChange={() => setDeleteTarget(null)}>
+      <Modal open={deleteTarget !== null} onOpenChange={closeDeleteModal}>
        <ModalContent size='sm'>
-          <ModalHeader>Delete Connector</ModalHeader>
+          <ModalHeader>Remove Connector</ModalHeader>
          <ModalBody>
            <p className='text-[var(--text-secondary)] text-sm'>
-              Are you sure you want to remove this connected source?{' '}
-              <span className='text-[var(--text-error)]'>
-                This will stop future syncs from this source.
-              </span>{' '}
-              <span className='text-[var(--text-tertiary)]'>
-                Documents already synced will remain in the knowledge base.
-              </span>
+              This will disconnect the source and stop future syncs. Documents already synced will
+              remain in the knowledge base unless you choose to delete them.
            </p>
+            <div className='mt-3 flex items-center gap-2'>
+              <Checkbox
+                id='delete-docs'
+                checked={deleteDocuments}
+                onCheckedChange={(checked) => setDeleteDocuments(checked === true)}
+              />
+              <label
+                htmlFor='delete-docs'
+                className='cursor-pointer text-[var(--text-secondary)] text-sm'
+              >
+                Also delete all synced documents
+              </label>
+            </div>
          </ModalBody>
          <ModalFooter>
-            <Button variant='default' onClick={() => setDeleteTarget(null)} disabled={isDeleting}>
+            <Button variant='default' onClick={closeDeleteModal} disabled={isDeleting}>
              Cancel
            </Button>
            <Button
@@ -248,23 +263,23 @@ export function ConnectorsSection({
              onClick={() => {
                if (deleteTarget) {
                  deleteConnector(
-                    { knowledgeBaseId, connectorId: deleteTarget },
+                    { knowledgeBaseId, connectorId: deleteTarget, deleteDocuments },
                    {
                      onSuccess: () => {
                        setError(null)
-                        setDeleteTarget(null)
+                        closeDeleteModal()
                      },
                      onError: (err) => {
                        logger.error('Delete connector failed', { error: err.message })
                        setError(err.message)
-                        setDeleteTarget(null)
+                        closeDeleteModal()
                      },
                    }
                  )
                }
              }}
            >
-              {isDeleting ? 'Deleting...' : 'Delete'}
+              {isDeleting ? 'Removing...' : 'Remove'}
            </Button>
          </ModalFooter>
        </ModalContent>
@@ -198,7 +198,7 @@ function SettingsTab({
  return (
    <div className='flex flex-col gap-3'>
      {connectorConfig?.configFields.map((field) => (
-        <div key={field.id} className='flex flex-col gap-1'>
+        <div key={field.id} className='flex flex-col gap-2'>
          <Label>
            {field.title}
            {field.required && <span className='ml-0.5 text-[var(--text-error)]'>*</span>}
@@ -227,7 +227,7 @@ function SettingsTab({
        </div>
      ))}

-      <div className='flex flex-col gap-1'>
+      <div className='flex flex-col gap-2'>
        <Label>Sync Frequency</Label>
        <ButtonGroup
          value={String(syncInterval)}
@@ -224,13 +224,16 @@ export function useUpdateConnector() {
 export interface DeleteConnectorParams {
  knowledgeBaseId: string
  connectorId: string
+  deleteDocuments?: boolean
 }

 async function deleteConnector({
  knowledgeBaseId,
  connectorId,
+  deleteDocuments,
 }: DeleteConnectorParams): Promise<void> {
-  const response = await fetch(`/api/knowledge/${knowledgeBaseId}/connectors/${connectorId}`, {
+  const base = `/api/knowledge/${knowledgeBaseId}/connectors/${connectorId}`
+  const response = await fetch(deleteDocuments ? `${base}?deleteDocuments=true` : base, {
    method: 'DELETE',
  })

@@ -1,28 +1,11 @@
 import { getExtensionFromMimeType } from '@/lib/uploads/utils/file-utils'
+import {
+  isAlphanumericExtension,
+  isSupportedExtension,
+  SUPPORTED_DOCUMENT_EXTENSIONS,
+} from '@/lib/uploads/utils/validation'

-const SUPPORTED_FILE_TYPES = [
-  'pdf',
-  'csv',
-  'docx',
-  'doc',
-  'txt',
-  'md',
-  'xlsx',
-  'xls',
-  'pptx',
-  'ppt',
-  'html',
-  'htm',
-  'json',
-  'yaml',
-  'yml',
-] as const
-
-const SUPPORTED_FILE_TYPES_TEXT = SUPPORTED_FILE_TYPES.join(', ')
-
-function isSupportedParserExtension(extension: string): boolean {
-  return SUPPORTED_FILE_TYPES.includes(extension as (typeof SUPPORTED_FILE_TYPES)[number])
-}
+const SUPPORTED_EXTENSIONS_TEXT = SUPPORTED_DOCUMENT_EXTENSIONS.join(', ')

 export function resolveParserExtension(
  filename: string,
@@ -30,14 +13,14 @@ export function resolveParserExtension(
  fallback?: string
 ): string {
  const raw = filename.includes('.') ? filename.split('.').pop()?.toLowerCase() : undefined
-  const filenameExtension = raw && /^[a-z0-9]+$/.test(raw) ? raw : undefined
+  const filenameExtension = raw && isAlphanumericExtension(raw) ? raw : undefined

-  if (filenameExtension && isSupportedParserExtension(filenameExtension)) {
+  if (filenameExtension && isSupportedExtension(filenameExtension)) {
    return filenameExtension
  }

  const mimeExtension = mimeType ? getExtensionFromMimeType(mimeType) : undefined
-  if (mimeExtension && isSupportedParserExtension(mimeExtension)) {
+  if (mimeExtension && isSupportedExtension(mimeExtension)) {
    return mimeExtension
  }

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

  if (filenameExtension) {
    throw new Error(
-      `Unsupported file type: ${filenameExtension}. Supported types are: ${SUPPORTED_FILE_TYPES_TEXT}`
+      `Unsupported file type: ${filenameExtension}. Supported types are: ${SUPPORTED_EXTENSIONS_TEXT}`
    )
  }

@@ -1,5 +1,14 @@
 import path from 'path'

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

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

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

  const isAudio = SUPPORTED_AUDIO_EXTENSIONS.includes(extension as SupportedAudioExtension)
  const isVideo = SUPPORTED_VIDEO_EXTENSIONS.includes(extension as SupportedVideoExtension)
@@ -0,0 +1,3 @@
+ALTER TABLE "document" DROP CONSTRAINT "document_connector_id_knowledge_connector_id_fk";
+--> statement-breakpoint
+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;
@@ -1268,6 +1268,13 @@
      "when": 1774411211528,
      "tag": "0181_dazzling_the_leader",
      "breakpoints": true
+    },
+    {
+      "idx": 182,
+      "version": "7",
+      "when": 1774722988437,
+      "tag": "0182_luxuriant_quasar",
+      "breakpoints": true
    }
  ]
 }
@@ -1275,7 +1275,7 @@ export const document = pgTable(

    // Connector-sourced document fields
    connectorId: text('connector_id').references(() => knowledgeConnector.id, {
-      onDelete: 'cascade',
+      onDelete: 'set null',
    }),
    externalId: text('external_id'),
    contentHash: text('content_hash'),