Skip to content

Commit d57e676

Browse files
Fix connector sync pause race
1 parent c4aaf57 commit d57e676

8 files changed

Lines changed: 137 additions & 12 deletions

File tree

apps/sim/background/knowledge-connector-sync.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ describe('knowledge connector sync worker', () => {
6060
connectorId: 'connector-1',
6161
requestId: 'request-1',
6262
fullSync: true,
63+
requireRunnable: true,
6364
billingAttribution: BILLING_ATTRIBUTION,
6465
})
6566

@@ -72,6 +73,7 @@ describe('knowledge connector sync worker', () => {
7273
expect(mockExecuteSync).toHaveBeenCalledWith('connector-1', {
7374
billingAttribution: BILLING_ATTRIBUTION,
7475
fullSync: true,
76+
requireRunnable: true,
7577
rehydrate: undefined,
7678
})
7779
})
@@ -94,6 +96,7 @@ describe('knowledge connector sync worker', () => {
9496
expect(mockExecuteSync).toHaveBeenCalledWith('connector-1', {
9597
billingAttribution: BILLING_ATTRIBUTION,
9698
fullSync: undefined,
99+
requireRunnable: undefined,
97100
rehydrate: true,
98101
})
99102
})

apps/sim/background/knowledge-connector-sync.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,18 @@ import { CONNECTOR_SYNC_MAX_DURATION_SECONDS } from '@/lib/knowledge/connectors/
1010
const logger = createLogger('TriggerKnowledgeConnectorSync')
1111

