diff --git a/packages/api/src/skill/skill-info.ts b/packages/api/src/skill/skill-info.ts index 759f367b58..88ba1f7325 100644 --- a/packages/api/src/skill/skill-info.ts +++ b/packages/api/src/skill/skill-info.ts @@ -26,8 +26,15 @@ export interface SkillInfo extends SkillMetadata { enabled: boolean; } -export interface SkillCreateOptions extends SkillMetadata { - content: string; +export interface SkillFolderInfo { + label: string; + badge: string; + icon?: string; + baseDirectory: string; +} + +export interface SkillFileContent extends SkillMetadata { + content?: string; } export const SKILL_SECTION = 'skills'; diff --git a/packages/main/src/plugin/skill/skill-manager.spec.ts b/packages/main/src/plugin/skill/skill-manager.spec.ts index e9031d99fb..ffbe30e94c 100644 --- a/packages/main/src/plugin/skill/skill-manager.spec.ts +++ b/packages/main/src/plugin/skill/skill-manager.spec.ts @@ -512,11 +512,10 @@ test('createSkill should create a SKILL.md and register the skill', async () => const skillManager = createSkillManager(); await skillManager.init(); - const skill = await skillManager.createSkill({ - name: 'new-skill', - description: 'A new skill', - content: '# New Skill\n\nInstructions here.', - }); + const skill = await skillManager.createSkill( + { name: 'new-skill', description: 'A new skill', content: '# New Skill\n\nInstructions here.' }, + SKILLS_DIR, + ); const expectedDir = join(SKILLS_DIR, 'new-skill'); expect(skill.name).toBe('new-skill'); @@ -533,15 +532,40 @@ test('createSkill should create a SKILL.md and register the skill', async () => expect(updateMock).toHaveBeenCalled(); }); +test('createSkill should strip existing frontmatter from content', async () => { + vi.mocked(existsSync).mockReturnValue(false); + vi.mocked(mkdir).mockResolvedValue(undefined); + vi.mocked(writeFile).mockResolvedValue(undefined); + + const skillManager = createSkillManager(); + await skillManager.init(); + await skillManager.createSkill( + { + name: 'imported-skill', + description: 'Overridden description', + content: '---\nname: old-name\ndescription: old desc\n---\n\n# Body content', + }, + SKILLS_DIR, + ); + + const writtenContent = vi.mocked(writeFile).mock.calls[0]![1] as string; + expect(writtenContent).toContain('name: imported-skill'); + expect(writtenContent).toContain('description: Overridden description'); + expect(writtenContent).not.toContain('old-name'); + expect(writtenContent).not.toContain('old desc'); + expect(writtenContent).toContain('# Body content'); +}); + test('createSkill should throw on duplicate name', async () => { vi.mocked(existsSync).mockReturnValue(true); vi.mocked(readFile).mockResolvedValue(validSkillMd); const skillManager = createSkillManager(); + await skillManager.init(); await skillManager.registerSkill(join(SKILLS_DIR, 'my-test-skill')); await expect( - skillManager.createSkill({ name: 'my-test-skill', description: 'Duplicate', content: '# Dup' }), + skillManager.createSkill({ name: 'my-test-skill', description: 'Duplicate', content: '# Dup' }, SKILLS_DIR), ).rejects.toThrow(`Skill with name 'my-test-skill' already registered`); }); @@ -549,28 +573,230 @@ test('createSkill should throw when directory already exists', async () => { vi.mocked(existsSync).mockReturnValue(true); const skillManager = createSkillManager(); + await skillManager.init(); await expect( - skillManager.createSkill({ name: 'existing-dir', description: 'Exists', content: '# Exists' }), + skillManager.createSkill({ name: 'existing-dir', description: 'Exists', content: '# Exists' }, SKILLS_DIR), ).rejects.toThrow('Skill directory already exists'); }); test('createSkill should throw on invalid name', async () => { + vi.mocked(existsSync).mockReturnValue(false); const skillManager = createSkillManager(); + await skillManager.init(); await expect( - skillManager.createSkill({ name: 'Invalid_Name', description: 'Bad name', content: '# Bad' }), + skillManager.createSkill({ name: 'Invalid_Name', description: 'Bad name', content: '# Bad' }, SKILLS_DIR), ).rejects.toThrow(`'name' must contain only lowercase letters, numbers, and hyphens`); }); test('createSkill should reject name containing colons', async () => { + vi.mocked(existsSync).mockReturnValue(false); const skillManager = createSkillManager(); + await skillManager.init(); await expect( - skillManager.createSkill({ name: 'team:my-skill', description: 'Colons are invalid', content: '# Bad' }), + skillManager.createSkill( + { name: 'team:my-skill', description: 'Colons are invalid', content: '# Bad' }, + SKILLS_DIR, + ), ).rejects.toThrow(`'name' must contain only lowercase letters, numbers, and hyphens`); }); +test('createSkill with extension-registered target should write to that target directory', async () => { + const CUSTOM_DIR = resolve('/custom/skills'); + vi.mocked(existsSync).mockReturnValue(false); + vi.mocked(mkdir).mockResolvedValue(undefined); + vi.mocked(writeFile).mockResolvedValue(undefined); + + const skillManager = createSkillManager(); + await skillManager.init(); + skillManager.registerSkillFolder({ + label: 'Custom Skills', + badge: 'Custom', + baseDirectory: CUSTOM_DIR, + }); + const skill = await skillManager.createSkill( + { name: 'new-skill', description: 'A custom skill', content: '# Custom Skill' }, + CUSTOM_DIR, + ); + + const expectedDir = join(CUSTOM_DIR, 'new-skill'); + expect(skill.path).toBe(expectedDir); + expect(mkdir).toHaveBeenCalledWith(expectedDir, { recursive: true }); + expect(writeFile).toHaveBeenCalledWith( + join(expectedDir, 'SKILL.md'), + expect.stringContaining('name: new-skill'), + 'utf-8', + ); +}); + +test('createSkill should throw when target is not registered', async () => { + vi.mocked(existsSync).mockReturnValue(false); + + const skillManager = createSkillManager(); + await skillManager.init(); + + await expect( + skillManager.createSkill({ name: 'some-skill', description: 'A skill', content: '# Content' }, '/unknown/target'), + ).rejects.toThrow(`Unknown skill folder: '/unknown/target'`); +}); + +test('registerSkillFolder should register and return disposable', async () => { + vi.mocked(existsSync).mockReturnValue(false); + + const skillManager = createSkillManager(); + await skillManager.init(); + + const disposable = skillManager.registerSkillFolder({ + label: 'Test Target', + badge: 'Test', + baseDirectory: '/test/target/skills', + }); + + expect(skillManager.listSkillFolders()).toHaveLength(2); + expect(skillManager.listSkillFolders().find(t => t.baseDirectory === '/test/target/skills')).toBeDefined(); + expect(apiSender.send).toHaveBeenCalledWith('skill-folders-update'); + + disposable.dispose(); + expect(skillManager.listSkillFolders()).toHaveLength(1); + expect(skillManager.listSkillFolders().find(t => t.baseDirectory === '/test/target/skills')).toBeUndefined(); +}); + +test('registerSkillFolder should throw on duplicate baseDirectory', async () => { + vi.mocked(existsSync).mockReturnValue(false); + + const skillManager = createSkillManager(); + await skillManager.init(); + + expect(() => + skillManager.registerSkillFolder({ + label: 'Duplicate', + badge: 'Dup', + baseDirectory: SKILLS_DIR, + }), + ).toThrow(`Skill folder '${SKILLS_DIR}' is already registered`); +}); + +test('listSkillFolders should include built-in kortex folder after init', async () => { + vi.mocked(existsSync).mockReturnValue(false); + + const skillManager = createSkillManager(); + await skillManager.init(); + + const targets = skillManager.listSkillFolders(); + expect(targets).toHaveLength(1); + expect(targets[0]).toEqual( + expect.objectContaining({ + label: 'Kortex Skills', + badge: 'Kortex', + baseDirectory: SKILLS_DIR, + }), + ); +}); + +test('registerSkillFolder should discover skills from its directory after init', async () => { + const EXTRA_DIR = resolve('/extra/skills'); + const EXTRA_SKILL_MD = `---\nname: extra-skill\ndescription: An extra skill\n---\n# Extra\n`; + + vi.mocked(existsSync).mockImplementation(p => { + const path = String(p); + return path === EXTRA_DIR || path === join(EXTRA_DIR, 'extra-skill', SKILL_FILE_NAME); + }); + vi.mocked(readdir).mockImplementation(async (p: unknown) => { + const path = String(p); + if (path === EXTRA_DIR) { + return [{ name: 'extra-skill', isDirectory: (): boolean => true }] as unknown as Awaited< + ReturnType + >; + } + return [] as unknown as Awaited>; + }); + vi.mocked(readFile).mockImplementation(async (p: unknown) => { + const path = String(p); + if (path === join(EXTRA_DIR, 'extra-skill', SKILL_FILE_NAME)) return EXTRA_SKILL_MD; + throw new Error(`File not found: ${path}`); + }); + + const skillManager = createSkillManager(); + await skillManager.init(); + skillManager.registerSkillFolder({ + label: 'Extra', + badge: 'Extra', + baseDirectory: EXTRA_DIR, + }); + + await vi.waitFor(() => { + const skills = skillManager.listSkills(); + expect(skills.some(s => s.path === join(EXTRA_DIR, 'extra-skill'))).toBe(true); + }); +}); + +test('disposing a skill folder should remove its skills from the list', async () => { + const EXTRA_DIR = resolve('/extra/skills'); + const EXTRA_SKILL_MD = `---\nname: extra-skill\ndescription: An extra skill\n---\n# Extra\n`; + + vi.mocked(existsSync).mockImplementation(p => { + const path = String(p); + return path === EXTRA_DIR || path === join(EXTRA_DIR, 'extra-skill', SKILL_FILE_NAME); + }); + vi.mocked(readdir).mockImplementation(async (p: unknown) => { + const path = String(p); + if (path === EXTRA_DIR) { + return [{ name: 'extra-skill', isDirectory: (): boolean => true }] as unknown as Awaited< + ReturnType + >; + } + return [] as unknown as Awaited>; + }); + vi.mocked(readFile).mockImplementation(async (p: unknown) => { + const path = String(p); + if (path === join(EXTRA_DIR, 'extra-skill', SKILL_FILE_NAME)) return EXTRA_SKILL_MD; + throw new Error(`File not found: ${path}`); + }); + + const skillManager = createSkillManager(); + await skillManager.init(); + + const disposable = skillManager.registerSkillFolder({ + label: 'Extra', + badge: 'Extra', + baseDirectory: EXTRA_DIR, + }); + + await vi.waitFor(() => { + expect(skillManager.listSkills().some(s => s.name === 'extra-skill')).toBe(true); + }); + + disposable.dispose(); + + expect(skillManager.listSkills().some(s => s.name === 'extra-skill')).toBe(false); + expect(skillManager.listSkillFolders().find(t => t.baseDirectory === EXTRA_DIR)).toBeUndefined(); + expect(apiSender.send).toHaveBeenCalledWith('skill-manager-update'); +}); + +test('getSkillFileContent should return parsed metadata and body from a SKILL.md file', async () => { + vi.mocked(readFile).mockResolvedValue(validSkillMd); + + const skillManager = createSkillManager(); + const result = await skillManager.getSkillFileContent('/path/to/SKILL.md'); + + expect(result).toEqual({ + name: 'my-test-skill', + description: 'A test skill for unit testing', + content: '# My Test Skill\n\nSome instructions here.', + }); + expect(readFile).toHaveBeenCalledWith('/path/to/SKILL.md', 'utf-8'); +}); + +test('getSkillFileContent should throw when file has no frontmatter', async () => { + vi.mocked(readFile).mockResolvedValue(noFrontmatterSkillMd); + + const skillManager = createSkillManager(); + + await expect(skillManager.getSkillFileContent('/path/to/SKILL.md')).rejects.toThrow('No metadata found'); +}); + test('saveSkillsToConfig should write only enabled skill names', async () => { vi.mocked(existsSync).mockReturnValue(true); vi.mocked(readFile).mockResolvedValueOnce(validSkillMd).mockResolvedValueOnce(secondSkillMd); diff --git a/packages/main/src/plugin/skill/skill-manager.ts b/packages/main/src/plugin/skill/skill-manager.ts index 7cfc46d910..5f532a75a4 100644 --- a/packages/main/src/plugin/skill/skill-manager.ts +++ b/packages/main/src/plugin/skill/skill-manager.ts @@ -18,7 +18,7 @@ import { existsSync } from 'node:fs'; import { mkdir, readdir, readFile, rm, writeFile } from 'node:fs/promises'; -import { join, resolve } from 'node:path'; +import { join, resolve, sep } from 'node:path'; import type { Configuration } from '@kortex-app/api'; import { inject, injectable, preDestroy } from 'inversify'; @@ -35,7 +35,8 @@ import { SKILL_FILE_NAME, SKILL_REGISTERED, SKILL_SECTION, - type SkillCreateOptions, + type SkillFileContent, + type SkillFolderInfo, type SkillInfo, type SkillMetadata, } from '/@api/skill/skill-info.js'; @@ -46,6 +47,7 @@ const XML_TAG_PATTERN = /<[a-zA-Z/][a-zA-Z0-9 "'=/]*>/; @injectable() export class SkillManager { private skills: SkillInfo[] = []; + private skillFolders: SkillFolderInfo[] = []; private configuration: Configuration | undefined; private disposables: IDisposable[] = []; @@ -81,12 +83,24 @@ export class SkillManager { this.disposables.push(this.configurationRegistry.registerConfigurations([skillsConfiguration])); this.configuration = this.configurationRegistry.getConfiguration(SKILL_SECTION); + this.skillFolders = [ + { + label: 'Kortex Skills', + badge: 'Kortex', + baseDirectory: this.directories.getSkillsDirectory(), + }, + ]; + await this.discoverSkills(); this.ipcHandle('skill-manager:listSkills', async (): Promise => { return this.listSkills(); }); + this.ipcHandle('skill-manager:listSkillFolders', async (): Promise => { + return this.listSkillFolders(); + }); + this.ipcHandle('skill-manager:registerSkill', async (_listener, folderPath: string): Promise => { return this.registerSkill(folderPath); }); @@ -103,9 +117,12 @@ export class SkillManager { return this.unregisterSkill(name); }); - this.ipcHandle('skill-manager:createSkill', async (_listener, options: SkillCreateOptions): Promise => { - return this.createSkill(options); - }); + this.ipcHandle( + 'skill-manager:createSkill', + async (_listener, options: SkillFileContent, targetDirectory: string): Promise => { + return this.createSkill(options, targetDirectory); + }, + ); this.ipcHandle('skill-manager:getSkillContent', async (_listener, name: string): Promise => { return this.getSkillContent(name); @@ -114,6 +131,13 @@ export class SkillManager { this.ipcHandle('skill-manager:listSkillFolderContent', async (_listener, name: string): Promise => { return this.listSkillFolderContent(name); }); + + this.ipcHandle( + 'skill-manager:getSkillFileContent', + async (_listener, filePath: string): Promise => { + return this.getSkillFileContent(filePath); + }, + ); } /** @@ -147,6 +171,15 @@ export class SkillManager { return parsed as SkillMetadata; } + private stripFrontmatter(content: string): string { + const trimmed = content.trimStart(); + const DELIMITER = '---'; + if (!trimmed.startsWith(DELIMITER)) return content; + const endIndex = trimmed.indexOf(`\n${DELIMITER}`, DELIMITER.length); + if (endIndex === -1) return content; + return trimmed.slice(endIndex + DELIMITER.length + 2).trim(); + } + private validateMetadata(metadata: SkillMetadata, filePath: string): void { if (typeof metadata.name !== 'string' || !metadata.name) { throw new Error(`Missing or invalid 'name' in ${filePath}`); @@ -192,11 +225,7 @@ export class SkillManager { } const metadata = await this.parseSkillFile(skillFilePath); - - const duplicate = this.skills.find(s => s.name === metadata.name); - if (duplicate) { - throw new Error(`Skill with name '${metadata.name}' already registered at path: ${duplicate.path}`); - } + this.assertNoDuplicate(metadata.name); const skill: SkillInfo = { ...metadata, @@ -205,8 +234,7 @@ export class SkillManager { }; this.skills = [...this.skills, skill]; - this.saveSkillsToConfig(); - this.apiSender.send('skill-manager-update'); + this.saveAndNotifySkills(); return skill; } @@ -215,17 +243,18 @@ export class SkillManager { * description, and content into the skills directory. The skill is * enabled immediately and persisted to config. */ - async createSkill(options: SkillCreateOptions): Promise { + async createSkill(options: SkillFileContent, targetDirectory: string): Promise { + if (!options.content) { + throw new Error('Content must be provided'); + } + + this.validateSkillFolder(targetDirectory); + const metadata: SkillMetadata = { name: options.name, description: options.description }; this.validateMetadata(metadata, SKILL_FILE_NAME); + this.assertNoDuplicate(metadata.name); - const duplicate = this.skills.find(s => s.name === metadata.name); - if (duplicate) { - throw new Error(`Skill with name '${metadata.name}' already registered at path: ${duplicate.path}`); - } - - const skillsDir = this.directories.getSkillsDirectory(); - const skillDir = join(skillsDir, metadata.name); + const skillDir = join(targetDirectory, metadata.name); if (existsSync(skillDir)) { throw new Error(`Skill directory already exists: ${skillDir}`); @@ -233,8 +262,9 @@ export class SkillManager { await mkdir(skillDir, { recursive: true }); + const body = this.stripFrontmatter(options.content); const frontmatter = dump({ name: metadata.name, description: metadata.description }).trimEnd(); - const fileContent = `---\n${frontmatter}\n---\n\n${options.content}`; + const fileContent = `---\n${frontmatter}\n---\n\n${body}`; await writeFile(join(skillDir, SKILL_FILE_NAME), fileContent, 'utf-8'); const skill: SkillInfo = { @@ -243,33 +273,50 @@ export class SkillManager { enabled: true, }; this.skills = [...this.skills, skill]; - this.saveSkillsToConfig(); - this.apiSender.send('skill-manager-update'); + this.saveAndNotifySkills(); return skill; } - /** - * Disables a skill by removing its name from the `skills.enabled` config. - * The skill folder remains on disk and the skill stays visible in the list. - */ + registerSkillFolder(folder: SkillFolderInfo): IDisposable { + if (this.skillFolders.some(f => f.baseDirectory === folder.baseDirectory)) { + throw new Error(`Skill folder '${folder.baseDirectory}' is already registered`); + } + this.skillFolders = [...this.skillFolders, folder]; + this.apiSender.send('skill-folders-update'); + + this.discoverSkillsInDirectory(folder.baseDirectory).catch(console.error); + + return { + dispose: (): void => { + this.skillFolders = this.skillFolders.filter(f => f.baseDirectory !== folder.baseDirectory); + const resolvedRoot = resolve(folder.baseDirectory); + this.skills = this.skills.filter(s => { + const resolvedPath = resolve(s.path); + return resolvedPath !== resolvedRoot && !resolvedPath.startsWith(`${resolvedRoot}${sep}`); + }); + this.saveSkillsToConfig(); + this.apiSender.send('skill-folders-update'); + this.apiSender.send('skill-manager-update'); + }, + }; + } + + listSkillFolders(): SkillFolderInfo[] { + return this.skillFolders; + } + + private validateSkillFolder(directory: string): void { + if (!this.listSkillFolders().some(f => f.baseDirectory === directory)) { + throw new Error(`Unknown skill folder: '${directory}'`); + } + } + disableSkill(name: string): void { - const skill = this.findSkillByName(name); - skill.enabled = false; - this.skills = [...this.skills]; - this.saveSkillsToConfig(); - this.apiSender.send('skill-manager-update'); + this.setSkillEnabled(name, false); } - /** - * Enables a previously disabled skill by adding its name back - * to the `skills.enabled` config. - */ enableSkill(name: string): void { - const skill = this.findSkillByName(name); - skill.enabled = true; - this.skills = [...this.skills]; - this.saveSkillsToConfig(); - this.apiSender.send('skill-manager-update'); + this.setSkillEnabled(name, true); } /** @@ -283,8 +330,7 @@ export class SkillManager { await rm(skill.path, { recursive: true, force: true }); } this.skills = this.skills.filter(s => s.name !== name); - this.saveSkillsToConfig(); - this.apiSender.send('skill-manager-update'); + this.saveAndNotifySkills(); } listSkills(): SkillInfo[] { @@ -298,6 +344,18 @@ export class SkillManager { return readFile(filePath, 'utf-8'); } + /** + * Helper to read a SKILL.md file by path and return parsed metadata + * and body content. Used by the renderer to preview and prefill the + * create-skill dialog before submission. + */ + async getSkillFileContent(filePath: string): Promise { + const rawContent = (await readFile(filePath, 'utf-8')).trimStart(); + const metadata = this.extractFrontmatter(rawContent, filePath); + const body = this.stripFrontmatter(rawContent); + return { name: metadata.name, description: metadata.description, content: body }; + } + /** Lists all file/directory names inside the skill's folder. */ async listSkillFolderContent(name: string): Promise { const skill = this.findSkillByName(name); @@ -312,34 +370,68 @@ export class SkillManager { return skill; } + private assertNoDuplicate(name: string): void { + const existing = this.skills.find(s => s.name === name); + if (existing) { + throw new Error(`Skill with name '${name}' already registered at path: ${existing.path}`); + } + } + + private setSkillEnabled(name: string, enabled: boolean): void { + const skill = this.findSkillByName(name); + skill.enabled = enabled; + this.skills = [...this.skills]; + this.saveAndNotifySkills(); + } + + private saveAndNotifySkills(): void { + this.saveSkillsToConfig(); + this.apiSender.send('skill-manager-update'); + } + private isManagedSkill(skill: SkillInfo): boolean { - const skillsDir = this.directories.getSkillsDirectory(); - return skill.path.startsWith(skillsDir); + const resolvedSkillPath = resolve(skill.path); + return this.skillFolders.some(folder => { + const resolvedRoot = resolve(folder.baseDirectory); + return resolvedSkillPath === resolvedRoot || resolvedSkillPath.startsWith(`${resolvedRoot}${sep}`); + }); } /** - * Discovers skills from both the managed skills directory and - * externally registered paths persisted in `skills.registered`. - * Newly discovered skills are enabled by default. + * Discovers skills from all registered folders and externally + * registered paths persisted in `skills.registered` config. */ - async discoverSkills(): Promise { - const enabledNames = new Set(this.configuration?.get(SKILL_ENABLED) ?? []); - const folderPaths: string[] = [...(this.configuration?.get(SKILL_REGISTERED) ?? [])]; + private async discoverSkills(): Promise { + for (const folder of this.skillFolders) { + await this.discoverSkillsInDirectory(folder.baseDirectory); + } + const externalPaths: string[] = this.configuration?.get(SKILL_REGISTERED) ?? []; + await this.processDiscoveredPaths(externalPaths); + } - const skillsDir = this.directories.getSkillsDirectory(); - if (existsSync(skillsDir)) { - try { - const entries = await readdir(skillsDir, { withFileTypes: true }); - for (const entry of entries) { - if (entry.isDirectory()) { - folderPaths.push(join(skillsDir, entry.name)); - } + /** + * Discovers skills from a single directory. Used by + * {@link registerSkillFolder} so that each new folder triggers + * discovery only for its own base directory. + */ + private async discoverSkillsInDirectory(directory: string): Promise { + if (!existsSync(directory)) return; + const folderPaths: string[] = []; + try { + const entries = await readdir(directory, { withFileTypes: true }); + for (const entry of entries) { + if (entry.isDirectory()) { + folderPaths.push(join(directory, entry.name)); } - } catch { - // ignore read errors on the managed directory } + } catch { + return; } + await this.processDiscoveredPaths(folderPaths); + } + private async processDiscoveredPaths(folderPaths: string[]): Promise { + const enabledNames = new Set(this.configuration?.get(SKILL_ENABLED) ?? []); const discovered: SkillInfo[] = []; for (const folderPath of folderPaths) { @@ -363,9 +455,17 @@ export class SkillManager { } } - this.skills = [...this.skills, ...discovered]; - if (discovered.some(s => !enabledNames.has(s.name))) { - this.saveSkillsToConfig(); + // Re-check against this.skills at merge time to guard against + // concurrent calls that may have appended the same skills. + const existingNames = new Set(this.skills.map(s => s.name)); + const unique = discovered.filter(s => !existingNames.has(s.name)); + + if (unique.length > 0) { + this.skills = [...this.skills, ...unique]; + if (unique.some(s => !enabledNames.has(s.name))) { + this.saveSkillsToConfig(); + } + this.apiSender.send('skill-manager-update'); } } diff --git a/packages/preload/src/index.ts b/packages/preload/src/index.ts index 771abc21a4..f04cc6fe5b 100644 --- a/packages/preload/src/index.ts +++ b/packages/preload/src/index.ts @@ -136,7 +136,7 @@ import type { ChunkProviderInfo } from '/@api/rag/chunk-provider-info'; import type { RagEnvironment } from '/@api/rag/rag-environment'; import type { ExtensionBanner, RecommendedRegistry } from '/@api/recommendations/recommendations'; import type { ReleaseNotesInfo } from '/@api/release-notes-info'; -import type { SkillCreateOptions, SkillInfo } from '/@api/skill/skill-info'; +import type { SkillFileContent, SkillFolderInfo, SkillInfo } from '/@api/skill/skill-info'; import type { StatusBarEntryDescriptor } from '/@api/status-bar'; import type { PinOption } from '/@api/status-bar/pin-option'; import type { TelemetryMessages } from '/@api/telemetry'; @@ -349,6 +349,10 @@ export function initExposure(): void { return ipcInvoke('skill-manager:listSkills'); }); + contextBridge.exposeInMainWorld('listSkillFolders', async (): Promise> => { + return ipcInvoke('skill-manager:listSkillFolders'); + }); + contextBridge.exposeInMainWorld('registerSkill', async (folderPath: string): Promise => { return ipcInvoke('skill-manager:registerSkill', folderPath); }); @@ -365,9 +369,12 @@ export function initExposure(): void { return ipcInvoke('skill-manager:unregisterSkill', name); }); - contextBridge.exposeInMainWorld('createSkill', async (options: SkillCreateOptions): Promise => { - return ipcInvoke('skill-manager:createSkill', options); - }); + contextBridge.exposeInMainWorld( + 'createSkill', + async (options: SkillFileContent, targetDirectory: string): Promise => { + return ipcInvoke('skill-manager:createSkill', options, targetDirectory); + }, + ); contextBridge.exposeInMainWorld('getSkillContent', async (name: string): Promise => { return ipcInvoke('skill-manager:getSkillContent', name); @@ -377,6 +384,10 @@ export function initExposure(): void { return ipcInvoke('skill-manager:listSkillFolderContent', name); }); + contextBridge.exposeInMainWorld('getSkillFileContent', async (filePath: string): Promise => { + return ipcInvoke('skill-manager:getSkillFileContent', filePath); + }); + contextBridge.exposeInMainWorld( 'deleteSchedule', async (schedulerName: string, scheduleId: string): Promise => { diff --git a/packages/renderer/src/lib/skills/SkillCreate.spec.ts b/packages/renderer/src/lib/skills/SkillCreate.spec.ts new file mode 100644 index 0000000000..dba19b85dd --- /dev/null +++ b/packages/renderer/src/lib/skills/SkillCreate.spec.ts @@ -0,0 +1,247 @@ +/********************************************************************** + * Copyright (C) 2026 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + ***********************************************************************/ + +import '@testing-library/jest-dom/vitest'; + +import { fireEvent, render, screen, waitFor } from '@testing-library/svelte'; +import userEvent from '@testing-library/user-event'; +import { beforeEach, expect, test, vi } from 'vitest'; + +import SkillCreate from './SkillCreate.svelte'; + +const closeMock = vi.fn(); + +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(window.listSkillFolders).mockResolvedValue([ + { label: 'Kortex Skills', badge: 'Kortex', baseDirectory: '/test/skills' }, + { label: 'Claude Skills', badge: 'Claude', baseDirectory: '/home/.claude/skills' }, + ]); + vi.mocked(window.createSkill).mockResolvedValue({ + name: 'test-skill', + description: 'A test skill', + path: '/skills/test-skill', + enabled: true, + }); +}); + +test('should render the Create Skill dialog title', async () => { + render(SkillCreate, { onclose: closeMock }); + + await waitFor(() => { + expect(screen.getByText('Create Skill')).toBeInTheDocument(); + }); +}); + +test('should render target cards after loading', async () => { + render(SkillCreate, { onclose: closeMock }); + + await waitFor(() => { + expect(screen.getByText('Kortex Skills')).toBeInTheDocument(); + expect(screen.getByText('Claude Skills')).toBeInTheDocument(); + }); +}); + +test('should render name, description, and content fields', async () => { + render(SkillCreate, { onclose: closeMock }); + + await waitFor(() => { + expect(screen.getByLabelText('Skill name')).toBeInTheDocument(); + expect(screen.getByLabelText('Skill description')).toBeInTheDocument(); + expect(screen.getByLabelText('Skill content')).toBeInTheDocument(); + }); +}); + +test('should render the drag/drop zone', async () => { + render(SkillCreate, { onclose: closeMock }); + + await waitFor(() => { + expect(screen.getByLabelText('Drop or click to select a SKILL.md file')).toBeInTheDocument(); + expect(screen.getByText(/Choose file/)).toBeInTheDocument(); + expect(screen.getByText('Supported formats: .md')).toBeInTheDocument(); + }); +}); + +test('should have Create button disabled when fields are empty', async () => { + render(SkillCreate, { onclose: closeMock }); + + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Create' })).toBeDisabled(); + }); +}); + +test('should call onclose when Cancel is clicked', async () => { + render(SkillCreate, { onclose: closeMock }); + + const cancelButton = await screen.findByRole('button', { name: 'Cancel' }); + await fireEvent.click(cancelButton); + + expect(closeMock).toHaveBeenCalled(); +}); + +test('should enable Create button when all fields are filled and target is auto-selected', async () => { + render(SkillCreate, { onclose: closeMock }); + + await waitFor(() => { + expect(screen.getByText('Kortex Skills')).toBeInTheDocument(); + }); + + const nameInput = screen.getByLabelText('Skill name'); + const descInput = screen.getByLabelText('Skill description'); + const contentArea = screen.getByLabelText('Skill content'); + + await userEvent.type(nameInput, 'test-skill'); + await userEvent.type(descInput, 'A test skill'); + await userEvent.type(contentArea, 'Some content'); + + const createButton = screen.getByRole('button', { name: 'Create' }); + expect(createButton).toBeEnabled(); +}); + +test('should call createSkill with correct parameters and close on success', async () => { + render(SkillCreate, { onclose: closeMock }); + + await waitFor(() => { + expect(screen.getByText('Kortex Skills')).toBeInTheDocument(); + }); + + const nameInput = screen.getByLabelText('Skill name'); + const descInput = screen.getByLabelText('Skill description'); + const contentArea = screen.getByLabelText('Skill content'); + + await userEvent.type(nameInput, 'test-skill'); + await userEvent.type(descInput, 'A test skill'); + await userEvent.type(contentArea, 'Some content'); + + const createButton = screen.getByRole('button', { name: 'Create' }); + await fireEvent.click(createButton); + + expect(window.createSkill).toHaveBeenCalledWith( + { name: 'test-skill', description: 'A test skill', content: 'Some content' }, + '/test/skills', + ); + expect(closeMock).toHaveBeenCalled(); +}); + +test('should call createSkill with claude target when Claude card is selected', async () => { + render(SkillCreate, { onclose: closeMock }); + + await waitFor(() => { + expect(screen.getByText('Claude Skills')).toBeInTheDocument(); + }); + + const claudeCard = screen.getByRole('button', { name: 'Claude Skills' }); + await fireEvent.click(claudeCard); + + const nameInput = screen.getByLabelText('Skill name'); + const descInput = screen.getByLabelText('Skill description'); + const contentArea = screen.getByLabelText('Skill content'); + + await userEvent.type(nameInput, 'test-skill'); + await userEvent.type(descInput, 'A test skill'); + await userEvent.type(contentArea, 'Some content'); + + const createButton = screen.getByRole('button', { name: 'Create' }); + await fireEvent.click(createButton); + + expect(window.createSkill).toHaveBeenCalledWith(expect.any(Object), '/home/.claude/skills'); +}); + +test('should display error when createSkill fails', async () => { + vi.mocked(window.createSkill).mockRejectedValue(new Error('Skill already exists')); + + render(SkillCreate, { onclose: closeMock }); + + await waitFor(() => { + expect(screen.getByText('Kortex Skills')).toBeInTheDocument(); + }); + + const nameInput = screen.getByLabelText('Skill name'); + const descInput = screen.getByLabelText('Skill description'); + const contentArea = screen.getByLabelText('Skill content'); + + await userEvent.type(nameInput, 'test-skill'); + await userEvent.type(descInput, 'A test skill'); + await userEvent.type(contentArea, 'Some content'); + + const createButton = screen.getByRole('button', { name: 'Create' }); + await fireEvent.click(createButton); + + expect(await screen.findByText(/Skill already exists/)).toBeInTheDocument(); + expect(closeMock).not.toHaveBeenCalled(); +}); + +test('should render drop zone with correct label', async () => { + render(SkillCreate, { onclose: closeMock }); + + const dropZone = await screen.findByRole('button', { name: 'Drop or click to select a SKILL.md file' }); + expect(dropZone).toBeInTheDocument(); +}); + +test('should open file dialog when drop zone is clicked', async () => { + vi.mocked(window.openDialog).mockResolvedValue(['/home/user/skills/SKILL.md']); + vi.mocked(window.getSkillFileContent).mockResolvedValue({ + name: 'parsed-skill', + description: 'Parsed description', + content: '# Body', + }); + + render(SkillCreate, { onclose: closeMock }); + + const dropZone = await screen.findByLabelText('Drop or click to select a SKILL.md file'); + await fireEvent.click(dropZone); + + expect(window.openDialog).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Select a SKILL.md file', + selectors: ['openFile'], + }), + ); +}); + +test('should prefill fields from parsed file when browsing', async () => { + vi.mocked(window.openDialog).mockResolvedValue(['/home/user/skills/SKILL.md']); + vi.mocked(window.getSkillFileContent).mockResolvedValue({ + name: 'parsed-skill', + description: 'Parsed description', + content: '# Body content', + }); + + render(SkillCreate, { onclose: closeMock }); + + const dropZone = await screen.findByLabelText('Drop or click to select a SKILL.md file'); + await fireEvent.click(dropZone); + + await waitFor(() => { + expect(screen.getByLabelText('Skill name')).toHaveValue('parsed-skill'); + expect(screen.getByLabelText('Skill description')).toHaveValue('Parsed description'); + expect(screen.getByLabelText('Skill content')).toHaveValue('# Body content'); + }); + + await waitFor(() => { + expect(screen.getByText('Kortex Skills')).toBeInTheDocument(); + }); + + const createButton = screen.getByRole('button', { name: 'Create' }); + await fireEvent.click(createButton); + + expect(window.createSkill).toHaveBeenCalledWith( + { name: 'parsed-skill', description: 'Parsed description', content: '# Body content' }, + '/test/skills', + ); +}); diff --git a/packages/renderer/src/lib/skills/SkillCreate.svelte b/packages/renderer/src/lib/skills/SkillCreate.svelte new file mode 100644 index 0000000000..6805468391 --- /dev/null +++ b/packages/renderer/src/lib/skills/SkillCreate.svelte @@ -0,0 +1,208 @@ + + + + {#snippet content()} +
+ + + {#if !selectedFile} + + +
+
+ or create manually +
+
+ {:else} +
+ +
+ + +
+
+ {/if} + + + + + + + + + + {#if error} + + {/if} +
+ {/snippet} + + {#snippet buttons()} + + + {/snippet} + diff --git a/packages/renderer/src/lib/skills/SkillEmptyScreen.svelte b/packages/renderer/src/lib/skills/SkillEmptyScreen.svelte index f56e37b832..f1c7c714a7 100644 --- a/packages/renderer/src/lib/skills/SkillEmptyScreen.svelte +++ b/packages/renderer/src/lib/skills/SkillEmptyScreen.svelte @@ -12,7 +12,7 @@ let { onclick }: Props = $props(); - diff --git a/packages/renderer/src/lib/skills/SkillFolderCards.svelte b/packages/renderer/src/lib/skills/SkillFolderCards.svelte new file mode 100644 index 0000000000..72bf6be442 --- /dev/null +++ b/packages/renderer/src/lib/skills/SkillFolderCards.svelte @@ -0,0 +1,35 @@ + + +{#if options.length > 0} + +{/if} diff --git a/packages/renderer/src/lib/skills/SkillsList.spec.ts b/packages/renderer/src/lib/skills/SkillsList.spec.ts index 3160ed3552..b9d68af0c7 100644 --- a/packages/renderer/src/lib/skills/SkillsList.spec.ts +++ b/packages/renderer/src/lib/skills/SkillsList.spec.ts @@ -18,7 +18,7 @@ import '@testing-library/jest-dom/vitest'; -import { render, screen } from '@testing-library/svelte'; +import { fireEvent, render, screen } from '@testing-library/svelte'; import { writable } from 'svelte/store'; import { beforeEach, expect, test, vi } from 'vitest'; @@ -27,12 +27,15 @@ import type { SkillInfo } from '/@api/skill/skill-info'; import SkillsList from './SkillsList.svelte'; -vi.mock('/@/stores/skills'); +vi.mock(import('/@/stores/skills')); beforeEach(() => { vi.resetAllMocks(); vi.mocked(skillsStore).filteredSkillInfos = writable([]); vi.mocked(skillsStore).skillSearchPattern = writable(''); + vi.mocked(window.listSkillFolders).mockResolvedValue([ + { label: 'Kortex Skills', badge: 'Kortex', baseDirectory: '/test/skills' }, + ]); }); test('should show empty screen when no skills', () => { @@ -60,3 +63,12 @@ test('should show table when skills exist', () => { expect(screen.getByText('skill-a')).toBeInTheDocument(); expect(screen.getByText('skill-b')).toBeInTheDocument(); }); + +test('should open the create dialog when "New skill" is clicked', async () => { + render(SkillsList); + + const buttons = screen.getAllByText('New skill'); + await fireEvent.click(buttons[0]); + + expect(screen.getByText('Create Skill')).toBeInTheDocument(); +}); diff --git a/packages/renderer/src/lib/skills/SkillsList.svelte b/packages/renderer/src/lib/skills/SkillsList.svelte index 7a7be2cd56..29774ec6bb 100644 --- a/packages/renderer/src/lib/skills/SkillsList.svelte +++ b/packages/renderer/src/lib/skills/SkillsList.svelte @@ -1,7 +1,6 @@ {#snippet additionalActions()} - {/snippet} @@ -64,7 +69,7 @@ function navigateToCreateSkill(): void { {#if searchTerm} {:else} - + {/if} {:else} {/snippet} + +{#if showCreateDialog} + +{/if} diff --git a/packages/renderer/src/lib/ui/CardSelector.spec.ts b/packages/renderer/src/lib/ui/CardSelector.spec.ts new file mode 100644 index 0000000000..17ffa94bbb --- /dev/null +++ b/packages/renderer/src/lib/ui/CardSelector.spec.ts @@ -0,0 +1,132 @@ +/********************************************************************** + * Copyright (C) 2026 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + ***********************************************************************/ + +import '@testing-library/jest-dom/vitest'; + +import { faFolder, faGears, faHome } from '@fortawesome/free-solid-svg-icons'; +import { fireEvent, render, screen } from '@testing-library/svelte'; +import { beforeEach, expect, test, vi } from 'vitest'; + +import CardSelector from './CardSelector.svelte'; + +const options = [ + { title: 'Option A', badge: 'Badge A', value: 'a', icon: faFolder }, + { title: 'Option B', badge: 'Badge B', value: 'b', icon: faHome }, + { title: 'Option C', badge: 'Badge C', value: 'c', icon: faGears, description: 'A description for C' }, +]; + +beforeEach(() => { + vi.resetAllMocks(); +}); + +test('Expect all option titles rendered', () => { + render(CardSelector, { options }); + + expect(screen.getByText('Option A')).toBeInTheDocument(); + expect(screen.getByText('Option B')).toBeInTheDocument(); + expect(screen.getByText('Option C')).toBeInTheDocument(); +}); + +test('Expect all badges rendered', () => { + render(CardSelector, { options }); + + expect(screen.getByText('Badge A')).toBeInTheDocument(); + expect(screen.getByText('Badge B')).toBeInTheDocument(); + expect(screen.getByText('Badge C')).toBeInTheDocument(); +}); + +test('Expect label rendered when provided', () => { + render(CardSelector, { options, label: 'My Label' }); + + expect(screen.getByText('My Label')).toBeInTheDocument(); +}); + +test('Expect no label rendered when not provided', () => { + render(CardSelector, { options }); + + expect(screen.queryByText('My Label')).not.toBeInTheDocument(); +}); + +test('Expect region has default aria-label when no label provided', () => { + render(CardSelector, { options }); + + expect(screen.getByRole('region', { name: 'Options' })).toBeInTheDocument(); +}); + +test('Expect region uses label as aria-label when provided', () => { + render(CardSelector, { options, label: 'Pick one' }); + + expect(screen.getByRole('region', { name: 'Pick one' })).toBeInTheDocument(); +}); + +test('Expect description rendered when provided', () => { + render(CardSelector, { options }); + + expect(screen.getByText('A description for C')).toBeInTheDocument(); +}); + +test('Expect clicking option selects it', async () => { + render(CardSelector, { options, selected: '' }); + + const buttonA = screen.getByRole('button', { name: 'Option A' }); + await fireEvent.click(buttonA); + + expect(buttonA.className).toContain('border-[var(--pd-content-card-border-selected)]'); +}); + +test('Expect clicking different option changes selection', async () => { + render(CardSelector, { options, selected: 'a' }); + + const buttonB = screen.getByRole('button', { name: 'Option B' }); + await fireEvent.click(buttonB); + + expect(buttonB.className).toContain('border-[var(--pd-content-card-border-selected)]'); + + const buttonA = screen.getByRole('button', { name: 'Option A' }); + expect(buttonA.className).toContain('border-[var(--pd-content-card-border)]'); +}); + +test('Expect each option has a button with aria-label using title', () => { + render(CardSelector, { options }); + + expect(screen.getByRole('button', { name: 'Option A' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Option B' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Option C' })).toBeInTheDocument(); +}); + +test('Expect radio indicator filled for selected option', () => { + const { container } = render(CardSelector, { options, selected: 'b' }); + + const buttons = container.querySelectorAll('button'); + const selectedButton = buttons[1]!; + const radioFill = selectedButton.querySelector('.w-2.h-2.rounded-full'); + expect(radioFill).toBeInTheDocument(); + + const unselectedButton = buttons[0]!; + const noRadioFill = unselectedButton.querySelector('.w-2.h-2.rounded-full'); + expect(noRadioFill).not.toBeInTheDocument(); +}); + +test('Expect deselection when clicking the already selected option', async () => { + render(CardSelector, { options, selected: 'a' }); + + const buttonA = screen.getByRole('button', { name: 'Option A' }); + await fireEvent.click(buttonA); + + expect(buttonA.className).toContain('border-[var(--pd-content-card-border)]'); +}); diff --git a/packages/renderer/src/lib/ui/CardSelector.svelte b/packages/renderer/src/lib/ui/CardSelector.svelte new file mode 100644 index 0000000000..3186ea68a2 --- /dev/null +++ b/packages/renderer/src/lib/ui/CardSelector.svelte @@ -0,0 +1,72 @@ + + +
+ {#if label} + {label} + {/if} +
+ {#each options as option (option.value)} + + {/each} +
+