Skip to content

Commit 283a789

Browse files
committed
refactor(knowledge): dedupe retry-cutoff computation, extract ownership-filter helper
/simplify pass: hoist the shared RETRY_WINDOW_DAYS cutoff into one computation reused by both the tombstone-retry bound and the stuck-document retry query, and pull the FOR-UPDATE-lock reconciliation's ID filtering into a pure, directly-unit-tested filterStillOwnedReconciliationIds function matching this file's existing convention for its other decision logic.
1 parent 6205cc8 commit 283a789

2 files changed

Lines changed: 66 additions & 5 deletions

File tree

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,38 @@ describe('partitionSyncReconciliation', () => {
277277
})
278278
})
279279

280+
describe('filterStillOwnedReconciliationIds', () => {
281+
it('keeps ids present in the ownership snapshot', async () => {
282+
const { filterStillOwnedReconciliationIds } = await import(
283+
'@/lib/knowledge/connectors/sync-engine'
284+
)
285+
286+
const result = filterStillOwnedReconciliationIds(['a'], ['b'], ['c'], new Set(['a', 'b', 'c']))
287+
288+
expect(result).toEqual({ resurrectIds: ['a'], softDeleteIds: ['b'], hardDeleteIds: ['c'] })
289+
})
290+
291+
it('drops ids a concurrent connector-delete already detached', async () => {
292+
const { filterStillOwnedReconciliationIds } = await import(
293+
'@/lib/knowledge/connectors/sync-engine'
294+
)
295+
296+
const result = filterStillOwnedReconciliationIds(['a'], ['b'], ['c'], new Set(['a']))
297+
298+
expect(result).toEqual({ resurrectIds: ['a'], softDeleteIds: [], hardDeleteIds: [] })
299+
})
300+
301+
it('returns all-empty lists when nothing is still owned', async () => {
302+
const { filterStillOwnedReconciliationIds } = await import(
303+
'@/lib/knowledge/connectors/sync-engine'
304+
)
305+
306+
const result = filterStillOwnedReconciliationIds(['a'], ['b'], ['c'], new Set())
307+
308+
expect(result).toEqual({ resurrectIds: [], softDeleteIds: [], hardDeleteIds: [] })
309+
})
310+
})
311+
280312
describe('resolveTagMapping', () => {
281313
beforeEach(() => {
282314
vi.clearAllMocks()

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

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,26 @@ export function partitionSyncReconciliation(
328328
return { resurrectIds, softDeleteIds: liveMissingIds, hardDeleteIds: tombstonedStillMissingIds }
329329
}
330330

331+
/**
332+
* Re-filters the three reconciliation ID lists against a fresh ownership
333+
* snapshot taken under the connector's `FOR UPDATE` lock, dropping any
334+
* document a concurrent "delete connector, keep documents" request already
335+
* detached (its `connectorId` no longer matches) since the lists were first
336+
* computed.
337+
*/
338+
export function filterStillOwnedReconciliationIds(
339+
resurrectIds: string[],
340+
softDeleteIds: string[],
341+
hardDeleteIds: string[],
342+
stillOwnedIds: Set<string>
343+
): { resurrectIds: string[]; softDeleteIds: string[]; hardDeleteIds: string[] } {
344+
return {
345+
resurrectIds: resurrectIds.filter((id) => stillOwnedIds.has(id)),
346+
softDeleteIds: softDeleteIds.filter((id) => stillOwnedIds.has(id)),
347+
hardDeleteIds: hardDeleteIds.filter((id) => stillOwnedIds.has(id)),
348+
}
349+
}
350+
331351
/**
332352
* Resolves tag values from connector metadata using the connector's mapTags function.
333353
* Translates semantic keys returned by mapTags to actual DB slots using the
@@ -516,6 +536,10 @@ export async function executeSync(
516536
let hasMore = true
517537
const syncContext: Record<string, unknown> = { syncRunId: generateId() }
518538

539+
// Shared cutoff for both the tombstone-retry bound below and the stuck-document
540+
// retry near the end of this sync — same RETRY_WINDOW_DAYS window, one computation.
541+
const retryCutoff = new Date(Date.now() - RETRY_WINDOW_DAYS * 24 * 60 * 60 * 1000)
542+
519543
/**
520544
* Bounded to the same retry window as the stuck-document retry below: a
521545
* document whose refresh keeps failing every sync (e.g. permanently
@@ -543,7 +567,7 @@ export async function executeSync(
543567
eq(document.connectorId, connectorId),
544568
isNull(document.archivedAt),
545569
isNotNull(document.deletedAt),
546-
gt(document.deletedAt, new Date(Date.now() - RETRY_WINDOW_DAYS * 24 * 60 * 60 * 1000))
570+
gt(document.deletedAt, retryCutoff)
547571
)
548572
)
549573
.limit(1)
@@ -991,9 +1015,15 @@ export async function executeSync(
9911015
).map((d) => d.id)
9921016
)
9931017

994-
safeResurrectIds = resurrectIds.filter((id) => stillOwned.has(id))
995-
safeSoftDeleteIds = gatedSoftDeleteIds.filter((id) => stillOwned.has(id))
996-
safeHardDeleteIds = gatedHardDeleteIds.filter((id) => stillOwned.has(id))
1018+
const stillOwnedResult = filterStillOwnedReconciliationIds(
1019+
resurrectIds,
1020+
gatedSoftDeleteIds,
1021+
gatedHardDeleteIds,
1022+
stillOwned
1023+
)
1024+
safeResurrectIds = stillOwnedResult.resurrectIds
1025+
safeSoftDeleteIds = stillOwnedResult.softDeleteIds
1026+
safeHardDeleteIds = stillOwnedResult.hardDeleteIds
9971027

9981028
/**
9991029
* A document reappearing at the source is trustworthy evidence on its
@@ -1055,7 +1085,6 @@ export async function executeSync(
10551085
// abandoned (e.g. the Trigger.dev task process exited before processing completed).
10561086
// Documents uploaded more than RETRY_WINDOW_DAYS ago are not retried.
10571087
const staleProcessingCutoff = new Date(Date.now() - STALE_PROCESSING_MINUTES * 60 * 1000)
1058-
const retryCutoff = new Date(Date.now() - RETRY_WINDOW_DAYS * 24 * 60 * 60 * 1000)
10591088
const stuckDocs = await db
10601089
.select({
10611090
id: document.id,

0 commit comments

Comments
 (0)