diff --git a/apps/sim/connectors/google-sheets/google-sheets.test.ts b/apps/sim/connectors/google-sheets/google-sheets.test.ts index 66eb84aa434..df1e6c898e4 100644 --- a/apps/sim/connectors/google-sheets/google-sheets.test.ts +++ b/apps/sim/connectors/google-sheets/google-sheets.test.ts @@ -116,26 +116,40 @@ describe('googleSheetsConnector trashed handling', () => { }) describe('listDocuments', () => { - it('returns an empty listing when the spreadsheet is trashed', async () => { + it('returns an empty listing and confirms the empty result when the spreadsheet is trashed', async () => { stubFetch({ drive: { status: 200, body: { trashed: true, modifiedTime: '2026-07-01T00:00:00.000Z' } }, }) + const syncContext: Record = {} - const result = await googleSheetsConnector.listDocuments(ACCESS_TOKEN, SOURCE_CONFIG) + const result = await googleSheetsConnector.listDocuments( + ACCESS_TOKEN, + SOURCE_CONFIG, + undefined, + syncContext + ) expect(result).toEqual({ documents: [], hasMore: false }) + expect(syncContext.sourceConfirmedEmpty).toBe(true) }) it('lists every tab when the trashed field is absent', async () => { stubFetch({ drive: { status: 200, body: { modifiedTime: '2026-07-01T00:00:00.000Z' } } }) + const syncContext: Record = {} - const result = await googleSheetsConnector.listDocuments(ACCESS_TOKEN, SOURCE_CONFIG) + const result = await googleSheetsConnector.listDocuments( + ACCESS_TOKEN, + SOURCE_CONFIG, + undefined, + syncContext + ) expect(result.documents.map((d) => d.externalId)).toEqual([ `${SPREADSHEET_ID}__sheet__0`, `${SPREADSHEET_ID}__sheet__7`, ]) expect(result.hasMore).toBe(false) + expect(syncContext.sourceConfirmedEmpty).toBeUndefined() }) it('lists every tab when trashed is explicitly false', async () => { @@ -148,13 +162,20 @@ describe('googleSheetsConnector trashed handling', () => { it('fails open and lists every tab when the Drive read fails', async () => { stubFetch({ drive: { status: 500, body: { error: 'backend error' } } }) + const syncContext: Record = {} - const result = await googleSheetsConnector.listDocuments(ACCESS_TOKEN, SOURCE_CONFIG) + const result = await googleSheetsConnector.listDocuments( + ACCESS_TOKEN, + SOURCE_CONFIG, + undefined, + syncContext + ) expect(result.documents.map((d) => d.externalId)).toEqual([ `${SPREADSHEET_ID}__sheet__0`, `${SPREADSHEET_ID}__sheet__7`, ]) + expect(syncContext.sourceConfirmedEmpty).toBeUndefined() }) it('fails open when the Drive body is not an object', async () => { @@ -164,6 +185,14 @@ describe('googleSheetsConnector trashed handling', () => { expect(result.documents).toHaveLength(2) }) + + it('does not throw when trashed and no syncContext is passed', async () => { + stubFetch({ drive: { status: 200, body: { trashed: true } } }) + + const result = await googleSheetsConnector.listDocuments(ACCESS_TOKEN, SOURCE_CONFIG) + + expect(result).toEqual({ documents: [], hasMore: false }) + }) }) describe('getDocument', () => { diff --git a/apps/sim/connectors/google-sheets/google-sheets.ts b/apps/sim/connectors/google-sheets/google-sheets.ts index b67a3ce79c9..11657e7842e 100644 --- a/apps/sim/connectors/google-sheets/google-sheets.ts +++ b/apps/sim/connectors/google-sheets/google-sheets.ts @@ -253,7 +253,7 @@ export const googleSheetsConnector: ConnectorConfig = { accessToken: string, sourceConfig: Record, _cursor?: string, - _syncContext?: Record + syncContext?: Record ): Promise => { const spreadsheetId = (sourceConfig.spreadsheetId as string)?.trim() if (!spreadsheetId) { @@ -269,18 +269,18 @@ export const googleSheetsConnector: ConnectorConfig = { /** * A trashed spreadsheet is no longer current content, so it drops out of the - * listing and stops being re-indexed. - * - * This does not by itself purge the stored tabs: an empty listing is - * indistinguishable from a provider outage, so the sync engine's - * zero-document guard deliberately skips reconciliation rather than risk - * wiping a knowledge base on a bad response. A forced full resync is the - * supported way to complete the cleanup. `validateConfig` reports the - * trashed state so the connector does not look healthy while serving tabs - * from a file its owner has thrown away. + * listing and stops being re-indexed. Unlike an ordinary empty listing page + * (which could equally mean the source is unreachable), this is a direct, + * single-resource confirmation from the Drive API that the spreadsheet itself + * is gone — so `sourceConfirmedEmpty` tells the sync engine's zero-document + * guard it's safe to reconcile (purge the stored tabs) on this sync, rather + * than requiring a forced full resync. `validateConfig` reports the trashed + * state so the connector does not look healthy while serving tabs from a file + * its owner has thrown away. */ if (isTrashedDriveFile(driveMetadata)) { logger.info('Spreadsheet is in the Drive trash; listing no documents', { spreadsheetId }) + if (syncContext) syncContext.sourceConfirmedEmpty = true return { documents: [], hasMore: false } } diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.test.ts b/apps/sim/lib/knowledge/connectors/sync-engine.test.ts index 83a91635bab..bdd197df0a3 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.test.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.test.ts @@ -76,6 +76,101 @@ describe('shouldReconcileDeletions', () => { }) }) +describe('shouldSkipEmptyListing', () => { + it('does not skip when the listing is non-empty', async () => { + const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine') + + expect(shouldSkipEmptyListing(1, 5, undefined, {})).toBe(false) + }) + + it('does not skip when there are no existing documents to lose', async () => { + const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine') + + expect(shouldSkipEmptyListing(0, 0, undefined, {})).toBe(false) + }) + + it('does not skip on a forced fullSync', async () => { + const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine') + + expect(shouldSkipEmptyListing(0, 5, true, {})).toBe(false) + }) + + it('skips by default on an empty listing with existing documents', async () => { + const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine') + + expect(shouldSkipEmptyListing(0, 5, undefined, {})).toBe(true) + expect(shouldSkipEmptyListing(0, 5, undefined, undefined)).toBe(true) + expect(shouldSkipEmptyListing(0, 5, false, {})).toBe(true) + }) + + it('does not skip when the connector confirms the empty result against the source', async () => { + const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine') + + expect(shouldSkipEmptyListing(0, 5, undefined, { sourceConfirmedEmpty: true })).toBe(false) + }) + + it('still skips when sourceConfirmedEmpty is falsy', async () => { + const { shouldSkipEmptyListing } = await import('@/lib/knowledge/connectors/sync-engine') + + expect(shouldSkipEmptyListing(0, 5, undefined, { sourceConfirmedEmpty: false })).toBe(true) + }) +}) + +describe('exceedsDeletionSafetyThreshold', () => { + it('does not block a small deletion, even above 50%', async () => { + const { exceedsDeletionSafetyThreshold } = await import( + '@/lib/knowledge/connectors/sync-engine' + ) + + expect(exceedsDeletionSafetyThreshold(4, 5, undefined, {})).toBe(false) + }) + + it('does not block a large deletion below 50%', async () => { + const { exceedsDeletionSafetyThreshold } = await import( + '@/lib/knowledge/connectors/sync-engine' + ) + + expect(exceedsDeletionSafetyThreshold(6, 20, undefined, {})).toBe(false) + }) + + it('blocks a deletion above both the ratio and count thresholds by default', async () => { + const { exceedsDeletionSafetyThreshold } = await import( + '@/lib/knowledge/connectors/sync-engine' + ) + + expect(exceedsDeletionSafetyThreshold(10, 10, undefined, {})).toBe(true) + expect(exceedsDeletionSafetyThreshold(10, 10, undefined, undefined)).toBe(true) + }) + + it('does not block on a forced fullSync', async () => { + const { exceedsDeletionSafetyThreshold } = await import( + '@/lib/knowledge/connectors/sync-engine' + ) + + expect(exceedsDeletionSafetyThreshold(10, 10, true, {})).toBe(false) + }) + + it('does not block when the connector confirms the deletion against the source', async () => { + const { exceedsDeletionSafetyThreshold } = await import( + '@/lib/knowledge/connectors/sync-engine' + ) + + expect(exceedsDeletionSafetyThreshold(10, 10, undefined, { sourceConfirmedEmpty: true })).toBe( + false + ) + }) + + it('still blocks when sourceConfirmedEmpty is falsy', async () => { + const { exceedsDeletionSafetyThreshold } = await import( + '@/lib/knowledge/connectors/sync-engine' + ) + + expect(exceedsDeletionSafetyThreshold(10, 10, undefined, { sourceConfirmedEmpty: false })).toBe( + true + ) + }) +}) + describe('resolveTagMapping', () => { beforeEach(() => { vi.clearAllMocks() diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.ts b/apps/sim/lib/knowledge/connectors/sync-engine.ts index a4e07e20f9b..67ecfe374b1 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.ts @@ -244,6 +244,47 @@ export function shouldReconcileDeletions( return !syncContext?.listingCapped || Boolean(fullSync) } +/** + * Decides whether a zero-document listing should skip deletion reconciliation. + * + * An empty listing is normally indistinguishable from a provider outage, so + * reconciliation is skipped by default rather than risk wiping a knowledge base on + * a bad response. A connector can set `sourceConfirmedEmpty` on `syncContext` to + * vouch that it verified the empty result directly against the source — not + * merely an empty listing page — e.g. a single-resource connector confirming its + * one source item was trashed/removed via a dedicated metadata lookup. + */ +export function shouldSkipEmptyListing( + externalDocsCount: number, + existingDocsCount: number, + fullSync: boolean | undefined, + syncContext: Record | undefined +): boolean { + if (externalDocsCount !== 0 || existingDocsCount === 0 || fullSync) return false + return !syncContext?.sourceConfirmedEmpty +} + +/** + * Decides whether a deletion should be blocked by the mass-deletion safety + * threshold (more than half of existing documents, over 5) instead of proceeding. + * + * This guards against a connector-side bug or transient glitch producing a + * listing that looks mostly empty. `sourceConfirmedEmpty` bypasses it the same + * way it bypasses `shouldSkipEmptyListing` — the connector positively verified + * the deletion against the source rather than merely inferring it from a + * listing, so the extra caution this threshold provides doesn't apply. + */ +export function exceedsDeletionSafetyThreshold( + removedCount: number, + existingCount: number, + fullSync: boolean | undefined, + syncContext: Record | undefined +): boolean { + if (fullSync || syncContext?.sourceConfirmedEmpty) return false + const deletionRatio = existingCount > 0 ? removedCount / existingCount : 0 + return deletionRatio > 0.5 && removedCount > 5 +} + /** * Resolves tag values from connector metadata using the connector's mapTags function. * Translates semantic keys returned by mapTags to actual DB slots using the @@ -537,7 +578,14 @@ export async function executeSync( const excludedExternalIds = new Set(excludedDocs.map((d) => d.externalId).filter(Boolean)) - if (externalDocs.length === 0 && existingDocs.length > 0 && !options?.fullSync) { + if ( + shouldSkipEmptyListing( + externalDocs.length, + existingDocs.length, + options?.fullSync, + syncContext + ) + ) { logger.warn( `Source returned 0 documents but ${existingDocs.length} exist — skipping reconciliation`, { connectorId } @@ -799,12 +847,20 @@ export async function executeSync( .map((d) => d.id) if (removedIds.length > 0) { - const deletionRatio = existingDocs.length > 0 ? removedIds.length / existingDocs.length : 0 - - if (deletionRatio > 0.5 && removedIds.length > 5 && !options?.fullSync) { + if ( + exceedsDeletionSafetyThreshold( + removedIds.length, + existingDocs.length, + options?.fullSync, + syncContext + ) + ) { logger.warn( `Skipping deletion of ${removedIds.length}/${existingDocs.length} docs — exceeds safety threshold. Trigger a full sync to force cleanup.`, - { connectorId, deletionRatio: Math.round(deletionRatio * 100) } + { + connectorId, + deletionRatio: Math.round((removedIds.length / existingDocs.length) * 100), + } ) } else { await hardDeleteDocuments(removedIds, syncLogId)