Skip to content

Commit 387fa37

Browse files
authored
fix(knowledge): purge trashed Google Sheets tabs on a normal sync (#5883)
* fix(knowledge): purge trashed Google Sheets tabs on a normal sync Trashing a spreadsheet made listDocuments return an empty listing, but the sync engine's zero-document guard skips deletion reconciliation whenever a listing comes back empty and documents already exist — it can't tell a genuinely empty source apart from a provider outage. For a single-spreadsheet connector, trashing its one source item empties the entire listing, so the guard always fired and the stale tabs never got cleaned up on a normal sync, contradicting the documented behavior. Add shouldSkipEmptyListing, mirroring shouldReconcileDeletions: a connector can now set syncContext.sourceConfirmedEmpty when it has positively confirmed the empty result against the source (not merely inferred it from an empty listing page), letting reconciliation proceed. The Google Sheets connector sets this flag when it confirms the spreadsheet is trashed via a direct Drive metadata lookup. No other connector sets it, so this doesn't change behavior anywhere else. * 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 874e742 commit 387fa37

4 files changed

Lines changed: 199 additions & 19 deletions

File tree

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

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

118118
describe('listDocuments', () => {
119-
it('returns an empty listing when the spreadsheet is trashed', async () => {
119+
it('returns an empty listing and confirms the empty result 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> = {}
123124

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

126132
expect(result).toEqual({ documents: [], hasMore: false })
133+
expect(syncContext.sourceConfirmedEmpty).toBe(true)
127134
})
128135

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

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

134147
expect(result.documents.map((d) => d.externalId)).toEqual([
135148
`${SPREADSHEET_ID}__sheet__0`,
136149
`${SPREADSHEET_ID}__sheet__7`,
137150
])
138151
expect(result.hasMore).toBe(false)
152+
expect(syncContext.sourceConfirmedEmpty).toBeUndefined()
139153
})
140154

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

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

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

154174
expect(result.documents.map((d) => d.externalId)).toEqual([
155175
`${SPREADSHEET_ID}__sheet__0`,
156176
`${SPREADSHEET_ID}__sheet__7`,
157177
])
178+
expect(syncContext.sourceConfirmedEmpty).toBeUndefined()
158179
})
159180

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

165186
expect(result.documents).toHaveLength(2)
166187
})
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+
})
167196
})
168197

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

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

Lines changed: 10 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,18 @@ 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.
273-
*
274-
* This does not by itself purge the stored tabs: an empty listing is
275-
* indistinguishable from a provider outage, so the sync engine's
276-
* zero-document guard deliberately skips reconciliation rather than risk
277-
* wiping a knowledge base on a bad response. A forced full resync is the
278-
* supported way to complete the cleanup. `validateConfig` reports the
279-
* trashed state so the connector does not look healthy while serving tabs
280-
* from a file its owner has thrown away.
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.
281280
*/
282281
if (isTrashedDriveFile(driveMetadata)) {
283282
logger.info('Spreadsheet is in the Drive trash; listing no documents', { spreadsheetId })
283+
if (syncContext) syncContext.sourceConfirmedEmpty = true
284284
return { documents: [], hasMore: false }
285285
}
286286

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,101 @@ 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')
82+
83+
expect(shouldSkipEmptyListing(1, 5, undefined, {})).toBe(false)
84+
})
85+
86+
it('does not skip when there are no existing documents to lose', async () => {
87+
const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine')
88+
89+
expect(shouldSkipEmptyListing(0, 0, undefined, {})).toBe(false)
90+
})
91+
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+
})
97+
98+
it('skips by default on an empty listing with existing documents', async () => {
99+
const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine')
100+
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)
104+
})
105+
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+
})
111+
112+
it('still skips when sourceConfirmedEmpty is falsy', async () => {
113+
const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine')
114+
115+
expect(shouldSkipEmptyListing(0, 5, undefined, { sourceConfirmedEmpty: false })).toBe(true)
116+
})
117+
})
118+
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+
79174
describe('resolveTagMapping', () => {
80175
beforeEach(() => {
81176
vi.clearAllMocks()

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

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,47 @@ export function shouldReconcileDeletions(
244244
return !syncContext?.listingCapped || Boolean(fullSync)
245245
}
246246

247+
/**
248+
* Decides whether a zero-document listing should skip deletion reconciliation.
249+
*
250+
* An empty listing is normally indistinguishable from a provider outage, so
251+
* reconciliation is skipped by default rather than risk wiping a knowledge base on
252+
* a bad response. A connector can set `sourceConfirmedEmpty` on `syncContext` to
253+
* vouch that it verified the empty result directly against the source — not
254+
* merely an empty listing page — e.g. a single-resource connector confirming its
255+
* one source item was trashed/removed via a dedicated metadata lookup.
256+
*/
257+
export function shouldSkipEmptyListing(
258+
externalDocsCount: number,
259+
existingDocsCount: number,
260+
fullSync: boolean | undefined,
261+
syncContext: Record<string, unknown> | undefined
262+
): boolean {
263+
if (externalDocsCount !== 0 || existingDocsCount === 0 || fullSync) return false
264+
return !syncContext?.sourceConfirmedEmpty
265+
}
266+
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+
247288
/**
248289
* Resolves tag values from connector metadata using the connector's mapTags function.
249290
* Translates semantic keys returned by mapTags to actual DB slots using the
@@ -537,7 +578,14 @@ export async function executeSync(
537578

538579
const excludedExternalIds = new Set(excludedDocs.map((d) => d.externalId).filter(Boolean))
539580

540-
if (externalDocs.length === 0 && existingDocs.length > 0 && !options?.fullSync) {
581+
if (
582+
shouldSkipEmptyListing(
583+
externalDocs.length,
584+
existingDocs.length,
585+
options?.fullSync,
586+
syncContext
587+
)
588+
) {
541589
logger.warn(
542590
`Source returned 0 documents but ${existingDocs.length} exist — skipping reconciliation`,
543591
{ connectorId }
@@ -799,12 +847,20 @@ export async function executeSync(
799847
.map((d) => d.id)
800848

801849
if (removedIds.length > 0) {
802-
const deletionRatio = existingDocs.length > 0 ? removedIds.length / existingDocs.length : 0
803-
804-
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+
) {
805858
logger.warn(
806859
`Skipping deletion of ${removedIds.length}/${existingDocs.length} docs — exceeds safety threshold. Trigger a full sync to force cleanup.`,
807-
{ connectorId, deletionRatio: Math.round(deletionRatio * 100) }
860+
{
861+
connectorId,
862+
deletionRatio: Math.round((removedIds.length / existingDocs.length) * 100),
863+
}
808864
)
809865
} else {
810866
await hardDeleteDocuments(removedIds, syncLogId)

0 commit comments

Comments
 (0)