Skip to content

Commit 12224be

Browse files
authored
fix(knowledge): preserve permission notices and fixture isolation (#7926)
* fix(knowledge): preserve permission notices and fixture isolation * fix(knowledge): make healthy sync fixtures explicit
1 parent 3060a84 commit 12224be

7 files changed

Lines changed: 113 additions & 12 deletions

File tree

.github/workflows/test-build.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ jobs:
231231
run: >-
232232
bunx vitest run --mode integration
233233
lib/knowledge/__integration__/search-source-progress.integration.ts
234+
lib/knowledge/__integration__/organization-search-overview.integration.ts
234235
lib/knowledge/__integration__/search-source-pagination.integration.ts
235236
lib/knowledge/__integration__/search-reference-batching.integration.ts
236237
lib/knowledge/__integration__/embedding-insert-batches.integration.ts

apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.test.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,15 @@ describe('organization source detail navigation', () => {
318318
}
319319
)
320320

321+
it('does not classify a provider message containing the permission text as its own notice', async () => {
322+
mocks.detail.mockReturnValue({
323+
data: { ...connector, lastSyncError: `Provider message: ${SOURCE_PERMISSION_ERROR}` },
324+
})
325+
await render()
326+
expect(container.textContent).toContain('Some connection updates are incomplete')
327+
expect(container.textContent).not.toContain('Permission verification incomplete')
328+
})
329+
321330
it.each(['', '?view=settings', '?view=history'])(
322331
'shows integration deactivation independently of source sync state at %s',
323332
async (searchParams) => {
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/** @vitest-environment node */
2+
import { describe, expect, it } from 'vitest'
3+
import { assertCodaLiveFixture } from '@/lib/knowledge/__integration__/coda-live-fixture'
4+
5+
const marker = 'SimConnector-fixture'
6+
const source = { name: `Sim Coda connector verification ${marker}`, owner: 'Owner@example.com' }
7+
8+
describe('Coda live fixture validation', () => {
9+
it.each(['Owner@example.com', 'owner@example.com', ' OWNER@EXAMPLE.COM '])(
10+
'rejects the owner as the second identity: %s',
11+
(secondEmail) => {
12+
expect(() => assertCodaLiveFixture(source, marker, secondEmail)).toThrow(
13+
'Refusing to change sharing'
14+
)
15+
}
16+
)
17+
18+
it('rejects a document outside the disposable fixture', () => {
19+
expect(() =>
20+
assertCodaLiveFixture({ ...source, name: 'Unrelated' }, marker, 'reader@example.com')
21+
).toThrow('Refusing to change sharing')
22+
})
23+
24+
it('accepts the disposable document with a distinct second identity', () => {
25+
expect(() => assertCodaLiveFixture(source, marker, 'reader@example.com')).not.toThrow()
26+
})
27+
})
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { normalizeEmail } from '@sim/utils/string'
2+
3+
/** Checks the disposable document and distinct identities before live sharing mutations. */
4+
export function assertCodaLiveFixture(
5+
source: { name: string; owner: string },
6+
marker: string,
7+
secondEmail: string
8+
): void {
9+
if (
10+
source.name !== `Sim Coda connector verification ${marker}` ||
11+
normalizeEmail(source.owner) === normalizeEmail(secondEmail)
12+
) {
13+
throw new Error('Refusing to change sharing on a non-fixture document')
14+
}
15+
}

apps/sim/lib/knowledge/__integration__/coda-live.integration.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
} from '@/lib/billing/core/billing-attribution'
5050
import { encryptSecret } from '@/lib/core/security/encryption'
5151
import { createOrganizationCredential } from '@/lib/credentials/application/organization-credentials'
52+
import { assertCodaLiveFixture } from '@/lib/knowledge/__integration__/coda-live-fixture'
5253
import { seedKnowledgeAclFixture } from '@/lib/knowledge/__integration__/seed-source-access-fixture'
5354
import { listKnowledgeChunks } from '@/lib/knowledge/application/chunks'
5455
import { createKnowledgeConnector } from '@/lib/knowledge/application/connectors'
@@ -168,12 +169,7 @@ describe
168169
codaDocPath(fixture.docId),
169170
z.object({ name: z.string(), owner: z.string().email() })
170171
)
171-
if (
172-
source.name !== `Sim Coda connector verification ${fixture.marker}` ||
173-
source.owner === secondEmail
174-
) {
175-
throw new Error('Refusing to change sharing on a non-fixture document')
176-
}
172+
assertCodaLiveFixture(source, fixture.marker, secondEmail!)
177173
fixtureValidated = true
178174
await revokeShare()
179175
await waitForAcl(false)

apps/sim/lib/knowledge/__integration__/organization-search-overview.integration.ts

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,15 @@ import {
1515
workspace,
1616
} from '@sim/db/schema'
1717
import { generateId } from '@sim/utils/id'
18-
import { eq, inArray } from 'drizzle-orm'
18+
import { eq, inArray, sql } from 'drizzle-orm'
1919
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'
2020
import {
2121
createKnowledgeAclFixtureIds,
2222
seedKnowledgeAclFixture,
2323
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
2424
import { readOrganizationSearchOverview } from '@/lib/knowledge/application/organization-search-overview'
2525
import { listSearchSources } from '@/lib/knowledge/application/search-sources'
26+
import { SOURCE_PERMISSION_ERROR } from '@/lib/knowledge/connectors/sync-limits'
2627

2728
const ids = createKnowledgeAclFixtureIds()
2829
const indexId = generateId()
@@ -177,6 +178,44 @@ async function provider(connectorType: string) {
177178
}
178179

179180
describe('organization operational overview with real SQL', () => {
181+
it.each([
182+
SOURCE_PERMISSION_ERROR,
183+
`Directory refresh incomplete: fixture\n${SOURCE_PERMISSION_ERROR}`,
184+
`${SOURCE_PERMISSION_ERROR}\nSource listing failed for a fixture account`,
185+
`Directory refresh incomplete: fixture\n${SOURCE_PERMISSION_ERROR}\nSource listing failed for a fixture account`,
186+
])(
187+
'recognizes a complete permission notice within composed diagnostics: %s',
188+
async (lastSyncError) => {
189+
await db
190+
.update(knowledgeConnector)
191+
.set({ lastSyncError })
192+
.where(eq(knowledgeConnector.id, driveId))
193+
expect(await provider('google_drive')).toMatchObject({
194+
status: 'needs_attention',
195+
issue: 'permission_sync_incomplete',
196+
})
197+
}
198+
)
199+
200+
it('does not classify a provider message containing the permission text as its own notice', async () => {
201+
await db
202+
.update(knowledgeConnector)
203+
.set({ lastSyncError: `Provider message: ${SOURCE_PERMISSION_ERROR}` })
204+
.where(eq(knowledgeConnector.id, driveId))
205+
expect(await provider('google_drive')).toMatchObject({
206+
status: 'needs_attention',
207+
issue: 'sync_failed',
208+
})
209+
})
210+
211+
it('ignores permission notices on paused sources', async () => {
212+
await db
213+
.update(knowledgeConnector)
214+
.set({ lastSyncError: `Directory refresh incomplete: fixture\n${SOURCE_PERMISSION_ERROR}` })
215+
.where(eq(knowledgeConnector.id, pausedDriveId))
216+
expect(await provider('google_drive')).toMatchObject({ status: 'active', issue: null })
217+
})
218+
180219
it('counts configured sources independently of viewer ACLs, and excludes workspace and untouched providers', async () => {
181220
const result = await readOrganizationSearchOverview.execute({ principal, input })
182221
expect(result.providers).toEqual(
@@ -188,6 +227,7 @@ describe('organization operational overview with real SQL', () => {
188227
status: 'active',
189228
issue: null,
190229
isSyncing: false,
230+
hasPendingSync: false,
191231
},
192232
{
193233
connectorType: 'gmail',
@@ -196,6 +236,7 @@ describe('organization operational overview with real SQL', () => {
196236
status: 'active',
197237
issue: null,
198238
isSyncing: false,
239+
hasPendingSync: false,
199240
},
200241
])
201242
)
@@ -237,7 +278,7 @@ describe('organization operational overview with real SQL', () => {
237278
.where(eq(knowledgeConnector.id, gmailId))
238279
expect(await provider('gmail')).toMatchObject({ status: 'waiting_for_connections' })
239280
})
240-
it('distinguishes normal member continuation from partial failure without treating idle as success', async () => {
281+
it('distinguishes queued member continuation from active indexing and partial failure', async () => {
241282
await db
242283
.update(knowledgeConnectorMember)
243284
.set({ listingCheckpoint: { cursor: 'fixture' } })
@@ -248,9 +289,15 @@ describe('organization operational overview with real SQL', () => {
248289
connectorId: gmailId,
249290
status: 'partial',
250291
membersIncomplete: 1,
292+
docsFailed: 0,
293+
processingDispatchFailed: 0,
251294
completedAt: new Date(),
252295
})
253-
expect(await provider('gmail')).toMatchObject({ status: 'indexing' })
296+
expect(await provider('gmail')).toMatchObject({
297+
status: 'active',
298+
isSyncing: false,
299+
hasPendingSync: true,
300+
})
254301
await db
255302
.update(knowledgeConnectorMemberSyncLog)
256303
.set({ membersFailed: 1 })
@@ -267,9 +314,13 @@ describe('organization operational overview with real SQL', () => {
267314
expect(await provider('gmail')).toMatchObject({ status: 'needs_attention' })
268315
await db
269316
.update(knowledgeConnector)
270-
.set({ nextMemberSyncAt: new Date() })
317+
.set({ nextMemberSyncAt: sql`statement_timestamp() - interval '1 second'` })
271318
.where(eq(knowledgeConnector.id, gmailId))
272-
expect(await provider('gmail')).toMatchObject({ status: 'indexing' })
319+
expect(await provider('gmail')).toMatchObject({
320+
status: 'active',
321+
isSyncing: false,
322+
hasPendingSync: true,
323+
})
273324
await db
274325
.update(knowledgeConnector)
275326
.set({ nextMemberSyncAt: null })
@@ -278,6 +329,8 @@ describe('organization operational overview with real SQL', () => {
278329
id: generateId(),
279330
connectorId: gmailId,
280331
status: 'completed',
332+
docsFailed: 0,
333+
processingDispatchFailed: 0,
281334
startedAt: new Date(Date.now() + 1000),
282335
completedAt: new Date(),
283336
})

apps/sim/lib/knowledge/application/organization-search-overview.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ export const readOrganizationSearchOverview = defineAuthorizedKnowledgeUseCase({
213213
))`,
214214
hasAccountError: sql<boolean>`bool_or(NOT ${paused} AND ${knowledgeConnector.accessMode} = 'members' AND ${hasMemberError})`,
215215
hasDocumentError: sql<boolean>`bool_or(NOT ${paused} AND ${hasDocumentsInState(failedDocumentCondition())})`,
216-
hasPermissionError: sql<boolean>`bool_or(NOT ${paused} AND ${knowledgeConnector.lastSyncError} = ${SOURCE_PERMISSION_ERROR})`,
216+
hasPermissionError: sql<boolean>`bool_or(NOT ${paused} AND ${SOURCE_PERMISSION_ERROR} = ANY(string_to_array(${knowledgeConnector.lastSyncError}, ${'\n'})))`,
217217
hasIndexing: sql<boolean>`bool_or(NOT ${paused}
218218
AND (${knowledgeConnector.accessMode} <> 'members' OR ${hasActiveMembers} OR ${knowledgeConnector.credentialId} IS NOT NULL)
219219
AND (

0 commit comments

Comments
 (0)