From 34ad25ea7bec05c4b795c9b0923768080075b6b8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 17 Sep 2026 00:51:30 -0700 Subject: [PATCH 1/2] fix(knowledge): preserve permission notices and fixture isolation --- .github/workflows/test-build.yml | 1 + .../[connectorId]/source-detail.test.tsx | 27 +++++++++ .../sources/[connectorId]/source-detail.tsx | 3 +- .../__integration__/coda-live-fixture.test.ts | 27 +++++++++ .../__integration__/coda-live-fixture.ts | 15 +++++ .../__integration__/coda-live.integration.ts | 8 +-- ...rganization-search-overview.integration.ts | 55 ++++++++++++++++++- .../organization-search-overview.ts | 2 +- 8 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 apps/sim/lib/knowledge/__integration__/coda-live-fixture.test.ts create mode 100644 apps/sim/lib/knowledge/__integration__/coda-live-fixture.ts diff --git a/.github/workflows/test-build.yml b/.github/workflows/test-build.yml index 07364732507..66694c9d63d 100644 --- a/.github/workflows/test-build.yml +++ b/.github/workflows/test-build.yml @@ -231,6 +231,7 @@ jobs: run: >- bunx vitest run --mode integration lib/knowledge/__integration__/search-source-progress.integration.ts + lib/knowledge/__integration__/organization-search-overview.integration.ts lib/knowledge/__integration__/search-source-pagination.integration.ts lib/knowledge/__integration__/search-reference-batching.integration.ts lib/knowledge/__integration__/embedding-insert-batches.integration.ts diff --git a/apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.test.tsx b/apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.test.tsx index 4b7cb5598ad..d6aff3c4abd 100644 --- a/apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.test.tsx +++ b/apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.test.tsx @@ -292,6 +292,33 @@ describe('organization source detail navigation', () => { } ) + it.each(['active', 'pending', 'syncing'] as const)( + 'preserves the permission warning among composed notices while %s', + async (status) => { + mocks.detail.mockReturnValue({ + data: { + ...connector, + status, + lastSyncError: `Directory refresh incomplete: private details\n${SOURCE_PERMISSION_ERROR}\nSource listing failed for a private account`, + }, + }) + await render() + expect(container.textContent).toContain('Permission verification incomplete') + expect(container.textContent).toContain(SOURCE_PERMISSION_ERROR) + expect(container.textContent).not.toContain('private') + expect(container.textContent).not.toContain('Review the connection settings') + } + ) + + it('does not classify a provider message containing the permission text as its own notice', async () => { + mocks.detail.mockReturnValue({ + data: { ...connector, lastSyncError: `Provider message: ${SOURCE_PERMISSION_ERROR}` }, + }) + await render() + expect(container.textContent).toContain('Some connection updates are incomplete') + expect(container.textContent).not.toContain('Permission verification incomplete') + }) + it.each(['', '?view=settings', '?view=history'])( 'shows integration deactivation independently of source sync state at %s', async (searchParams) => { diff --git a/apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.tsx b/apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.tsx index e2ad6985e22..ae654870d01 100644 --- a/apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.tsx +++ b/apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.tsx @@ -187,7 +187,8 @@ function SourceDetailContent({ ? describeSearchSource(meta, connector.sourceConfig) || meta.name : 'Connection' const { effectiveStatus, lastSyncError } = getConnectorSyncState(connector) - const permissionsIncomplete = connector.lastSyncError === SOURCE_PERMISSION_ERROR + const permissionsIncomplete = + connector.lastSyncError?.split('\n').includes(SOURCE_PERMISSION_ERROR) ?? false const status = effectiveStatus === 'paused' ? 'Sync paused' diff --git a/apps/sim/lib/knowledge/__integration__/coda-live-fixture.test.ts b/apps/sim/lib/knowledge/__integration__/coda-live-fixture.test.ts new file mode 100644 index 00000000000..0df3c74c15b --- /dev/null +++ b/apps/sim/lib/knowledge/__integration__/coda-live-fixture.test.ts @@ -0,0 +1,27 @@ +/** @vitest-environment node */ +import { describe, expect, it } from 'vitest' +import { assertCodaLiveFixture } from '@/lib/knowledge/__integration__/coda-live-fixture' + +const marker = 'SimConnector-fixture' +const source = { name: `Sim Coda connector verification ${marker}`, owner: 'Owner@example.com' } + +describe('Coda live fixture validation', () => { + it.each(['Owner@example.com', 'owner@example.com', ' OWNER@EXAMPLE.COM '])( + 'rejects the owner as the second identity: %s', + (secondEmail) => { + expect(() => assertCodaLiveFixture(source, marker, secondEmail)).toThrow( + 'Refusing to change sharing' + ) + } + ) + + it('rejects a document outside the disposable fixture', () => { + expect(() => + assertCodaLiveFixture({ ...source, name: 'Unrelated' }, marker, 'reader@example.com') + ).toThrow('Refusing to change sharing') + }) + + it('accepts the disposable document with a distinct second identity', () => { + expect(() => assertCodaLiveFixture(source, marker, 'reader@example.com')).not.toThrow() + }) +}) diff --git a/apps/sim/lib/knowledge/__integration__/coda-live-fixture.ts b/apps/sim/lib/knowledge/__integration__/coda-live-fixture.ts new file mode 100644 index 00000000000..95a99c0bab1 --- /dev/null +++ b/apps/sim/lib/knowledge/__integration__/coda-live-fixture.ts @@ -0,0 +1,15 @@ +import { normalizeEmail } from '@sim/utils/string' + +/** Checks the disposable document and distinct identities before live sharing mutations. */ +export function assertCodaLiveFixture( + source: { name: string; owner: string }, + marker: string, + secondEmail: string +): void { + if ( + source.name !== `Sim Coda connector verification ${marker}` || + normalizeEmail(source.owner) === normalizeEmail(secondEmail) + ) { + throw new Error('Refusing to change sharing on a non-fixture document') + } +} diff --git a/apps/sim/lib/knowledge/__integration__/coda-live.integration.ts b/apps/sim/lib/knowledge/__integration__/coda-live.integration.ts index a83f45d0c3b..7b2674547b9 100644 --- a/apps/sim/lib/knowledge/__integration__/coda-live.integration.ts +++ b/apps/sim/lib/knowledge/__integration__/coda-live.integration.ts @@ -49,6 +49,7 @@ import { } from '@/lib/billing/core/billing-attribution' import { encryptSecret } from '@/lib/core/security/encryption' import { createOrganizationCredential } from '@/lib/credentials/application/organization-credentials' +import { assertCodaLiveFixture } from '@/lib/knowledge/__integration__/coda-live-fixture' import { seedKnowledgeAclFixture } from '@/lib/knowledge/__integration__/seed-source-access-fixture' import { listKnowledgeChunks } from '@/lib/knowledge/application/chunks' import { createKnowledgeConnector } from '@/lib/knowledge/application/connectors' @@ -168,12 +169,7 @@ describe codaDocPath(fixture.docId), z.object({ name: z.string(), owner: z.string().email() }) ) - if ( - source.name !== `Sim Coda connector verification ${fixture.marker}` || - source.owner === secondEmail - ) { - throw new Error('Refusing to change sharing on a non-fixture document') - } + assertCodaLiveFixture(source, fixture.marker, secondEmail!) fixtureValidated = true await revokeShare() await waitForAcl(false) diff --git a/apps/sim/lib/knowledge/__integration__/organization-search-overview.integration.ts b/apps/sim/lib/knowledge/__integration__/organization-search-overview.integration.ts index 1e16c3f52c4..46488912c22 100644 --- a/apps/sim/lib/knowledge/__integration__/organization-search-overview.integration.ts +++ b/apps/sim/lib/knowledge/__integration__/organization-search-overview.integration.ts @@ -23,6 +23,7 @@ import { } from '@/lib/knowledge/__integration__/seed-source-access-fixture' import { readOrganizationSearchOverview } from '@/lib/knowledge/application/organization-search-overview' import { listSearchSources } from '@/lib/knowledge/application/search-sources' +import { SOURCE_PERMISSION_ERROR } from '@/lib/knowledge/connectors/sync-limits' const ids = createKnowledgeAclFixtureIds() const indexId = generateId() @@ -177,6 +178,44 @@ async function provider(connectorType: string) { } describe('organization operational overview with real SQL', () => { + it.each([ + SOURCE_PERMISSION_ERROR, + `Directory refresh incomplete: fixture\n${SOURCE_PERMISSION_ERROR}`, + `${SOURCE_PERMISSION_ERROR}\nSource listing failed for a fixture account`, + `Directory refresh incomplete: fixture\n${SOURCE_PERMISSION_ERROR}\nSource listing failed for a fixture account`, + ])( + 'recognizes a complete permission notice within composed diagnostics: %s', + async (lastSyncError) => { + await db + .update(knowledgeConnector) + .set({ lastSyncError }) + .where(eq(knowledgeConnector.id, driveId)) + expect(await provider('google_drive')).toMatchObject({ + status: 'needs_attention', + issue: 'permission_sync_incomplete', + }) + } + ) + + it('does not classify a provider message containing the permission text as its own notice', async () => { + await db + .update(knowledgeConnector) + .set({ lastSyncError: `Provider message: ${SOURCE_PERMISSION_ERROR}` }) + .where(eq(knowledgeConnector.id, driveId)) + expect(await provider('google_drive')).toMatchObject({ + status: 'needs_attention', + issue: 'sync_failed', + }) + }) + + it('ignores permission notices on paused sources', async () => { + await db + .update(knowledgeConnector) + .set({ lastSyncError: `Directory refresh incomplete: fixture\n${SOURCE_PERMISSION_ERROR}` }) + .where(eq(knowledgeConnector.id, pausedDriveId)) + expect(await provider('google_drive')).toMatchObject({ status: 'active', issue: null }) + }) + it('counts configured sources independently of viewer ACLs, and excludes workspace and untouched providers', async () => { const result = await readOrganizationSearchOverview.execute({ principal, input }) expect(result.providers).toEqual( @@ -188,6 +227,7 @@ describe('organization operational overview with real SQL', () => { status: 'active', issue: null, isSyncing: false, + hasPendingSync: false, }, { connectorType: 'gmail', @@ -196,6 +236,7 @@ describe('organization operational overview with real SQL', () => { status: 'active', issue: null, isSyncing: false, + hasPendingSync: false, }, ]) ) @@ -237,7 +278,7 @@ describe('organization operational overview with real SQL', () => { .where(eq(knowledgeConnector.id, gmailId)) expect(await provider('gmail')).toMatchObject({ status: 'waiting_for_connections' }) }) - it('distinguishes normal member continuation from partial failure without treating idle as success', async () => { + it('distinguishes queued member continuation from active indexing and partial failure', async () => { await db .update(knowledgeConnectorMember) .set({ listingCheckpoint: { cursor: 'fixture' } }) @@ -250,7 +291,11 @@ describe('organization operational overview with real SQL', () => { membersIncomplete: 1, completedAt: new Date(), }) - expect(await provider('gmail')).toMatchObject({ status: 'indexing' }) + expect(await provider('gmail')).toMatchObject({ + status: 'active', + isSyncing: false, + hasPendingSync: true, + }) await db .update(knowledgeConnectorMemberSyncLog) .set({ membersFailed: 1 }) @@ -269,7 +314,11 @@ describe('organization operational overview with real SQL', () => { .update(knowledgeConnector) .set({ nextMemberSyncAt: new Date() }) .where(eq(knowledgeConnector.id, gmailId)) - expect(await provider('gmail')).toMatchObject({ status: 'indexing' }) + expect(await provider('gmail')).toMatchObject({ + status: 'active', + isSyncing: false, + hasPendingSync: true, + }) await db .update(knowledgeConnector) .set({ nextMemberSyncAt: null }) diff --git a/apps/sim/lib/knowledge/application/organization-search-overview.ts b/apps/sim/lib/knowledge/application/organization-search-overview.ts index 890fc5c5cb2..085e0537462 100644 --- a/apps/sim/lib/knowledge/application/organization-search-overview.ts +++ b/apps/sim/lib/knowledge/application/organization-search-overview.ts @@ -213,7 +213,7 @@ export const readOrganizationSearchOverview = defineAuthorizedKnowledgeUseCase({ ))`, hasAccountError: sql`bool_or(NOT ${paused} AND ${knowledgeConnector.accessMode} = 'members' AND ${hasMemberError})`, hasDocumentError: sql`bool_or(NOT ${paused} AND ${hasDocumentsInState(failedDocumentCondition())})`, - hasPermissionError: sql`bool_or(NOT ${paused} AND ${knowledgeConnector.lastSyncError} = ${SOURCE_PERMISSION_ERROR})`, + hasPermissionError: sql`bool_or(NOT ${paused} AND ${SOURCE_PERMISSION_ERROR} = ANY(string_to_array(${knowledgeConnector.lastSyncError}, ${'\n'})))`, hasIndexing: sql`bool_or(NOT ${paused} AND (${knowledgeConnector.accessMode} <> 'members' OR ${hasActiveMembers} OR ${knowledgeConnector.credentialId} IS NOT NULL) AND ( From 99eda3c25d816391d45e6a9eacda883ac7c3f0bd Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 17 Sep 2026 01:06:44 -0700 Subject: [PATCH 2/2] fix(knowledge): make healthy sync fixtures explicit --- .../organization-search-overview.integration.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/knowledge/__integration__/organization-search-overview.integration.ts b/apps/sim/lib/knowledge/__integration__/organization-search-overview.integration.ts index 46488912c22..f9565f1097e 100644 --- a/apps/sim/lib/knowledge/__integration__/organization-search-overview.integration.ts +++ b/apps/sim/lib/knowledge/__integration__/organization-search-overview.integration.ts @@ -15,7 +15,7 @@ import { workspace, } from '@sim/db/schema' import { generateId } from '@sim/utils/id' -import { eq, inArray } from 'drizzle-orm' +import { eq, inArray, sql } from 'drizzle-orm' import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest' import { createKnowledgeAclFixtureIds, @@ -289,6 +289,8 @@ describe('organization operational overview with real SQL', () => { connectorId: gmailId, status: 'partial', membersIncomplete: 1, + docsFailed: 0, + processingDispatchFailed: 0, completedAt: new Date(), }) expect(await provider('gmail')).toMatchObject({ @@ -312,7 +314,7 @@ describe('organization operational overview with real SQL', () => { expect(await provider('gmail')).toMatchObject({ status: 'needs_attention' }) await db .update(knowledgeConnector) - .set({ nextMemberSyncAt: new Date() }) + .set({ nextMemberSyncAt: sql`statement_timestamp() - interval '1 second'` }) .where(eq(knowledgeConnector.id, gmailId)) expect(await provider('gmail')).toMatchObject({ status: 'active', @@ -327,6 +329,8 @@ describe('organization operational overview with real SQL', () => { id: generateId(), connectorId: gmailId, status: 'completed', + docsFailed: 0, + processingDispatchFailed: 0, startedAt: new Date(Date.now() + 1000), completedAt: new Date(), })