Skip to content

Commit e472f19

Browse files
committed
fix(knowledge): exclude fulfilled-but-unverified hydration outcomes from resurrection too
Both bots independently found a third instance of the same bug class: when deferred hydration for an update fulfills but has no usable content (skipped as oversized, or an empty re-fetch), the code falls back to keeping the stored content as last-known-good and counts it unchanged — correct for an already-visible document, but for a tombstoned one it means content was never actually verified as current. That fallback wasn't added to failedExternalIds, so reconciliation still resurrected it with stale pre-tombstone content despite hydration never actually confirming anything. Fixed by adding both fallback branches to failedExternalIds, same as the rejected-promise cases. Verified across every connector's skippedReason call site that this only ever happens inside getDocument (the deferred hydration path already covered here) — no connector sets skippedReason directly in listDocuments, so there's no equivalent listing-time gap to fix.
1 parent 536f56b commit e472f19

1 file changed

Lines changed: 18 additions & 8 deletions

File tree

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

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -619,11 +619,15 @@ export async function executeSync(
619619

620620
const seenExternalIds = new Set<string>()
621621
/**
622-
* externalIds whose content update was attempted but failed (hydration
623-
* error, or the write itself rejected) — as opposed to `docsFailed` counts
624-
* elsewhere, this specifically excludes them from resurrection below: a
625-
* tombstoned document whose refresh failed must stay tombstoned rather
626-
* than come back visible while still serving stale pre-tombstone content.
622+
* externalIds whose content was never verified as current: a hydration
623+
* error, a rejected write, or a fulfilled-but-unusable hydration (skipped
624+
* as oversized, or an empty re-fetch) that falls back to keeping the
625+
* stored content as last-known-good. That fallback is fine for an
626+
* already-visible document, but for a tombstoned one it means we still
627+
* don't have confirmed-current content — so this excludes them from
628+
* resurrection below: a tombstoned document whose refresh didn't actually
629+
* land must stay tombstoned rather than come back visible while still
630+
* serving stale pre-tombstone content.
627631
*/
628632
const failedExternalIds = new Set<string>()
629633

@@ -710,15 +714,21 @@ export async function executeSync(
710714
})
711715
} else if (op.type === 'update') {
712716
// Already-indexed file is kept as last-known-good (not downgraded), so it
713-
// counts as unchanged rather than slipping past every result counter.
717+
// counts as unchanged rather than slipping past every result counter. Not a
718+
// verified refresh, though — see failedExternalIds below.
714719
result.docsUnchanged++
720+
failedExternalIds.add(op.extDoc.externalId)
715721
}
716722
return null
717723
}
718724
if (!fullDoc?.content.trim()) {
719725
// An empty re-fetch leaves an already-indexed update as last-known-good; count
720-
// it as unchanged so the totals still reconcile with documents seen.
721-
if (op.type === 'update') result.docsUnchanged++
726+
// it as unchanged so the totals still reconcile with documents seen. Not a
727+
// verified refresh, though — see failedExternalIds below.
728+
if (op.type === 'update') {
729+
result.docsUnchanged++
730+
failedExternalIds.add(op.extDoc.externalId)
731+
}
722732
return null
723733
}
724734
const hydratedHash = fullDoc.contentHash ?? op.extDoc.contentHash

0 commit comments

Comments
 (0)