fix: Repair tsc error and guard external edits in github skill sync

- Fix TS2352 in github.spec mirror-cleanup test: build the existing-skill mock via makeSkill with authorName instead of an under-typed 'as CreateSkillInput' cast (this was the failing TypeScript CI check on f00ce3c5a).
- 808: commitExistingRemoteSkillAfterFileSync re-reads to clear our own file-sync version bumps, but now compares refreshed content against the pre-sync snapshot (body/name/description/always-apply) and throws SKILL_CONFLICT on a concurrent external edit instead of overwriting it.
This commit is contained in:
Danny Avila 2026-05-30 13:15:56 -04:00
parent 204a8dfbca
commit 7fedb5c027
No known key found for this signature in database
GPG key ID: BF31EEB2C5CA0956
2 changed files with 34 additions and 11 deletions

View file

@ -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),

View file

@ -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 });
}