Skip to content

Commit de993af

Browse files
committed
feat(knowledge): refresh mirrored directories on their own clock
Group membership decides who can read an already-indexed document, so it has to move independently of the corpus: someone leaving a group should lose access in minutes, not on whatever schedule their documents happen to be re-crawled on. Until now the only thing that refreshed a directory was the admin crawl itself, which made the five-minute interval a ceiling rather than a cadence — on a connector syncing daily, a revoked membership stood for a day, bounded only by the staleness ratchet. A scheduler now offers every admin-mode connector each tick and lets `syncExternalDirectoryGroups` decide whether its directory is actually due. The crawl keeps its own refresh, which is a floor rather than a duplicate: it is what guarantees a crawl never publishes grants against membership nobody has read. Connectors sharing a directory cost one refresh between them — the first brings it up to date and the rest skip on the interval gate. Two ticks overlapping on one directory would both enumerate and write the same rows, which is wasteful and never wrong, so it takes no lease to prevent; every write on this path is idempotent, and a lease would be new state to keep correct for no behavioural gain. Failure is contained per connector. One workspace whose credential lapsed must not stop the tick refreshing every other workspace's directory, and a test covers exactly that. The refresh moves out of the sync engine so both callers share it, and the Confluence connector drops a concurrency helper it should never have had — `mapWithConcurrency` already existed in `lib/core/utils`.
1 parent 972422f commit de993af

6 files changed

Lines changed: 326 additions & 75 deletions

