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.
2026-09-24 15:45:35 +08:00 · 2026-07-22 23:14:42 -07:00
parent 874e742a47
commit 387fa378a5
4 changed files with 199 additions and 19 deletions
@@ -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<string, unknown> = {}

-      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<string, unknown> = {}

-      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<string, unknown> = {}

-      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', () => {
@@ -253,7 +253,7 @@ export const googleSheetsConnector: ConnectorConfig = {
    accessToken: string,
    sourceConfig: Record<string, unknown>,
    _cursor?: string,
-    _syncContext?: Record<string, unknown>
+    syncContext?: Record<string, unknown>
  ): Promise<ExternalDocumentList> => {
    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 }
    }

@@ -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()
@@ -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<string, unknown> | 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<string, unknown> | 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)