diff --git a/packages/api/src/skills/sync/github.spec.ts b/packages/api/src/skills/sync/github.spec.ts index e0acc27305..ed2f41d342 100644 --- a/packages/api/src/skills/sync/github.spec.ts +++ b/packages/api/src/skills/sync/github.spec.ts @@ -308,17 +308,18 @@ describe('createGitHubSkillSyncRunner', () => { it('scopes mirror cleanup to the current source and deletes only its absent upstream skills', async () => { const keptId = new Types.ObjectId(); const staleId = new Types.ObjectId(); - const existingSkill = (upstreamId: string, _id: Types.ObjectId) => - ({ - ...makeSkill({ - name: 'research', - description: 'Research things', - author: new Types.ObjectId(), - source: 'github', - sourceMetadata: { provider: 'github', sourceId: 'librechat-skills', upstreamId }, - } as CreateSkillInput), - _id, - }) as ISkill & { _id: Types.ObjectId }; + const existingSkill = (upstreamId: string, _id: Types.ObjectId) => { + const skill = makeSkill({ + name: 'research', + description: 'Research things', + author: new Types.ObjectId(), + authorName: 'GitHub Sync', + source: 'github', + sourceMetadata: { provider: 'github', sourceId: 'librechat-skills', upstreamId }, + }); + skill._id = _id; + return skill; + }; const listSkillsBySource = jest.fn(async () => [ existingSkill('librechat-skills:LibreChat/skills:skills/research', keptId), existingSkill('librechat-skills:LibreChat/skills:skills/removed', staleId), diff --git a/packages/api/src/skills/sync/github.ts b/packages/api/src/skills/sync/github.ts index 8ce41eed2a..af333c6be7 100644 --- a/packages/api/src/skills/sync/github.ts +++ b/packages/api/src/skills/sync/github.ts @@ -797,6 +797,22 @@ async function commitRemoteSkill( return { skill: created.skill, created: true }; } +/** + * File sync bumps the parent skill's `version` (via file upserts/deletes) but + * never changes its authored content, so we must re-read to get past our own + * version bumps. A plain re-read would also silently accept and overwrite a + * concurrent external edit; compare the refreshed content against the pre-sync + * snapshot and treat a changed body/name/description/always-apply as a conflict. + */ +function hasExternalSkillEdit(before: ISkill, after: ISkill): boolean { + return ( + before.body !== after.body || + before.name !== after.name || + before.description !== after.description || + (before.alwaysApply ?? false) !== (after.alwaysApply ?? false) + ); +} + async function commitExistingRemoteSkillAfterFileSync( deps: GitHubSkillSyncDeps, prepared: PreparedExistingRemoteSkill, @@ -808,6 +824,12 @@ async function commitExistingRemoteSkillAfterFileSync( `Previously synced skill "${prepared.existing.name}" was removed`, ); } + if (hasExternalSkillEdit(prepared.existing, refreshed)) { + throw new SkillSyncError( + 'SKILL_CONFLICT', + `Skill "${prepared.existing.name}" was modified during sync`, + ); + } return commitRemoteSkill(deps, { ...prepared, existing: refreshed }); }