diff --git a/api/server/routes/admin/skills.js b/api/server/routes/admin/skills.js index 74a81059d9..4c2e8f7bde 100644 --- a/api/server/routes/admin/skills.js +++ b/api/server/routes/admin/skills.js @@ -4,12 +4,13 @@ const { SystemCapabilities } = require('@librechat/data-schemas'); const { hasCapability, requireCapability } = require('~/server/middleware/roles/capabilities'); const { requireJwtAuth } = require('~/server/middleware'); const { upsertSkillSyncCredential, deleteSkillSyncCredential } = require('~/models'); -const { getGitHubSkillSyncRunner } = require('~/server/services/Skills/sync'); +const { getGitHubSkillSyncRunnerForRequest } = require('~/server/services/Skills/sync'); +const configMiddleware = require('~/server/middleware/config/app'); const router = express.Router(); const requireAdminAccess = requireCapability(SystemCapabilities.ACCESS_ADMIN); -function requirePlatformCapability(capability) { +function requireSkillCapability(capability, { platformOnly = false } = {}) { return async (req, res, next) => { try { const id = req.user?.id ?? req.user?._id?.toString?.(); @@ -19,6 +20,7 @@ function requirePlatformCapability(capability) { const user = { id, role: req.user?.role ?? '', + ...(platformOnly ? {} : { tenantId: req.user?.tenantId }), }; if (await hasCapability(user, capability)) { return next(); @@ -30,19 +32,22 @@ function requirePlatformCapability(capability) { }; } -const requirePlatformReadSkills = requirePlatformCapability(SystemCapabilities.READ_SKILLS); -const requirePlatformManageSkills = requirePlatformCapability(SystemCapabilities.MANAGE_SKILLS); +const requireReadSkills = requireSkillCapability(SystemCapabilities.READ_SKILLS); +const requireManageSkills = requireSkillCapability(SystemCapabilities.MANAGE_SKILLS); +const requirePlatformManageSkills = requireSkillCapability(SystemCapabilities.MANAGE_SKILLS, { + platformOnly: true, +}); const handlers = createAdminSkillsSyncHandlers({ - runner: getGitHubSkillSyncRunner(), + getRunner: getGitHubSkillSyncRunnerForRequest, upsertCredential: upsertSkillSyncCredential, deleteCredential: deleteSkillSyncCredential, }); -router.use(requireJwtAuth, requireAdminAccess); +router.use(requireJwtAuth, requireAdminAccess, configMiddleware); -router.get('/sync/status', requirePlatformReadSkills, handlers.getSyncStatus); -router.post('/sync/run', requirePlatformManageSkills, handlers.runSync); +router.get('/sync/status', requireReadSkills, handlers.getSyncStatus); +router.post('/sync/run', requireManageSkills, handlers.runSync); router.put('/sync/credentials/:credentialKey', requirePlatformManageSkills, handlers.setCredential); router.delete( '/sync/credentials/:credentialKey', diff --git a/api/server/routes/admin/skills.test.js b/api/server/routes/admin/skills.test.js index 59fe33f529..2301c915af 100644 --- a/api/server/routes/admin/skills.test.js +++ b/api/server/routes/admin/skills.test.js @@ -8,6 +8,11 @@ const mockRequireJwtAuth = jest.fn((req, res, next) => { const mockCapabilityMiddleware = jest.fn((req, res, next) => next()); const mockRequireCapability = jest.fn(() => mockCapabilityMiddleware); const mockHasCapability = jest.fn().mockResolvedValue(true); +const mockConfigMiddleware = jest.fn((req, res, next) => { + req.config = { skillSync: { github: { enabled: false, sources: [] } } }; + next(); +}); +const mockGetGitHubSkillSyncRunnerForRequest = jest.fn(); jest.mock('@librechat/data-schemas', () => ({ SystemCapabilities: { @@ -35,16 +40,15 @@ jest.mock('~/server/middleware', () => ({ requireJwtAuth: mockRequireJwtAuth, })); +jest.mock('~/server/middleware/config/app', () => mockConfigMiddleware); + jest.mock('~/models', () => ({ upsertSkillSyncCredential: jest.fn(), deleteSkillSyncCredential: jest.fn(), })); jest.mock('~/server/services/Skills/sync', () => ({ - getGitHubSkillSyncRunner: jest.fn(() => ({ - getStatus: jest.fn(), - runOnce: jest.fn(), - })), + getGitHubSkillSyncRunnerForRequest: mockGetGitHubSkillSyncRunnerForRequest, })); describe('admin skills sync routes', () => { @@ -62,11 +66,23 @@ describe('admin skills sync routes', () => { expect(mockRequireCapability).toHaveBeenCalledWith('access:admin'); expect(mockRequireJwtAuth).toHaveBeenCalled(); expect(mockCapabilityMiddleware).toHaveBeenCalled(); + expect(mockConfigMiddleware).toHaveBeenCalled(); expect(mockHasCapability).toHaveBeenCalledTimes(4); - expect(mockHasCapability).toHaveBeenCalledWith({ id: 'user-1', role: 'ADMIN' }, 'read:skills'); + expect(mockHasCapability).toHaveBeenCalledWith( + { id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' }, + 'read:skills', + ); + expect(mockHasCapability).toHaveBeenCalledWith( + { id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' }, + 'manage:skills', + ); expect(mockHasCapability).toHaveBeenCalledWith( { id: 'user-1', role: 'ADMIN' }, 'manage:skills', ); + const { createAdminSkillsSyncHandlers } = require('@librechat/api'); + expect(createAdminSkillsSyncHandlers).toHaveBeenCalledWith( + expect.objectContaining({ getRunner: mockGetGitHubSkillSyncRunnerForRequest }), + ); }); }); diff --git a/api/server/services/Skills/sync.js b/api/server/services/Skills/sync.js index 57d1cbefa2..cb397d4c0c 100644 --- a/api/server/services/Skills/sync.js +++ b/api/server/services/Skills/sync.js @@ -12,6 +12,7 @@ const { getFileStrategy } = require('~/server/utils/getFileStrategy'); const SYSTEM_USER_ID = '000000000000000000000000'; const REQUEST_SYNC_MIN_INTERVAL_MS = 5 * 60 * 1000; +const REQUEST_SYNC_STALE_RUNNING_MS = 35 * 60 * 1000; let appConfigRef; let runner; @@ -167,6 +168,25 @@ function isSameSkillSyncConfig(left, right) { return JSON.stringify(left ?? null) === JSON.stringify(right ?? null); } +function getRequestTenantId(user) { + return typeof user?.tenantId === 'string' && user.tenantId ? user.tenantId : undefined; +} + +function withRequestTenant(config, user, { disableRunOnStartup = false } = {}) { + const tenantId = getRequestTenantId(user); + return { + ...config, + github: { + ...config.github, + ...(disableRunOnStartup ? { runOnStartup: false } : {}), + sources: config.github.sources.map((source) => ({ + ...source, + tenantId, + })), + }, + }; +} + function getRequestSkillSyncConfig(appConfig, user) { const resolved = parseSkillSyncConfig(appConfig?.skillSync); if (!resolved?.github?.enabled || resolved.github.sources.length === 0) { @@ -178,18 +198,21 @@ function getRequestSkillSyncConfig(appConfig, user) { return undefined; } - const tenantId = typeof user?.tenantId === 'string' && user.tenantId ? user.tenantId : undefined; - return { - ...resolved, - github: { - ...resolved.github, - runOnStartup: false, - sources: resolved.github.sources.map((source) => ({ - ...source, - tenantId, - })), - }, - }; + return withRequestTenant(resolved, user, { disableRunOnStartup: true }); +} + +function getAdminRequestSkillSyncConfig(appConfig, user) { + const resolved = parseSkillSyncConfig(appConfig?.skillSync); + if (!resolved?.github) { + return resolved; + } + + const base = parseSkillSyncConfig(appConfig?.config?.skillSync); + if (isSameSkillSyncConfig(resolved, base)) { + return resolved; + } + + return withRequestTenant(resolved, user); } function toTimestamp(value) { @@ -224,13 +247,22 @@ function shouldRunRequestSync(status) { const now = Date.now(); return status.sources.some((source) => { if (source.status === 'running') { - return false; + const startedAt = toTimestamp(source.startedAt); + return Boolean(startedAt && now - startedAt >= REQUEST_SYNC_STALE_RUNNING_MS); } const lastAttemptAt = getLastAttemptAt(source); return !lastAttemptAt || now - lastAttemptAt >= intervalMs; }); } +function getGitHubSkillSyncRunnerForRequest(req) { + const config = getAdminRequestSkillSyncConfig(req.config, req.user); + return createRunner({ + getConfig: async () => config, + loadAppConfig: async () => req.config, + }); +} + function getRequestSyncKey(config, user) { const tenantId = user?.tenantId ?? ''; const sources = config.github.sources @@ -296,6 +328,7 @@ function stopGitHubSkillSyncScheduler() { module.exports = { initializeGitHubSkillSync, getGitHubSkillSyncRunner, + getGitHubSkillSyncRunnerForRequest, maybeRunGitHubSkillSyncForRequest, stopGitHubSkillSyncScheduler, }; diff --git a/api/server/services/Skills/sync.test.js b/api/server/services/Skills/sync.test.js index f77f952ee1..61757453ee 100644 --- a/api/server/services/Skills/sync.test.js +++ b/api/server/services/Skills/sync.test.js @@ -220,6 +220,41 @@ describe('GitHub skill sync service', () => { expect(mockCreatedRunners).toHaveLength(0); }); + it('creates an admin request runner from resolved skillSync config overrides', async () => { + const skillSync = { + github: { + enabled: true, + intervalMinutes: 60, + runOnStartup: true, + sources: [ + { + id: 'tenant-skills', + owner: 'LibreChat', + repo: 'skills', + ref: 'main', + paths: ['skills'], + token: '${GITHUB_SKILLS_TOKEN}', + tenantId: 'other-tenant', + }, + ], + }, + }; + + const service = require('./sync'); + const runner = service.getGitHubSkillSyncRunnerForRequest({ + config: { skillSync, config: {} }, + user: { id: 'user-1', tenantId: 'tenant-a' }, + }); + const config = await mockCreatedRunners[0].deps.getConfig(); + + expect(runner.runOnce).toBe(mockCreatedRunners[0].runner.runOnce); + expect(runner.getStatus).toBe(mockCreatedRunners[0].runner.getStatus); + expect(config.github.runOnStartup).toBe(true); + expect(config.github.sources[0]).toEqual( + expect.objectContaining({ id: 'tenant-skills', tenantId: 'tenant-a' }), + ); + }); + it('does not start a request-scoped sync when the configured source is already running', async () => { const skillSync = { github: { @@ -271,6 +306,57 @@ describe('GitHub skill sync service', () => { expect(mockCreatedRunners[0].runner.runOnce).not.toHaveBeenCalled(); }); + it('retries a request-scoped sync when a running source status is stale', async () => { + const skillSync = { + github: { + enabled: true, + intervalMinutes: 60, + runOnStartup: false, + sources: [ + { + id: 'tenant-skills', + owner: 'LibreChat', + repo: 'skills', + ref: 'main', + paths: ['skills'], + token: '${GITHUB_SKILLS_TOKEN}', + }, + ], + }, + }; + mockRunnerStatus = { + enabled: true, + intervalMinutes: 60, + runOnStartup: false, + sources: [ + { + provider: 'github', + sourceId: 'tenant-skills', + status: 'running', + owner: 'LibreChat', + repo: 'skills', + ref: 'main', + paths: ['skills'], + startedAt: new Date(Date.now() - 40 * 60 * 1000), + syncedSkillCount: 0, + syncedFileCount: 0, + deletedSkillCount: 0, + deletedFileCount: 0, + }, + ], + credentials: [], + }; + + const service = require('./sync'); + const started = await service.maybeRunGitHubSkillSyncForRequest({ + config: { skillSync, config: {} }, + user: { id: 'user-1', tenantId: 'tenant-a' }, + }); + + expect(started).toBe(true); + expect(mockCreatedRunners[0].runner.runOnce).toHaveBeenCalledTimes(1); + }); + it('uses the file owner when deleting synced files from storage', async () => { const deleteFile = jest.fn(async () => undefined); const ownerId = '507f1f77bcf86cd799439011'; diff --git a/packages/api/src/admin/skills.ts b/packages/api/src/admin/skills.ts index dcf4630b37..c40e1c5d2f 100644 --- a/packages/api/src/admin/skills.ts +++ b/packages/api/src/admin/skills.ts @@ -22,7 +22,8 @@ type AdminSkillsRequest = Request & { }; export type AdminSkillSyncDeps = { - runner: GitHubSkillSyncRunner; + runner?: GitHubSkillSyncRunner; + getRunner?: (req: Request) => GitHubSkillSyncRunner; upsertCredential: (input: UpsertSkillSyncCredentialInput) => Promise; deleteCredential: ( provider: SkillSyncProvider, @@ -55,6 +56,7 @@ function serializeSourceStatus( return { provider: status.provider, sourceId: status.sourceId, + tenantId: status.tenantId, status: status.status, credentialKey: status.credentialKey, credentialPresent: status.credentialPresent ?? false, @@ -86,8 +88,16 @@ function getUserObjectId(req: AdminSkillsRequest): Types.ObjectId | undefined { } export function createAdminSkillsSyncHandlers(deps: AdminSkillSyncDeps) { - async function getSyncStatus(_req: Request, res: Response) { - const status = await deps.runner.getStatus(); + function getRunner(req: Request): GitHubSkillSyncRunner { + const runner = deps.getRunner?.(req) ?? deps.runner; + if (!runner) { + throw new Error('GitHub skill sync runner is not configured'); + } + return runner; + } + + async function getSyncStatus(req: Request, res: Response) { + const status = await getRunner(req).getStatus(); const response: TGitHubSkillSyncStatusResponse = { enabled: status.enabled, intervalMinutes: status.intervalMinutes, @@ -99,8 +109,8 @@ export function createAdminSkillsSyncHandlers(deps: AdminSkillSyncDeps) { return res.status(200).json(response); } - async function runSync(_req: Request, res: Response) { - const result = await deps.runner.runOnce(); + async function runSync(req: Request, res: Response) { + const result = await getRunner(req).runOnce(); const response: TGitHubSkillSyncManualRunResponse = { status: result.status, message: result.message, diff --git a/packages/api/src/skills/sync/github.spec.ts b/packages/api/src/skills/sync/github.spec.ts index 029942c2fc..5ab88a8a98 100644 --- a/packages/api/src/skills/sync/github.spec.ts +++ b/packages/api/src/skills/sync/github.spec.ts @@ -181,6 +181,7 @@ function createDeps( const status: ISkillSyncStatus = { provider: input.provider, sourceId: input.sourceId, + tenantId: input.tenantId, status: input.status, credentialKey: input.credentialKey, owner: input.owner, @@ -507,6 +508,69 @@ describe('createGitHubSkillSyncRunner', () => { expect(deps.createSkill).toHaveBeenCalledWith( expect.objectContaining({ name: 'research', tenantId: 'tenant-a' }), ); + expect(deps.upsertStatus).toHaveBeenCalledWith( + expect.objectContaining({ sourceId: 'librechat-skills', tenantId: 'tenant-a' }), + ); + expect(deps.tryAcquireLock).toHaveBeenCalledWith( + expect.objectContaining({ tenantId: 'tenant-a' }), + ); + }); + + it('matches stored source status by tenant and source id', async () => { + const deps = createDeps({ + listStatuses: jest.fn(async () => [ + { + provider: 'github', + sourceId: 'librechat-skills', + tenantId: 'tenant-a', + status: 'succeeded', + syncedSkillCount: 1, + syncedFileCount: 2, + deletedSkillCount: 0, + deletedFileCount: 0, + } as ISkillSyncStatus, + { + provider: 'github', + sourceId: 'librechat-skills', + tenantId: 'tenant-b', + status: 'failed', + errorCode: 'OTHER_TENANT', + syncedSkillCount: 0, + syncedFileCount: 0, + deletedSkillCount: 0, + deletedFileCount: 0, + } as ISkillSyncStatus, + ]), + getConfig: () => ({ + github: { + enabled: true, + intervalMinutes: 60, + runOnStartup: false, + sources: [ + { + id: 'librechat-skills', + owner: 'LibreChat', + repo: 'skills', + ref: 'main', + paths: ['skills'], + credentialKey: 'github-skills-prod', + tenantId: 'tenant-b', + }, + ], + }, + }), + }); + + const status = await createGitHubSkillSyncRunner(deps).getStatus(); + + expect(status.sources[0]).toEqual( + expect.objectContaining({ + sourceId: 'librechat-skills', + tenantId: 'tenant-b', + status: 'failed', + errorCode: 'OTHER_TENANT', + }), + ); }); it('runs in the ambient context when a source has no configured tenantId', async () => { diff --git a/packages/api/src/skills/sync/github.ts b/packages/api/src/skills/sync/github.ts index f71212f31c..a04b9eb1eb 100644 --- a/packages/api/src/skills/sync/github.ts +++ b/packages/api/src/skills/sync/github.ts @@ -140,13 +140,19 @@ export type GitHubSkillSyncDeps = { provider: SkillSyncProvider; lockOwner: string; leaseMs: number; + tenantId?: string; }) => Promise; refreshLock: (params: { provider: SkillSyncProvider; lockOwner: string; leaseMs: number; + tenantId?: string; }) => Promise; - releaseLock: (params: { provider: SkillSyncProvider; lockOwner: string }) => Promise; + releaseLock: (params: { + provider: SkillSyncProvider; + lockOwner: string; + tenantId?: string; + }) => Promise; createSkill: (data: CreateSkillInput) => Promise; updateSkill: (params: { id: string; @@ -731,6 +737,7 @@ function makeStatusInput(params: { return { provider: PROVIDER, sourceId: params.source.id, + tenantId: params.source.tenantId, status: params.status, credentialKey: params.source.credentialKey, owner: params.source.owner, @@ -748,6 +755,15 @@ function makeStatusInput(params: { }; } +function makeStatusKey(sourceId: string, tenantId?: string): string { + return `${tenantId ?? ''}:${sourceId}`; +} + +function getLockTenantId(sources: SkillSyncGitHubSourceConfig[]): string | undefined { + const tenantIds = new Set(sources.map((source) => source.tenantId).filter(Boolean)); + return tenantIds.size === 1 ? [...tenantIds][0] : undefined; +} + async function ensurePublicViewer( deps: GitHubSkillSyncDeps, skillId: Types.ObjectId, @@ -1454,18 +1470,21 @@ export function createGitHubSkillSyncRunner(deps: GitHubSkillSyncDeps) { deps.listStatuses(PROVIDER), deps.listCredentials(PROVIDER), ]); - const statusBySourceId = new Map(storedStatuses.map((status) => [status.sourceId, status])); + const statusBySourceId = new Map( + storedStatuses.map((status) => [makeStatusKey(status.sourceId, status.tenantId), status]), + ); const credentialByKey = new Map( credentials.map((credential) => [credential.credentialKey, credential]), ); const sources = github.sources.map((source) => { - const stored = statusBySourceId.get(source.id); + const stored = statusBySourceId.get(makeStatusKey(source.id, source.tenantId)); const credential = source.credentialKey ? credentialByKey.get(source.credentialKey) : null; const tokenEnvVar = getTokenEnvVarName(source.token); const envTokenPresent = tokenEnvVar ? Boolean(process.env[tokenEnvVar]?.trim()) : false; return { provider: PROVIDER, sourceId: source.id, + tenantId: source.tenantId, status: stored?.status ?? 'idle', credentialKey: source.credentialKey, credentialPresent: envTokenPresent || Boolean(credential), @@ -1503,10 +1522,12 @@ export function createGitHubSkillSyncRunner(deps: GitHubSkillSyncDeps) { return { status: 'skipped', message: 'GitHub skill sync is disabled', sources: [] }; } const lockOwner = `${lockOwnerPrefix}:${crypto.randomUUID().replace(/-/g, '').slice(0, 12)}`; + const lockTenantId = getLockTenantId(github.sources); const acquired = await deps.tryAcquireLock({ provider: PROVIDER, lockOwner, leaseMs: LOCK_LEASE_MS, + tenantId: lockTenantId, }); if (!acquired) { const status = await getStatus(); @@ -1525,7 +1546,12 @@ export function createGitHubSkillSyncRunner(deps: GitHubSkillSyncDeps) { const refreshTimer = setInterval( () => { deps - .refreshLock({ provider: PROVIDER, lockOwner, leaseMs: LOCK_LEASE_MS }) + .refreshLock({ + provider: PROVIDER, + lockOwner, + leaseMs: LOCK_LEASE_MS, + tenantId: lockTenantId, + }) .then((refreshed) => { if (!refreshed) { lockLost = true; @@ -1558,7 +1584,7 @@ export function createGitHubSkillSyncRunner(deps: GitHubSkillSyncDeps) { }; } finally { clearInterval(refreshTimer); - await deps.releaseLock({ provider: PROVIDER, lockOwner }); + await deps.releaseLock({ provider: PROVIDER, lockOwner, tenantId: lockTenantId }); } } diff --git a/packages/data-provider/src/types/skills.ts b/packages/data-provider/src/types/skills.ts index cfcf27e407..4103b6e536 100644 --- a/packages/data-provider/src/types/skills.ts +++ b/packages/data-provider/src/types/skills.ts @@ -216,6 +216,7 @@ export type TGitHubSkillSyncCredentialSummary = { export type TGitHubSkillSyncSourceStatus = { provider: 'github'; sourceId: string; + tenantId?: string; status: 'idle' | 'running' | 'succeeded' | 'failed' | 'skipped'; credentialKey?: string; credentialPresent: boolean; diff --git a/packages/data-schemas/src/methods/skillSync.spec.ts b/packages/data-schemas/src/methods/skillSync.spec.ts index 0ceac31d42..8ab5354aa2 100644 --- a/packages/data-schemas/src/methods/skillSync.spec.ts +++ b/packages/data-schemas/src/methods/skillSync.spec.ts @@ -131,4 +131,64 @@ describe('createSkillSyncMethods', () => { expect(success.errorCode).toBeUndefined(); expect(success.errorMessage).toBeUndefined(); }); + + it('keeps status rows separate for the same source id in different tenants', async () => { + await methods.upsertSkillSyncStatus({ + provider: 'github', + sourceId: 'shared-source', + tenantId: 'tenant-a', + status: 'succeeded', + syncedSkillCount: 1, + }); + await methods.upsertSkillSyncStatus({ + provider: 'github', + sourceId: 'shared-source', + tenantId: 'tenant-b', + status: 'failed', + errorCode: 'TENANT_B_FAILURE', + }); + + const tenantA = await methods.getSkillSyncStatus('github', 'shared-source', 'tenant-a'); + const tenantB = await methods.getSkillSyncStatus('github', 'shared-source', 'tenant-b'); + + expect(tenantA).toMatchObject({ + sourceId: 'shared-source', + tenantId: 'tenant-a', + status: 'succeeded', + syncedSkillCount: 1, + }); + expect(tenantB).toMatchObject({ + sourceId: 'shared-source', + tenantId: 'tenant-b', + status: 'failed', + errorCode: 'TENANT_B_FAILURE', + }); + }); + + it('keeps sync locks separate per tenant', async () => { + await expect( + methods.tryAcquireSkillSyncLock({ + provider: 'github', + tenantId: 'tenant-a', + lockOwner: 'worker-a', + leaseMs: 60_000, + }), + ).resolves.toBe(true); + await expect( + methods.tryAcquireSkillSyncLock({ + provider: 'github', + tenantId: 'tenant-b', + lockOwner: 'worker-b', + leaseMs: 60_000, + }), + ).resolves.toBe(true); + await expect( + methods.tryAcquireSkillSyncLock({ + provider: 'github', + tenantId: 'tenant-a', + lockOwner: 'worker-c', + leaseMs: 60_000, + }), + ).resolves.toBe(false); + }); }); diff --git a/packages/data-schemas/src/methods/skillSync.ts b/packages/data-schemas/src/methods/skillSync.ts index bcd97694ea..e662614956 100644 --- a/packages/data-schemas/src/methods/skillSync.ts +++ b/packages/data-schemas/src/methods/skillSync.ts @@ -31,6 +31,7 @@ export type UpsertSkillSyncCredentialInput = { export type SkillSyncStatusInput = { provider: SkillSyncProvider; sourceId: string; + tenantId?: string; status: SkillSyncRunStatus; credentialKey?: string; owner?: string; @@ -51,6 +52,10 @@ function hashToken(token: string): string { return createHash('sha256').update(token).digest('hex'); } +function tenantStatusCondition(tenantId?: string) { + return tenantId ? { tenantId } : { tenantId: { $exists: false } }; +} + function summarizeCredential( credential: Pick< ISkillSyncCredential, @@ -155,9 +160,14 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { async function getSkillSyncStatus( provider: SkillSyncProvider, sourceId: string, + tenantId?: string, ): Promise { const Status = mongoose.models.SkillSyncStatus as Model; - const row = await Status.findOne({ provider, sourceId }).lean(); + const row = await Status.findOne({ + provider, + sourceId, + ...tenantStatusCondition(tenantId), + }).lean(); return row ?? null; } @@ -188,13 +198,18 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { } const unsetPayload = failure ? {} : { errorCode: '', errorMessage: '' }; const row = await Status.findOneAndUpdate( - { provider: input.provider, sourceId: input.sourceId }, + { + provider: input.provider, + sourceId: input.sourceId, + ...tenantStatusCondition(input.tenantId), + }, { $set: setPayload, ...(failure ? {} : { $unset: unsetPayload }), $setOnInsert: { provider: input.provider, sourceId: input.sourceId, + ...(input.tenantId ? { tenantId: input.tenantId } : {}), }, }, { upsert: true, new: true, setDefaultsOnInsert: true }, @@ -206,6 +221,7 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { provider: SkillSyncProvider; lockOwner: string; leaseMs: number; + tenantId?: string; }): Promise { const Status = mongoose.models.SkillSyncStatus as Model; const now = new Date(); @@ -213,6 +229,7 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { const existing = await Status.findOne({ provider: params.provider, sourceId: LOCK_SOURCE_ID, + ...tenantStatusCondition(params.tenantId), }).lean(); if (existing?.lockOwner && existing.lockExpiresAt && existing.lockExpiresAt > now) { return false; @@ -222,6 +239,7 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { await Status.create({ provider: params.provider, sourceId: LOCK_SOURCE_ID, + ...(params.tenantId ? { tenantId: params.tenantId } : {}), status: 'running', lockOwner: params.lockOwner, lockExpiresAt, @@ -240,6 +258,7 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { { provider: params.provider, sourceId: LOCK_SOURCE_ID, + ...tenantStatusCondition(params.tenantId), $or: [{ lockExpiresAt: { $exists: false } }, { lockExpiresAt: { $lte: now } }], }, { @@ -252,6 +271,7 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { $setOnInsert: { provider: params.provider, sourceId: LOCK_SOURCE_ID, + ...(params.tenantId ? { tenantId: params.tenantId } : {}), }, }, { upsert: true, new: true, setDefaultsOnInsert: true }, @@ -269,6 +289,7 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { provider: SkillSyncProvider; lockOwner: string; leaseMs: number; + tenantId?: string; }): Promise { const Status = mongoose.models.SkillSyncStatus as Model; const now = new Date(); @@ -276,6 +297,7 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { { provider: params.provider, sourceId: LOCK_SOURCE_ID, + ...tenantStatusCondition(params.tenantId), lockOwner: params.lockOwner, lockExpiresAt: { $gt: now }, }, @@ -293,10 +315,16 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { async function releaseSkillSyncLock(params: { provider: SkillSyncProvider; lockOwner: string; + tenantId?: string; }): Promise { const Status = mongoose.models.SkillSyncStatus as Model; await Status.updateOne( - { provider: params.provider, sourceId: LOCK_SOURCE_ID, lockOwner: params.lockOwner }, + { + provider: params.provider, + sourceId: LOCK_SOURCE_ID, + ...tenantStatusCondition(params.tenantId), + lockOwner: params.lockOwner, + }, { $set: { status: 'idle', diff --git a/packages/data-schemas/src/migrations/tenantIndexes.spec.ts b/packages/data-schemas/src/migrations/tenantIndexes.spec.ts index a2fb2fef01..986adb5ab8 100644 --- a/packages/data-schemas/src/migrations/tenantIndexes.spec.ts +++ b/packages/data-schemas/src/migrations/tenantIndexes.spec.ts @@ -113,6 +113,11 @@ describe('dropSupersededTenantIndexes', () => { { idOnTheSource: 1, source: 1 }, { unique: true, name: 'idOnTheSource_1_source_1' }, ); + + await db.createCollection('skillsyncstatuses'); + await db + .collection('skillsyncstatuses') + .createIndex({ provider: 1, sourceId: 1 }, { unique: true, name: 'provider_1_sourceId_1' }); }); it('drops all superseded indexes', async () => { @@ -160,6 +165,13 @@ describe('dropSupersededTenantIndexes', () => { expect(indexNames).not.toContain('conversationId_1_user_1'); }); + + it('old skill sync status unique index is gone', async () => { + const indexes = await mongoose.connection.db!.collection('skillsyncstatuses').indexes(); + const indexNames = indexes.map((idx) => idx.name); + + expect(indexNames).not.toContain('provider_1_sourceId_1'); + }); }); describe('multi-tenant writes after migration', () => { @@ -287,6 +299,7 @@ describe('dropSupersededTenantIndexes', () => { 'mcpservers', 'files', 'groups', + 'skillsyncstatuses', ]; for (const col of expectedCollections) { diff --git a/packages/data-schemas/src/migrations/tenantIndexes.ts b/packages/data-schemas/src/migrations/tenantIndexes.ts index e511bb1736..9a84997c5c 100644 --- a/packages/data-schemas/src/migrations/tenantIndexes.ts +++ b/packages/data-schemas/src/migrations/tenantIndexes.ts @@ -35,6 +35,7 @@ const SUPERSEDED_INDEXES: Record = { mcpservers: ['serverName_1'], files: ['filename_1_conversationId_1_context_1'], groups: ['idOnTheSource_1_source_1'], + skillsyncstatuses: ['provider_1_sourceId_1'], }; interface MigrationResult { diff --git a/packages/data-schemas/src/models/skillSyncStatus.ts b/packages/data-schemas/src/models/skillSyncStatus.ts index 64d0dc3cc2..408d8e7a18 100644 --- a/packages/data-schemas/src/models/skillSyncStatus.ts +++ b/packages/data-schemas/src/models/skillSyncStatus.ts @@ -2,8 +2,8 @@ import skillSyncStatusSchema from '~/schema/skillSyncStatus'; import type { ISkillSyncStatusDocument } from '~/types/skillSync'; export function createSkillSyncStatusModel(mongoose: typeof import('mongoose')) { - // GitHub skill sync status is intentionally app-wide in v1, matching the - // shared synced skills and the single scheduler lock across app processes. + // GitHub skill sync status supports app-wide YAML sources and tenant-scoped + // resolved config sources from admin overrides. return ( mongoose.models.SkillSyncStatus || mongoose.model('SkillSyncStatus', skillSyncStatusSchema) diff --git a/packages/data-schemas/src/schema/skillSyncStatus.ts b/packages/data-schemas/src/schema/skillSyncStatus.ts index 3a611fd7b5..0413f9d02e 100644 --- a/packages/data-schemas/src/schema/skillSyncStatus.ts +++ b/packages/data-schemas/src/schema/skillSyncStatus.ts @@ -15,6 +15,10 @@ const skillSyncStatusSchema: Schema = new Schema( maxlength: 128, index: true, }, + tenantId: { + type: String, + index: true, + }, status: { type: String, enum: ['idle', 'running', 'succeeded', 'failed', 'skipped'], @@ -88,6 +92,6 @@ const skillSyncStatusSchema: Schema = new Schema( }, ); -skillSyncStatusSchema.index({ provider: 1, sourceId: 1 }, { unique: true }); +skillSyncStatusSchema.index({ provider: 1, sourceId: 1, tenantId: 1 }, { unique: true }); export default skillSyncStatusSchema; diff --git a/packages/data-schemas/src/types/skillSync.ts b/packages/data-schemas/src/types/skillSync.ts index e3e85fe579..edd3bf5704 100644 --- a/packages/data-schemas/src/types/skillSync.ts +++ b/packages/data-schemas/src/types/skillSync.ts @@ -19,6 +19,7 @@ export interface ISkillSyncCredentialDocument extends ISkillSyncCredential, Docu export interface ISkillSyncStatus { provider: SkillSyncProvider; sourceId: string; + tenantId?: string; status: SkillSyncRunStatus; credentialKey?: string; owner?: string;