Skip to content

Commit 573b5e2

Browse files
committed
fix(search): retain sync failures and resume reconnected accounts
1 parent 5dbb1c2 commit 573b5e2

14 files changed

Lines changed: 233 additions & 14 deletions

File tree

‎apps/sim/app/credential-groups/complete/completion-handoff.test.tsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ afterEach(() => {
1010
})
1111

1212
describe('credential group OAuth completion', () => {
13-
it.each([undefined, 'denied', 'configuration_changed'] as const)(
14-
'publishes %s to only its initiating tab and closes',
13+
it.each([undefined, 'failed', 'denied', 'configuration_changed'] as const)(
14+
'publishes %s to only its initiating tab and keeps failures visible',
1515
(failure) => {
1616
const postMessage = vi.fn()
1717
const closeChannel = vi.fn()
@@ -40,7 +40,8 @@ describe('credential group OAuth completion', () => {
4040
expect(names).toEqual([`sim:credential-group-oauth:${completionId}`])
4141
expect(postMessage).toHaveBeenCalledExactlyOnceWith(failure ?? 'connected')
4242
expect(closeChannel).toHaveBeenCalledOnce()
43-
expect(closeWindow).toHaveBeenCalledOnce()
43+
if (failure) expect(closeWindow).not.toHaveBeenCalled()
44+
else expect(closeWindow).toHaveBeenCalledOnce()
4445
} finally {
4546
act(() => root.unmount())
4647
}

‎apps/sim/app/credential-groups/complete/completion-handoff.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ export function CredentialGroupCompletionHandoff({
2020
const channel = new BroadcastChannel(credentialGroupOAuthCompletionChannel(completionId))
2121
channel.postMessage(failure ?? 'connected')
2222
channel.close()
23-
window.close()
23+
/** Keep the authorization failure visible while the initiating chat shows its retry action. */
24+
if (!failure) window.close()
2425
}, [completionId, failure])
2526
return null
2627
}

‎apps/sim/hooks/use-search-integration-connection.test.tsx‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,37 @@ describe('Search connection card lifecycle', () => {
211211
expect(m.mutate).toHaveBeenCalledTimes(2)
212212
expect(connection().pending).toBe(true)
213213
})
214+
it('surfaces callback failures and retries with a fresh completion receipt', async () => {
215+
render()
216+
await act(async () => {
217+
await connection().connect()
218+
})
219+
const firstId = m.mutate.mock.calls[0][0].oauthCompletionId
220+
const failureChannel = m.channels.find(
221+
(channel) => channel.name === `sim:credential-group-oauth:${firstId}`
222+
)
223+
act(() => failureChannel?.onmessage?.(new MessageEvent('message', { data: 'failed' })))
224+
expect(connection().pending).toBe(false)
225+
expect(connection().connected).toBe(false)
226+
expect(connection().error).toContain('Account authorization did not complete')
227+
expect(windows[0].close).not.toHaveBeenCalled()
228+
229+
await act(async () => {
230+
await connection().connect()
231+
})
232+
const secondId = m.mutate.mock.calls[1][0].oauthCompletionId
233+
expect(secondId).not.toBe(firstId)
234+
expect(connection().pending).toBe(true)
235+
expect(connection().error).toBeNull()
236+
m.accounts = [{ credentialId: 'mine', status: 'connected' }]
237+
m.receipts.set(firstId, 'mine')
238+
render()
239+
expect(connection().connected).toBe(false)
240+
m.receipts.set(secondId, 'mine')
241+
render()
242+
expect(connection().connected).toBe(true)
243+
expect(windows[1].close).toHaveBeenCalled()
244+
})
214245
it('retries the exact source created by the first attempt after cancellation or reload', async () => {
215246
m.requestedTarget = { type: 'link', provider: 'gmail', connectorType: 'gmail' }
216247
render()

‎apps/sim/lib/credential-groups/oauth.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ describe('credential group OAuth persistence', () => {
278278
expect(dispatchMemberSyncsForCredentialOption).toHaveBeenCalledWith({
279279
organizationId: 'org-1',
280280
credentialGroupOptionId: 'option-1',
281+
connectedCredentialId: 'credential-1',
281282
})
282283
expect(eq).toHaveBeenCalledWith(
283284
schemaMock.credentialGroupEnrollment.invitationTokenHash,

‎apps/sim/lib/credential-groups/oauth.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,7 @@ async function persistGrant(
359359
await dispatchMemberSyncsForCredentialOption({
360360
...resourceScopeFields(resourceScopeFromOwner(context)),
361361
credentialGroupOptionId: context.option.id,
362+
connectedCredentialId: completion.credentialId,
362363
})
363364
} catch (error) {
364365
logger.warn('Failed to queue member syncs after an account connected', {

‎apps/sim/lib/credentials/managed-oauth.test.ts‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,38 @@ describe('managed OAuth token resolution', () => {
132132
expect(mocks.decryptSecret).toHaveBeenCalledWith('encrypted-token-set')
133133
})
134134

135+
it('keeps an active GitHub grant connected when the worker lacks its OAuth app configuration', async () => {
136+
setEnv({ GITHUB_APP_CLIENT_ID: 'fixture-client', GITHUB_APP_CLIENT_SECRET: 'fixture-secret' })
137+
const adapter = createStandardOAuthCredentialGroupProviderAdapter('github-repositories')
138+
const policy = await adapter.getPolicy(undefined, { workspaceId: 'workspace-1' })
139+
mocks.getAdapter.mockReturnValue(adapter)
140+
const row = {
141+
...mondayCredentialRow(),
142+
providerId: policy.providerId,
143+
authorizationAppId: policy.authorizationAppId,
144+
managedOauthScopeVersion: policy.scopeVersion,
145+
grantedScopes: [],
146+
accessTokenExpiresAt: new Date('2026-09-01T13:00:00Z'),
147+
}
148+
dbChainMockFns.limit.mockResolvedValue([row])
149+
const params = {
150+
...mondayTokenResolutionParams(),
151+
expectedProviderId: policy.providerId,
152+
requiredScopes: [],
153+
}
154+
setEnv({ GITHUB_APP_CLIENT_ID: undefined, GITHUB_APP_CLIENT_SECRET: undefined })
155+
await expect(resolveManagedOAuthToken(params)).rejects.toMatchObject({
156+
code: 'MANAGED_CREDENTIAL_CONFIGURATION_UNAVAILABLE',
157+
statusCode: 503,
158+
})
159+
expect(dbChainMockFns.set).not.toHaveBeenCalled()
160+
expect(mocks.decryptSecret).not.toHaveBeenCalled()
161+
162+
setEnv({ GITHUB_APP_CLIENT_ID: 'fixture-client', GITHUB_APP_CLIENT_SECRET: 'fixture-secret' })
163+
await expect(resolveManagedOAuthToken(params)).resolves.toMatchObject({ refreshed: false })
164+
expect(dbChainMockFns.set).not.toHaveBeenCalled()
165+
})
166+
135167
it.each([
136168
{ grant: 'drive', request: 'drive.readonly', allowed: true },
137169
{ grant: 'drive.readonly', request: 'drive.readonly', allowed: true },
@@ -279,7 +311,10 @@ describe('managed OAuth token resolution', () => {
279311
expectedProviderId: 'slack',
280312
requiredScopes: [...SLACK_SEARCH_USER_SCOPES],
281313
})
282-
).rejects.toMatchObject({ code: 'MANAGED_CREDENTIAL_NEEDS_REAUTH' })
314+
).rejects.toMatchObject({
315+
code: 'MANAGED_CREDENTIAL_CONFIGURATION_UNAVAILABLE',
316+
statusCode: 503,
317+
})
283318
expect(mocks.decryptSecret).not.toHaveBeenCalled()
284319
})
285320

‎apps/sim/lib/credentials/managed-oauth.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export type ManagedOAuthCredentialErrorCode =
3838
| 'MANAGED_CREDENTIAL_NEEDS_REAUTH'
3939
| 'MANAGED_CREDENTIAL_INSUFFICIENT_SCOPE'
4040
| 'MANAGED_CREDENTIAL_INVALID_TOKEN_SET'
41+
| 'MANAGED_CREDENTIAL_CONFIGURATION_UNAVAILABLE'
4142
| 'MANAGED_CREDENTIAL_REFRESH_FAILED'
4243

4344
export class ManagedOAuthCredentialError extends Error {
@@ -258,9 +259,9 @@ async function assertManagedCredentialUsable(
258259
} catch (error) {
259260
if (!(error instanceof CredentialGroupProviderConfigurationError)) throw error
260261
throw new ManagedOAuthCredentialError(
261-
'MANAGED_CREDENTIAL_NEEDS_REAUTH',
262+
'MANAGED_CREDENTIAL_CONFIGURATION_UNAVAILABLE',
262263
'Managed credential authorization app is unavailable',
263-
401
264+
503
264265
)
265266
}
266267
if (

‎apps/sim/lib/knowledge/application/search-source-progress.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { requirePrincipalSubjectUserId } from '@sim/auth/principal'
12
import { db } from '@sim/db'
23
import { document, knowledgeBase, knowledgeConnector } from '@sim/db/schema'
34
import { and, eq, exists, inArray, isNull, type SQL, sql } from 'drizzle-orm'
@@ -9,6 +10,7 @@ import { createKnowledgeAccessProvider } from '@/lib/knowledge/access/scope'
910
import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/authorized-knowledge-use-case'
1011
import { resolveKnowledgeOwnerContext } from '@/lib/knowledge/application/contexts'
1112
import { knowledgeOperations } from '@/lib/knowledge/application/operations'
13+
import { hasViewerMemberSyncError } from '@/lib/knowledge/connectors/viewer-member-sync-error'
1214
import { MAX_SEARCH_SOURCE_PROGRESS_ITEMS } from '@/lib/knowledge/constants'
1315
import { failedDocumentCondition } from '@/lib/knowledge/documents/processing-status'
1416
import { searchIntegrationAccessCondition } from '@/lib/knowledge/search/integration-policy'
@@ -60,6 +62,9 @@ export const readSearchSourceProgress = defineAuthorizedKnowledgeUseCase({
6062
accessMode: knowledgeConnector.accessMode,
6163
memberSyncStatus: knowledgeConnector.memberSyncStatus,
6264
hasRetainedSyncError: sql<boolean>`${knowledgeConnector.lastSyncError} IS NOT NULL`,
65+
hasViewerMemberSyncError: hasViewerMemberSyncError(
66+
requirePrincipalSubjectUserId(principal)
67+
),
6368
approved: sql<boolean>`${searchIntegrationAccessCondition()}`,
6469
isIndexing: hasDocumentsInState(
6570
inArray(document.processingStatus, ['pending', 'processing'])
@@ -96,6 +101,7 @@ export const readSearchSourceProgress = defineAuthorizedKnowledgeUseCase({
96101
hasSyncError:
97102
row.status === 'error' ||
98103
row.hasRetainedSyncError === true ||
104+
row.hasViewerMemberSyncError === true ||
99105
(row.accessMode === 'members' && row.memberSyncStatus === 'error'),
100106
hasIndexingError: row.hasIndexingError,
101107
})),

‎apps/sim/lib/knowledge/application/search-sources.test.ts‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
/** @vitest-environment node */
22
import {
3+
credentialGroupEnrollment,
34
document,
45
embedding,
56
knowledgeBase,
67
knowledgeConnector,
8+
knowledgeConnectorMember,
79
member,
810
organizationSearchIntegration,
911
user,
@@ -96,6 +98,7 @@ function source(id: string, connectorType = 'google_drive', accessMode = 'admin'
9698
memberSyncStatus: 'idle',
9799
lastSyncAt: LAST_SYNC as Date | null,
98100
hasRetainedSyncError: false,
101+
hasViewerMemberSyncError: false,
99102
lastMemberSyncAt: null as Date | null,
100103
credentialGroupId: 'group-secret',
101104
credentialGroupOptionId: 'option-secret',
@@ -129,6 +132,26 @@ beforeEach(() => {
129132
})
130133

131134
describe('Search source summaries', () => {
135+
it('retains the viewer account failure after an otherwise successful empty run', async () => {
136+
seed([{ ...source('github', 'google_drive', 'members'), hasViewerMemberSyncError: true }])
137+
const result = await listSearchSources.execute({ principal, input })
138+
expect(result.sources[0]).toMatchObject({ hasSyncError: true, isSyncing: false })
139+
expect(dbChainMockFns.where).toHaveBeenCalledWith(
140+
expect.objectContaining({
141+
type: 'and',
142+
conditions: expect.arrayContaining([
143+
{ type: 'eq', left: credentialGroupEnrollment.userId, right: principal.userId },
144+
{
145+
type: 'eq',
146+
left: knowledgeConnectorMember.connectorId,
147+
right: knowledgeConnector.id,
148+
},
149+
{ type: 'isNotNull', column: knowledgeConnectorMember.lastError },
150+
]),
151+
})
152+
)
153+
})
154+
132155
it.each(['read', 'write', 'admin'])(
133156
'allows a current workspace %s without exposing credentials or other members',
134157
async (role) => {
@@ -297,7 +320,7 @@ describe('Search source summaries', () => {
297320
sources: [],
298321
nextCursor: null,
299322
})
300-
expect(dbChainMockFns.where.mock.calls[0][0]).toEqual({
323+
expect(dbChainMockFns.where).toHaveBeenCalledWith({
301324
type: 'and',
302325
conditions: expect.arrayContaining([
303326
{
@@ -493,6 +516,26 @@ describe('organization Search source summaries', () => {
493516
})
494517

495518
describe('bounded Search progress', () => {
519+
it('keeps reporting an unresolved viewer account failure between retries', async () => {
520+
queueTableRows(knowledgeConnector, [
521+
{
522+
connectorId: 'github',
523+
approved: true,
524+
status: 'active',
525+
accessMode: 'members',
526+
memberSyncStatus: 'idle',
527+
hasViewerMemberSyncError: true,
528+
isIndexing: false,
529+
hasIndexingError: false,
530+
},
531+
])
532+
const result = await readSearchSourceProgress.execute({
533+
principal,
534+
input: { ...input, connectorIds: ['github'] },
535+
})
536+
expect(result.sources[0]).toMatchObject({ hasSyncError: true, isSyncing: false })
537+
})
538+
496539
it('reports visible pending and failed work without counting documents or chunks', async () => {
497540
queueTableRows(knowledgeConnector, [
498541
{

‎apps/sim/lib/knowledge/application/search-sources.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/au
1717
import { resolveKnowledgeOwnerContext } from '@/lib/knowledge/application/contexts'
1818
import { knowledgeOperations } from '@/lib/knowledge/application/operations'
1919
import { resolveViewerConnectorMemberships } from '@/lib/knowledge/connectors/member-provisioning'
20+
import { hasViewerMemberSyncError } from '@/lib/knowledge/connectors/viewer-member-sync-error'
2021
import { resolveViewerSourceAccounts } from '@/lib/knowledge/connectors/viewer-source-accounts'
2122
import {
2223
SEARCH_SOURCE_CANDIDATE_PAGE_SIZE,
@@ -84,6 +85,7 @@ export const listSearchSources = defineAuthorizedKnowledgeUseCase({
8485
memberSyncStatus: knowledgeConnector.memberSyncStatus,
8586
lastSyncAt: knowledgeConnector.lastSyncAt,
8687
hasRetainedSyncError: sql<boolean>`${knowledgeConnector.lastSyncError} IS NOT NULL`,
88+
hasViewerMemberSyncError: hasViewerMemberSyncError(userId),
8789
lastMemberSyncAt: knowledgeConnector.lastMemberSyncAt,
8890
credentialGroupId: knowledgeConnector.credentialGroupId,
8991
credentialGroupOptionId: knowledgeConnector.credentialGroupOptionId,
@@ -274,6 +276,7 @@ export const listSearchSources = defineAuthorizedKnowledgeUseCase({
274276
hasSyncError:
275277
row.status === 'error' ||
276278
row.hasRetainedSyncError === true ||
279+
row.hasViewerMemberSyncError === true ||
277280
(row.accessMode === 'members' && row.memberSyncStatus === 'error'),
278281
hasViewerDocuments: available && state?.hasDocuments === true,
279282
viewerFailedDocumentCount: available ? (state?.failedCount ?? 0) : 0,

0 commit comments

Comments
 (0)