fix(connectors): purge archived/deleted source items in KB connectors#5880
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Confluence requests Asana, Outlook, and ServiceNow also set User-facing docs add a “Content Removed From View” section and expand the deleted-document FAQ. Reviewed by Cursor Bugbot for commit f54b125. Configure here. |
Greptile SummaryThis PR removes non-current source content from knowledge-base connector listings. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "fix(connectors): key removal on explicit..." | Re-trigger Greptile |
|
@greptileai review |
|
bugbot run |
… reconciliation purges them
…e KB connectors The sync engine only purges a knowledge-base document when its source item is absent from a full-sync listing, so any connector that keeps listing archived/trashed/canceled items never drops them. An audit of all 51 connectors found seven with this bug: - asana: list only non-archived projects (the API returns both when `archived` is omitted), so tasks under archived projects stop being re-listed - google-sheets: skip a spreadsheet Drive reports as trashed, which stays readable by id for 30 days before the Sheets call starts 404ing - incidentio: exclude canceled incidents by default (cancelling is incident.io's documented stand-in for deletion), with an explicit opt-in to sync them - outlook: exclude Deleted Items from the all-mail listing, which Graph otherwise includes - servicenow: drop retired knowledge articles, which the Table API returns with no implicit state filter - webflow: drop archived CMS items, which the staged items endpoint always returns and offers no way to filter - youtube: drop playlist entries whose video was deleted or made private, which the API keeps returning as placeholder items Every exclusion keys off an explicit non-current signal and fails open on a missing field or a failed metadata read, since wrongly excluding a live item would hard-delete it. Explicit user filter selections are still honoured verbatim; the new defaults apply only when nothing is configured. Also flag truncated listings as capped in asana, outlook, and servicenow. All three silently cut a listing short at their configured item cap without setting `syncContext.listingCapped`, so reconciliation read the untraversed tail as deleted at the source and hard-deleted it.
… path listDocuments deliberately keeps syncing a project the user pinned via the `project` config field even once it is archived, but getDocument ignored sourceConfig and applied the all-parents-archived exclusion unconditionally. For a pinned archived project the listing kept emitting its tasks while every hydration returned null, so new tasks were dropped as empty and already-indexed ones were frozen at their last content. isTaskUnderActiveProject now takes the pinned project gid and keeps any task reachable through it, matching the listing exactly. The unpinned path is unchanged and still fails open on missing/non-boolean archived values.
1f4c2fb to
d2ee39e
Compare
|
@cursor review |
…ence
Follow-up to the connector purge fixes, from an independent audit.
YouTube inferred deletion from absence: a playlist entry whose id was missing
from a `videos.list` response was dropped, so a well-formed 200 that returned 49
of 50 requested ids hard-deleted the 50th. Playlist items instead carry a
documented `status.privacyStatus`, available as a free part on a call the
connector already makes, so the extra `videos.list` request is gone along with
its quota-failure and pagination-wedge risks. An item is now excluded only on an
explicit `private`; missing, empty, or unrecognized values keep it.
ServiceNow read every record through a guard requiring a string `sys_id`, but
the listing requests `sysparm_display_value=all`, under which every field —
`sys_id` included — comes back as `{display_value, value}`. The guard rejected
every record, so the retired-article filter was unreachable and the sys_id
object would have leaked into `externalId` and `title` had it not been. Records
are now read through the existing `rawValue` normalizer, which accepts both wire
shapes, and the fixtures use the shape the API actually returns.
Also: resolve the ServiceNow cap ambiguity with `X-Total-Count` so a table that
ends exactly on a page boundary is not read as truncated; stop the Google Sheets
comment claiming a purge the engine's zero-document guard prevents; and assert
the Outlook junk-mail invariant instead of comparing a constant to itself.
Document the behavior change: content archived, retired, or trashed at the
source is now removed from the knowledge base, and restoring it re-ingests it.
|
@greptileai review |
|
bugbot run |
There was a problem hiding this comment.
✅ 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 f54b125. Configure here.
Summary
A knowledge-base document is only purged when its source item is absent from a full-sync listing — the sync engine has no other removal signal. So any connector that keeps listing archived/trashed/canceled items never drops them, and they stay embedded and keep getting returned by search. This shipped as a real incident: an internal support bot cited an archived Confluence VPN article to a user.
Confluence was the reported case; auditing all 51 other connectors turned up seven more with the same bug.
Confluence — v2
GET /spaces/{id}/pagesincludesarchivedpages by default ("By default,currentandarchivedare used"), so archived pages never left the listing. Now requestsstatus=current, with a client-side filter for the CQL path (CQL has no status field) and ingetDocumentto close the archive-between-list-and-hydrate race.Seven more connectors:
GET /projectsreturns archived and active whenarchivedis omittedGET /me/messagesincludes on the all-mail pathplaylistItems.listkeeps returning as placeholder itemsEvery exclusion keys off an explicit non-current signal and fails open on a missing field, a non-boolean value, or a failed metadata read — wrongly excluding a live item would hard-delete it, which is worse than the bug being fixed. Explicit user filter selections are honoured verbatim; the new defaults apply only when nothing is configured. incident.io gains an explicit "All (including canceled)" option so there is still a way to sync everything.
Also fixed: three silent truncation bugs
asana, outlook, and servicenow each cut a listing short at their configured item cap without setting
syncContext.listingCapped, so reconciliation treated the untraversed tail as deleted at the source and hard-deleted it. All three now flag the cap. This is pre-existing and independent of the archived-item bug, but it lives in the same functions and is the more dangerous of the two.Deploy note
The first full sync after deploy removes previously-indexed archived/deleted items — that is the intended cleanup. A knowledge base where this would delete >50% of documents (and >5) is held back by the engine's existing safety threshold and needs a manual full resync to complete.
Type of Change
Testing
414 connector tests pass, including new coverage per connector for the exclusion helpers, the
listDocuments/getDocumentcall sites, and the fail-open paths. Lint and typecheck clean. Every API-semantics claim above was verified against the service's live documentation or OpenAPI spec; not live-tested against real tenants.Checklist