Skip to content

Commit a5c804f

Browse files
committed
fix(knowledge): lock the connector row for stuck-doc retry ownership too
Cursor + Greptile (same root cause, two reports): the previous round's fix re-checked connectorId via a separate SELECT before the embedding delete and processing-state reset, but a bare re-SELECT only narrows a TOCTOU window, it never closes it — a concurrent "delete connector, keep documents" request could still commit its detach in between. Wraps the ownership re-check and both writes in a transaction that takes the same knowledge_connector FOR UPDATE lock the DELETE route takes before nulling connectorId, so the two requests serialize instead of racing, matching the pattern already used by the reconciliation transaction elsewhere in this PR.
1 parent 0e160a7 commit a5c804f

1 file changed

Lines changed: 44 additions & 29 deletions

File tree

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

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,42 +1120,57 @@ export async function executeSync(
11201120
logger.info(`Retrying ${stuckDocs.length} stuck documents`, { connectorId })
11211121
try {
11221122
const stuckDocIds = stuckDocs.map((doc) => doc.id)
1123+
let retryDocs: typeof stuckDocs = []
11231124

11241125
/**
1125-
* Re-verify connectorId at write time — a concurrent "delete connector,
1126-
* keep documents" request can null it out between the select above and
1127-
* these writes, and resetting/re-enqueuing a detached document's
1128-
* processing state would corrupt a standalone KB entry the connector no
1129-
* longer owns.
1126+
* Takes the same `knowledge_connector` FOR UPDATE lock the DELETE route
1127+
* takes before nulling connectorId on detached documents, so the two
1128+
* requests serialize instead of racing — a plain re-SELECT only
1129+
* narrows the window between the ownership check and these writes, it
1130+
* never closes it, since a concurrent detach can still commit in
1131+
* between. Embedding cleanup and the processing-state reset happen
1132+
* inside the same locked transaction so a document already claimed by
1133+
* a detach never gets its embeddings wiped or is reprocessed as if
1134+
* still connector-owned.
11301135
*/
1131-
const stillOwnedIds = new Set(
1132-
(
1133-
await db
1134-
.select({ id: document.id })
1135-
.from(document)
1136-
.where(and(inArray(document.id, stuckDocIds), eq(document.connectorId, connectorId)))
1137-
).map((d) => d.id)
1138-
)
1139-
const retryDocs = stuckDocs.filter((doc) => stillOwnedIds.has(doc.id))
1136+
await db.transaction(async (tx) => {
1137+
await tx.execute(
1138+
sql`SELECT 1 FROM knowledge_connector WHERE id = ${connectorId} FOR UPDATE`
1139+
)
11401140

1141-
if (retryDocs.length > 0) {
1142-
const retryDocIds = retryDocs.map((doc) => doc.id)
1141+
const stillOwnedIds = new Set(
1142+
(
1143+
await tx
1144+
.select({ id: document.id })
1145+
.from(document)
1146+
.where(
1147+
and(inArray(document.id, stuckDocIds), eq(document.connectorId, connectorId))
1148+
)
1149+
).map((d) => d.id)
1150+
)
1151+
retryDocs = stuckDocs.filter((doc) => stillOwnedIds.has(doc.id))
11431152

1144-
await db.delete(embedding).where(inArray(embedding.documentId, retryDocIds))
1153+
if (retryDocs.length > 0) {
1154+
const retryDocIds = retryDocs.map((doc) => doc.id)
11451155

1146-
await db
1147-
.update(document)
1148-
.set({
1149-
processingStatus: 'pending',
1150-
processingStartedAt: null,
1151-
processingCompletedAt: null,
1152-
processingError: null,
1153-
chunkCount: 0,
1154-
tokenCount: 0,
1155-
characterCount: 0,
1156-
})
1157-
.where(inArray(document.id, retryDocIds))
1156+
await tx.delete(embedding).where(inArray(embedding.documentId, retryDocIds))
11581157

1158+
await tx
1159+
.update(document)
1160+
.set({
1161+
processingStatus: 'pending',
1162+
processingStartedAt: null,
1163+
processingCompletedAt: null,
1164+
processingError: null,
1165+
chunkCount: 0,
1166+
tokenCount: 0,
1167+
characterCount: 0,
1168+
})
1169+
.where(inArray(document.id, retryDocIds))
1170+
}
1171+
})
1172+
1173+
if (retryDocs.length > 0) {
11591174
await processDocumentsWithQueue(
11601175
retryDocs.map((doc) => ({
11611176
documentId: doc.id,

0 commit comments

Comments
 (0)