mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-10 16:23:44 +00:00
fix: Scope resolved skill sync by tenant
This commit is contained in:
parent
d7f8dba177
commit
4251fa4903
15 changed files with 390 additions and 42 deletions
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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 }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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';
|
||||
|
|
|
|||
|
|
@ -22,7 +22,8 @@ type AdminSkillsRequest = Request & {
|
|||
};
|
||||
|
||||
export type AdminSkillSyncDeps = {
|
||||
runner: GitHubSkillSyncRunner;
|
||||
runner?: GitHubSkillSyncRunner;
|
||||
getRunner?: (req: Request) => GitHubSkillSyncRunner;
|
||||
upsertCredential: (input: UpsertSkillSyncCredentialInput) => Promise<SkillSyncCredentialSummary>;
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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 () => {
|
||||
|
|
|
|||
|
|
@ -140,13 +140,19 @@ export type GitHubSkillSyncDeps = {
|
|||
provider: SkillSyncProvider;
|
||||
lockOwner: string;
|
||||
leaseMs: number;
|
||||
tenantId?: string;
|
||||
}) => Promise<boolean>;
|
||||
refreshLock: (params: {
|
||||
provider: SkillSyncProvider;
|
||||
lockOwner: string;
|
||||
leaseMs: number;
|
||||
tenantId?: string;
|
||||
}) => Promise<boolean>;
|
||||
releaseLock: (params: { provider: SkillSyncProvider; lockOwner: string }) => Promise<void>;
|
||||
releaseLock: (params: {
|
||||
provider: SkillSyncProvider;
|
||||
lockOwner: string;
|
||||
tenantId?: string;
|
||||
}) => Promise<void>;
|
||||
createSkill: (data: CreateSkillInput) => Promise<CreateSkillResult>;
|
||||
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 });
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<ISkillSyncStatus | null> {
|
||||
const Status = mongoose.models.SkillSyncStatus as Model<ISkillSyncStatusDocument>;
|
||||
const row = await Status.findOne({ provider, sourceId }).lean<ISkillSyncStatus | null>();
|
||||
const row = await Status.findOne({
|
||||
provider,
|
||||
sourceId,
|
||||
...tenantStatusCondition(tenantId),
|
||||
}).lean<ISkillSyncStatus | null>();
|
||||
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<boolean> {
|
||||
const Status = mongoose.models.SkillSyncStatus as Model<ISkillSyncStatusDocument>;
|
||||
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<ISkillSyncStatus | null>();
|
||||
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<boolean> {
|
||||
const Status = mongoose.models.SkillSyncStatus as Model<ISkillSyncStatusDocument>;
|
||||
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<void> {
|
||||
const Status = mongoose.models.SkillSyncStatus as Model<ISkillSyncStatusDocument>;
|
||||
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',
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ const SUPERSEDED_INDEXES: Record<string, string[]> = {
|
|||
mcpservers: ['serverName_1'],
|
||||
files: ['filename_1_conversationId_1_context_1'],
|
||||
groups: ['idOnTheSource_1_source_1'],
|
||||
skillsyncstatuses: ['provider_1_sourceId_1'],
|
||||
};
|
||||
|
||||
interface MigrationResult {
|
||||
|
|
|
|||
|
|
@ -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<ISkillSyncStatusDocument>('SkillSyncStatus', skillSyncStatusSchema)
|
||||
|
|
|
|||
|
|
@ -15,6 +15,10 @@ const skillSyncStatusSchema: Schema<ISkillSyncStatusDocument> = 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<ISkillSyncStatusDocument> = new Schema(
|
|||
},
|
||||
);
|
||||
|
||||
skillSyncStatusSchema.index({ provider: 1, sourceId: 1 }, { unique: true });
|
||||
skillSyncStatusSchema.index({ provider: 1, sourceId: 1, tenantId: 1 }, { unique: true });
|
||||
|
||||
export default skillSyncStatusSchema;
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue