Skip to content

Commit f875d4e

Browse files
committed
fix(knowledge): re-verify connectorId at the actual hard-delete, fix docsDeleted count
Cursor findings: expectedConnectorId was only checked on the pre-transaction SELECT in hardDeleteDocumentBatch, not on the DELETE itself — the billing lookups and KB locking in between are async and can span a concurrent "delete connector, keep documents" request, so the delete (and its embedding cleanup) now re-verifies against a fresh in-transaction snapshot instead of the stale existingIds. Also fixed docsDeleted to use hardDeleteDocuments' actual returned count instead of the pre-filter candidate count, so a sync log no longer overreports deletions that expectedConnectorId skipped. /cleanup: dropped two comments that only restated the line below them.
1 parent 283a789 commit f875d4e

2 files changed

Lines changed: 24 additions & 6 deletions

File tree

apps/sim/lib/knowledge/connectors/sync-engine.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,11 +1065,9 @@ export async function executeSync(
10651065
// query — the FOR UPDATE lock above only covers the window up to its
10661066
// own commit; this closes the remaining gap between that commit and
10671067
// this call.
1068-
await hardDeleteDocuments(safeHardDeleteIds, syncLogId, connectorId)
1069-
result.docsDeleted += safeHardDeleteIds.length
1068+
result.docsDeleted += await hardDeleteDocuments(safeHardDeleteIds, syncLogId, connectorId)
10701069
}
10711070

1072-
// Check if connector/KB were deleted before retrying stuck documents
10731071
const postBatchLiveness = await checkSyncLiveness(connectorId, connector.knowledgeBaseId)
10741072
if (postBatchLiveness.connectorDeleted) {
10751073
throw new ConnectorDeletedException(connectorId)
@@ -1489,7 +1487,6 @@ async function updateDocument(
14891487
kbOwner: KnowledgeBaseOwner,
14901488
sourceConfig?: Record<string, unknown>
14911489
): Promise<DocumentData> {
1492-
// Fetch old file URL before uploading replacement
14931490
const existingRows = await db
14941491
.select({ fileUrl: document.fileUrl })
14951492
.from(document)

apps/sim/lib/knowledge/documents/service.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2451,10 +2451,31 @@ async function hardDeleteDocumentBatch(
24512451
}
24522452
}
24532453

2454-
await tx.delete(embedding).where(inArray(embedding.documentId, existingIds))
2454+
/**
2455+
* Re-verify `expectedConnectorId` here too, not only on the pre-transaction
2456+
* SELECT above — the billing lookups and KB locking between that SELECT
2457+
* and this delete are async and can span a concurrent "delete connector,
2458+
* keep documents" request that clears these rows' `connectorId` in
2459+
* between. Deleting a detached document's embeddings would corrupt its
2460+
* search index even if the document row itself were spared, so both the
2461+
* embedding delete and the document delete are scoped to this re-verified
2462+
* ID set rather than the stale `existingIds`.
2463+
*/
2464+
const stillTargetedIds = expectedConnectorId
2465+
? (
2466+
await tx
2467+
.select({ id: document.id })
2468+
.from(document)
2469+
.where(
2470+
and(inArray(document.id, existingIds), eq(document.connectorId, expectedConnectorId))
2471+
)
2472+
).map((d) => d.id)
2473+
: existingIds
2474+
2475+
await tx.delete(embedding).where(inArray(embedding.documentId, stillTargetedIds))
24552476
const deletedRows = await tx
24562477
.delete(document)
2457-
.where(inArray(document.id, existingIds))
2478+
.where(inArray(document.id, stillTargetedIds))
24582479
.returning({ id: document.id })
24592480

24602481
const deletedIds = new Set(deletedRows.map((row) => row.id))

0 commit comments

Comments
 (0)