fix(skills): validate installs before writing

Preserve bundled files and normalize fallback names across skill installation paths.

Validate complete batches before writing and reject existing targets to avoid partial installs.

Keep project metadata and make folder selection tolerant of casing and cancelled dialogs.
This commit is contained in:
Haileyesus
2026-06-22 14:21:03 +03:00
parent 333625bdab
commit dc6208bc75
5 changed files with 130 additions and 30 deletions

View File

@@ -197,6 +197,7 @@ const parseProviderSkillCreatePayload = (payload: unknown): ProviderSkillCreateI
content: body.content, content: body.content,
directoryName: body.directoryName, directoryName: body.directoryName,
fileName: body.fileName, fileName: body.fileName,
files: body.files,
}] }]
: null; : null;

View File

@@ -1,5 +1,5 @@
import path from 'node:path'; 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 { IProviderSkills } from '@/shared/interfaces.js';
import type { import type {
@@ -33,6 +33,30 @@ const normalizeSkillDirectoryName = (value: string): string => (
.replace(/^-+|-+$/g, '') .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<boolean> => {
try {
await stat(targetPath);
return true;
} catch (error) {
if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
return false;
}
throw error;
}
};
const resolveSkillSupportingFilePath = ( const resolveSkillSupportingFilePath = (
skillDirectoryPath: string, skillDirectoryPath: string,
relativePath: 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<string>(); const seenSkillPaths = new Set<string>();
const pendingInstalls: PendingSkillInstall[] = [];
for (const [index, entry] of input.entries.entries()) { for (const [index, entry] of input.entries.entries()) {
const content = typeof entry.content === 'string' ? entry.content.trim() : ''; 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 fileNameFallback = readOptionalString(entry.fileName);
const requestedDirectoryName = readOptionalString(entry.directoryName); const requestedDirectoryName = readOptionalString(entry.directoryName);
const fallbackSkillName = requestedDirectoryName const fallbackSkillName = normalizeSkillDirectoryName(
?? (fileNameFallback ? stripMarkdownExtension(fileNameFallback) : `skill-${index + 1}`); requestedDirectoryName
?? (fileNameFallback ? stripMarkdownExtension(fileNameFallback) : `skill-${index + 1}`),
);
const definition = readProviderSkillMarkdownDefinitionFromContent(content, fallbackSkillName); const definition = readProviderSkillMarkdownDefinitionFromContent(content, fallbackSkillName);
const resolvedDirectoryName = normalizeSkillDirectoryName( const resolvedDirectoryName = normalizeSkillDirectoryName(
requestedDirectoryName ?? definition.name, requestedDirectoryName ?? definition.name,
@@ -172,6 +196,13 @@ export abstract class SkillsProvider implements IProviderSkills {
} }
seenSkillPaths.add(normalizedSkillPath); 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) => ({ const supportingFiles = (entry.files ?? []).map((file) => ({
targetPath: resolveSkillSupportingFilePath(skillDirectoryPath, file.relativePath, index), targetPath: resolveSkillSupportingFilePath(skillDirectoryPath, file.relativePath, index),
content: file.encoding === 'base64' content: file.encoding === 'base64'
@@ -189,30 +220,38 @@ export abstract class SkillsProvider implements IProviderSkills {
seenSupportingPaths.add(file.targetPath); 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 const command = globalSkillSource.commandForSkill
? globalSkillSource.commandForSkill(definition.name) ? globalSkillSource.commandForSkill(definition.name)
: `${globalSkillSource.commandPrefix ?? '/'}${definition.name}`; : `${globalSkillSource.commandPrefix ?? '/'}${definition.name}`;
createdSkills.push({ pendingInstalls.push({
provider: this.provider, skillDirectoryPath,
name: definition.name, skillPath,
description: definition.description, content,
command, supportingFiles,
scope: globalSkillSource.scope, skill: {
sourcePath: skillPath, provider: this.provider,
pluginName: globalSkillSource.pluginName, name: definition.name,
pluginId: globalSkillSource.pluginId, 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<ProviderSkillSource[]>; protected abstract getSkillSources(workspacePath: string): Promise<ProviderSkillSource[]>;

View File

@@ -568,6 +568,54 @@ test('providerSkillsService adds global skills for claude, codex, gemini, and cu
'console.log("codex skill");\n', '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', { const createdGeminiSkills = await providerSkillsService.addProviderSkills('gemini', {
entries: [ entries: [
{ {

View File

@@ -111,8 +111,12 @@ const normalizeSkill = (
sourcePath: String(skill.sourcePath ?? ''), sourcePath: String(skill.sourcePath ?? ''),
pluginName: typeof skill.pluginName === 'string' ? skill.pluginName : undefined, pluginName: typeof skill.pluginName === 'string' ? skill.pluginName : undefined,
pluginId: typeof skill.pluginId === 'string' ? skill.pluginId : undefined, pluginId: typeof skill.pluginId === 'string' ? skill.pluginId : undefined,
projectDisplayName: shouldAttachProject ? project?.displayName : undefined, projectDisplayName: shouldAttachProject
projectPath: shouldAttachProject ? project?.path : undefined, ? project?.displayName ?? skill.projectDisplayName
: skill.projectDisplayName,
projectPath: shouldAttachProject
? project?.path ?? skill.projectPath
: skill.projectPath,
}; };
}; };

View File

@@ -170,14 +170,18 @@ const buildQueuedSkillFolders = (selectedFiles: File[]): QueuedSkillFile[] => {
return skillRoots.map((skillRoot) => { return skillRoots.map((skillRoot) => {
const skillFiles = files.filter(({ relativePath }) => { const skillFiles = files.filter(({ relativePath }) => {
const owningRoot = skillRoots.find((candidateRoot) => ( const owningRoot = skillRoots.find((candidateRoot) => {
relativePath === `${candidateRoot}/SKILL.md` const normalizedRelativePath = relativePath.toLowerCase();
|| relativePath.startsWith(`${candidateRoot}/`) const normalizedSkillPath = `${candidateRoot}/skill.md`.toLowerCase();
)); return normalizedRelativePath === normalizedSkillPath
|| relativePath.startsWith(`${candidateRoot}/`);
});
return owningRoot === skillRoot; return owningRoot === skillRoot;
}); });
const skillSourceFile = skillFiles.find( const skillSourceFile = skillFiles.find(
({ relativePath }) => relativePath === `${skillRoot}/SKILL.md`, ({ relativePath }) => (
relativePath.toLowerCase() === `${skillRoot}/skill.md`.toLowerCase()
),
); );
if (!skillSourceFile) { if (!skillSourceFile) {
throw new Error(`Could not read SKILL.md from ${getBaseName(skillRoot)}.`); throw new Error(`Could not read SKILL.md from ${getBaseName(skillRoot)}.`);
@@ -303,6 +307,10 @@ export default function ProviderSkills({ selectedProvider, currentProjects }: Pr
}, [queueSkillFolders]); }, [queueSkillFolders]);
const handleFolderSelection = useCallback((selectedFiles: File[]) => { const handleFolderSelection = useCallback((selectedFiles: File[]) => {
if (selectedFiles.length === 0) {
return;
}
try { try {
queueSkillFolders(selectedFiles); queueSkillFolders(selectedFiles);
setSubmitError(null); setSubmitError(null);