fix(search): close authorization races and repair integration fixtures

Author: waleedlatif1

apps/sim/app/api/v1/knowledge/search/route.test.ts

@@ -16,6 +16,7 @@ const {
   mockExecuteKnowledgeSearch,
   mockGenerateSearchEmbedding,
   mockGetDocumentMetadataByIds,
+  mockGetDocumentTagDefinitions,
   mockAuthenticateRequest,
   mockValidateWorkspaceAccess,
   mockResolveBillingAttribution,
@@ -26,6 +27,7 @@ const {
   mockExecuteKnowledgeSearch: vi.fn(),
   mockGenerateSearchEmbedding: vi.fn(),
   mockGetDocumentMetadataByIds: vi.fn(),
+  mockGetDocumentTagDefinitions: vi.fn(),
   mockAuthenticateRequest: vi.fn(),
   mockValidateWorkspaceAccess: vi.fn(),
   mockResolveBillingAttribution: vi.fn(),
@@ -91,7 +93,7 @@ vi.mock('@/app/api/v1/knowledge/utils', () => ({
 }))
 
 vi.mock('@/lib/knowledge/tags/service', () => ({
-  getDocumentTagDefinitions: vi.fn().mockResolvedValue([]),
+  getDocumentTagDefinitions: mockGetDocumentTagDefinitions,
 }))
 
 import { POST } from '@/app/api/v1/knowledge/search/route'
@@ -124,6 +126,7 @@ describe('v1 knowledge search route — per-KB embedding model', () => {
     })
     mockExecuteKnowledgeSearch.mockResolvedValue([])
     mockGetDocumentMetadataByIds.mockResolvedValue({})
+    mockGetDocumentTagDefinitions.mockResolvedValue([])
     mockResolveBillingAttribution.mockImplementation(
       ({ actorUserId, workspaceId }: { actorUserId: string; workspaceId: string }) =>
         Promise.resolve({
@@ -165,6 +168,82 @@ describe('v1 knowledge search route — per-KB embedding model', () => {
     expect(mockGetDocumentMetadataByIds).toHaveBeenCalledWith([], access, provider)
   })
 
+  it.each([
+    ['query', false],
+    ['query', true],
+    ['filters', false],
+    ['filters', true],
+  ] as const)(
+    'omits newly denied content from %s results and counts when all denied is %s',
+    async (mode, allDenied) => {
+      const access = { kind: 'user' as const, userId: 'user-1', tokens: ['reader-token'] }
+      const provider = {
+        get: vi.fn().mockResolvedValue(access),
+        getForConnectors: vi.fn(),
+        getForDocuments: vi.fn(),
+      }
+      mockResolveV1KnowledgeReadAccess.mockResolvedValue(provider)
+      mockCheckKnowledgeBaseAccess.mockResolvedValueOnce({
+        hasAccess: true,
+        knowledgeBase: baseKb('kb-1', 'text-embedding-3-small'),
+      })
+      mockGetDocumentTagDefinitions.mockResolvedValue([
+        { tagSlot: 'tag1', displayName: 'category', fieldType: 'text' },
+      ])
+      mockExecuteKnowledgeSearch.mockResolvedValue([
+        {
+          documentId: 'revoked-document',
+          knowledgeBaseId: 'kb-1',
+          content: 'revoked page content',
+          tag1: 'revoked tag',
+          chunkIndex: 0,
+          distance: 0.1,
+        },
+        {
+          documentId: 'allowed-document',
+          knowledgeBaseId: 'kb-1',
+          content: 'allowed page content',
+          tag1: 'docs',
+          chunkIndex: 0,
+          distance: 0.2,
+        },
+      ])
+      mockGetDocumentMetadataByIds.mockResolvedValue(
+        allDenied ? {} : { 'allowed-document': { filename: 'Allowed page', sourceUrl: null } }
+      )
+      const response = await POST(
+        createMockRequest('POST', {
+          workspaceId: 'ws-1',
+          knowledgeBaseIds: 'kb-1',
+          ...(mode === 'query'
+            ? { query: 'docs' }
+            : { tagFilters: [{ tagName: 'category', operator: 'eq', value: 'docs' }] }),
+        })
+      )
+      const body = await response.json()
+      expect(response.status).toBe(200)
+      expect(mockGetDocumentMetadataByIds).toHaveBeenCalledWith(
+        ['revoked-document', 'allowed-document'],
+        access,
+        provider
+      )
+      expect(body.data.results).toEqual(
+        allDenied
+          ? []
+          : [
+              expect.objectContaining({
+                documentId: 'allowed-document',
+                documentName: 'Allowed page',
+                content: 'allowed page content',
+                metadata: { category: 'docs' },
+              }),
+            ]
+      )
+      expect(body.data.totalResults).toBe(allDenied ? 0 : 1)
+      expect(JSON.stringify(body)).not.toContain('revoked')
+    }
+  )
+
   it('passes the KB embedding model into generateSearchEmbedding', async () => {
     mockCheckKnowledgeBaseAccess.mockResolvedValueOnce({
       hasAccess: true,

apps/sim/app/api/v1/knowledge/search/route.ts

@@ -312,11 +312,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
 
     const documentIds = results.map((r) => r.documentId)
     const documentMetadataMap = await getDocumentMetadataByIds(documentIds, access, accessProvider)
+    const readableResults = results.filter((result) => documentMetadataMap[result.documentId])
 
     return NextResponse.json({
       success: true,
       data: {
-        results: results.map((result) => {
+        results: readableResults.map((result) => {
           const kbTagMap = tagDefinitionsMap[result.knowledgeBaseId] || {}
           const tags: Record<string, string | number | boolean | Date | null> = {}
 
@@ -342,7 +343,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
         query: query || '',
         knowledgeBaseIds: accessibleKbIds,
         topK,
-        totalResults: results.length,
+        totalResults: readableResults.length,
       },
     })
   } catch (error) {

apps/sim/hooks/use-member-enrollment.test.tsx

@@ -211,6 +211,201 @@ describe('useMemberEnrollment', () => {
     )
   })
 
+  it.each([
+    ['existing', 'account_mismatch'],
+    ['existing', 'denied'],
+    ['existing', 'expired'],
+    ['new', 'account_mismatch'],
+    ['new', 'denied'],
+    ['new', 'expired'],
+  ] as const)(
+    'ignores the previous %s source’s %s while its retry request is pending',
+    (source, failure) => {
+      mount(new Set(), true, mocks.connectionError)
+      const mutation = source === 'existing' ? mocks.enrollmentMutate : mocks.sourceConnectionMutate
+      const connect = () => {
+        if (source === 'existing') enrollment().connect('kb-1', 'connector-1')
+        else enrollment().connectSource('workspace-1', 'jira', { projectKey: 'ENG' })
+      }
+      act(connect)
+      act(() =>
+        mutation.mock.calls[0][1].onSuccess({
+          url: 'https://provider.test/previous',
+          connectorId: 'connector-1',
+        })
+      )
+      act(() => vi.advanceTimersByTime(9 * 60_000))
+      act(connect)
+      if (failure !== 'expired') {
+        act(() => mocks.channels[0].onmessage?.(new MessageEvent('message', { data: failure })))
+      }
+      act(() => vi.advanceTimersByTime(60_000))
+      expect(mocks.connectionError).not.toHaveBeenCalled()
+      expect(enrollment().error).toBeNull()
+      act(() =>
+        mutation.mock.calls[1][1].onSuccess({
+          url: 'https://provider.test/retry',
+          connectorId: 'connector-1',
+        })
+      )
+      expect(enrollment().isAwaiting('connector-1')).toBe(true)
+      expect(mocks.channels[1].close).not.toHaveBeenCalled()
+    }
+  )
+
+  it.each([
+    ['existing', 'success'],
+    ['existing', 'failure'],
+    ['new', 'success'],
+    ['new', 'failure'],
+  ] as const)('ignores a superseded %s source request’s late %s', (source, outcome) => {
+    mount(new Set(), true, mocks.connectionError)
+    const mutation = source === 'existing' ? mocks.enrollmentMutate : mocks.sourceConnectionMutate
+    const retryTab = { location: { href: '' }, closed: false, close: vi.fn() }
+    vi.mocked(window.open)
+      .mockReturnValueOnce(enrollmentTab as unknown as Window)
+      .mockReturnValueOnce(retryTab as unknown as Window)
+    for (let index = 0; index < 2; index += 1) {
+      act(() => {
+        if (source === 'existing') enrollment().connect('kb-1', 'connector-1')
+        else enrollment().connectSource('workspace-1', 'jira', { projectKey: 'ENG' })
+      })
+    }
+    act(() =>
+      mutation.mock.calls[1][1].onSuccess({
+        url: 'https://provider.test/retry',
+        connectorId: 'connector-1',
+      })
+    )
+    act(() => {
+      if (outcome === 'failure') {
+        mutation.mock.calls[0][1].onError(new Error('Previous request failed'))
+      } else {
+        mutation.mock.calls[0][1].onSuccess({
+          url: 'https://provider.test/previous',
+          connectorId: 'connector-1',
+        })
+      }
+    })
+    expect(enrollmentTab.location.href).toBe('')
+    expect(retryTab.location.href).toBe('https://provider.test/retry')
+    expect(retryTab.close).not.toHaveBeenCalled()
+    expect(enrollment().isAwaiting('connector-1')).toBe(true)
+    expect(mocks.channels[1].close).not.toHaveBeenCalled()
+    expect(mocks.connectionError).not.toHaveBeenCalled()
+    expect(enrollment().error).toBeNull()
+  })
+
+  it('retires a first-source authorization when retrying its resolved connector', () => {
+    mount(new Set(), true, mocks.connectionError)
+    act(() => enrollment().connectSource('workspace-1', 'jira', { projectKey: 'ENG' }))
+    act(() =>
+      mocks.sourceConnectionMutate.mock.calls[0][1].onSuccess({
+        url: 'https://provider.test/previous',
+        connectorId: 'connector-1',
+      })
+    )
+    act(() => enrollment().connect('kb-1', 'connector-1'))
+    act(() => mocks.channels[0].onmessage?.(new MessageEvent('message', { data: 'denied' })))
+    expect(mocks.channels[0].close).toHaveBeenCalledOnce()
+    expect(mocks.connectionError).not.toHaveBeenCalled()
+  })
+
+  it('does not let a delayed first-source response replace its newer connector authorization', () => {
+    mount(new Set(), true, mocks.connectionError)
+    act(() => enrollment().connectSource('workspace-1', 'jira', { projectKey: 'ENG' }))
+    act(() => enrollment().connect('kb-1', 'connector-1'))
+    act(() =>
+      mocks.enrollmentMutate.mock.calls[0][1].onSuccess({ url: 'https://provider.test/retry' })
+    )
+    act(() =>
+      mocks.sourceConnectionMutate.mock.calls[0][1].onSuccess({
+        url: 'https://provider.test/previous',
+        connectorId: 'connector-1',
+      })
+    )
+    expect(enrollmentTab.location.href).toBe('https://provider.test/retry')
+    expect(mocks.channels[1].close).not.toHaveBeenCalled()
+    expect(enrollment().isAwaiting('connector-1')).toBe(true)
+  })
+
+  it('ignores first-source success while a newer request for its connector is still pending', () => {
+    mount(new Set(), true, mocks.connectionError)
+    const retryTab = { location: { href: '' }, closed: false, close: vi.fn() }
+    vi.mocked(window.open)
+      .mockReturnValueOnce(enrollmentTab as unknown as Window)
+      .mockReturnValueOnce(retryTab as unknown as Window)
+    act(() => enrollment().connectSource('workspace-1', 'jira', { projectKey: 'ENG' }))
+    act(() => enrollment().connect('kb-1', 'connector-1'))
+    act(() =>
+      mocks.sourceConnectionMutate.mock.calls[0][1].onSuccess({
+        url: 'https://provider.test/previous',
+        connectorId: 'connector-1',
+      })
+    )
+    expect(enrollmentTab.location.href).toBe('')
+    expect(enrollment().isAwaiting('connector-1')).toBe(false)
+    act(() => mocks.channels[0].onmessage?.(new MessageEvent('message', { data: 'denied' })))
+    expect(mocks.connectionError).not.toHaveBeenCalled()
+    act(() =>
+      mocks.enrollmentMutate.mock.calls[0][1].onSuccess({ url: 'https://provider.test/retry' })
+    )
+    expect(retryTab.location.href).toBe('https://provider.test/retry')
+    expect(enrollment().isAwaiting('connector-1')).toBe(true)
+    expect(mocks.channels[1].close).not.toHaveBeenCalled()
+  })
+
+  it('retires a pending connector request when a newer first-source request resolves to it', () => {
+    mount(new Set(), true, mocks.connectionError)
+    act(() => enrollment().connect('kb-1', 'connector-1'))
+    act(() => enrollment().connectSource('workspace-1', 'jira', { projectKey: 'ENG' }))
+    act(() =>
+      mocks.sourceConnectionMutate.mock.calls[0][1].onSuccess({
+        url: 'https://provider.test/retry',
+        connectorId: 'connector-1',
+      })
+    )
+    act(() => mocks.enrollmentMutate.mock.calls[0][1].onError(new Error('Previous request failed')))
+    expect(mocks.connectionError).not.toHaveBeenCalled()
+    expect(enrollment().isAwaiting('connector-1')).toBe(true)
+    expect(mocks.channels[0].close).toHaveBeenCalledOnce()
+    expect(mocks.channels[1].close).not.toHaveBeenCalled()
+  })
+
+  it('keeps the previous authorization active when the retry popup is blocked', () => {
+    mount(new Set(), true, mocks.connectionError)
+    act(() => enrollment().connect('kb-1', 'connector-1'))
+    act(() =>
+      mocks.enrollmentMutate.mock.calls[0][1].onSuccess({ url: 'https://provider.test/previous' })
+    )
+    vi.mocked(window.open).mockReturnValueOnce(null)
+    act(() => enrollment().connect('kb-1', 'connector-1'))
+    expect(mocks.channels[0].close).not.toHaveBeenCalled()
+    expect(enrollment().isAwaiting('connector-1')).toBe(true)
+    expect(mocks.enrollmentMutate).toHaveBeenCalledOnce()
+  })
+
+  it('keeps pending source requests with different scopes or configurations independent', () => {
+    mount(new Set(), true, mocks.connectionError)
+    for (const [owner, projectKey] of [
+      ['workspace-1', 'ENG'],
+      ['workspace-1', 'SUPPORT'],
+      ['workspace-2', 'ENG'],
+    ]) {
+      act(() => enrollment().connectSource(owner, 'jira', { projectKey }))
+    }
+    for (let index = 0; index < 3; index += 1) {
+      act(() =>
+        mocks.sourceConnectionMutate.mock.calls[index][1].onSuccess({
+          url: `https://provider.test/attempt-${index}`,
+          connectorId: `connector-${index}`,
+        })
+      )
+      expect(enrollment().isAwaiting(`connector-${index}`)).toBe(true)
+      expect(mocks.channels[index].close).not.toHaveBeenCalled()
+    }
+  })
+
   it('reports a blocked popup once without starting a connection', () => {
     mount(new Set(), true, mocks.connectionError)
     vi.mocked(window.open).mockReturnValueOnce(null)