diff --git a/cli/src/modules/common/handlers/skills.ts b/cli/src/modules/common/handlers/skills.ts index ea719a76..87a45bc5 100644 --- a/cli/src/modules/common/handlers/skills.ts +++ b/cli/src/modules/common/handlers/skills.ts @@ -3,12 +3,12 @@ import type { RpcHandlerManager } from '@/api/rpc/RpcHandlerManager' import { listSkills, type ListSkillsRequest, type ListSkillsResponse } from '../skills' import { getErrorMessage, rpcError } from '../rpcResponses' -export function registerSkillsHandlers(rpcHandlerManager: RpcHandlerManager): void { +export function registerSkillsHandlers(rpcHandlerManager: RpcHandlerManager, workingDirectory: string): void { rpcHandlerManager.registerHandler('listSkills', async () => { logger.debug('List skills request') try { - const skills = await listSkills() + const skills = await listSkills(workingDirectory) return { success: true, skills } } catch (error) { logger.debug('Failed to list skills:', error) @@ -16,4 +16,3 @@ export function registerSkillsHandlers(rpcHandlerManager: RpcHandlerManager): vo } }) } - diff --git a/cli/src/modules/common/registerCommonHandlers.ts b/cli/src/modules/common/registerCommonHandlers.ts index 705c956c..91fa20d6 100644 --- a/cli/src/modules/common/registerCommonHandlers.ts +++ b/cli/src/modules/common/registerCommonHandlers.ts @@ -16,7 +16,7 @@ export function registerCommonHandlers(rpcHandlerManager: RpcHandlerManager, wor registerRipgrepHandlers(rpcHandlerManager, workingDirectory) registerDifftasticHandlers(rpcHandlerManager, workingDirectory) registerSlashCommandHandlers(rpcHandlerManager, workingDirectory) - registerSkillsHandlers(rpcHandlerManager) + registerSkillsHandlers(rpcHandlerManager, workingDirectory) registerGitHandlers(rpcHandlerManager, workingDirectory) registerUploadHandlers(rpcHandlerManager) } diff --git a/cli/src/modules/common/skills.test.ts b/cli/src/modules/common/skills.test.ts index 365b2487..2679cbc3 100644 --- a/cli/src/modules/common/skills.test.ts +++ b/cli/src/modules/common/skills.test.ts @@ -1,90 +1,119 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { mkdtemp, mkdir, writeFile, rm } from 'fs/promises'; -import { tmpdir } from 'os'; -import { join } from 'path'; -import { listSkills } from './skills'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { listSkills } from './skills' -describe('skills', () => { - const originalCodexHome = process.env.CODEX_HOME; - let codexHome: string; +async function writeSkill(skillDir: string, name: string, description: string): Promise { + await mkdir(skillDir, { recursive: true }) + await writeFile(join(skillDir, 'SKILL.md'), [ + '---', + `name: ${name}`, + `description: ${description}`, + '---', + '', + `# ${name}`, + ].join('\n')) +} + +describe('listSkills', () => { + const originalHome = process.env.HOME + let sandboxDir: string + let homeDir: string beforeEach(async () => { - codexHome = await mkdtemp(join(tmpdir(), 'hapi-skills-')); - process.env.CODEX_HOME = codexHome; - }); + sandboxDir = await mkdtemp(join(tmpdir(), 'hapi-skills-')) + homeDir = join(sandboxDir, 'home') + process.env.HOME = homeDir + await mkdir(homeDir, { recursive: true }) + }) afterEach(async () => { - if (originalCodexHome === undefined) { - delete process.env.CODEX_HOME; + if (originalHome === undefined) { + delete process.env.HOME } else { - process.env.CODEX_HOME = originalCodexHome; + process.env.HOME = originalHome } - await rm(codexHome, { recursive: true, force: true }); - }); - it('returns empty list when skills directory is missing', async () => { - const skills = await listSkills(); - expect(skills).toEqual([]); - }); + await rm(sandboxDir, { recursive: true, force: true }) + }) - it('lists only top-level skills and .system children', async () => { - const skillsRoot = join(codexHome, 'skills'); - await mkdir(skillsRoot, { recursive: true }); + it('returns empty list when skills directories are missing', async () => { + await expect(listSkills()).resolves.toEqual([]) + }) - const amisDir = join(skillsRoot, 'amis'); - await mkdir(amisDir, { recursive: true }); - await writeFile(join(amisDir, 'SKILL.md'), [ - '---', - 'name: amis', - 'description: AMIS guide', - '---', - '', - '# AMIS', - ].join('\n')); + it('lists user skills from ~/.agents only', async () => { + await writeSkill(join(homeDir, '.agents', 'skills', 'amis'), 'amis', 'AMIS guide') - const helloAgentsDir = join(skillsRoot, 'hello-agents'); - await mkdir(join(helloAgentsDir, 'analyze'), { recursive: true }); - await writeFile(join(helloAgentsDir, 'SKILL.md'), [ - '---', - 'name: helloagents', - 'description: Main skill', - '---', - '', - '# HelloAGENTS', - ].join('\n')); - await writeFile(join(helloAgentsDir, 'analyze', 'SKILL.md'), [ - '---', - 'name: analyze', - 'description: Sub skill', - '---', - '', - '# Analyze', - ].join('\n')); + const skills = await listSkills() - const systemRoot = join(skillsRoot, '.system'); - const systemSkillDir = join(systemRoot, 'skill-creator'); - await mkdir(systemSkillDir, { recursive: true }); - await writeFile(join(systemSkillDir, 'SKILL.md'), [ - '---', - 'name: skill-creator', - 'description: Create skills', - '---', - '', - '# Skill Creator', - ].join('\n')); + expect(skills.map((skill) => skill.name)).toEqual(['amis']) + }) - const skills = await listSkills(); - expect(skills.map((s) => s.name)).toEqual(['amis', 'helloagents', 'skill-creator']); - }); + it('ignores legacy ~/.codex skills', async () => { + await writeSkill(join(homeDir, '.agents', 'skills', 'amis'), 'amis', 'AMIS guide') + await writeSkill(join(homeDir, '.codex', 'skills', 'hello-agents'), 'helloagents', 'Main skill') + await writeSkill(join(homeDir, '.codex', 'skills', '.system', 'skill-creator'), 'skill-creator', 'Create skills') + + const skills = await listSkills() + + expect(skills.map((skill) => skill.name)).toEqual(['amis']) + }) it('falls back to directory name when frontmatter is missing', async () => { - const skillsRoot = join(codexHome, 'skills'); - const fallbackDir = join(skillsRoot, 'no-frontmatter'); - await mkdir(fallbackDir, { recursive: true }); - await writeFile(join(fallbackDir, 'SKILL.md'), '# No Frontmatter\n'); + const skillDir = join(homeDir, '.agents', 'skills', 'no-frontmatter') + await mkdir(skillDir, { recursive: true }) + await writeFile(join(skillDir, 'SKILL.md'), '# No Frontmatter\n') - const skills = await listSkills(); - expect(skills).toEqual([{ name: 'no-frontmatter', description: undefined }]); - }); -}); + await expect(listSkills()).resolves.toEqual([ + { name: 'no-frontmatter', description: undefined } + ]) + }) + it('loads project skills from cwd up to repo root', async () => { + const repoRoot = join(sandboxDir, 'repo') + const packageDir = join(repoRoot, 'packages') + const workingDirectory = join(packageDir, 'app') + + await mkdir(join(repoRoot, '.git'), { recursive: true }) + await writeSkill(join(repoRoot, '.agents', 'skills', 'root-skill'), 'root-skill', 'Repo root skill') + await writeSkill(join(packageDir, '.agents', 'skills', 'package-skill'), 'package-skill', 'Package skill') + await writeSkill(join(workingDirectory, '.agents', 'skills', 'local-skill'), 'local-skill', 'Local skill') + await writeSkill(join(sandboxDir, '.agents', 'skills', 'outside-skill'), 'outside-skill', 'Outside repo skill') + + const skills = await listSkills(workingDirectory) + + expect(skills.map((skill) => skill.name)).toEqual(['local-skill', 'package-skill', 'root-skill']) + }) + + it('uses only cwd project skills outside a git repository', async () => { + const parentDirectory = join(sandboxDir, 'workspace') + const workingDirectory = join(parentDirectory, 'feature') + + await writeSkill(join(parentDirectory, '.agents', 'skills', 'parent-skill'), 'parent-skill', 'Parent skill') + await writeSkill(join(workingDirectory, '.agents', 'skills', 'local-skill'), 'local-skill', 'Local skill') + + const skills = await listSkills(workingDirectory) + + expect(skills.map((skill) => skill.name)).toEqual(['local-skill']) + }) + + it('prefers nearest project skill over parent and user duplicates', async () => { + const repoRoot = join(sandboxDir, 'repo') + const workingDirectory = join(repoRoot, 'apps', 'web') + + await mkdir(join(repoRoot, '.git'), { recursive: true }) + await writeSkill(join(homeDir, '.agents', 'skills', 'shared'), 'shared', 'User shared skill') + await writeSkill(join(repoRoot, '.agents', 'skills', 'shared'), 'shared', 'Repo shared skill') + await writeSkill(join(workingDirectory, '.agents', 'skills', 'shared'), 'shared', 'Local shared skill') + + const skills = await listSkills(workingDirectory) + const sharedSkills = skills.filter((skill) => skill.name === 'shared') + + expect(sharedSkills).toHaveLength(1) + expect(sharedSkills[0]).toEqual({ + name: 'shared', + description: 'Local shared skill' + }) + }) +}) diff --git a/cli/src/modules/common/skills.ts b/cli/src/modules/common/skills.ts index 613e9eeb..9090987a 100644 --- a/cli/src/modules/common/skills.ts +++ b/cli/src/modules/common/skills.ts @@ -1,5 +1,5 @@ -import { readdir, readFile } from 'fs/promises'; -import { join, basename } from 'path'; +import { access, readdir, readFile } from 'fs/promises'; +import { basename, dirname, join, resolve } from 'path'; import { homedir } from 'os'; import { parse as parseYaml } from 'yaml'; @@ -17,9 +17,53 @@ export interface ListSkillsResponse { error?: string; } -function getSkillsRoot(): string { - const codexHome = process.env.CODEX_HOME ?? join(homedir(), '.codex'); - return join(codexHome, 'skills'); +function getHomeDirectory(): string { + return process.env.HOME ?? process.env.USERPROFILE ?? homedir(); +} + +function getUserSkillsRoot(): string { + return join(getHomeDirectory(), '.agents', 'skills'); +} + +function getAdminSkillsRoot(): string { + return join('/etc', 'codex', 'skills'); +} + +function getProjectSkillsRoot(directory: string): string { + return join(directory, '.agents', 'skills'); +} + +async function pathExists(path: string): Promise { + try { + await access(path); + return true; + } catch { + return false; + } +} + +async function listProjectSkillsRoots(workingDirectory?: string): Promise { + if (!workingDirectory) { + return []; + } + + const resolvedWorkingDirectory = resolve(workingDirectory); + const directories = [resolvedWorkingDirectory]; + let currentDirectory = resolvedWorkingDirectory; + + while (true) { + if (await pathExists(join(currentDirectory, '.git'))) { + return directories.map(getProjectSkillsRoot); + } + + const parentDirectory = dirname(currentDirectory); + if (parentDirectory === currentDirectory) { + return [getProjectSkillsRoot(resolvedWorkingDirectory)]; + } + + currentDirectory = parentDirectory; + directories.push(currentDirectory); + } } function parseFrontmatter(fileContent: string): { frontmatter?: Record; body: string } { @@ -59,23 +103,7 @@ async function listTopLevelSkillDirs(skillsRoot: string): Promise { const result: string[] = []; for (const entry of entries) { - if (!entry.isDirectory()) { - continue; - } - - if (entry.name === '.system') { - const systemRoot = join(skillsRoot, entry.name); - try { - const systemEntries = await readdir(systemRoot, { withFileTypes: true }); - for (const systemEntry of systemEntries) { - if (!systemEntry.isDirectory()) { - continue; - } - result.push(join(systemRoot, systemEntry.name)); - } - } catch { - // ignore unreadable .system - } + if (!entry.isDirectory() || entry.name.startsWith('.')) { continue; } @@ -88,13 +116,7 @@ async function listTopLevelSkillDirs(skillsRoot: string): Promise { } } -export async function listSkills(): Promise { - const skillsRoot = getSkillsRoot(); - const skillDirs = await listTopLevelSkillDirs(skillsRoot); - if (skillDirs.length === 0) { - return []; - } - +async function readSkillsFromDirs(skillDirs: string[]): Promise { const skills = await Promise.all(skillDirs.map(async (dir): Promise => { const filePath = join(dir, 'SKILL.md'); try { @@ -105,8 +127,33 @@ export async function listSkills(): Promise { } })); - return skills - .filter((skill): skill is SkillSummary => skill !== null) - .sort((a, b) => a.name.localeCompare(b.name)); + return skills.filter((skill): skill is SkillSummary => skill !== null); } +export async function listSkills(workingDirectory?: string): Promise { + const projectRoots = await listProjectSkillsRoots(workingDirectory); + const [projectSkillDirs, userSkillDirs, adminSkillDirs] = await Promise.all([ + Promise.all(projectRoots.map(async (root) => await listTopLevelSkillDirs(root))).then((dirs) => dirs.flat()), + listTopLevelSkillDirs(getUserSkillsRoot()), + listTopLevelSkillDirs(getAdminSkillsRoot()), + ]); + + const [projectSkills, userSkills, adminSkills] = await Promise.all([ + readSkillsFromDirs(projectSkillDirs), + readSkillsFromDirs(userSkillDirs), + readSkillsFromDirs(adminSkillDirs), + ]); + + const dedupedSkills = new Map(); + for (const skill of [ + ...projectSkills, + ...userSkills, + ...adminSkills, + ]) { + if (!dedupedSkills.has(skill.name)) { + dedupedSkills.set(skill.name, skill); + } + } + + return [...dedupedSkills.values()].sort((a, b) => a.name.localeCompare(b.name)); +}