1212
export async function executeConnectorSyncJob(payload: unknown) {
13-
const { connectorId, fullSync, rehydrate, requestId, billingAttribution } =
13+
const { connectorId, fullSync, requireRunnable, rehydrate, requestId, billingAttribution } =
1414
assertConnectorSyncPayload(payload)
1515

1616
logger.info(`[${requestId}] Starting connector sync: ${connectorId}`)
1717

1818
try {
19-
const result = await executeSync(connectorId, { billingAttribution, fullSync, rehydrate })
19+
const result = await executeSync(connectorId, {
20+
billingAttribution,
21+
fullSync,
22+
requireRunnable,
23+
rehydrate,
24+
})
2025

2126
logger.info(`[${requestId}] Connector sync completed`, {
2227
connectorId,

apps/sim/lib/knowledge/connectors/queue.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ vi.mock('@/lib/knowledge/documents/service', () => ({
2121
}))
2222
vi.mock('@/lib/knowledge/connectors/sync-engine', () => ({
2323
executeSync: mockExecuteSync,
24+
isConnectorRunnableStatus: (status: string) => status === 'active' || status === 'error',
2425
}))
2526

2627
import { assertConnectorSyncPayload, dispatchSync } from '@/lib/knowledge/connectors/queue'
@@ -53,6 +54,7 @@ describe('connector sync queue', () => {
5354
queueTableRows(schemaMock.knowledgeConnector, [
5455
{
5556
knowledgeBaseId: 'knowledge-base-1',
57+
connectorStatus: 'active',
5658
connectorArchivedAt: null,
5759
connectorDeletedAt: null,
5860
workspaceId: 'workspace-paid',
@@ -80,6 +82,7 @@ describe('connector sync queue', () => {
8082
{
8183
connectorId: 'connector-1',
8284
fullSync: true,
85+
requireRunnable: undefined,
8386
rehydrate: undefined,
8487
requestId: 'request-1',
8588
billingAttribution: BILLING_ATTRIBUTION,
@@ -110,6 +113,43 @@ describe('connector sync queue', () => {
110113
)
111114
})
112115

116+
it('carries the runnable requirement into the queued payload', async () => {
117+
await dispatchSync('connector-1', {
118+
billingAttribution: BILLING_ATTRIBUTION,
119+
requireRunnable: true,
120+
requestId: 'request-1',
121+
})
122+
123+
expect(mockTrigger).toHaveBeenCalledWith(
124+
'knowledge-connector-sync',
125+
expect.objectContaining({ connectorId: 'connector-1', requireRunnable: true }),
126+
expect.anything()
127+
)
128+
})
129+
130+
it('skips automatic dispatch when the connector was paused concurrently', async () => {
131+
resetDbChainMock()
132+
queueTableRows(schemaMock.knowledgeConnector, [
133+
{
134+
knowledgeBaseId: 'knowledge-base-1',
135+
connectorStatus: 'paused',
136+
connectorArchivedAt: null,
137+
connectorDeletedAt: null,
138+
workspaceId: 'workspace-paid',
139+
kbDeletedAt: null,
140+
},
141+
])
142+
143+
await dispatchSync('connector-1', {
144+
billingAttribution: BILLING_ATTRIBUTION,
145+
requireRunnable: true,
146+
requestId: 'request-1',
147+
})
148+
149+
expect(mockTrigger).not.toHaveBeenCalled()
150+
expect(mockExecuteSync).not.toHaveBeenCalled()
151+
})
152+
113153
it('rejects legacy payloads without billing attribution', () => {
114154
expect(() =>
115155
assertConnectorSyncPayload({

apps/sim/lib/knowledge/connectors/queue.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,16 @@ import {
1111
type BillingAttributionSnapshot,
1212
} from '@/lib/billing/core/billing-attribution'
1313
import { resolveTriggerRegion } from '@/lib/core/async-jobs/region'
14-
import { executeSync } from '@/lib/knowledge/connectors/sync-engine'
14+
import { executeSync, isConnectorRunnableStatus } from '@/lib/knowledge/connectors/sync-engine'
1515
import { isTriggerAvailable } from '@/lib/knowledge/documents/service'
1616

1717
const logger = createLogger('ConnectorSyncQueue')
1818

1919
export interface ConnectorSyncPayload {
2020
connectorId: string
2121
fullSync?: boolean
22+
/** Skip automatic work if the connector is paused or disabled before execution starts. */
23+
requireRunnable?: boolean
2224
/**
2325
* Force re-hydration + re-indexing of already-synced documents for connectors
2426
* whose rendered content can drift without a hash change (see
@@ -34,6 +36,7 @@ export interface ConnectorSyncPayload {
3436
export interface DispatchSyncOptions {
3537
billingAttribution: BillingAttributionSnapshot
3638
fullSync?: boolean
39+
requireRunnable?: boolean
3740
rehydrate?: boolean
3841
requestId?: string
3942
}
@@ -55,6 +58,9 @@ export function assertConnectorSyncPayload(value: unknown): ConnectorSyncPayload
5558
if (value.fullSync !== undefined && typeof value.fullSync !== 'boolean') {
5659
throw new Error('Connector sync payload fullSync must be a boolean when provided')
5760
}
61+
if (value.requireRunnable !== undefined && typeof value.requireRunnable !== 'boolean') {
62+
throw new Error('Connector sync payload requireRunnable must be a boolean when provided')
63+
}
5864
if (value.rehydrate !== undefined && typeof value.rehydrate !== 'boolean') {
5965
throw new Error('Connector sync payload rehydrate must be a boolean when provided')
6066
}
@@ -65,6 +71,7 @@ export function assertConnectorSyncPayload(value: unknown): ConnectorSyncPayload
6571
return {
6672
connectorId: value.connectorId,
6773
fullSync: value.fullSync as boolean | undefined,
74+
requireRunnable: value.requireRunnable as boolean | undefined,
6875
rehydrate: value.rehydrate as boolean | undefined,
6976
requestId: value.requestId,
7077
billingAttribution: assertBillingAttributionSnapshot(value.billingAttribution),
@@ -87,6 +94,7 @@ export async function dispatchSync(
8794
const payload = assertConnectorSyncPayload({
8895
connectorId,
8996
fullSync: options?.fullSync,
97+
requireRunnable: options?.requireRunnable,
9098
rehydrate: options?.rehydrate,
9199
requestId,
92100
billingAttribution: options?.billingAttribution,
@@ -95,6 +103,7 @@ export async function dispatchSync(
95103
const connectorRows = await db
96104
.select({
97105
knowledgeBaseId: knowledgeConnector.knowledgeBaseId,
106+
connectorStatus: knowledgeConnector.status,
98107
connectorArchivedAt: knowledgeConnector.archivedAt,
99108
connectorDeletedAt: knowledgeConnector.deletedAt,
100109
workspaceId: knowledgeBase.workspaceId,
@@ -134,6 +143,14 @@ export async function dispatchSync(
134143
})
135144
return
136145
}
146+
if (payload.requireRunnable && !isConnectorRunnableStatus(row.connectorStatus)) {
147+
logger.info('Skipping automatic sync dispatch: connector is not runnable', {
148+
connectorId,
149+
status: row.connectorStatus,
150+
requestId,
151+
})
152+
return
153+
}
137154
if (!row.workspaceId) {
138155
throw new Error(`Connector ${connectorId} is missing workspace billing context`)
139156
}
@@ -161,6 +178,7 @@ export async function dispatchSync(
161178

162179
executeSync(connectorId, {
163180
fullSync: payload.fullSync,
181+
requireRunnable: payload.requireRunnable,
164182
rehydrate: payload.rehydrate,
165183
billingAttribution: payload.billingAttribution,
166184
}).catch((error) => {

apps/sim/lib/knowledge/connectors/sync-engine.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
77
import {
88
classifySuspectListing,
99
evaluateListingSafety,
10+
isConnectorRunnableStatus,
1011
isStuckDocumentSweepEligible,
1112
mergeHydratedDocument,
1213
type PreviousListingObservation,
@@ -44,6 +45,16 @@ vi.mock('@/connectors/registry.server', () => ({
4445
},
4546
}))
4647

48+
describe('isConnectorRunnableStatus', () => {
49+
it.each(['active', 'error'])('allows automatic sync from %s', (status) => {
50+
expect(isConnectorRunnableStatus(status)).toBe(true)
51+
})
52+
53+
it.each(['paused', 'disabled', 'syncing'])('blocks automatic sync from %s', (status) => {
54+
expect(isConnectorRunnableStatus(status)).toBe(false)
55+
})
56+
})
57+
4758
describe('shouldReconcileDeletions', () => {
4859
it('runs on a clean full listing', async () => {
4960
const { shouldReconcileDeletions } = await import('@/lib/knowledge/connectors/sync-engine')

apps/sim/lib/knowledge/connectors/sync-engine.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,12 @@ const QUEUED_DISPATCH_GRACE_MINUTES = Math.ceil(
9191
)
9292
const RETRY_WINDOW_DAYS = 7
9393
const MAX_CONSECUTIVE_FAILURES = 10
94+
const RUNNABLE_CONNECTOR_STATUSES = ['active', 'error'] as const
95+
96+
/** Whether an automatic connector sync may begin from this persisted state. */
97+
export function isConnectorRunnableStatus(status: string): boolean {
98+
return RUNNABLE_CONNECTOR_STATUSES.some((runnableStatus) => runnableStatus === status)
99+
}
94100

95101
/** The processing state the stuck-document sweep decides on, one row at a time. */
96102
export interface StuckDocumentSweepCandidate {
@@ -777,6 +783,7 @@ export async function executeSync(
777783
options: {
778784
billingAttribution: BillingAttributionSnapshot
779785
fullSync?: boolean
786+
requireRunnable?: boolean
780787
rehydrate?: boolean
781788
}
782789
): Promise<SyncResult> {
@@ -808,6 +815,14 @@ export async function executeSync(
808815

809816
const connector = connectorRows[0]
810817

818+
if (options.requireRunnable && !isConnectorRunnableStatus(connector.status)) {
819+
logger.info('Skipping automatic sync: connector is not runnable', {
820+
connectorId,
821+
status: connector.status,
822+
})
823+
return result
824+
}
825+
811826
const connectorConfig = CONNECTOR_REGISTRY[connector.connectorType]
812827
if (!connectorConfig) {
813828
throw new Error(`Unknown connector type: ${connector.connectorType}`)
@@ -857,7 +872,9 @@ export async function executeSync(
857872
.where(
858873
and(
859874
eq(knowledgeConnector.id, connectorId),
860-
ne(knowledgeConnector.status, 'syncing'),
875+
options.requireRunnable
876+
? inArray(knowledgeConnector.status, RUNNABLE_CONNECTOR_STATUSES)
877+
: ne(knowledgeConnector.status, 'syncing'),
861878
isNull(knowledgeConnector.archivedAt),
862879
isNull(knowledgeConnector.deletedAt)
863880
)

apps/sim/lib/knowledge/orchestration/connectors.test.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ describe('performUpdateKnowledgeConnector', () => {
274274
expect(mockDispatchSync).toHaveBeenCalledWith('conn-1', {
275275
billingAttribution: BILLING,
276276
requestId: 'req-1',
277+
requireRunnable: true,
277278
})
278279
})
279280

@@ -361,10 +362,10 @@ describe('performUpdateKnowledgeConnector', () => {
361362
expect(mockDispatchSync).not.toHaveBeenCalled()
362363
})
363364

364-
it('rejects a source replacement that races with synchronization', async () => {
365+
it('rejects a source replacement that races with a pause', async () => {
365366
dbChainMockFns.limit
366367
.mockResolvedValueOnce([{ id: 'conn-1', connectorType: 'notion', status: 'active' }])
367-
.mockResolvedValueOnce([{ id: 'conn-1', connectorType: 'notion', status: 'syncing' }])
368+
.mockResolvedValueOnce([{ id: 'conn-1', connectorType: 'notion', status: 'paused' }])
368369
dbChainMockFns.returning.mockResolvedValueOnce([])
369370

370371
const outcome = await performUpdateKnowledgeConnector({
@@ -379,6 +380,24 @@ describe('performUpdateKnowledgeConnector', () => {
379380
expect(outcome).toMatchObject({ success: false, errorCode: 'conflict' })
380381
expect(mockDispatchSync).not.toHaveBeenCalled()
381382
})
383+
384+
it('rejects a pause that races with synchronization startup', async () => {
385+
dbChainMockFns.limit
386+
.mockResolvedValueOnce([{ id: 'conn-1', connectorType: 'notion', status: 'active' }])
387+
.mockResolvedValueOnce([{ id: 'conn-1', connectorType: 'notion', status: 'syncing' }])
388+
dbChainMockFns.returning.mockResolvedValueOnce([])
389+
390+
const outcome = await performUpdateKnowledgeConnector({
391+
...ACTOR,
392+
knowledgeBase: KB,
393+
connectorId: 'conn-1',
394+
updates: { status: 'paused' },
395+
resolveBillingAttribution,
396+
})
397+
398+
expect(outcome).toMatchObject({ success: false, errorCode: 'conflict' })
399+
expect(mockDispatchSync).not.toHaveBeenCalled()
400+
})
382401
})
383402

384403
describe('performSyncKnowledgeConnector', () => {

apps/sim/lib/knowledge/orchestration/connectors.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
} from '@sim/db/schema'
1010
import { createLogger } from '@sim/logger'
1111
import { generateId } from '@sim/utils/id'
12-
import { and, eq, inArray, isNull, ne, sql } from 'drizzle-orm'
12+
import { and, eq, inArray, isNull, sql } from 'drizzle-orm'
1313
import { encryptApiKey } from '@/lib/api-key/crypto'
1414
import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution'
1515
import { hasWorkspaceLiveSyncAccess } from '@/lib/billing/core/subscription'
@@ -443,6 +443,9 @@ export async function performUpdateKnowledgeConnector(
443443
'conflict'
444444
)
445445
}
446+
if (updates.status !== undefined && existing.status === 'syncing') {
447+
return fail('Cannot change connector status while synchronization is in progress', 'conflict')
448+
}
446449

447450
if (updates.syncIntervalMinutes !== undefined) {
448451
if (!kb.workspaceId && updates.syncIntervalMinutes > 0 && updates.syncIntervalMinutes < 60) {
@@ -512,8 +515,8 @@ export async function performUpdateKnowledgeConnector(
512515
isNull(knowledgeConnector.archivedAt),
513516
isNull(knowledgeConnector.deletedAt),
514517
]
515-
if (updates.sourceConfig !== undefined) {
516-
updateConditions.push(ne(knowledgeConnector.status, 'syncing'))
518+
if (updates.sourceConfig !== undefined || updates.status !== undefined) {
519+
updateConditions.push(eq(knowledgeConnector.status, existing.status))
517520
}
518521

519522
const [row] = await db
@@ -523,14 +526,19 @@ export async function performUpdateKnowledgeConnector(
523526
.returning()
524527

525528
if (!row) {
526-
if (updates.sourceConfig !== undefined) {
529+
if (updates.sourceConfig !== undefined || updates.status !== undefined) {
527530
const current = await getKnowledgeConnector(kb.id, connectorId)
528531
if (current?.status === 'syncing') {
529532
return fail(
530-
'Cannot update source configuration while connector synchronization is in progress',
533+
updates.sourceConfig !== undefined
534+
? 'Cannot update source configuration while connector synchronization is in progress'
535+
: 'Cannot change connector status while synchronization is in progress',
531536
'conflict'
532537
)
533538
}
539+
if (current) {
540+
return fail('Connector status changed during the update; retry the request', 'conflict')
541+
}
534542
}
535543
return fail('Connector not found', 'not_found')
536544
}
@@ -564,7 +572,11 @@ export async function performUpdateKnowledgeConnector(
564572
}
565573

566574
if (dispatchSourceSync && billingAttribution) {
567-
dispatchSourceSync(connectorId, { billingAttribution, requestId }).catch((error) => {
575+
dispatchSourceSync(connectorId, {
576+
billingAttribution,
577+
requestId,
578+
requireRunnable: true,
579+
}).catch((error) => {
568580
logger.error(
569581
`[${requestId}] Failed to dispatch source-change sync for connector ${connectorId}`,
570582
error

0 commit comments

Comments
 (0)