feat: Trigger skill sync from resolved config

This commit is contained in:
Danny Avila 2026-06-01 19:51:22 -04:00
parent d79712bf5d
commit d7f8dba177
7 changed files with 355 additions and 24 deletions

View file

@ -40,6 +40,7 @@ const {
} = require('~/server/services/PermissionService');
const { getStrategyFunctions } = require('~/server/services/Files/strategies');
const { createFileLimiters } = require('~/server/middleware/limiters/uploadLimiters');
const { maybeRunGitHubSkillSyncForRequest } = require('~/server/services/Skills/sync');
const configMiddleware = require('~/server/middleware/config/app');
const { getFileStrategy } = require('~/server/utils/getFileStrategy');
@ -272,6 +273,14 @@ async function uploadFileHandler(req, res) {
// ---------------------------------------------------------------------------
// Routes
// ---------------------------------------------------------------------------
async function maybeStartRequestSkillSync(req, _res, next) {
try {
await maybeRunGitHubSkillSyncForRequest(req);
} catch (error) {
logger.error('[GET /skills] Failed to start request-scoped skill sync:', error);
}
next();
}
// Import: accepts .md / .zip / .skill via multipart
router.post(
@ -284,7 +293,7 @@ router.post(
importHandler,
);
router.get('/', handlers.list);
router.get('/', maybeStartRequestSkillSync, handlers.list);
router.post('/', checkSkillCreate, handlers.create);
router.get(

View file

@ -33,6 +33,7 @@ const {
} = require('librechat-data-provider');
let mockFileConfig;
const mockMaybeRunGitHubSkillSyncForRequest = jest.fn(async () => false);
jest.mock('~/server/services/Config', () => ({
getCachedTools: jest.fn().mockResolvedValue({}),
@ -68,6 +69,10 @@ jest.mock('~/server/utils/getFileStrategy', () => ({
getFileStrategy: jest.fn().mockReturnValue('local'),
}));
jest.mock('~/server/services/Skills/sync', () => ({
maybeRunGitHubSkillSyncForRequest: mockMaybeRunGitHubSkillSyncForRequest,
}));
jest.mock('~/models', () => {
const mongoose = require('mongoose');
const { createMethods } = require('@librechat/data-schemas');
@ -152,6 +157,7 @@ afterEach(async () => {
await AclEntry.deleteMany({});
currentTestUser = testUsers.owner;
mockFileConfig = undefined;
mockMaybeRunGitHubSkillSyncForRequest.mockClear();
});
afterAll(async () => {
@ -409,6 +415,12 @@ describe('Skill routes', () => {
setTestUser(testUsers.owner);
const res = await request(app).get('/api/skills');
expect(res.status).toBe(200);
expect(mockMaybeRunGitHubSkillSyncForRequest).toHaveBeenCalledWith(
expect.objectContaining({
config: expect.objectContaining({ fileStrategy: 'local' }),
user: expect.objectContaining({ id: testUsers.owner._id.toString() }),
}),
);
expect(res.body.skills.length).toBe(1);
expect(res.body.skills[0].name).toBe('mine-skill');
});

View file

@ -1,20 +1,22 @@
const { FileContext } = require('librechat-data-provider');
const { FileContext, skillSyncConfigSchema } = require('librechat-data-provider');
const {
getStorageMetadata,
createGitHubSkillSyncRunner,
startGitHubSkillSyncScheduler,
} = require('@librechat/api');
const { runAsSystem } = require('@librechat/data-schemas');
const { logger, runAsSystem } = require('@librechat/data-schemas');
const db = require('~/models');
const { getAppConfig } = require('~/server/services/Config');
const { getStrategyFunctions } = require('~/server/services/Files/strategies');
const { getFileStrategy } = require('~/server/utils/getFileStrategy');
const SYSTEM_USER_ID = '000000000000000000000000';
const REQUEST_SYNC_MIN_INTERVAL_MS = 5 * 60 * 1000;
let appConfigRef;
let runner;
let scheduler;
const requestSyncsInFlight = new Set();
async function loadCurrentAppConfig() {
try {
@ -29,13 +31,13 @@ async function loadCurrentAppConfig() {
}
}
async function getSyncConfig() {
const appConfig = await loadCurrentAppConfig();
async function getSyncConfig(loadAppConfig = loadCurrentAppConfig) {
const appConfig = await loadAppConfig();
return appConfig?.skillSync;
}
async function resolveSkillStorage({ isImage = false } = {}) {
const appConfig = await loadCurrentAppConfig();
async function resolveSkillStorage({ isImage = false, loadAppConfig = loadCurrentAppConfig } = {}) {
const appConfig = await loadAppConfig();
const source = getFileStrategy(appConfig, { context: FileContext.skill_file, isImage });
const strategy = getStrategyFunctions(source);
if (!strategy.saveBuffer) {
@ -44,8 +46,8 @@ async function resolveSkillStorage({ isImage = false } = {}) {
return { source, saveBuffer: strategy.saveBuffer };
}
async function getSyntheticReq({ userId = SYSTEM_USER_ID, tenantId } = {}) {
const appConfig = await loadCurrentAppConfig();
async function getSyntheticReq({ userId = SYSTEM_USER_ID, tenantId, loadAppConfig } = {}) {
const appConfig = await (loadAppConfig ?? loadCurrentAppConfig)();
return {
config: appConfig,
user: {
@ -56,9 +58,11 @@ async function getSyntheticReq({ userId = SYSTEM_USER_ID, tenantId } = {}) {
};
}
function createRunner() {
function createRunner({ getConfig, loadAppConfig } = {}) {
const resolveAppConfig = loadAppConfig ?? loadCurrentAppConfig;
const resolveConfig = getConfig ?? (() => getSyncConfig(resolveAppConfig));
const createdRunner = createGitHubSkillSyncRunner({
getConfig: getSyncConfig,
getConfig: resolveConfig,
getCredentialToken: db.getSkillSyncCredentialToken,
getCredentialSummary: db.getSkillSyncCredentialSummary,
listCredentials: db.listSkillSyncCredentials,
@ -110,7 +114,7 @@ function createRunner() {
);
},
saveBuffer: async ({ userId, buffer, fileName, basePath, isImage, tenantId }) => {
const storage = await resolveSkillStorage({ isImage });
const storage = await resolveSkillStorage({ isImage, loadAppConfig: resolveAppConfig });
const filepath = await storage.saveBuffer({
userId: userId ?? SYSTEM_USER_ID,
buffer,
@ -133,6 +137,7 @@ function createRunner() {
await getSyntheticReq({
userId: file.user?.toString?.() ?? file.user ?? SYSTEM_USER_ID,
tenantId: file.tenantId,
loadAppConfig: resolveAppConfig,
}),
file,
);
@ -144,6 +149,126 @@ function createRunner() {
};
}
function parseSkillSyncConfig(raw) {
if (!raw || typeof raw !== 'object') {
return undefined;
}
const parsed = skillSyncConfigSchema.safeParse(raw);
if (!parsed.success) {
logger.warn('[GitHubSkillSync] Ignoring invalid request-scoped skill sync config', {
issues: parsed.error.flatten(),
});
return undefined;
}
return parsed.data;
}
function isSameSkillSyncConfig(left, right) {
return JSON.stringify(left ?? null) === JSON.stringify(right ?? null);
}
function getRequestSkillSyncConfig(appConfig, user) {
const resolved = parseSkillSyncConfig(appConfig?.skillSync);
if (!resolved?.github?.enabled || resolved.github.sources.length === 0) {
return undefined;
}
const base = parseSkillSyncConfig(appConfig?.config?.skillSync);
if (isSameSkillSyncConfig(resolved, base)) {
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,
})),
},
};
}
function toTimestamp(value) {
if (!value) {
return 0;
}
if (value instanceof Date) {
return value.getTime();
}
const parsed = Date.parse(String(value));
return Number.isNaN(parsed) ? 0 : parsed;
}
function getLastAttemptAt(source) {
return Math.max(
toTimestamp(source.finishedAt),
toTimestamp(source.startedAt),
toTimestamp(source.updatedAt),
toTimestamp(source.lastSuccessAt),
toTimestamp(source.lastFailureAt),
);
}
function shouldRunRequestSync(status) {
if (!status.enabled || status.sources.length === 0) {
return false;
}
const intervalMs = Math.max(
REQUEST_SYNC_MIN_INTERVAL_MS,
(status.intervalMinutes ?? 60) * 60 * 1000,
);
const now = Date.now();
return status.sources.some((source) => {
if (source.status === 'running') {
return false;
}
const lastAttemptAt = getLastAttemptAt(source);
return !lastAttemptAt || now - lastAttemptAt >= intervalMs;
});
}
function getRequestSyncKey(config, user) {
const tenantId = user?.tenantId ?? '';
const sources = config.github.sources
.map((source) => source.id)
.sort()
.join(',');
return `${tenantId}:${sources}`;
}
async function maybeRunGitHubSkillSyncForRequest(req) {
const config = getRequestSkillSyncConfig(req.config, req.user);
if (!config) {
return false;
}
const syncKey = getRequestSyncKey(config, req.user);
if (requestSyncsInFlight.has(syncKey)) {
return false;
}
const loadAppConfig = async () => req.config;
const requestRunner = createRunner({
getConfig: async () => config,
loadAppConfig,
});
const status = await requestRunner.getStatus();
if (!shouldRunRequestSync(status)) {
return false;
}
requestSyncsInFlight.add(syncKey);
void requestRunner
.runOnce()
.catch((error) => logger.error('[GitHubSkillSync] Request-scoped sync failed:', error))
.finally(() => requestSyncsInFlight.delete(syncKey));
return true;
}
function initializeGitHubSkillSync(appConfig) {
appConfigRef = appConfig;
runner = createRunner();
@ -171,5 +296,6 @@ function stopGitHubSkillSyncScheduler() {
module.exports = {
initializeGitHubSkillSync,
getGitHubSkillSyncRunner,
maybeRunGitHubSkillSyncForRequest,
stopGitHubSkillSyncScheduler,
};

View file

@ -4,6 +4,8 @@ const mockGetFileStrategy = jest.fn();
const mockFindRoleByIdentifier = jest.fn();
const mockGrantPermission = jest.fn();
let mockRunnerDeps;
let mockRunnerStatus;
const mockCreatedRunners = [];
jest.mock('~/server/services/Config', () => ({
getAppConfig: mockGetAppConfig,
@ -12,22 +14,71 @@ jest.mock('~/server/services/Config', () => ({
jest.mock('@librechat/api', () => ({
createGitHubSkillSyncRunner: jest.fn((deps) => {
mockRunnerDeps = deps;
return {
getStatus: jest.fn(async () => deps.getConfig()),
const runner = {
getStatus: jest.fn(async () => {
if (mockRunnerStatus) {
return mockRunnerStatus;
}
const config = await deps.getConfig();
const github = config?.github ?? {};
return {
enabled: github.enabled ?? false,
intervalMinutes: github.intervalMinutes ?? 60,
runOnStartup: github.runOnStartup ?? false,
sources: (github.sources ?? []).map((source) => ({
provider: 'github',
sourceId: source.id,
status: 'idle',
owner: source.owner,
repo: source.repo,
ref: source.ref,
paths: source.paths,
syncedSkillCount: 0,
syncedFileCount: 0,
deletedSkillCount: 0,
deletedFileCount: 0,
})),
credentials: [],
};
}),
runOnce: jest.fn(async () => deps.getConfig()),
};
mockCreatedRunners.push({ deps, runner });
return runner;
}),
getStorageMetadata: jest.fn(() => ({})),
startGitHubSkillSyncScheduler: jest.fn(() => ({ stop: jest.fn() })),
}));
jest.mock('@librechat/data-schemas', () => ({
logger: {
error: jest.fn(),
warn: jest.fn(),
},
runAsSystem: jest.fn((fn) => fn()),
}));
jest.mock('~/models', () => ({
findRoleByIdentifier: mockFindRoleByIdentifier,
grantPermission: mockGrantPermission,
getSkillSyncCredentialToken: jest.fn(),
getSkillSyncCredentialSummary: jest.fn(),
listSkillSyncCredentials: jest.fn(async () => []),
listSkillSyncStatuses: jest.fn(async () => []),
upsertSkillSyncStatus: jest.fn(),
tryAcquireSkillSyncLock: jest.fn(),
refreshSkillSyncLock: jest.fn(),
releaseSkillSyncLock: jest.fn(),
createSkill: jest.fn(),
updateSkill: jest.fn(),
getSkillById: jest.fn(),
findSkillBySourceIdentity: jest.fn(),
listSkillsBySource: jest.fn(),
listSkillFiles: jest.fn(),
getSkillFileByPath: jest.fn(),
upsertSkillFile: jest.fn(),
deleteSkillFile: jest.fn(),
deleteSkill: jest.fn(),
}));
jest.mock('~/server/services/Files/strategies', () => ({
getStrategyFunctions: mockGetStrategyFunctions,
@ -43,6 +94,8 @@ describe('GitHub skill sync service', () => {
mockFindRoleByIdentifier.mockReset();
mockGrantPermission.mockReset();
mockRunnerDeps = undefined;
mockRunnerStatus = undefined;
mockCreatedRunners.length = 0;
});
it('resolves sync config from fresh base app config for runner operations', async () => {
@ -99,6 +152,125 @@ describe('GitHub skill sync service', () => {
expect(mockGetAppConfig).toHaveBeenCalledWith({ baseOnly: true });
});
it('starts a request-scoped sync for resolved admin skillSync config', 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 started = await service.maybeRunGitHubSkillSyncForRequest({
config: { skillSync, config: {} },
user: { id: 'user-1', tenantId: 'tenant-a' },
});
const requestRunner = mockCreatedRunners[0].runner;
const requestConfig = await mockCreatedRunners[0].deps.getConfig();
expect(started).toBe(true);
expect(requestRunner.runOnce).toHaveBeenCalledTimes(1);
expect(requestConfig.github.runOnStartup).toBe(false);
expect(requestConfig.github.sources[0]).toEqual(
expect.objectContaining({
id: 'tenant-skills',
tenantId: 'tenant-a',
}),
);
});
it('does not start a request-scoped sync for base YAML skillSync config', async () => {
const skillSync = {
github: {
enabled: true,
intervalMinutes: 60,
runOnStartup: true,
sources: [
{
id: 'base-skills',
owner: 'LibreChat',
repo: 'skills',
ref: 'main',
paths: ['skills'],
token: '${GITHUB_SKILLS_TOKEN}',
},
],
},
};
const service = require('./sync');
const started = await service.maybeRunGitHubSkillSyncForRequest({
config: { skillSync, config: { skillSync } },
user: { id: 'user-1', tenantId: 'tenant-a' },
});
expect(started).toBe(false);
expect(mockCreatedRunners).toHaveLength(0);
});
it('does not start a request-scoped sync when the configured source is already running', 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(),
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(false);
expect(mockCreatedRunners[0].runner.runOnce).not.toHaveBeenCalled();
});
it('uses the file owner when deleting synced files from storage', async () => {
const deleteFile = jest.fn(async () => undefined);
const ownerId = '507f1f77bcf86cd799439011';

View file

@ -191,7 +191,7 @@ describe('createAdminConfigHandlers', () => {
expect(savedOverrides.interface).toEqual({ modelSelect: false });
});
it('strips base-only sections from admin overrides', async () => {
it('preserves skillSync sections in admin overrides', async () => {
const { handlers, deps } = createHandlers({
upsertConfig: jest.fn().mockResolvedValue({ _id: 'c1', configVersion: 1 }),
});
@ -210,7 +210,7 @@ describe('createAdminConfigHandlers', () => {
expect(res.statusCode).toBe(201);
const savedOverrides = deps.upsertConfig.mock.calls[0][3];
expect(savedOverrides.skillSync).toBeUndefined();
expect(savedOverrides.skillSync).toEqual({ github: { enabled: true } });
expect(savedOverrides.interface).toEqual({ modelSelect: false });
});
@ -358,7 +358,7 @@ describe('createAdminConfigHandlers', () => {
expect(deps.unsetConfigField).not.toHaveBeenCalled();
});
it('returns 200 no-op for base-only field path', async () => {
it('allows deleting skillSync field paths', async () => {
const { handlers, deps } = createHandlers();
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
@ -369,8 +369,11 @@ describe('createAdminConfigHandlers', () => {
await handlers.deleteConfigField(req, res);
expect(res.statusCode).toBe(200);
expect(res.body!.message).toBeDefined();
expect(deps.unsetConfigField).not.toHaveBeenCalled();
expect(deps.unsetConfigField).toHaveBeenCalledWith(
'role',
'admin',
'skillSync.github.enabled',
);
});
it('allows deleting interface UI field paths', async () => {
@ -452,7 +455,7 @@ describe('createAdminConfigHandlers', () => {
expect(patchedFields['interface.prompts']).toBeUndefined();
});
it('strips base-only field entries from patches', async () => {
it('preserves skillSync field entries in patches', async () => {
const { handlers, deps } = createHandlers();
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
@ -469,7 +472,7 @@ describe('createAdminConfigHandlers', () => {
expect(res.statusCode).toBe(200);
const patchedFields = deps.patchConfigFields.mock.calls[0][3];
expect(patchedFields['skillSync.github.enabled']).toBeUndefined();
expect(patchedFields['skillSync.github.enabled']).toBe(true);
expect(patchedFields['interface.modelSelect']).toBe(false);
});

View file

@ -18,7 +18,7 @@ import { REFILL_INTERVAL_UNITS } from './balance';
export const defaultSocialLogins = ['google', 'facebook', 'openid', 'github', 'discord', 'saml'];
export const BASE_ONLY_CONFIG_SECTIONS = ['skillSync'] as const;
export const BASE_ONLY_CONFIG_SECTIONS = [] as const;
export const defaultRetrievalModels = [
'gpt-4o',

View file

@ -342,7 +342,7 @@ describe('mergeConfigOverrides', () => {
expect(iface.parameters).toBe(true);
});
it('ignores base-only config sections from DB overrides', () => {
it('merges skillSync config sections from DB overrides', () => {
const base = {
skillSync: {
github: {
@ -389,7 +389,16 @@ describe('mergeConfigOverrides', () => {
const result = mergeConfigOverrides(base, configs);
expect(result.skillSync).toEqual(base.skillSync);
expect(result.skillSync?.github?.enabled).toBe(false);
expect(result.skillSync?.github?.sources).toEqual([
{
id: 'override-source',
owner: 'other',
repo: 'skills',
paths: ['skills'],
token: '${OTHER_TOKEN}',
},
]);
expect(result.interfaceConfig?.modelSelect).toBe(false);
});