Skip to content

fix(knowledge): purge trashed Google Sheets tabs on a normal sync#5883

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/gsheets-trashed-empty-guard
Jul 23, 2026
Merged

fix(knowledge): purge trashed Google Sheets tabs on a normal sync#5883
waleedlatif1 merged 2 commits into
stagingfrom
fix/gsheets-trashed-empty-guard

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Follow-up to fix(connectors): purge archived/deleted source items in KB connectors #5880 — Cursor Bugbot flagged ( v0.7.43: ci and connector fixes #5882 (comment)) that a trashed Google Sheets spreadsheet never actually gets purged on a normal sync, contradicting the docs.
  • Root cause: the sync engine's zero-document guard skips deletion reconciliation whenever a listing comes back empty and documents already exist, since it can't tell a genuinely empty source apart from a transient provider outage. Google Sheets is a single-spreadsheet-per-connector integration, so trashing the one source item empties the entire listing — the guard fired every time, and only a forced full resync could complete the cleanup.
  • Added shouldSkipEmptyListing to sync-engine.ts, mirroring the existing shouldReconcileDeletions helper: a connector can set syncContext.sourceConfirmedEmpty to vouch that it positively confirmed the empty result against the source (not merely inferred it from an empty listing page). Google Sheets sets this flag when its direct Drive metadata lookup confirms the spreadsheet is trashed.
  • No other connector sets this flag, so behavior everywhere else is unchanged.

Type of Change

  • Bug fix

Testing

  • Added unit tests for shouldSkipEmptyListing covering: non-empty listing, no existing docs, forced fullSync, default skip behavior, and the new sourceConfirmedEmpty override (including a falsy-value case).
  • Updated Google Sheets connector tests to assert sourceConfirmedEmpty is set only on the trashed path, and left unset on the absent/false/fail-open paths.
  • bun run lint, tsc --noEmit, and the full connectors + lib/knowledge/connectors test suites (411 tests) all pass.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@waleedlatif1
fix(knowledge): purge trashed Google Sheets tabs on a normal sync
c93c12f 
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.
@vercel

vercel Bot commented Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 23, 2026 5:52am

Request Review

@cursor

cursor Bot commented Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes knowledge-base deletion reconciliation and can hard-delete stored documents when a connector sets sourceConfirmedEmpty; scope is narrow (only Google Sheets today) but touches data-loss safety guards.

Overview
Fixes trashed Google Sheets never purging on a normal sync — the sync engine treated an empty listing like a possible outage and skipped deletion reconciliation (and could block large deletions), so only a forced full resync cleaned up.

Connectors can set syncContext.sourceConfirmedEmpty when they positively verify the source is empty (not just an empty page). The sync engine refactors this into shouldSkipEmptyListing and exceedsDeletionSafetyThreshold, both of which honor that flag. Google Sheets sets it when Drive metadata confirms the spreadsheet is trashed. Other connectors are unchanged.

Reviewed by Cursor Bugbot for commit 147572d. Configure here.

cursor[bot]
cursor Bot reviewed Jul 23, 2026
View reviewed changes
Comment thread apps/sim/lib/knowledge/connectors/sync-engine.ts
@waleedlatif1 waleedlatif1 mentioned this pull request Jul 23, 2026
Merged
@greptile-apps

greptile-apps Bot commented Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR purges indexed Google Sheets tabs during a normal sync after the source spreadsheet is trashed. The main changes are:

  • Marks an empty listing as confirmed after Drive reports the spreadsheet as trashed.
  • Lets confirmed empty sources pass both deletion safety guards.
  • Adds tests for connector flag propagation and guard behavior.

Confidence Score: 5/5

This looks safe to merge.

  • The confirmed-empty flag now bypasses both guards that blocked the purge.
  • The flag is scoped to one sync and requires an explicit trashed response.
  • Existing incremental and truncated-listing protections remain in place.

Important Files Changed

Filename Overview
apps/sim/connectors/google-sheets/google-sheets.ts Confirms an empty source only when Drive explicitly reports the configured spreadsheet as trashed.
apps/sim/lib/knowledge/connectors/sync-engine.ts Allows a confirmed empty source to pass the empty-listing and mass-deletion guards.
apps/sim/connectors/google-sheets/google-sheets.test.ts Tests confirmed-empty state for trashed, active, malformed, and failed metadata responses.
apps/sim/lib/knowledge/connectors/sync-engine.test.ts Tests the new guard helpers, including confirmed-empty and forced-sync behavior.

Reviews (2): Last reviewed commit: "fix(knowledge): let sourceConfirmedEmpty..." | Re-trigger Greptile

greptile-apps[bot]
greptile-apps Bot reviewed Jul 23, 2026
View reviewed changes
Comment thread apps/sim/lib/knowledge/connectors/sync-engine.ts
@waleedlatif1
fix(knowledge): let sourceConfirmedEmpty also bypass the mass-deletio…
147572d 
…n 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.
@waleedlatif1

waleedlatif1 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

waleedlatif1 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

cursor[bot]
cursor Bot reviewed Jul 23, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 147572d. Configure here.

@waleedlatif1
waleedlatif1 merged commit 387fa37 into staging Jul 23, 2026
20 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/gsheets-trashed-empty-guard branch July 23, 2026 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments
@cursor cursor[bot] cursor[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1