Skip to content

Commit 147572d

Browse files
committed
fix(knowledge): let sourceConfirmedEmpty also bypass the mass-deletion safety threshold
The zero-document guard bypass alone wasn't enough: for a trashed spreadsheet with more than 5 tabs, reconciliation would proceed but the separate mass-deletion ratio guard (>50% deleted, >5 docs) still blocked the actual delete on a normal sync, requiring a forced full resync anyway. Extracted the ratio guard into exceedsDeletionSafetyThreshold, mirroring shouldSkipEmptyListing, so a connector's positive source confirmation bypasses both guards consistently.
1 parent c93c12f commit 147572d

2 files changed

Lines changed: 88 additions & 4 deletions

File tree

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,61 @@ describe('shouldSkipEmptyListing', () => {
116116
})
117117
})
118118

119+
describe('exceedsDeletionSafetyThreshold', () => {
120+
it('does not block a small deletion, even above 50%', async () => {
121+
const { exceedsDeletionSafetyThreshold } = await import(
122+
'@/lib/knowledge/connectors/sync-engine'
123+
)
124+
125+
expect(exceedsDeletionSafetyThreshold(4, 5, undefined, {})).toBe(false)
126+
})
127+
128+
it('does not block a large deletion below 50%', async () => {
129+
const { exceedsDeletionSafetyThreshold } = await import(
130+
'@/lib/knowledge/connectors/sync-engine'
131+
)
132+
133+
expect(exceedsDeletionSafetyThreshold(6, 20, undefined, {})).toBe(false)
134+
})
135+
136+
it('blocks a deletion above both the ratio and count thresholds by default', async () => {
137+
const { exceedsDeletionSafetyThreshold } = await import(
138+
'@/lib/knowledge/connectors/sync-engine'
139+
)
140+
141+
expect(exceedsDeletionSafetyThreshold(10, 10, undefined, {})).toBe(true)
142+
expect(exceedsDeletionSafetyThreshold(10, 10, undefined, undefined)).toBe(true)
143+
})
144+
145+
it('does not block on a forced fullSync', async () => {
146+
const { exceedsDeletionSafetyThreshold } = await import(
147+
'@/lib/knowledge/connectors/sync-engine'
148+
)
149+
150+
expect(exceedsDeletionSafetyThreshold(10, 10, true, {})).toBe(false)
151+
})
152+
153+
it('does not block when the connector confirms the deletion against the source', async () => {
154+
const { exceedsDeletionSafetyThreshold } = await import(
155+
'@/lib/knowledge/connectors/sync-engine'
156+
)
157+
158+
expect(exceedsDeletionSafetyThreshold(10, 10, undefined, { sourceConfirmedEmpty: true })).toBe(
159+
false
160+
)
161+
})
162+
163+
it('still blocks when sourceConfirmedEmpty is falsy', async () => {
164+
const { exceedsDeletionSafetyThreshold } = await import(
165+
'@/lib/knowledge/connectors/sync-engine'
166+
)
167+
168+
expect(exceedsDeletionSafetyThreshold(10, 10, undefined, { sourceConfirmedEmpty: false })).toBe(
169+
true
170+
)
171+
})
172+
})
173+
119174
describe('resolveTagMapping', () => {
120175
beforeEach(() => {
121176
vi.clearAllMocks()

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

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,27 @@ export function shouldSkipEmptyListing(
264264
return !syncContext?.sourceConfirmedEmpty
265265
}
266266

267+
/**
268+
* Decides whether a deletion should be blocked by the mass-deletion safety
269+
* threshold (more than half of existing documents, over 5) instead of proceeding.
270+
*
271+
* This guards against a connector-side bug or transient glitch producing a
272+
* listing that looks mostly empty. `sourceConfirmedEmpty` bypasses it the same
273+
* way it bypasses `shouldSkipEmptyListing` — the connector positively verified
274+
* the deletion against the source rather than merely inferring it from a
275+
* listing, so the extra caution this threshold provides doesn't apply.
276+
*/
277+
export function exceedsDeletionSafetyThreshold(
278+
removedCount: number,
279+
existingCount: number,
280+
fullSync: boolean | undefined,
281+
syncContext: Record<string, unknown> | undefined
282+
): boolean {
283+
if (fullSync || syncContext?.sourceConfirmedEmpty) return false
284+
const deletionRatio = existingCount > 0 ? removedCount / existingCount : 0
285+
return deletionRatio > 0.5 && removedCount > 5
286+
}
287+
267288
/**
268289
* Resolves tag values from connector metadata using the connector's mapTags function.
269290
* Translates semantic keys returned by mapTags to actual DB slots using the
@@ -826,12 +847,20 @@ export async function executeSync(
826847
.map((d) => d.id)
827848

828849
if (removedIds.length > 0) {
829-
const deletionRatio = existingDocs.length > 0 ? removedIds.length / existingDocs.length : 0
830-
831-
if (deletionRatio > 0.5 && removedIds.length > 5 && !options?.fullSync) {
850+
if (
851+
exceedsDeletionSafetyThreshold(
852+
removedIds.length,
853+
existingDocs.length,
854+
options?.fullSync,
855+
syncContext
856+
)
857+
) {
832858
logger.warn(
833859
`Skipping deletion of ${removedIds.length}/${existingDocs.length} docs — exceeds safety threshold. Trigger a full sync to force cleanup.`,
834-
{ connectorId, deletionRatio: Math.round(deletionRatio * 100) }
860+
{
861+
connectorId,
862+
deletionRatio: Math.round((removedIds.length / existingDocs.length) * 100),
863+
}
835864
)
836865
} else {
837866
await hardDeleteDocuments(removedIds, syncLogId)

0 commit comments

Comments
 (0)