File tree

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { createMockRequest } from '@sim/testing'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const {
8+
mockVerifyCronAuth,
9+
mockConnectorRows,
10+
mockResolveIdentity,
11+
mockResolveToken,
12+
mockRefreshDirectory,
13+
} = vi.hoisted(() => ({
14+
mockVerifyCronAuth: vi.fn(() => null),
15+
mockConnectorRows: vi.fn(),
16+
mockResolveIdentity: vi.fn(),
17+
mockResolveToken: vi.fn(),
18+
mockRefreshDirectory: vi.fn(),
19+
}))
20+
21+
vi.mock('@/lib/auth/internal', () => ({ verifyCronAuth: mockVerifyCronAuth }))
22+
vi.mock('@/lib/credentials/access', () => ({
23+
resolveCredentialTokenIdentity: mockResolveIdentity,
24+
}))
25+
vi.mock('@/lib/knowledge/connectors/access-token', () => ({
26+
resolveConnectorAccessToken: mockResolveToken,
27+
}))
28+
vi.mock('@/lib/knowledge/connectors/external-group-sync', () => ({
29+
refreshMirroredDirectory: mockRefreshDirectory,
30+
}))
31+
vi.mock('@/connectors/registry.server', () => ({
32+
CONNECTOR_REGISTRY: {
33+
google_drive: { auth: { mode: 'oauth', provider: 'google-drive' }, openDirectory: vi.fn() },
34+
notion: { auth: { mode: 'oauth', provider: 'notion' } },
35+
},
36+
}))
37+
vi.mock('@sim/db', () => ({
38+
db: {
39+
select: () => ({
40+
from: () => ({
41+
innerJoin: () => ({ where: () => ({ limit: () => mockConnectorRows() }) }),
42+
}),
43+
}),
44+
},
45+
}))
46+
47+
import { GET } from '@/app/api/knowledge/connectors/directory-sync/route'
48+
49+
function connector(overrides: Record<string, unknown> = {}) {
50+
return {
51+
id: 'connector-1',
52+
connectorType: 'google_drive',
53+
credentialId: 'credential-1',
54+
encryptedApiKey: null,
55+
sourceConfig: { adminEmail: 'admin@corp.com' },
56+
workspaceId: 'ws-1',
57+
knowledgeBaseOwnerId: 'owner-1',
58+
...overrides,
59+
}
60+
}
61+
62+
async function run() {
63+
const response = await GET(createMockRequest('GET'))
64+
return response.json()
65+
}
66+
67+
describe('connector directory sync scheduler', () => {
68+
beforeEach(() => {
69+
vi.clearAllMocks()
70+
mockVerifyCronAuth.mockReturnValue(null)
71+
mockResolveIdentity.mockResolvedValue({ kind: 'service_account' })
72+
mockResolveToken.mockResolvedValue({ accessToken: 'token' })
73+
mockRefreshDirectory.mockResolvedValue(undefined)
74+
})
75+
76+
it('refreshes the directory of every admin-mode connector it finds', async () => {
77+
mockConnectorRows.mockResolvedValue([connector(), connector({ id: 'connector-2' })])
78+
79+
await expect(run()).resolves.toMatchObject({ considered: 2, refreshed: 2, failed: 0 })
80+
expect(mockRefreshDirectory).toHaveBeenCalledTimes(2)
81+
})
82+
83+
/**
84+
* The tick refreshes every workspace's directory, so one workspace's lapsed
85+
* credential or unreachable source must not stop the others.
86+
*/
87+
it('contains a failure to the connector that caused it', async () => {
88+
mockConnectorRows.mockResolvedValue([connector(), connector({ id: 'connector-2' })])
89+
mockRefreshDirectory.mockRejectedValueOnce(new Error('directory unreachable'))
90+
91+
await expect(run()).resolves.toMatchObject({ considered: 2, refreshed: 1, failed: 1 })
92+
})
93+
94+
it('reports a connector whose credential no longer resolves rather than failing', async () => {
95+
mockConnectorRows.mockResolvedValue([connector()])
96+
mockResolveToken.mockResolvedValue(null)
97+
98+
await expect(run()).resolves.toMatchObject({ unusable: 1, refreshed: 0 })
99+
expect(mockRefreshDirectory).not.toHaveBeenCalled()
100+
})
101+
102+
it('skips a connector whose source has no directory to read', async () => {
103+
mockConnectorRows.mockResolvedValue([connector({ connectorType: 'notion' })])
104+
105+
await expect(run()).resolves.toMatchObject({ skipped: 1, refreshed: 0 })
106+
expect(mockRefreshDirectory).not.toHaveBeenCalled()
107+
})
108+
109+
/**
110+
* Token reads are scoped to the credential's own account owner, not the
111+
* knowledge base owner, who is routinely a different member.
112+
*/
113+
it('resolves the token as the credential owner for an OAuth credential', async () => {
114+
mockConnectorRows.mockResolvedValue([connector()])
115+
mockResolveIdentity.mockResolvedValue({ kind: 'oauth', userId: 'credential-owner' })
116+
117+
await run()
118+
119+
expect(mockResolveToken).toHaveBeenCalledWith(
120+
expect.objectContaining({ userId: 'credential-owner' })
121+
)
122+
})
123+
124+
it('refuses an unauthenticated tick', async () => {
125+
mockVerifyCronAuth.mockReturnValue(new Response('nope', { status: 401 }))
126+
127+
const response = await GET(createMockRequest('GET'))
128+
129+
expect(response.status).toBe(401)
130+
expect(mockConnectorRows).not.toHaveBeenCalled()
131+
})
132+
})
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import { db } from '@sim/db'
2+
import { knowledgeBase, knowledgeConnector } from '@sim/db/schema'
3+
import { createLogger } from '@sim/logger'
4+
import { getErrorMessage } from '@sim/utils/errors'
5+
import { and, eq, isNull } from 'drizzle-orm'
6+
import type { NextRequest } from 'next/server'
7+
import { verifyCronAuth } from '@/lib/auth/internal'
8+
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
9+
import { generateRequestId } from '@/lib/core/utils/request'
10+
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
11+
import { resolveCredentialTokenIdentity } from '@/lib/credentials/access'
12+
import { resolveConnectorAccessToken } from '@/lib/knowledge/connectors/access-token'
13+
import { refreshMirroredDirectory } from '@/lib/knowledge/connectors/external-group-sync'
14+
import { CONNECTOR_REGISTRY } from '@/connectors/registry.server'
15+
16+
export const dynamic = 'force-dynamic'
17+
18+
const logger = createLogger('ConnectorDirectorySyncSchedulerAPI')
19+
20+
/** Directories refreshed per tick, and how many at once. */
21+
const MAX_DIRECTORIES_PER_TICK = 200
22+
const REFRESH_CONCURRENCY = 4
23+
24+
/**
25+
* Refreshes the external directories that admin-mode connectors mirror.
26+
*
27+
* Group membership decides who can read an already-indexed document, so it has
28+
* to move on its own clock: someone leaving a group should lose access in
29+
* minutes, not on whatever schedule the corpus happens to be re-crawled on. The
30+
* admin crawl refreshes the directory too — so a crawl can never publish grants
31+
* against membership nobody has read — but that is a floor, not the cadence.
32+
*
33+
* Every eligible connector is offered each tick; `syncExternalDirectoryGroups`
34+
* decides whether its directory is actually due. Connectors sharing a directory
35+
* therefore cost one refresh between them: the first brings it up to date and
36+
* the rest skip. Two ticks overlapping on one directory would both enumerate
37+
* and write the same rows — wasteful, never wrong, and not worth a lease to
38+
* prevent, since every write here is idempotent.
39+
*/
40+
export const GET = withRouteHandler(async (request: NextRequest) => {
41+
const requestId = generateRequestId()
42+
logger.info(`[${requestId}] Connector directory sync scheduler triggered`)
43+
44+
const authError = verifyCronAuth(request, 'Connector directory sync scheduler')
45+
if (authError) return authError
46+
47+
const connectors = await db
48+
.select({
49+
id: knowledgeConnector.id,
50+
connectorType: knowledgeConnector.connectorType,
51+
credentialId: knowledgeConnector.credentialId,
52+
encryptedApiKey: knowledgeConnector.encryptedApiKey,
53+
sourceConfig: knowledgeConnector.sourceConfig,
54+
workspaceId: knowledgeBase.workspaceId,
55+
knowledgeBaseOwnerId: knowledgeBase.userId,
56+
})
57+
.from(knowledgeConnector)
58+
.innerJoin(knowledgeBase, eq(knowledgeConnector.knowledgeBaseId, knowledgeBase.id))
59+
.where(
60+
and(
61+
eq(knowledgeConnector.accessMode, 'admin'),
62+
isNull(knowledgeConnector.archivedAt),
63+
isNull(knowledgeConnector.deletedAt),
64+
isNull(knowledgeBase.deletedAt)
65+
)
66+
)
67+
.limit(MAX_DIRECTORIES_PER_TICK)
68+
69+
const outcomes = await mapWithConcurrency(connectors, REFRESH_CONCURRENCY, async (connector) => {
70+
/**
71+
* Every failure is contained here. One workspace whose credential lapsed
72+
* must not stop the tick refreshing every other workspace's directory.
73+
*/
74+
try {
75+
if (!connector.workspaceId) return 'skipped'
76+
const connectorConfig = CONNECTOR_REGISTRY[connector.connectorType]
77+
if (!connectorConfig?.openDirectory) return 'skipped'
78+
79+
/**
80+
* The credential's own account owner, not the knowledge base owner —
81+
* token reads are scoped to `account.userId`, and a service account
82+
* ignores the argument entirely.
83+
*/
84+
let credentialUserId = connector.knowledgeBaseOwnerId
85+
if (connector.credentialId) {
86+
const identity = await resolveCredentialTokenIdentity(
87+
connector.credentialId,
88+
connector.workspaceId
89+
)
90+
if (!identity) return 'unusable'
91+
if (identity.kind === 'oauth') credentialUserId = identity.userId
92+
}
93+
94+
const sourceConfig = connector.sourceConfig as Record<string, unknown>
95+
const token = await resolveConnectorAccessToken({
96+
auth: connectorConfig.auth,
97+
connector,
98+
userId: credentialUserId,
99+
requestId,
100+
sourceConfig,
101+
})
102+
if (!token) return 'unusable'
103+
104+
await refreshMirroredDirectory({
105+
workspaceId: connector.workspaceId,
106+
connectorConfig,
107+
sourceConfig,
108+
syncContext: {},
109+
accessToken: token.accessToken,
110+
})
111+
return 'refreshed'
112+
} catch (error) {
113+
logger.error(`[${requestId}] Directory refresh failed for a connector`, {
114+
connectorId: connector.id,
115+
error: getErrorMessage(error),
116+
})
117+
return 'failed'
118+
}
119+
})
120+
121+
const summary = {
122+
considered: connectors.length,
123+
refreshed: outcomes.filter((outcome) => outcome === 'refreshed').length,
124+
skipped: outcomes.filter((outcome) => outcome === 'skipped').length,
125+
unusable: outcomes.filter((outcome) => outcome === 'unusable').length,
126+
failed: outcomes.filter((outcome) => outcome === 'failed').length,
127+
}
128+
logger.info(`[${requestId}] Connector directory sync scheduler finished`, summary)
129+
return Response.json({ success: true, ...summary })
130+
})

