diff --git a/server/modules/providers/shared/skills/skills.provider.ts b/server/modules/providers/shared/skills/skills.provider.ts index e42237b7..20e7b3c5 100644 --- a/server/modules/providers/shared/skills/skills.provider.ts +++ b/server/modules/providers/shared/skills/skills.provider.ts @@ -1,5 +1,5 @@ import path from 'node:path'; -import { mkdir, stat, writeFile } from 'node:fs/promises'; +import { mkdir, rm, writeFile } from 'node:fs/promises'; import type { IProviderSkills } from '@/shared/interfaces.js'; import type { @@ -44,19 +44,6 @@ type PendingSkillInstall = { skill: ProviderSkill; }; -const pathExists = async (targetPath: string): Promise => { - try { - await stat(targetPath); - return true; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - return false; - } - - throw error; - } -}; - const resolveSkillSupportingFilePath = ( skillDirectoryPath: string, relativePath: string, @@ -196,13 +183,6 @@ export abstract class SkillsProvider implements IProviderSkills { } seenSkillPaths.add(normalizedSkillPath); - if (await pathExists(skillDirectoryPath)) { - throw new AppError(`Skill target "${resolvedDirectoryName}" already exists.`, { - code: 'PROVIDER_SKILL_ALREADY_EXISTS', - statusCode: 409, - }); - } - const supportingFiles = (entry.files ?? []).map((file) => ({ targetPath: resolveSkillSupportingFilePath(skillDirectoryPath, file.relativePath, index), content: file.encoding === 'base64' @@ -243,6 +223,8 @@ export abstract class SkillsProvider implements IProviderSkills { } for (const install of pendingInstalls) { + // Replace the complete skill directory so removed scripts or assets do not remain stale. + await rm(install.skillDirectoryPath, { recursive: true, force: true }); await mkdir(install.skillDirectoryPath, { recursive: true }); await writeFile(install.skillPath, `${install.content}\n`, 'utf8'); for (const file of install.supportingFiles) { diff --git a/server/modules/providers/tests/skills.test.ts b/server/modules/providers/tests/skills.test.ts index eea13945..79a3d9af 100644 --- a/server/modules/providers/tests/skills.test.ts +++ b/server/modules/providers/tests/skills.test.ts @@ -585,18 +585,20 @@ test('providerSkillsService adds global skills for claude, codex, gemini, and cu true, ); + const replacedCodexSkills = await providerSkillsService.addProviderSkills('codex', { + entries: [ + { + directoryName: 'uploaded-codex-folder', + content: '---\nname: replacement\ndescription: Replacement skill\n---\n\nReplacement body.\n', + }, + ], + }); + assert.equal(replacedCodexSkills[0]?.command, '$replacement'); + assert.match(await fs.readFile(createdCodexSkill.sourcePath, 'utf8'), /Replacement body\./); await assert.rejects( - providerSkillsService.addProviderSkills('codex', { - entries: [ - { - directoryName: 'uploaded-codex-folder', - content: '---\nname: replacement\n---\n\nReplacement body.\n', - }, - ], - }), - /already exists/i, + fs.stat(path.join(path.dirname(createdCodexSkill.sourcePath), 'scripts', 'run.js')), + { code: 'ENOENT' }, ); - assert.match(await fs.readFile(createdCodexSkill.sourcePath, 'utf8'), /Codex body\./); const pendingBatchSkillPath = path.join(tempRoot, '.agents', 'skills', 'pending-batch', 'SKILL.md'); await assert.rejects( @@ -652,7 +654,7 @@ test('providerSkillsService adds global skills for claude, codex, gemini, and cu assert.equal(listedClaudeSkills.some((skill) => skill.name === 'claude-global'), true); const listedCodexSkills = await providerSkillsService.listProviderSkills('codex'); - assert.equal(listedCodexSkills.some((skill) => skill.name === 'codex-global'), true); + assert.equal(listedCodexSkills.some((skill) => skill.name === 'replacement'), true); const listedGeminiSkills = await providerSkillsService.listProviderSkills('gemini'); assert.equal(listedGeminiSkills.some((skill) => skill.name === 'gemini-global'), true);