Skip to content

Commit ecf2ae2

Browse files
fix(knowledge): skip verified empty Slack threads (#7734)
* fix(knowledge): skip verified empty Slack threads * chore(knowledge): format Slack sync integration fixture
1 parent d69eb72 commit ecf2ae2

3 files changed

Lines changed: 312 additions & 11 deletions

File tree

apps/sim/connectors/slack/slack.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,66 @@ describe('Slack thread indexing through provider APIs', () => {
410410
})
411411
})
412412

413+
describe('Slack threads without indexable text', () => {
414+
beforeEach(() => {
415+
pageSize = 1
416+
channels = [
417+
{
418+
channel: GENERAL,
419+
readers: ['alice'],
420+
messages: [{ ...root(''), thread_ts: ROOT }],
421+
replies: { [ROOT]: [{ ...root(''), thread_ts: ROOT }, reply('')] },
422+
},
423+
]
424+
})
425+
426+
it('explicitly skips a listed thread only after reading all its reply pages', async () => {
427+
const listed = await listAll('alice')
428+
expect(listed.documents).toHaveLength(1)
429+
const document = await slackConnector.getDocument('alice', {}, id(GENERAL.id), listed.context)
430+
expect(document).toMatchObject({
431+
externalId: listed.documents[0].externalId,
432+
content: '',
433+
contentDeferred: false,
434+
skippedReason: 'Document contains no extractable text',
435+
skippedExistingDisposition: 'replace',
436+
metadata: { messageCount: 0, rootTs: ROOT, channelId: GENERAL.id, teamId: TEAM },
437+
})
438+
expect(calls.filter((call) => call.method === 'conversations.replies')).toHaveLength(2)
439+
expect(calls.some((call) => call.method === 'chat.getPermalink')).toBe(false)
440+
})
441+
442+
it('indexes a later reply edit even when the root and reply count have not changed', async () => {
443+
const empty = await slackConnector.getDocument('alice', {}, id(GENERAL.id))
444+
channels[0].replies[ROOT][1] = reply('Orion has a launch date')
445+
const document = await slackConnector.getDocument('alice', {}, id(GENERAL.id))
446+
expect(document?.externalId).toBe(empty?.externalId)
447+
expect(document?.content).toContain('Orion has a launch date')
448+
expect(document?.contentHash).not.toBe(empty?.contentHash)
449+
expect(document?.skippedReason).toBeUndefined()
450+
})
451+
452+
it('does not classify a missing root as verified empty content', async () => {
453+
replacement = (call) =>
454+
call.method === 'conversations.replies' ? { ok: true, messages: [] } : undefined
455+
expect(await slackConnector.getDocument('alice', {}, id(GENERAL.id))).toBeNull()
456+
})
457+
458+
it.each([
459+
[{ ok: true, messages: [reply('')], is_limited: true }, 'only part'],
460+
[{ ok: true, messages: [reply('')], has_more: true }, 'continuation cursor'],
461+
[{ ok: false, error: 'missing_scope' }, 'missing_scope'],
462+
[{ ok: true }, 'invalid message page'],
463+
])(
464+
'does not skip an empty thread when a later page is incomplete: %j',
465+
async (response, error) => {
466+
replacement = (call) =>
467+
call.method === 'conversations.replies' && call.params.has('cursor') ? response : undefined
468+
await expect(slackConnector.getDocument('alice', {}, id(GENERAL.id))).rejects.toThrow(error)
469+
}
470+
)
471+
})
472+
413473
describe('Slack incomplete and unsafe provider responses', () => {
414474
it('does not complete a member observation when channel access disappears between history pages', async () => {
415475
pageSize = 1

apps/sim/connectors/slack/slack.ts

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
BoundedLines,
2121
CONNECTOR_TEXT_DOCUMENT_MAX_BYTES,
2222
ConnectorFileTooLargeError,
23+
markSkipped,
2324
parseDefaultedUnlimitedSafeInteger,
2425
parseMultiValue,
2526
parseTagDate,
@@ -693,22 +694,14 @@ async function getDocument(
693694
cursor = continuation
694695
}
695696
if (!exhausted) throw new Error(`Slack thread exceeds ${MAX_THREAD_PAGES} reply pages`)
696-
if (!root || lines.count === 0) return null
697-
const link = await slackApiGet('chat.getPermalink', accessToken, {
698-
channel: channelId,
699-
message_ts: rootTs,
700-
})
701-
if (typeof link.permalink !== 'string' || !link.permalink.startsWith('https://')) {
702-
throw new Error('Slack did not return a message permalink')
703-
}
704-
const content = lines.join()
705-
return {
697+
if (!root) return null
698+
const content = lines.count > 0 ? lines.join() : ''
699+
const document: ExternalDocument = {
706700
externalId,
707701
title: messageTitle(channel, root),
708702
content,
709703
contentDeferred: false,
710704
mimeType: 'text/plain',
711-
sourceUrl: link.permalink,
712705
contentHash: `slack-content:v4:${createHash('sha256').update(content).digest('hex')}`,
713706
metadata: {
714707
channelName: channel.name,
@@ -720,6 +713,21 @@ async function getDocument(
720713
lastActivity: new Date(Number(lastActivity) * 1000).toISOString(),
721714
},
722715
}
716+
/** Only a fully read thread can authoritatively replace previously indexed text with a skip. */
717+
if (lines.count === 0) {
718+
return {
719+
...markSkipped(document, 'Document contains no extractable text'),
720+
skippedExistingDisposition: 'replace',
721+
}
722+
}
723+
const link = await slackApiGet('chat.getPermalink', accessToken, {
724+
channel: channelId,
725+
message_ts: rootTs,
726+
})
727+
if (typeof link.permalink !== 'string' || !link.permalink.startsWith('https://')) {
728+
throw new Error('Slack did not return a message permalink')
729+
}
730+
return { ...document, sourceUrl: link.permalink }
723731
} catch (error) {
724732
if (
725733
error instanceof SlackApiError &&
Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
/** Real sync jobs, PostgreSQL, storage, indexing and authorized search; Slack and embedding responses are synthetic. */
2+
import { mkdtempSync } from 'node:fs'
3+
import { rm } from 'node:fs/promises'
4+
import { tmpdir } from 'node:os'
5+
import path from 'node:path'
6+
import { db } from '@sim/db'
7+
import {
8+
document,
9+
embedding,
10+
knowledgeConnector,
11+
organization,
12+
user,
13+
workspace,
14+
} from '@sim/db/schema'
15+
import { generateId } from '@sim/utils/id'
16+
import { and, eq, inArray } from 'drizzle-orm'
17+
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
18+
19+
const fixture = vi.hoisted(() => ({ storageRoot: '', embeddingCalls: 0 }))
20+
vi.mock('@/lib/uploads/core/setup.server', () => ({
21+
get UPLOAD_DIR_SERVER() {
22+
return fixture.storageRoot
23+
},
24+
}))
25+
vi.mock('@/lib/embeddings', async () => ({
26+
...(await import('@/lib/embeddings/client')),
27+
assertKnowledgeEmbeddingCapacity: async () => {},
28+
embedKnowledge: async (texts: string[]) => {
29+
fixture.embeddingCalls++
30+
return {
31+
embeddings: texts.map(() => [1, ...Array<number>(1535).fill(0)]),
32+
totalTokens: texts.length,
33+
billableTokens: 0,
34+
isBYOK: true,
35+
modelName: 'text-embedding-3-small',
36+
pricingId: 'text-embedding-3-small',
37+
}
38+
},
39+
}))
40+
41+
import { resolveBillingAttribution } from '@/lib/billing/core/billing-attribution'
42+
import {
43+
createKnowledgeAclFixtureIds,
44+
seedKnowledgeAclFixture,
45+
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
46+
import { searchKnowledge } from '@/lib/knowledge/application/search'
47+
import * as connectorTokens from '@/lib/knowledge/connectors/access-token'
48+
import { executeConnectorSyncJob } from '@/background/knowledge-connector-sync'
49+
50+
const TEAM = 'T0FIXTURE'
51+
const CHANNEL = 'C0GENERAL'
52+
const ROOT = '1700000100.000100'
53+
const REPLY = '1700000200.000100'
54+
const EXTERNAL_ID = `slack:v4:${TEAM}:${CHANNEL}:${ROOT}`
55+
const EMPTY_REASON = 'Document contains no extractable text'
56+
57+
describe('Slack empty threads through sync jobs, indexing and search', () => {
58+
const ids = createKnowledgeAclFixtureIds()
59+
const documentId = generateId()
60+
let billing: Awaited<ReturnType<typeof resolveBillingAttribution>>
61+
let replyText = ''
62+
let incomplete = false
63+
let missingRoot = false
64+
const channel = { id: CHANNEL, name: 'general', is_archived: false }
65+
const root = () => ({ type: 'message', ts: ROOT, thread_ts: ROOT, text: '', reply_count: 1 })
66+
67+
async function providerFetch(input: string | URL | Request, init?: RequestInit) {
68+
const url = new URL(
69+
typeof input === 'string' ? input : input instanceof URL ? input.href : input.url
70+
)
71+
expect(url.origin).toBe('https://slack.com')
72+
expect(new Headers(init?.headers).get('Authorization')).toBe('Bearer fixture-slack-token')
73+
const method = url.pathname.split('/').at(-1)
74+
switch (method) {
75+
case 'auth.test':
76+
return Response.json({ ok: true, team_id: TEAM })
77+
case 'conversations.list':
78+
return Response.json({ ok: true, channels: [channel] })
79+
case 'conversations.info':
80+
return Response.json({ ok: true, channel })
81+
case 'conversations.history':
82+
return Response.json({ ok: true, messages: [root()] })
83+
case 'conversations.replies':
84+
if (missingRoot) return Response.json({ ok: true, messages: [] })
85+
if (!url.searchParams.has('cursor')) {
86+
return Response.json({
87+
ok: true,
88+
messages: [root()],
89+
has_more: true,
90+
response_metadata: { next_cursor: 'reply' },
91+
})
92+
}
93+
return Response.json({
94+
ok: true,
95+
messages: [{ type: 'message', ts: REPLY, thread_ts: ROOT, text: replyText }],
96+
is_limited: incomplete,
97+
})
98+
case 'chat.getPermalink':
99+
return Response.json({
100+
ok: true,
101+
permalink: `https://fixture.slack.com/archives/${CHANNEL}/p${ROOT.replace('.', '')}`,
102+
})
103+
default:
104+
throw new Error('Unexpected fixture Slack endpoint')
105+
}
106+
}
107+
async function sync() {
108+
return executeConnectorSyncJob({
109+
connectorId: ids.connectorId,
110+
requestId: 'slack-empty-fixture',
111+
fullSync: true,
112+
billingAttribution: billing,
113+
})
114+
}
115+
async function row() {
116+
const [stored] = await db
117+
.select()
118+
.from(document)
119+
.where(and(eq(document.connectorId, ids.connectorId), eq(document.externalId, EXTERNAL_ID)))
120+
expect(stored?.id).toBe(documentId)
121+
return stored!
122+
}
123+
async function vectors() {
124+
return db
125+
.select({ content: embedding.content })
126+
.from(embedding)
127+
.where(eq(embedding.documentId, documentId))
128+
}
129+
async function search() {
130+
const result = await searchKnowledge.execute({
131+
principal: { kind: 'session', userId: ids.aliceId, sessionId: 'slack-empty-fixture' },
132+
input: {
133+
workspaceId: ids.workspaceId,
134+
knowledgeBaseIds: [ids.knowledgeBaseId],
135+
query: 'Orion',
136+
searchMode: 'hybrid',
137+
topK: 10,
138+
},
139+
})
140+
return result.results.map((result) => result.documentId)
141+
}
142+
beforeAll(async () => {
143+
fixture.storageRoot = mkdtempSync(path.join(tmpdir(), 'sim-slack-empty-'))
144+
await seedKnowledgeAclFixture(ids)
145+
billing = await resolveBillingAttribution({
146+
actorUserId: ids.aliceId,
147+
workspaceId: ids.workspaceId,
148+
})
149+
await db
150+
.update(knowledgeConnector)
151+
.set({
152+
connectorType: 'slack',
153+
sourceConfig: { channel: CHANNEL, maxMessages: 0 },
154+
accessMode: 'workspace',
155+
status: 'active',
156+
syncLockToken: null,
157+
})
158+
.where(eq(knowledgeConnector.id, ids.connectorId))
159+
vi.spyOn(connectorTokens, 'resolveConnectorAccessToken').mockResolvedValue({
160+
accessToken: 'fixture-slack-token',
161+
})
162+
vi.stubGlobal('fetch', providerFetch)
163+
await db.insert(document).values({
164+
id: documentId,
165+
knowledgeBaseId: ids.knowledgeBaseId,
166+
connectorId: ids.connectorId,
167+
externalId: EXTERNAL_ID,
168+
filename: 'Thread.txt',
169+
mimeType: 'text/plain',
170+
fileUrl: '',
171+
fileSize: 0,
172+
processingStatus: 'failed',
173+
processingError: 'Synthetic previous source failure',
174+
})
175+
})
176+
afterAll(async () => {
177+
await db.delete(workspace).where(eq(workspace.id, ids.workspaceId))
178+
await db.delete(organization).where(eq(organization.id, ids.organizationId))
179+
await db.delete(user).where(inArray(user.id, [ids.aliceId, ids.bobId]))
180+
await rm(fixture.storageRoot, { recursive: true, force: true })
181+
vi.restoreAllMocks()
182+
vi.unstubAllGlobals()
183+
await db.$client.end()
184+
})
185+
186+
it('completes empty-thread syncs, recovers reply edits and removes stale searchable text', async () => {
187+
expect(await sync()).toMatchObject({
188+
outcome: 'completed',
189+
docsFailed: 0,
190+
docsSkipped: 1,
191+
processingDispatch: { requested: 0, failed: 0 },
192+
})
193+
expect(await row()).toMatchObject({
194+
processingStatus: 'failed',
195+
processingError: EMPTY_REASON,
196+
storageKey: null,
197+
})
198+
expect(await vectors()).toEqual([])
199+
expect(fixture.embeddingCalls).toBe(0)
200+
expect(await sync()).toMatchObject({ outcome: 'completed', docsFailed: 0 })
201+
expect(fixture.embeddingCalls).toBe(0)
202+
203+
replyText = 'Orion launch is scheduled for Friday.'
204+
expect(await sync()).toMatchObject({ outcome: 'completed', docsFailed: 0, docsUpdated: 1 })
205+
expect(await row()).toMatchObject({ processingStatus: 'completed', processingError: null })
206+
expect((await vectors()).map((row) => row.content).join(' ')).toContain(replyText)
207+
expect(await search()).toContain(documentId)
208+
const embedded = fixture.embeddingCalls
209+
expect(await sync()).toMatchObject({ outcome: 'completed', docsUnchanged: 1 })
210+
expect(fixture.embeddingCalls).toBe(embedded)
211+
212+
replyText = ''
213+
expect(await sync()).toMatchObject({ outcome: 'completed', docsFailed: 0, docsSkipped: 1 })
214+
expect(await row()).toMatchObject({ processingError: EMPTY_REASON, storageKey: null })
215+
expect(await vectors()).toEqual([])
216+
expect(fixture.embeddingCalls).toBe(embedded)
217+
expect(await search()).not.toContain(documentId)
218+
219+
replyText = 'Orion launch moved to Monday.'
220+
expect(await sync()).toMatchObject({ outcome: 'completed', docsUpdated: 1 })
221+
expect(await search()).toContain(documentId)
222+
const restored = await vectors()
223+
incomplete = true
224+
replyText = ''
225+
await expect(sync()).rejects.toThrow('1 source failures')
226+
expect(await vectors()).toEqual(restored)
227+
expect((await row()).storageKey).not.toBeNull()
228+
incomplete = false
229+
missingRoot = true
230+
await expect(sync()).rejects.toThrow('1 source failures')
231+
expect(await vectors()).toEqual(restored)
232+
}, 60000)
233+
})

0 commit comments

Comments
 (0)