diff --git a/server/modules/providers/provider.routes.ts b/server/modules/providers/provider.routes.ts index 74592beb..9b9fb576 100644 --- a/server/modules/providers/provider.routes.ts +++ b/server/modules/providers/provider.routes.ts @@ -197,6 +197,7 @@ const parseProviderSkillCreatePayload = (payload: unknown): ProviderSkillCreateI content: body.content, directoryName: body.directoryName, fileName: body.fileName, + files: body.files, }] : null; diff --git a/server/modules/providers/shared/skills/skills.provider.ts b/server/modules/providers/shared/skills/skills.provider.ts index 55f88184..e42237b7 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, writeFile } from 'node:fs/promises'; +import { mkdir, stat, writeFile } from 'node:fs/promises'; import type { IProviderSkills } from '@/shared/interfaces.js'; import type { @@ -33,6 +33,30 @@ const normalizeSkillDirectoryName = (value: string): string => ( .replace(/^-+|-+$/g, '') ); +type PendingSkillInstall = { + skillDirectoryPath: string; + skillPath: string; + content: string; + supportingFiles: Array<{ + targetPath: string; + content: string | Buffer; + }>; + 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, @@ -131,10 +155,8 @@ export abstract class SkillsProvider implements IProviderSkills { }); } - await mkdir(globalSkillSource.rootDir, { recursive: true }); - - const createdSkills: ProviderSkill[] = []; const seenSkillPaths = new Set(); + const pendingInstalls: PendingSkillInstall[] = []; for (const [index, entry] of input.entries.entries()) { const content = typeof entry.content === 'string' ? entry.content.trim() : ''; @@ -147,8 +169,10 @@ export abstract class SkillsProvider implements IProviderSkills { const fileNameFallback = readOptionalString(entry.fileName); const requestedDirectoryName = readOptionalString(entry.directoryName); - const fallbackSkillName = requestedDirectoryName - ?? (fileNameFallback ? stripMarkdownExtension(fileNameFallback) : `skill-${index + 1}`); + const fallbackSkillName = normalizeSkillDirectoryName( + requestedDirectoryName + ?? (fileNameFallback ? stripMarkdownExtension(fileNameFallback) : `skill-${index + 1}`), + ); const definition = readProviderSkillMarkdownDefinitionFromContent(content, fallbackSkillName); const resolvedDirectoryName = normalizeSkillDirectoryName( requestedDirectoryName ?? definition.name, @@ -172,6 +196,13 @@ 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' @@ -189,30 +220,38 @@ export abstract class SkillsProvider implements IProviderSkills { seenSupportingPaths.add(file.targetPath); } - await mkdir(skillDirectoryPath, { recursive: true }); - await writeFile(skillPath, `${content}\n`, 'utf8'); - for (const file of supportingFiles) { - await mkdir(path.dirname(file.targetPath), { recursive: true }); - await writeFile(file.targetPath, file.content); - } - const command = globalSkillSource.commandForSkill ? globalSkillSource.commandForSkill(definition.name) : `${globalSkillSource.commandPrefix ?? '/'}${definition.name}`; - createdSkills.push({ - provider: this.provider, - name: definition.name, - description: definition.description, - command, - scope: globalSkillSource.scope, - sourcePath: skillPath, - pluginName: globalSkillSource.pluginName, - pluginId: globalSkillSource.pluginId, + pendingInstalls.push({ + skillDirectoryPath, + skillPath, + content, + supportingFiles, + skill: { + provider: this.provider, + name: definition.name, + description: definition.description, + command, + scope: globalSkillSource.scope, + sourcePath: skillPath, + pluginName: globalSkillSource.pluginName, + pluginId: globalSkillSource.pluginId, + }, }); } - return createdSkills; + for (const install of pendingInstalls) { + await mkdir(install.skillDirectoryPath, { recursive: true }); + await writeFile(install.skillPath, `${install.content}\n`, 'utf8'); + for (const file of install.supportingFiles) { + await mkdir(path.dirname(file.targetPath), { recursive: true }); + await writeFile(file.targetPath, file.content); + } + } + + return pendingInstalls.map((install) => install.skill); } protected abstract getSkillSources(workspacePath: string): Promise; diff --git a/server/modules/providers/tests/skills.test.ts b/server/modules/providers/tests/skills.test.ts index 16c660ae..eea13945 100644 --- a/server/modules/providers/tests/skills.test.ts +++ b/server/modules/providers/tests/skills.test.ts @@ -568,6 +568,54 @@ test('providerSkillsService adds global skills for claude, codex, gemini, and cu 'console.log("codex skill");\n', ); + const fallbackNamedSkills = await providerSkillsService.addProviderSkills('codex', { + entries: [ + { + fileName: 'fallback / skill.md', + content: '---\ndescription: Normalized fallback skill\n---\n\nFallback body.\n', + }, + ], + }); + const fallbackNamedSkill = fallbackNamedSkills[0]; + assert.ok(fallbackNamedSkill); + assert.equal(fallbackNamedSkill.name, 'fallback-skill'); + assert.equal(fallbackNamedSkill.command, '$fallback-skill'); + assert.equal( + fallbackNamedSkill.sourcePath.endsWith(path.join('.agents', 'skills', 'fallback-skill', 'SKILL.md')), + true, + ); + + await assert.rejects( + providerSkillsService.addProviderSkills('codex', { + entries: [ + { + directoryName: 'uploaded-codex-folder', + content: '---\nname: replacement\n---\n\nReplacement body.\n', + }, + ], + }), + /already exists/i, + ); + assert.match(await fs.readFile(createdCodexSkill.sourcePath, 'utf8'), /Codex body\./); + + const pendingBatchSkillPath = path.join(tempRoot, '.agents', 'skills', 'pending-batch', 'SKILL.md'); + await assert.rejects( + providerSkillsService.addProviderSkills('codex', { + entries: [ + { + directoryName: 'pending-batch', + content: '---\nname: pending-batch\n---\n\nPending body.\n', + }, + { + directoryName: 'pending-batch', + content: '---\nname: duplicate-batch\n---\n\nDuplicate body.\n', + }, + ], + }), + /duplicate skill target/i, + ); + await assert.rejects(fs.stat(pendingBatchSkillPath), { code: 'ENOENT' }); + const createdGeminiSkills = await providerSkillsService.addProviderSkills('gemini', { entries: [ { diff --git a/src/components/skills/hooks/useProviderSkills.ts b/src/components/skills/hooks/useProviderSkills.ts index 225ca3c2..4655acd5 100644 --- a/src/components/skills/hooks/useProviderSkills.ts +++ b/src/components/skills/hooks/useProviderSkills.ts @@ -111,8 +111,12 @@ const normalizeSkill = ( sourcePath: String(skill.sourcePath ?? ''), pluginName: typeof skill.pluginName === 'string' ? skill.pluginName : undefined, pluginId: typeof skill.pluginId === 'string' ? skill.pluginId : undefined, - projectDisplayName: shouldAttachProject ? project?.displayName : undefined, - projectPath: shouldAttachProject ? project?.path : undefined, + projectDisplayName: shouldAttachProject + ? project?.displayName ?? skill.projectDisplayName + : skill.projectDisplayName, + projectPath: shouldAttachProject + ? project?.path ?? skill.projectPath + : skill.projectPath, }; }; diff --git a/src/components/skills/view/ProviderSkills.tsx b/src/components/skills/view/ProviderSkills.tsx index 8c7f9b22..186b6d35 100644 --- a/src/components/skills/view/ProviderSkills.tsx +++ b/src/components/skills/view/ProviderSkills.tsx @@ -170,14 +170,18 @@ const buildQueuedSkillFolders = (selectedFiles: File[]): QueuedSkillFile[] => { return skillRoots.map((skillRoot) => { const skillFiles = files.filter(({ relativePath }) => { - const owningRoot = skillRoots.find((candidateRoot) => ( - relativePath === `${candidateRoot}/SKILL.md` - || relativePath.startsWith(`${candidateRoot}/`) - )); + const owningRoot = skillRoots.find((candidateRoot) => { + const normalizedRelativePath = relativePath.toLowerCase(); + const normalizedSkillPath = `${candidateRoot}/skill.md`.toLowerCase(); + return normalizedRelativePath === normalizedSkillPath + || relativePath.startsWith(`${candidateRoot}/`); + }); return owningRoot === skillRoot; }); const skillSourceFile = skillFiles.find( - ({ relativePath }) => relativePath === `${skillRoot}/SKILL.md`, + ({ relativePath }) => ( + relativePath.toLowerCase() === `${skillRoot}/skill.md`.toLowerCase() + ), ); if (!skillSourceFile) { throw new Error(`Could not read SKILL.md from ${getBaseName(skillRoot)}.`); @@ -303,6 +307,10 @@ export default function ProviderSkills({ selectedProvider, currentProjects }: Pr }, [queueSkillFolders]); const handleFolderSelection = useCallback((selectedFiles: File[]) => { + if (selectedFiles.length === 0) { + return; + } + try { queueSkillFolders(selectedFiles); setSubmitError(null);