apps/sim/connectors/confluence/confluence.ts

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
AtlassianSiteNotAccessibleError,
66
AtlassianSiteNotMatchedError,
77
} from '@/lib/atlassian/discovery'
8+
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
89
import {
910
type ConfluencePrincipal,
1011
type ConfluenceRestriction,
@@ -424,23 +425,6 @@ async function resolveConfluenceAcls(
424425
return acls
425426
}
426427

427-
/** Runs `worker` over `items`, at most `limit` at a time, preserving no order. */
428-
async function mapWithConcurrency<T>(
429-
items: readonly T[],
430-
limit: number,
431-
worker: (item: T) => Promise<void>
432-
): Promise<void> {
433-
let cursor = 0
434-
const runners = Array.from({ length: Math.min(limit, items.length) }, async () => {
435-
for (;;) {
436-
const index = cursor++
437-
if (index >= items.length) return
438-
await worker(items[index])
439-
}
440-
})
441-
await Promise.all(runners)
442-
}
443-
444428
export const confluenceConnector: ConnectorConfig = {
445429
...confluenceConnectorMeta,
446430

apps/sim/lib/knowledge/connectors/external-group-sync.ts

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import { getErrorMessage } from '@sim/utils/errors'
55
import { generateId } from '@sim/utils/id'
66
import { and, eq, isNull, lt, notInArray, or } from 'drizzle-orm'
77
import { EXTERNAL_GROUP_SYNC_INTERVAL_MS } from '@/lib/knowledge/access/external-groups'
8-
import type { ConnectorDirectory, ConnectorDirectoryGroup } from '@/connectors/types'
8+
import type {
9+
ConnectorConfig,
10+
ConnectorDirectory,
11+
ConnectorDirectoryGroup,
12+
} from '@/connectors/types'
913

1014
const logger = createLogger('ExternalGroupSync')
1115

@@ -229,3 +233,58 @@ async function pruneRemovedGroups(input: {
229233
.returning({ id: knowledgeExternalGroup.id })
230234
return removed.length
231235
}
236+
237+
/**
238+
* Refreshes the directory groups the mirrored ACLs refer to.
239+
*
240+
* A `g:` token grants nobody until the directory says who is in that group, so
241+
* the refresh runs in the same pass that writes the tokens — a crawl can never
242+
* publish grants against membership this workspace has never read.
243+
*
244+
* It is rate-limited on its own clock rather than the connector's, so a
245+
* frequently-syncing connector does not re-read the whole directory every run.
246+
* A failure is logged rather than thrown: last-known-good membership is still
247+
* serving reads, and failing the content sync over it would strand the
248+
* documents as well as the groups.
249+
*/
250+
export async function refreshMirroredDirectory(input: {
251+
workspaceId: string
252+
connectorConfig: ConnectorConfig
253+
sourceConfig: Record<string, unknown>
254+
syncContext: Record<string, unknown>
255+
accessToken: string
256+
}): Promise<void> {
257+
const { workspaceId, connectorConfig } = input
258+
if (connectorConfig.auth.mode !== 'oauth' || !connectorConfig.openDirectory) return
259+
260+
try {
261+
const directory = await connectorConfig.openDirectory(
262+
input.accessToken,
263+
input.sourceConfig,
264+
input.syncContext
265+
)
266+
if (!directory) {
267+
logger.warn('Skipping directory refresh: the connector names no directory', {
268+
workspaceId,
269+
connector: connectorConfig.id,
270+
})
271+
return
272+
}
273+
const result = await syncExternalDirectoryGroups({
274+
workspaceId,
275+
providerId: connectorConfig.auth.provider,
276+
directory,
277+
})
278+
logger.info('Refreshed mirrored directory groups', {
279+
workspaceId,
280+
tenantId: directory.tenantId,
281+
...result,
282+
})
283+
} catch (error) {
284+
logger.error('Directory refresh failed; serving last-known-good group membership', {
285+
workspaceId,
286+
connector: connectorConfig.id,
287+
error: getErrorMessage(error),
288+
})
289+
}
290+
}

0 commit comments

Comments
 (0)