Skip to content

Commit f54b125

Browse files
committed
fix(connectors): key removal on explicit source signals, never on absence
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.
1 parent d2ee39e commit f54b125

7 files changed

Lines changed: 369 additions & 335 deletions

File tree

apps/docs/content/docs/en/knowledgebase/connectors.mdx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,12 @@ To reverse an exclusion, switch to the **Excluded** tab and click **Restore**
162162

163163
On each run the connector fetches documents from the source and compares them against what's already stored. Only changed documents are reprocessed — new content is added, updated content is re-chunked and re-embedded, deleted content is removed. A connector syncing thousands of documents will only do real work when something actually changes.
164164

165+
### Content Removed From View
166+
167+
Content that still exists at the source but is no longer current is treated the same as deleted, and is removed from your knowledge base. This covers an archived Confluence page, a task in an archived Asana project, an archived Webflow CMS item, a retired ServiceNow knowledge article, a spreadsheet in the Google Drive trash, a message moved to Outlook's Deleted Items, and a cancelled incident.io incident.
168+
169+
This keeps agents from citing content your team has already retired. Restoring the item at the source brings it back on the next sync as a new document.
170+
165171
### Connector Status
166172

167173
| Status | Meaning |
@@ -196,7 +202,7 @@ You can add as many connectors as you need to a single knowledge base. Each mana
196202

