Add project skills for codex, close #241

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<ListSkillsRequest, ListSkillsResponse>('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
         }
     })
 }
-

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)
 }

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<void> {
+    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'
+        })
+    })
+})

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<boolean> {
+    try {
+        await access(path);
+        return true;
+    } catch {
+        return false;
+    }
+}
+
+async function listProjectSkillsRoots(workingDirectory?: string): Promise<string[]> {
+    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<string, unknown>; body: string } {
@@ -59,23 +103,7 @@ async function listTopLevelSkillDirs(skillsRoot: string): Promise<string[]> {
         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<string[]> {
     }
 }
 
-export async function listSkills(): Promise<SkillSummary[]> {
-    const skillsRoot = getSkillsRoot();
-    const skillDirs = await listTopLevelSkillDirs(skillsRoot);
-    if (skillDirs.length === 0) {
-        return [];
-    }
-
+async function readSkillsFromDirs(skillDirs: string[]): Promise<SkillSummary[]> {
     const skills = await Promise.all(skillDirs.map(async (dir): Promise<SkillSummary | null> => {
         const filePath = join(dir, 'SKILL.md');
         try {
@@ -105,8 +127,33 @@ export async function listSkills(): Promise<SkillSummary[]> {
         }
     }));
 
-    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<SkillSummary[]> {
+    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<string, SkillSummary>();
+    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));
+}