fix(knowledge): give users choice to keep or delete documents when removing connector

[waleedlatif1]

Summary

• Fix misleading delete connector modal — previously said "documents will remain" but actually hard-deleted everything
• Add checkbox "Also delete all synced documents" to the remove connector modal (unchecked by default)
• When unchecked (default): connector is removed, documents are kept by nullifying their connectorId foreign key
• When checked: connector and all associated documents, embeddings, and storage files are deleted (previous behavior)
• Backend accepts ?deleteDocuments=true query param on DELETE endpoint

Type of Change

• Bug fix

Testing

Tested manually

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

[cursor]

PR Summary

Medium Risk
Changes connector deletion semantics and a DB foreign-key constraint, which affects how documents are retained or deleted and could lead to unexpected data loss/retention if misused. Limited scope but touches data lifecycle, audit metadata, and UI flows.

Overview
Connector removal now supports keeping or deleting synced documents. The DELETE /api/knowledge/[id]/connectors/[connectorId] endpoint accepts ?deleteDocuments=true to hard-delete associated documents/embeddings and storage files; without it, the connector is deleted while documents are retained (via FK ON DELETE SET NULL).

The workspace UI updates the removal modal to reflect this behavior and adds a checkbox to optionally delete all synced documents, wiring the flag through the useDeleteConnector hook. Audit/logging is updated to record documents deleted vs kept, and tests/migration are adjusted to match the new behavior.

^{Written by Cursor Bugbot for commit a2959f5. Configure here.}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
docs	Ready	Preview, Comment	Mar 31, 2026 2:33am

[greptile-apps]

Greptile Summary

This PR fixes a misleading connector-removal modal that previously claimed "documents will remain" while actually hard-deleting them. It adds an opt-in checkbox ("Also delete all synced documents", unchecked by default) to the UI, threads ?deleteDocuments=true through the frontend hook to the DELETE API route, and backs the safe default with a DB-level FK change from ON DELETE CASCADE to ON DELETE SET NULL on document.connector_id.

Key changes:

• DB migration (0182_luxuriant_quasar.sql / schema.ts): document.connectorId FK changed from cascade → set null, so deleting a connector automatically nullifies the column without requiring an explicit UPDATE.
• API route (route.ts): reads ?deleteDocuments=true; when set, manually deletes embeddings + documents and runs storage/tag cleanup (old behaviour); when absent, skips those steps and lets the FK cascade handle nullification.
• UI (connectors-section.tsx): adds a Checkbox with a closeDeleteModal callback that correctly resets both deleteTarget and deleteDocuments on close, cancel, and error paths.
• Hook (connectors.ts): appends ?deleteDocuments=true to the fetch URL only when the flag is explicitly true — safe default.
• Refactor (parser-extension.ts / validation.ts): extracts isAlphanumericExtension and reuses isSupportedExtension / SUPPORTED_DOCUMENT_EXTENSIONS from validation.ts, eliminating a duplicated extension list.

Confidence Score: 5/5

Safe to merge — the core logic is correct, the FK migration is well-formed, and the only finding is a stale test name with missing coverage for the deletion branch.

All remaining findings are P2. The prior dangling-FK concern was fully addressed by switching to ON DELETE SET NULL. The UI/API/hook changes are straightforward and correctly implement the described behaviour. The only gap is that the ?deleteDocuments=true code path lacks a dedicated test and the existing test's name is now a misnomer — neither blocks correct production behaviour.

apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.test.ts — test name is stale and ?deleteDocuments=true path is untested.

Important Files Changed

Filename	Overview
apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts	DELETE handler now reads ?deleteDocuments=true to choose between hard-delete (old behavior) and simply deleting the connector (letting the new FK cascade null out connectorId); logic is clean and the FOR UPDATE lock is retained.
apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.test.ts	Only one DELETE test; its name ("hard-delete") is now a misnomer since the default request tests the keep-documents path, and no test covers the ?deleteDocuments=true branch.
apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.tsx	Adds checkbox UI with closeDeleteModal callback that correctly resets both deleteTarget and deleteDocuments state on close/cancel/error.
apps/sim/hooks/queries/kb/connectors.ts	Adds optional deleteDocuments param; appends ?deleteDocuments=true to the URL only when the flag is explicitly true — safe default.
packages/db/migrations/0182_luxuriant_quasar.sql	Correctly changes the document.connector_id FK from CASCADE to SET NULL, enabling the keep-documents behaviour at the DB level.
packages/db/schema.ts	Schema reflects the FK change to onDelete: 'set null', matching the migration.
apps/sim/lib/knowledge/documents/parser-extension.ts	Refactored to reuse isAlphanumericExtension, isSupportedExtension, and SUPPORTED_DOCUMENT_EXTENSIONS from validation.ts, eliminating duplication.
apps/sim/lib/uploads/utils/validation.ts	Extracts isAlphanumericExtension into an exported function used by both parser-extension.ts and the existing validation helpers.

Sequence Diagram

sequenceDiagram
actor User
participant UI as ConnectorsSection
participant Hook as useDeleteConnector
participant API as DELETE /connectors/:id
participant DB as PostgreSQL
User->>UI: Click "Remove Connector"
UI->>User: Show modal (checkbox unchecked)
User->>UI: (Optionally) Check "Also delete all synced documents"
User->>UI: Click "Remove"
UI->>Hook: deleteConnector({ knowledgeBaseId, connectorId, deleteDocuments })
alt deleteDocuments = false (default)
Hook->>API: DELETE /connectors/:id
API->>DB: BEGIN TRANSACTION
API->>DB: SELECT docs (active only)
API->>DB: DELETE FROM knowledge_connector WHERE id = :connectorId
DB-->>DB: ON DELETE SET NULL → document.connectorId = NULL
API->>DB: COMMIT
API-->>Hook: { success: true }
else deleteDocuments = true
Hook->>API: DELETE /connectors/:id?deleteDocuments=true
API->>DB: BEGIN TRANSACTION
API->>DB: SELECT docs (active only)
API->>DB: DELETE FROM embedding WHERE documentId IN (docs)
API->>DB: DELETE FROM document WHERE id IN (docs)
API->>DB: DELETE FROM knowledge_connector WHERE id = :connectorId
API->>DB: COMMIT
API->>API: deleteDocumentStorageFiles(deletedDocs)
API->>API: cleanupUnusedTagDefinitions(knowledgeBaseId)
API-->>Hook: { success: true }
end
Hook-->>UI: onSuccess → closeDeleteModal()

Loading

_{Reviews (2): Last reviewed commit: "fix(knowledge): fix connector delete tes..." | Re-trigger Greptile}

[waleedlatif1]

@cursor review

[waleedlatif1]

@greptile

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR