Skip to content

Commit 4e32f47

Browse files
committed
fix(knowledge): replace deletion-safety heuristics with a two-phase tombstone
PR #5883 merged an interim fix (sourceConfirmedEmpty bypassing two static safety heuristics) before this follow-up redesign was ready. This supersedes that approach with a properly general fix, matching how production sync systems (Entra Connect, SCIM/Entra ID deprovisioning, Cassandra/Couchbase tombstones) handle this exact problem: never let a single observation trigger an irreversible mass deletion, no matter how confident the signal looks. A document missing from a normal sync's listing is now soft-deleted (marked pending-removal) rather than hard-deleted immediately. It's only actually purged once a *later* sync confirms it's still absent. If it reappears in between, it's resurrected automatically — this self-heals a transient outage or a bad API response without needing to distinguish 'real' emptiness from 'ambiguous' emptiness at all, which is what the removed heuristics were trying (and failing) to do from a single observation. This removes shouldSkipEmptyListing, exceedsDeletionSafetyThreshold, and sourceConfirmedEmpty entirely — Google Sheets no longer needs a connector-specific bypass flag, since a genuinely trashed spreadsheet now reconciles through the exact same general path as every other connector, with no special-casing and no new misuse surface for future connectors. A forced fullSync still purges everything absent in one pass, preserving the existing 'trigger a full sync to force cleanup' escape hatch. Uses the existing (previously unused for individual documents) document.deletedAt column as the tombstone marker — no schema migration required. shouldReconcileDeletions (the isIncremental / listingCapped / listingTruncated gate) is unchanged; it still governs whether reconciliation may run at all. Resurrection runs unconditionally even when that gate is closed, since presence is trustworthy evidence regardless of whether the listing was complete.
1 parent 0199508 commit 4e32f47

4 files changed

Lines changed: 161 additions & 207 deletions

File tree

apps/sim/connectors/google-sheets/google-sheets.test.ts

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -116,40 +116,26 @@ describe('googleSheetsConnector trashed handling', () => {
116116
})
117117

118118
describe('listDocuments', () => {
119-
it('returns an empty listing and confirms the empty result when the spreadsheet is trashed', async () => {
119+
it('returns an empty listing when the spreadsheet is trashed', async () => {
120120
stubFetch({
121121
drive: { status: 200, body: { trashed: true, modifiedTime: '2026-07-01T00:00:00.000Z' } },
122122
})
123-
const syncContext: Record<string, unknown> = {}
124123

125-
const result = await googleSheetsConnector.listDocuments(
126-
ACCESS_TOKEN,
127-
SOURCE_CONFIG,
128-
undefined,
129-
syncContext
130-
)
124+
const result = await googleSheetsConnector.listDocuments(ACCESS_TOKEN, SOURCE_CONFIG)
131125

132126
expect(result).toEqual({ documents: [], hasMore: false })
133-
expect(syncContext.sourceConfirmedEmpty).toBe(true)
134127
})
135128

136129
it('lists every tab when the trashed field is absent', async () => {
137130
stubFetch({ drive: { status: 200, body: { modifiedTime: '2026-07-01T00:00:00.000Z' } } })
138-
const syncContext: Record<string, unknown> = {}
139131

140-
const result = await googleSheetsConnector.listDocuments(
141-
ACCESS_TOKEN,
142-
SOURCE_CONFIG,
143-
undefined,
144-
syncContext
145-
)
132+
const result = await googleSheetsConnector.listDocuments(ACCESS_TOKEN, SOURCE_CONFIG)
146133

147134
expect(result.documents.map((d) => d.externalId)).toEqual([
148135
`${SPREADSHEET_ID}__sheet__0`,
149136
`${SPREADSHEET_ID}__sheet__7`,
150137
])
151138
expect(result.hasMore).toBe(false)
152-
expect(syncContext.sourceConfirmedEmpty).toBeUndefined()
153139
})
154140

155141
it('lists every tab when trashed is explicitly false', async () => {
@@ -162,20 +148,13 @@ describe('googleSheetsConnector trashed handling', () => {
162148

163149
it('fails open and lists every tab when the Drive read fails', async () => {
164150
stubFetch({ drive: { status: 500, body: { error: 'backend error' } } })
165-
const syncContext: Record<string, unknown> = {}
166151

167-
const result = await googleSheetsConnector.listDocuments(
168-
ACCESS_TOKEN,
169-
SOURCE_CONFIG,
170-
undefined,
171-
syncContext
172-
)
152+
const result = await googleSheetsConnector.listDocuments(ACCESS_TOKEN, SOURCE_CONFIG)
173153

174154
expect(result.documents.map((d) => d.externalId)).toEqual([
175155
`${SPREADSHEET_ID}__sheet__0`,
176156
`${SPREADSHEET_ID}__sheet__7`,
177157
])
178-
expect(syncContext.sourceConfirmedEmpty).toBeUndefined()
179158
})
180159

181160
it('fails open when the Drive body is not an object', async () => {
@@ -185,14 +164,6 @@ describe('googleSheetsConnector trashed handling', () => {
185164

186165
expect(result.documents).toHaveLength(2)
187166
})
188-
189-
it('does not throw when trashed and no syncContext is passed', async () => {
190-
stubFetch({ drive: { status: 200, body: { trashed: true } } })
191-
192-
const result = await googleSheetsConnector.listDocuments(ACCESS_TOKEN, SOURCE_CONFIG)
193-
194-
expect(result).toEqual({ documents: [], hasMore: false })
195-
})
196167
})
197168

198169
describe('getDocument', () => {

apps/sim/connectors/google-sheets/google-sheets.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ export const googleSheetsConnector: ConnectorConfig = {
253253
accessToken: string,
254254
sourceConfig: Record<string, unknown>,
255255
_cursor?: string,
256-
syncContext?: Record<string, unknown>
256+
_syncContext?: Record<string, unknown>
257257
): Promise<ExternalDocumentList> => {
258258
const spreadsheetId = (sourceConfig.spreadsheetId as string)?.trim()
259259
if (!spreadsheetId) {
@@ -269,18 +269,14 @@ export const googleSheetsConnector: ConnectorConfig = {
269269

270270
/**
271271
* A trashed spreadsheet is no longer current content, so it drops out of the
272-
* listing and stops being re-indexed. Unlike an ordinary empty listing page
273-
* (which could equally mean the source is unreachable), this is a direct,
274-
* single-resource confirmation from the Drive API that the spreadsheet itself
275-
* is gone — so `sourceConfirmedEmpty` tells the sync engine's zero-document
276-
* guard it's safe to reconcile (purge the stored tabs) on this sync, rather
277-
* than requiring a forced full resync. `validateConfig` reports the trashed
278-
* state so the connector does not look healthy while serving tabs from a file
279-
* its owner has thrown away.
272+
* listing and stops being re-indexed. The sync engine reconciles its absence
273+
* the same way it does for every connector: pending-removal on the first
274+
* sync that doesn't see it, purged once a later sync confirms it's still
275+
* gone. `validateConfig` reports the trashed state so the connector does not
276+
* look healthy while serving tabs from a file its owner has thrown away.
280277
*/
281278
if (isTrashedDriveFile(driveMetadata)) {
282279
logger.info('Spreadsheet is in the Drive trash; listing no documents', { spreadsheetId })
283-
if (syncContext) syncContext.sourceConfirmedEmpty = true
284280
return { documents: [], hasMore: false }
285281
}
286282

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

Lines changed: 51 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -76,98 +76,86 @@ describe('shouldReconcileDeletions', () => {
7676
})
7777
})
7878

79-
describe('shouldSkipEmptyListing', () => {
80-
it('does not skip when the listing is non-empty', async () => {
81-
const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine')
79+
describe('partitionSyncReconciliation', () => {
80+
const live = (id: string, externalId: string | null = id) => ({ id, externalId })
8281

83-
expect(shouldSkipEmptyListing(1, 5, undefined, {})).toBe(false)
84-
})
82+
it('marks a live document missing from the listing as pending removal, not hard-deleted', async () => {
83+
const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
8584

86-
it('does not skip when there are no existing documents to lose', async () => {
87-
const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine')
85+
const result = partitionSyncReconciliation([live('a')], [], new Set(), undefined)
8886

89-
expect(shouldSkipEmptyListing(0, 0, undefined, {})).toBe(false)
87+
expect(result).toEqual({ resurrectIds: [], softDeleteIds: ['a'], hardDeleteIds: [] })
9088
})
9189

92-
it('does not skip on a forced fullSync', async () => {
93-
const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine')
94-
95-
expect(shouldSkipEmptyListing(0, 5, true, {})).toBe(false)
96-
})
90+
it('hard-deletes a document already pending removal that is still absent', async () => {
91+
const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
9792

98-
it('skips by default on an empty listing with existing documents', async () => {
99-
const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine')
93+
const result = partitionSyncReconciliation([], [live('a')], new Set(), undefined)
10094

101-
expect(shouldSkipEmptyListing(0, 5, undefined, {})).toBe(true)
102-
expect(shouldSkipEmptyListing(0, 5, undefined, undefined)).toBe(true)
103-
expect(shouldSkipEmptyListing(0, 5, false, {})).toBe(true)
95+
expect(result).toEqual({ resurrectIds: [], softDeleteIds: [], hardDeleteIds: ['a'] })
10496
})
10597

106-
it('does not skip when the connector confirms the empty result against the source', async () => {
107-
const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine')
108-
109-
expect(shouldSkipEmptyListing(0, 5, undefined, { sourceConfirmedEmpty: true })).toBe(false)
110-
})
98+
it('resurrects a pending-removal document that reappears in the listing', async () => {
99+
const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
111100

112-
it('still skips when sourceConfirmedEmpty is falsy', async () => {
113-
const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine')
101+
const result = partitionSyncReconciliation([], [live('a')], new Set(['a']), undefined)
114102

115-
expect(shouldSkipEmptyListing(0, 5, undefined, { sourceConfirmedEmpty: false })).toBe(true)
103+
expect(result).toEqual({ resurrectIds: ['a'], softDeleteIds: [], hardDeleteIds: [] })
116104
})
117-
})
118105

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-
)
106+
it('leaves a document untouched when it is still present in the listing', async () => {
107+
const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
124108

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-
)
109+
const result = partitionSyncReconciliation([live('a')], [], new Set(['a']), undefined)
132110

133-
expect(exceedsDeletionSafetyThreshold(6, 20, undefined, {})).toBe(false)
111+
expect(result).toEqual({ resurrectIds: [], softDeleteIds: [], hardDeleteIds: [] })
134112
})
135113

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-
)
114+
it('resurrects even on a forced fullSync', async () => {
115+
const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
116+
117+
const result = partitionSyncReconciliation([], [live('a')], new Set(['a']), true)
140118

141-
expect(exceedsDeletionSafetyThreshold(10, 10, undefined, {})).toBe(true)
142-
expect(exceedsDeletionSafetyThreshold(10, 10, undefined, undefined)).toBe(true)
119+
expect(result.resurrectIds).toEqual(['a'])
143120
})
144121

145-
it('does not block on a forced fullSync', async () => {
146-
const { exceedsDeletionSafetyThreshold } = await import(
147-
'@/lib/knowledge/connectors/sync-engine'
148-
)
122+
it('hard-deletes both live and pending-removal documents immediately on a forced fullSync', async () => {
123+
const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
149124

150-
expect(exceedsDeletionSafetyThreshold(10, 10, true, {})).toBe(false)
125+
const result = partitionSyncReconciliation([live('a')], [live('b')], new Set(), true)
126+
127+
expect(result.softDeleteIds).toEqual([])
128+
expect(result.hardDeleteIds.sort()).toEqual(['a', 'b'])
151129
})
152130

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-
)
131+
it('handles a mixed batch of every outcome in one pass', async () => {
132+
const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
157133

158-
expect(exceedsDeletionSafetyThreshold(10, 10, undefined, { sourceConfirmedEmpty: true })).toBe(
159-
false
134+
const result = partitionSyncReconciliation(
135+
[live('kept'), live('newly-missing')],
136+
[live('resurrected'), live('confirmed-gone')],
137+
new Set(['kept', 'resurrected']),
138+
undefined
160139
)
140+
141+
expect(result).toEqual({
142+
resurrectIds: ['resurrected'],
143+
softDeleteIds: ['newly-missing'],
144+
hardDeleteIds: ['confirmed-gone'],
145+
})
161146
})
162147

163-
it('still blocks when sourceConfirmedEmpty is falsy', async () => {
164-
const { exceedsDeletionSafetyThreshold } = await import(
165-
'@/lib/knowledge/connectors/sync-engine'
166-
)
148+
it('ignores documents with a null externalId', async () => {
149+
const { partitionSyncReconciliation } = await import('@/lib/knowledge/connectors/sync-engine')
167150

168-
expect(exceedsDeletionSafetyThreshold(10, 10, undefined, { sourceConfirmedEmpty: false })).toBe(
169-
true
151+
const result = partitionSyncReconciliation(
152+
[live('a', null)],
153+
[live('b', null)],
154+
new Set(),
155+
undefined
170156
)
157+
158+
expect(result).toEqual({ resurrectIds: [], softDeleteIds: [], hardDeleteIds: [] })
171159
})
172160
})
173161

0 commit comments

Comments
 (0)