197203
<FAQ items={[
198204
{ question: "How often do connectors sync?", answer: "You choose from hourly, every 6 hours, daily (default), weekly, or manual-only. Sub-hourly frequencies require a Max or Enterprise plan. Each connector has its own schedule." },
199-
{ question: "What happens if a source document is deleted?", answer: "On the next sync the connector detects the document is gone and removes it from your knowledge base automatically." },
205+
{ question: "What happens if a source document is deleted?", answer: "On the next sync the connector detects the document is gone and removes it from your knowledge base automatically. Content that is archived, retired, trashed, or otherwise removed from view at the source is treated the same way — restoring it at the source brings it back on the next sync." },
200206
{ question: "What happens when I delete a connector?", answer: "The connector is removed and future syncs stop. You're given the option to also delete all documents that were synced by that connector. If you don't check that option, they stay in the knowledge base as-is." },
201207
{ question: "What does the Disabled status mean?", answer: "After 10 consecutive full-sync failures, the connector is automatically disabled to stop retrying. Reconnect the OAuth account or click Resume to re-enable it." },
202208
{ question: "Do metadata tags count against a limit?", answer: "Yes. Tag slots are shared across all documents in a knowledge base — 17 slots total. Multiple connectors draw from the same pool, so plan accordingly if several connectors each auto-populate tags." },

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,16 @@ export const googleSheetsConnector: ConnectorConfig = {
268268
])
269269

270270
/**
271-
* A trashed spreadsheet is no longer current content, so it must drop out of
272-
* the full listing for deletion reconciliation to purge its stored tabs.
271+
* 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.
273281
*/
274282
if (isTrashedDriveFile(driveMetadata)) {
275283
logger.info('Spreadsheet is in the Drive trash; listing no documents', { spreadsheetId })

apps/sim/connectors/outlook/outlook.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
const DELETED_ITEMS_ID = 'deleted-items-id'
1515
const DELETED_SUBFOLDER_ID = 'deleted-subfolder-id'
1616
const INBOX_ID = 'inbox-id'
17+
const JUNK_EMAIL_ID = 'junk-email-id'
1718

1819
interface JsonResponseInit {
1920
status?: number
@@ -109,7 +110,7 @@ describe('isAllMailSync', () => {
109110
})
110111

111112
describe('DELETED_ITEMS_FOLDER', () => {
112-
it('excludes Deleted Items only, never Junk Email', () => {
113+
it('pins the well-known folder name interpolated into the Graph URL', () => {
113114
expect(DELETED_ITEMS_FOLDER).toBe('deleteditems')
114115
})
115116
})
@@ -196,6 +197,29 @@ describe('listDocuments folder exclusion', () => {
196197
expect(result.documents.map((d) => d.externalId)).toEqual(['live'])
197198
})
198199

200+
it('keeps junk mail, which is a spam classification rather than a deletion', async () => {
201+
routeFetch([
202+
deletedItemsRoute,
203+
childFoldersRoute,
204+
[
205+
/\/me\/messages\?/,
206+
() =>
207+
jsonResponse({
208+
value: [
209+
message({ id: 'm1', conversationId: 'live', parentFolderId: INBOX_ID }),
210+
message({ id: 'm2', conversationId: 'junked', parentFolderId: JUNK_EMAIL_ID }),
211+
],
212+
}),
213+
],
214+
])
215+
216+
const result = await outlookConnector.listDocuments('token', { folder: 'all' }, undefined, {})
217+
218+
expect(result.documents.map((d) => d.externalId).sort()).toEqual(['junked', 'live'])
219+
const requested = fetchMock.mock.calls.map((call) => String(call[0]))
220+
expect(requested.some((url) => url.includes('junkemail'))).toBe(false)
221+
})
222+
199223
it('keeps every conversation when the Deleted Items lookup fails with a non-2xx', async () => {
200224
routeFetch([
201225
[/mailFolders\/deleteditems\?/, () => jsonResponse({ error: 'forbidden' }, { status: 403 })],

apps/sim/connectors/servicenow/servicenow.test.ts

Lines changed: 80 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,27 @@ const KB_CONFIG = {
2020
contentType: 'kb_knowledge',
2121
} as const
2222

23-
interface FakeRecord {
24-
sys_id: string
25-
workflow_state?: unknown
26-
short_description?: string
27-
text?: string
23+
/**
24+
* Wraps a value the way `sysparm_display_value=all` does. Under `all` the Table
25+
* API returns EVERY column — `sys_id` included — as `{ display_value, value }`,
26+
* so fixtures must use this shape to exercise the real listing path.
27+
*/
28+
function field(value: string): { display_value: string; value: string } {
29+
return { display_value: value, value }
30+
}
31+
32+
/** Builds a `kb_knowledge` row in the `sysparm_display_value=all` wire shape. */
33+
function kbRecord(sysId: string, workflowState?: string): Record<string, unknown> {
34+
return {
35+
sys_id: field(sysId),
36+
...(workflowState === undefined ? {} : { workflow_state: field(workflowState) }),
37+
short_description: field(`Article ${sysId.slice(0, 4)}`),
38+
text: field(`Body of ${sysId.slice(0, 4)}`),
39+
}
2840
}
2941

30-
function kbRecord(sysId: string, workflowState?: unknown): FakeRecord {
42+
/** Builds the same row in the plain-string shape (`display_value` absent/true/false). */
43+
function kbRecordPlain(sysId: string, workflowState?: string): Record<string, unknown> {
3144
return {
3245
sys_id: sysId,
3346
...(workflowState === undefined ? {} : { workflow_state: workflowState }),
@@ -136,6 +149,47 @@ describe('shouldIngestKBArticle', () => {
136149
})
137150

138151
describe('servicenowConnector.listDocuments', () => {
152+
it('accepts the sysparm_display_value=all object shape, where sys_id is an object', async () => {
153+
mockFetch.mockResolvedValueOnce(jsonResponse({ result: [kbRecord(SYS_ID_A, 'published')] }))
154+
155+
const list = await servicenowConnector.listDocuments('key', { ...KB_CONFIG })
156+
157+
expect(lastRequestUrl().searchParams.get('sysparm_display_value')).toBe('all')
158+
expect(list.documents).toHaveLength(1)
159+
expect(list.documents[0].externalId).toBe(SYS_ID_A)
160+
expect(list.documents[0].title).toBe(`Article ${SYS_ID_A.slice(0, 4)}`)
161+
expect(list.documents[0].sourceUrl).toBe(`${INSTANCE_URL}/kb_view.do?sys_kb_id=${SYS_ID_A}`)
162+
expect(list.documents[0].metadata.workflowState).toBe('published')
163+
})
164+
165+
it('still accepts the plain-string shape', async () => {
166+
mockFetch.mockResolvedValueOnce(
167+
jsonResponse({
168+
result: [kbRecordPlain(SYS_ID_A, 'published'), kbRecordPlain(SYS_ID_B, 'retired')],
169+
})
170+
)
171+
172+
const list = await servicenowConnector.listDocuments('key', { ...KB_CONFIG })
173+
174+
expect(list.documents.map((doc) => doc.externalId)).toEqual([SYS_ID_A])
175+
expect(list.documents[0].title).toBe(`Article ${SYS_ID_A.slice(0, 4)}`)
176+
})
177+
178+
it('skips records whose sys_id object carries no usable value', async () => {
179+
mockFetch.mockResolvedValueOnce(
180+
jsonResponse({
181+
result: [
182+
{ ...kbRecord(SYS_ID_A, 'published'), sys_id: { display_value: null, value: '' } },
183+
kbRecord(SYS_ID_B, 'published'),
184+
],
185+
})
186+
)
187+
188+
const list = await servicenowConnector.listDocuments('key', { ...KB_CONFIG })
189+
190+
expect(list.documents.map((doc) => doc.externalId)).toEqual([SYS_ID_B])
191+
})
192+
139193
it('drops retired KB records and keeps every other state', async () => {
140194
mockFetch.mockResolvedValueOnce(
141195
jsonResponse({
@@ -200,12 +254,17 @@ describe('servicenowConnector.listDocuments', () => {
200254
mockFetch.mockResolvedValueOnce(
201255
jsonResponse({
202256
result: [
203-
{ sys_id: SYS_ID_A, number: 'INC001', short_description: 'Down', state: '7' },
204257
{
205-
sys_id: SYS_ID_B,
206-
number: 'INC002',
207-
short_description: 'Up',
208-
workflow_state: 'retired',
258+
sys_id: field(SYS_ID_A),
259+
number: field('INC001'),
260+
short_description: field('Down'),
261+
state: { display_value: 'Closed', value: '7' },
262+
},
263+
{
264+
sys_id: field(SYS_ID_B),
265+
number: field('INC002'),
266+
short_description: field('Up'),
267+
workflow_state: field('retired'),
209268
},
210269
],
211270
})
@@ -280,7 +339,17 @@ describe('servicenowConnector.getDocument', () => {
280339
const doc = await servicenowConnector.getDocument('key', { ...KB_CONFIG }, SYS_ID_A)
281340

282341
expect(doc?.externalId).toBe(SYS_ID_A)
342+
expect(doc?.sourceUrl).toBe(`${INSTANCE_URL}/kb_view.do?sys_kb_id=${SYS_ID_A}`)
283343
expect(lastRequestUrl().pathname).toBe(`/api/now/table/kb_knowledge/${SYS_ID_A}`)
344+
expect(lastRequestUrl().searchParams.get('sysparm_display_value')).toBe('all')
345+
})
346+
347+
it('hydrates an article returned in the plain-string shape', async () => {
348+
mockFetch.mockResolvedValueOnce(jsonResponse({ result: kbRecordPlain(SYS_ID_A, 'published') }))
349+
350+
const doc = await servicenowConnector.getDocument('key', { ...KB_CONFIG }, SYS_ID_A)
351+
352+
expect(doc?.externalId).toBe(SYS_ID_A)
284353
})
285354

286355
it('rejects a malformed sys_id without issuing a request', async () => {

0 commit comments

Comments
 (0)