diff --git a/api/server/services/Skills/sync.js b/api/server/services/Skills/sync.js index 2391e6784d..46d985e4c9 100644 --- a/api/server/services/Skills/sync.js +++ b/api/server/services/Skills/sync.js @@ -48,6 +48,7 @@ function createRunner() { listStatuses: db.listSkillSyncStatuses, upsertStatus: db.upsertSkillSyncStatus, tryAcquireLock: db.tryAcquireSkillSyncLock, + refreshLock: db.refreshSkillSyncLock, releaseLock: db.releaseSkillSyncLock, createSkill: db.createSkill, updateSkill: db.updateSkill, diff --git a/packages/api/src/skills/__tests__/import.test.ts b/packages/api/src/skills/__tests__/import.test.ts index d577ee1556..57e3441474 100644 --- a/packages/api/src/skills/__tests__/import.test.ts +++ b/packages/api/src/skills/__tests__/import.test.ts @@ -128,6 +128,13 @@ describe('parseFrontmatter', () => { expect(result.invalidBooleans).toEqual(['always-apply']); }); + it('flags legacy YAML boolean aliases as invalid', () => { + const raw = `---\nname: n\ndescription: d\nalways-apply: on\n---\n\nbody`; + const result = parseFrontmatter(raw); + expect(result.alwaysApply).toBeUndefined(); + expect(result.invalidBooleans).toEqual(['always-apply']); + }); + it('does not flag always-apply when the key is absent', () => { const raw = `---\nname: n\ndescription: d\n---\n\nbody`; expect(parseFrontmatter(raw).invalidBooleans).toEqual([]); diff --git a/packages/api/src/skills/parse.ts b/packages/api/src/skills/parse.ts index 2332d518b1..c5b5f08b30 100644 --- a/packages/api/src/skills/parse.ts +++ b/packages/api/src/skills/parse.ts @@ -30,6 +30,29 @@ function getCaseInsensitive(frontmatter: Record, key: string): return entry?.[1]; } +function getRawFrontmatterValue(block: string, key: string): string | undefined { + const escapedKey = key.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const pattern = new RegExp(`^\\s*${escapedKey}\\s*:\\s*(.*)$`, 'i'); + const line = block.split('\n').find((candidate) => pattern.test(candidate)); + const match = line?.match(pattern); + return match?.[1]; +} + +function stripInlineComment(value: string): string { + let quote: '"' | "'" | null = null; + for (let i = 0; i < value.length; i++) { + const char = value[i]; + if ((char === '"' || char === "'") && (!quote || quote === char)) { + quote = quote ? null : char; + continue; + } + if (char === '#' && !quote) { + return value.slice(0, i).trim(); + } + } + return value.trim(); +} + function normalizeFrontmatterKeys(frontmatter: Record): Record { return Object.entries(frontmatter).reduce>((acc, [key, value]) => { acc[key.toLowerCase()] = value; @@ -37,9 +60,10 @@ function normalizeFrontmatterKeys(frontmatter: Record): Record< }, {}); } -function parseBoolean(value: unknown): boolean | undefined { +function parseBoolean(value: unknown, rawValue?: string): boolean | undefined { + const raw = rawValue === undefined ? undefined : stripInlineComment(rawValue).toLowerCase(); if (typeof value === 'boolean') { - return value; + return raw === 'true' || raw === 'false' ? value : undefined; } if (typeof value !== 'string') { return undefined; @@ -75,6 +99,7 @@ export function parseSkillMarkdown(raw: string): ParsedSkillMarkdown { const descriptionValue = getCaseInsensitive(frontmatter, 'description'); const whenToUseValue = getCaseInsensitive(frontmatter, 'when-to-use'); const alwaysApplyValue = getCaseInsensitive(frontmatter, 'always-apply'); + const rawAlwaysApplyValue = getRawFrontmatterValue(block, 'always-apply'); const name = typeof nameValue === 'string' ? nameValue : ''; let description = ''; if (typeof descriptionValue === 'string') { @@ -85,7 +110,7 @@ export function parseSkillMarkdown(raw: string): ParsedSkillMarkdown { let alwaysApply: boolean | undefined; const invalidBooleans: string[] = []; if (alwaysApplyValue !== undefined) { - alwaysApply = parseBoolean(alwaysApplyValue); + alwaysApply = parseBoolean(alwaysApplyValue, rawAlwaysApplyValue); if (alwaysApply === undefined && alwaysApplyValue !== null && alwaysApplyValue !== '') { invalidBooleans.push('always-apply'); } diff --git a/packages/api/src/skills/sync/github.spec.ts b/packages/api/src/skills/sync/github.spec.ts index a583310194..e3c20fa892 100644 --- a/packages/api/src/skills/sync/github.spec.ts +++ b/packages/api/src/skills/sync/github.spec.ts @@ -145,6 +145,7 @@ function createDeps( return status; }), tryAcquireLock: jest.fn(async () => true), + refreshLock: jest.fn(async () => true), releaseLock: jest.fn(async () => undefined), createSkill: jest.fn(async (input: CreateSkillInput): Promise => { return { skill: makeSkill(input), warnings: [] }; diff --git a/packages/api/src/skills/sync/github.ts b/packages/api/src/skills/sync/github.ts index 6f3f468dd3..77a1a7e102 100644 --- a/packages/api/src/skills/sync/github.ts +++ b/packages/api/src/skills/sync/github.ts @@ -99,6 +99,11 @@ export type GitHubSkillSyncDeps = { lockOwner: string; leaseMs: number; }) => Promise; + refreshLock: (params: { + provider: SkillSyncProvider; + lockOwner: string; + leaseMs: number; + }) => Promise; releaseLock: (params: { provider: SkillSyncProvider; lockOwner: string }) => Promise; createSkill: (data: CreateSkillInput) => Promise; updateSkill: (params: { @@ -879,6 +884,22 @@ export function createGitHubSkillSyncRunner(deps: GitHubSkillSyncDeps) { sources: statuses, }; } + const refreshTimer = setInterval( + () => { + deps + .refreshLock({ provider: PROVIDER, lockOwner, leaseMs: LOCK_LEASE_MS }) + .then((refreshed) => { + if (!refreshed) { + logger.warn('[GitHubSkillSync] Failed to refresh active sync lock'); + } + }) + .catch((error) => { + logger.error('[GitHubSkillSync] Failed to refresh active sync lock:', error); + }); + }, + Math.max(60_000, Math.floor(LOCK_LEASE_MS / 3)), + ); + refreshTimer.unref?.(); try { const sources: ISkillSyncStatus[] = []; for (const source of github.sources) { @@ -890,6 +911,7 @@ export function createGitHubSkillSyncRunner(deps: GitHubSkillSyncDeps) { sources, }; } finally { + clearInterval(refreshTimer); await deps.releaseLock({ provider: PROVIDER, lockOwner }); } } diff --git a/packages/data-schemas/src/methods/skillSync.spec.ts b/packages/data-schemas/src/methods/skillSync.spec.ts index 8f10be3a0d..0ceac31d42 100644 --- a/packages/data-schemas/src/methods/skillSync.spec.ts +++ b/packages/data-schemas/src/methods/skillSync.spec.ts @@ -79,6 +79,13 @@ describe('createSkillSyncMethods', () => { leaseMs: 60_000, }), ).resolves.toBe(true); + await expect( + methods.tryAcquireSkillSyncLock({ + provider: 'github', + lockOwner: 'worker-a', + leaseMs: 60_000, + }), + ).resolves.toBe(false); await expect( methods.tryAcquireSkillSyncLock({ provider: 'github', @@ -86,6 +93,13 @@ describe('createSkillSyncMethods', () => { leaseMs: 60_000, }), ).resolves.toBe(false); + await expect( + methods.refreshSkillSyncLock({ + provider: 'github', + lockOwner: 'worker-a', + leaseMs: 60_000, + }), + ).resolves.toBe(true); await methods.releaseSkillSyncLock({ provider: 'github', lockOwner: 'worker-a' }); await expect( methods.tryAcquireSkillSyncLock({ @@ -95,4 +109,26 @@ describe('createSkillSyncMethods', () => { }), ).resolves.toBe(true); }); + + it('clears stale source errors after a successful sync status update', async () => { + await methods.upsertSkillSyncStatus({ + provider: 'github', + sourceId: 'librechat-skills', + status: 'failed', + errorCode: 'SKILL_PARSE_FAILED', + errorMessage: 'bad frontmatter', + }); + + const success = await methods.upsertSkillSyncStatus({ + provider: 'github', + sourceId: 'librechat-skills', + status: 'succeeded', + syncedSkillCount: 1, + syncedFileCount: 2, + }); + + expect(success.status).toBe('succeeded'); + expect(success.errorCode).toBeUndefined(); + expect(success.errorMessage).toBeUndefined(); + }); }); diff --git a/packages/data-schemas/src/methods/skillSync.ts b/packages/data-schemas/src/methods/skillSync.ts index 53e2304459..bcd97694ea 100644 --- a/packages/data-schemas/src/methods/skillSync.ts +++ b/packages/data-schemas/src/methods/skillSync.ts @@ -175,8 +175,6 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { paths: input.paths, startedAt: input.startedAt, finishedAt: input.finishedAt, - errorCode: input.errorCode, - errorMessage: input.errorMessage, syncedSkillCount: input.syncedSkillCount ?? 0, syncedFileCount: input.syncedFileCount ?? 0, deletedSkillCount: input.deletedSkillCount ?? 0, @@ -184,10 +182,16 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { ...(success ? { lastSuccessAt: input.finishedAt ?? now } : {}), ...(failure ? { lastFailureAt: input.finishedAt ?? now } : {}), }; + if (failure) { + setPayload.errorCode = input.errorCode; + setPayload.errorMessage = input.errorMessage; + } + const unsetPayload = failure ? {} : { errorCode: '', errorMessage: '' }; const row = await Status.findOneAndUpdate( { provider: input.provider, sourceId: input.sourceId }, { $set: setPayload, + ...(failure ? {} : { $unset: unsetPayload }), $setOnInsert: { provider: input.provider, sourceId: input.sourceId, @@ -210,12 +214,7 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { provider: params.provider, sourceId: LOCK_SOURCE_ID, }).lean(); - if ( - existing?.lockOwner && - existing.lockOwner !== params.lockOwner && - existing.lockExpiresAt && - existing.lockExpiresAt > now - ) { + if (existing?.lockOwner && existing.lockExpiresAt && existing.lockExpiresAt > now) { return false; } if (!existing) { @@ -241,11 +240,7 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { { provider: params.provider, sourceId: LOCK_SOURCE_ID, - $or: [ - { lockExpiresAt: { $exists: false } }, - { lockExpiresAt: { $lte: now } }, - { lockOwner: params.lockOwner }, - ], + $or: [{ lockExpiresAt: { $exists: false } }, { lockExpiresAt: { $lte: now } }], }, { $set: { @@ -270,6 +265,31 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { } } + async function refreshSkillSyncLock(params: { + provider: SkillSyncProvider; + lockOwner: string; + leaseMs: number; + }): Promise { + const Status = mongoose.models.SkillSyncStatus as Model; + const now = new Date(); + const row = await Status.findOneAndUpdate( + { + provider: params.provider, + sourceId: LOCK_SOURCE_ID, + lockOwner: params.lockOwner, + lockExpiresAt: { $gt: now }, + }, + { + $set: { + status: 'running', + lockExpiresAt: new Date(now.getTime() + params.leaseMs), + }, + }, + { new: true }, + ).lean(); + return Boolean(row); + } + async function releaseSkillSyncLock(params: { provider: SkillSyncProvider; lockOwner: string; @@ -300,6 +320,7 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')) { getSkillSyncStatus, upsertSkillSyncStatus, tryAcquireSkillSyncLock, + refreshSkillSyncLock, releaseSkillSyncLock, }; }