Skip to content

Commit 9549f17

Browse files
committed
fix(knowledge): tombstone and resurrect only under the lease
The document lifecycle's tombstone and resurrection writes ran outside the lease guard, so a run reclaimed after its ACL transaction could hide a document the replacement restored or expose one it removed. They now run in the same lease-guarded transaction as every other member write, and a run whose lease is lost while it disables itself ends as superseded rather than rejecting.
1 parent 087d6d2 commit 9549f17

2 files changed

Lines changed: 52 additions & 37 deletions

File tree

apps/sim/lib/knowledge/connectors/member-observations.ts

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,8 @@ export async function applyMemberDocumentLifecycle(input: {
254254
knowledgeBaseId: string
255255
runId: string
256256
lease: Pick<SyncRunLease, 'beatIfDue'>
257+
/** Runs the tombstone and resurrection writes only while the run still holds its lease. */
258+
withLease: <T>(fn: (tx: DbOrTx) => Promise<T>) => Promise<T>
257259
/** External ids whose refresh did not land this run; withheld from resurrection. */
258260
failedExternalIds: ReadonlySet<string>
259261
/**
@@ -266,52 +268,55 @@ export async function applyMemberDocumentLifecycle(input: {
266268
const { connectorId, knowledgeBaseId, runId } = input
267269
const now = new Date()
268270

269-
const tombstoned = !input.allowRemoval
270-
? []
271-
: await db
272-
.update(document)
273-
.set({ deletedAt: now })
274-
.where(
275-
and(
276-
eq(document.connectorId, connectorId),
277-
eq(document.userExcluded, false),
278-
isNull(document.archivedAt),
279-
isNull(document.deletedAt),
280-
hasNoObservation()
281-
)
282-
)
283-
.returning({ id: document.id })
284-
285-
const resurrectionCandidates = await db
286-
.select({ id: document.id, externalId: document.externalId })
287-
.from(document)
288-
.where(
289-
and(
290-
eq(document.connectorId, connectorId),
291-
isNull(document.archivedAt),
292-
isNotNull(document.deletedAt),
293-
hasObservation()
294-
)
295-
)
296-
const resurrectIds = resurrectionCandidates
297-
.filter((row) => !row.externalId || !input.failedExternalIds.has(row.externalId))
298-
.map((row) => row.id)
299-
const resurrected =
300-
resurrectIds.length === 0
271+
const { tombstoned, resurrected } = await input.withLease(async (tx) => {
272+
const tombstoned = !input.allowRemoval
301273
? []
302-
: await db
274+
: await tx
303275
.update(document)
304-
.set({ deletedAt: null })
276+
.set({ deletedAt: now })
305277
.where(
306278
and(
307-
inArray(document.id, resurrectIds),
308279
eq(document.connectorId, connectorId),
280+
eq(document.userExcluded, false),
309281
isNull(document.archivedAt),
310-
isNotNull(document.deletedAt)
282+
isNull(document.deletedAt),
283+
hasNoObservation()
311284
)
312285
)
313286
.returning({ id: document.id })
314287

288+
const resurrectionCandidates = await tx
289+
.select({ id: document.id, externalId: document.externalId })
290+
.from(document)
291+
.where(
292+
and(
293+
eq(document.connectorId, connectorId),
294+
isNull(document.archivedAt),
295+
isNotNull(document.deletedAt),
296+
hasObservation()
297+
)
298+
)
299+
const resurrectIds = resurrectionCandidates
300+
.filter((row) => !row.externalId || !input.failedExternalIds.has(row.externalId))
301+
.map((row) => row.id)
302+
const resurrected =
303+
resurrectIds.length === 0
304+
? []
305+
: await tx
306+
.update(document)
307+
.set({ deletedAt: null })
308+
.where(
309+
and(
310+
inArray(document.id, resurrectIds),
311+
eq(document.connectorId, connectorId),
312+
isNull(document.archivedAt),
313+
isNotNull(document.deletedAt)
314+
)
315+
)
316+
.returning({ id: document.id })
317+
return { tombstoned, resurrected }
318+
})
319+
315320
const purgeCutoff = new Date(now.getTime() - MEMBER_TOMBSTONE_PURGE_DAYS * 24 * 60 * 60 * 1000)
316321
const purgeCandidates = input.allowRemoval
317322
? await db

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1539,6 +1539,7 @@ export async function executeMemberSync(
15391539
knowledgeBaseId: connector.knowledgeBaseId,
15401540
runId,
15411541
lease: run.lease,
1542+
withLease: (fn) => withMemberLease(run, fn),
15421543
failedExternalIds: state.failedExternalIds,
15431544
allowRemoval: (listed?.count ?? 0) > 0,
15441545
})
@@ -1590,7 +1591,16 @@ export async function executeMemberSync(
15901591
return skipped(result, 'connector_deleted_during_sync')
15911592
}
15921593
if (error instanceof MemberBindingGoneError) {
1593-
await disableMemberSync(run, error.message)
1594+
try {
1595+
await disableMemberSync(run, error.message)
1596+
} catch (disableError) {
1597+
if (!(disableError instanceof SyncLockLostException)) throw disableError
1598+
logger.warn('Member sync abandoned — lock was reclaimed before it could be disabled', {
1599+
connectorId,
1600+
runId,
1601+
})
1602+
return skipped(result, 'sync_superseded')
1603+
}
15941604
return { ...skipped(result, 'connector_not_syncable'), error: error.message }
15951605
}
15961606

0 commit comments

Comments
 (0)