fix(knowledge): don't resurrect a tombstoned document whose content refresh failed

Critical race found by adversarial audit: resurrectIds was derived
purely from 'externalId was seen in the listing', independent of
whether the paired content update actually succeeded. If updateDocument
threw (hydration failure, storage upload failure, or any other
transient error) or the deferred-hydration fetch itself failed, the
document's row was never touched — yet the separate unconditional
resurrect step still cleared deletedAt for it anyway, reproducing the
exact bug this PR fixes (visible again, serving stale pre-tombstone
content), just triggered by a failed write instead of a gated one.

Track externalIds whose refresh attempt failed (hydration rejection or
write rejection) and exclude them from partitionSyncReconciliation's
resurrectIds. A failed refresh leaves the document tombstoned as-is —
not soft-deleted, not hard-deleted, not resurrected — so a later sync
gets a clean retry instead of the row landing in an inconsistent state
either way.

Author: waleedlatif1

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

@@ -78,51 +78,70 @@ describe('shouldReconcileDeletions', () => {
 
 describe('partitionSyncReconciliation', () => {
   const live = (id: string, externalId: string | null = id) => ({ id, externalId })
+  const noFailures = new Set<string>()
 
   it('marks a live document missing from the listing as pending removal, not hard-deleted', async () => {
     const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
 
-    const result = partitionSyncReconciliation([live('a')], [], new Set(), undefined)
+    const result = partitionSyncReconciliation([live('a')], [], new Set(), noFailures, undefined)
 
     expect(result).toEqual({ resurrectIds: [], softDeleteIds: ['a'], hardDeleteIds: [] })
   })
 
   it('hard-deletes a document already pending removal that is still absent', async () => {
     const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
 
-    const result = partitionSyncReconciliation([], [live('a')], new Set(), undefined)
+    const result = partitionSyncReconciliation([], [live('a')], new Set(), noFailures, undefined)
 
     expect(result).toEqual({ resurrectIds: [], softDeleteIds: [], hardDeleteIds: ['a'] })
   })
 
   it('resurrects a pending-removal document that reappears in the listing', async () => {
     const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
 
-    const result = partitionSyncReconciliation([], [live('a')], new Set(['a']), undefined)
+    const result = partitionSyncReconciliation(
+      [],
+      [live('a')],
+      new Set(['a']),
+      noFailures,
+      undefined
+    )
 
     expect(result).toEqual({ resurrectIds: ['a'], softDeleteIds: [], hardDeleteIds: [] })
   })
 
   it('leaves a document untouched when it is still present in the listing', async () => {
     const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
 
-    const result = partitionSyncReconciliation([live('a')], [], new Set(['a']), undefined)
+    const result = partitionSyncReconciliation(
+      [live('a')],
+      [],
+      new Set(['a']),
+      noFailures,
+      undefined
+    )
 
     expect(result).toEqual({ resurrectIds: [], softDeleteIds: [], hardDeleteIds: [] })
   })
 
   it('resurrects even on a forced fullSync', async () => {
     const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
 
-    const result = partitionSyncReconciliation([], [live('a')], new Set(['a']), true)
+    const result = partitionSyncReconciliation([], [live('a')], new Set(['a']), noFailures, true)
 
     expect(result.resurrectIds).toEqual(['a'])
   })
 
   it('hard-deletes both live and pending-removal documents immediately on a forced fullSync', async () => {
     const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
 
-    const result = partitionSyncReconciliation([live('a')], [live('b')], new Set(), true)
+    const result = partitionSyncReconciliation(
+      [live('a')],
+      [live('b')],
+      new Set(),
+      noFailures,
+      true
+    )
 
     expect(result.softDeleteIds).toEqual([])
     expect(result.hardDeleteIds.sort()).toEqual(['a', 'b'])
@@ -135,6 +154,7 @@ describe('partitionSyncReconciliation', () => {
       [live('kept'), live('newly-missing')],
       [live('resurrected'), live('confirmed-gone')],
       new Set(['kept', 'resurrected']),
+      noFailures,
       undefined
     )
 
@@ -152,11 +172,54 @@ describe('partitionSyncReconciliation', () => {
       [live('a', null)],
       [live('b', null)],
       new Set(),
+      noFailures,
       undefined
     )
 
     expect(result).toEqual({ resurrectIds: [], softDeleteIds: [], hardDeleteIds: [] })
   })
+
+  it('does not resurrect a reappearing document whose content refresh failed', async () => {
+    const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
+
+    const result = partitionSyncReconciliation(
+      [],
+      [live('a')],
+      new Set(['a']),
+      new Set(['a']),
+      undefined
+    )
+
+    expect(result).toEqual({ resurrectIds: [], softDeleteIds: [], hardDeleteIds: [] })
+  })
+
+  it('still refuses to resurrect a failed refresh even on a forced fullSync', async () => {
+    const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
+
+    const result = partitionSyncReconciliation(
+      [],
+      [live('a')],
+      new Set(['a']),
+      new Set(['a']),
+      true
+    )
+
+    expect(result.resurrectIds).toEqual([])
+  })
+
+  it('resurrects the ones that succeeded while excluding the one that failed', async () => {
+    const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
+
+    const result = partitionSyncReconciliation(
+      [],
+      [live('ok'), live('failed')],
+      new Set(['ok', 'failed']),
+      new Set(['failed']),
+      undefined
+    )
+
+    expect(result.resurrectIds).toEqual(['ok'])
+  })
 })
 
 describe('resolveTagMapping', () => {

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

@@ -260,7 +260,11 @@ type ReconciliationDoc = { id: string; externalId: string | null }
  * was already pending-removal (`tombstonedDocs`) coming into this sync. A
  * document that reappears while pending-removal is resurrected
  * (`resurrectIds`) regardless of `fullSync`, since presence — unlike absence —
- * is trustworthy evidence even from a partial listing.
+ * is trustworthy evidence even from a partial listing. A document whose
+ * content refresh was attempted but failed (`failedExternalIds`) is excluded
+ * from resurrection even though it was seen — surfacing it now would show
+ * known-stale pre-tombstone content; it stays tombstoned for a later sync to
+ * retry.
  *
  * A forced `fullSync` is an explicit request to reconcile right now: it skips
  * the grace period and purges everything absent in one pass.
@@ -269,10 +273,14 @@ export function partitionSyncReconciliation(
   existingDocs: ReconciliationDoc[],
   tombstonedDocs: ReconciliationDoc[],
   seenExternalIds: Set<string>,
+  failedExternalIds: Set<string>,
   fullSync: boolean | undefined
 ): { resurrectIds: string[]; softDeleteIds: string[]; hardDeleteIds: string[] } {
   const resurrectIds = tombstonedDocs
-    .filter((d) => d.externalId && seenExternalIds.has(d.externalId))
+    .filter(
+      (d) =>
+        d.externalId && seenExternalIds.has(d.externalId) && !failedExternalIds.has(d.externalId)
+    )
     .map((d) => d.id)
   const liveMissingIds = existingDocs
     .filter((d) => d.externalId && !seenExternalIds.has(d.externalId))
@@ -610,6 +618,14 @@ export async function executeSync(
     )
 
     const seenExternalIds = new Set<string>()
+    /**
+     * externalIds whose content update was attempted but failed (hydration
+     * error, or the write itself rejected) — as opposed to `docsFailed` counts
+     * elsewhere, this specifically excludes them from resurrection below: a
+     * tombstoned document whose refresh failed must stay tombstoned rather
+     * than come back visible while still serving stale pre-tombstone content.
+     */
+    const failedExternalIds = new Set<string>()
 
     const pendingOps: DocOp[] = []
     for (const extDoc of externalDocs) {
@@ -735,13 +751,16 @@ export async function executeSync(
           })
         )
 
-        for (const outcome of hydrated) {
+        for (let i = 0; i < hydrated.length; i++) {
+          const outcome = hydrated[i]
           if (outcome.status === 'fulfilled' && outcome.value) {
             readyOps.push(outcome.value)
           } else if (outcome.status === 'rejected') {
             result.docsFailed++
+            failedExternalIds.add(deferredOps[i].extDoc.externalId)
             logger.error('Failed to hydrate deferred document', {
               connectorId,
+              externalId: deferredOps[i].extDoc.externalId,
               error: getErrorMessage(outcome.reason),
             })
           }
@@ -804,6 +823,7 @@ export async function executeSync(
           else result.docsUpdated++
         } else {
           result.docsFailed++
+          failedExternalIds.add(batch[j].extDoc.externalId)
           logger.error('Failed to process document', {
             connectorId,
             externalId: batch[j].extDoc.externalId,
@@ -835,6 +855,7 @@ export async function executeSync(
       existingDocs,
       tombstonedDocs,
       seenExternalIds,
+      failedExternalIds,
       options?.fullSync
     )