diff --git a/src/agent/skills.ts b/src/agent/skills.ts index 0c3e403..753d410 100644 --- a/src/agent/skills.ts +++ b/src/agent/skills.ts @@ -9,7 +9,7 @@ */ import { existsSync } from "node:fs"; -import { readdir, readFile, stat } from "node:fs/promises"; +import { readdir, readFile, realpath, stat } from "node:fs/promises"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { parseFrontmatter } from "../utils/frontmatter"; @@ -242,31 +242,68 @@ async function findSkillFiles( skills: Skill[], errors: SkillDiscoveryError[], source: SkillSource, + visitedRealPaths: Set = new Set(), ): Promise { + try { + const resolvedPath = await realpath(currentPath); + if (visitedRealPaths.has(resolvedPath)) { + return; + } + visitedRealPaths.add(resolvedPath); + } catch (error) { + errors.push({ + path: currentPath, + message: `Failed to resolve directory path: ${error instanceof Error ? error.message : String(error)}`, + }); + return; + } + try { const entries = await readdir(currentPath, { withFileTypes: true }); for (const entry of entries) { const fullPath = join(currentPath, entry.name); - // Follows symlinks to detect directories and files correctly - const entryStat = await stat(fullPath); - if (entryStat.isDirectory()) { - // Recursively search subdirectories - await findSkillFiles(fullPath, rootPath, skills, errors, source); - } else if (entry.isFile() && entry.name.toUpperCase() === "SKILL.MD") { - // Found a SKILL.MD file - try { - const skill = await parseSkillFile(fullPath, rootPath, source); - if (skill) { - skills.push(skill); - } - } catch (error) { - errors.push({ - path: fullPath, - message: error instanceof Error ? error.message : String(error), - }); + try { + let isDirectory = entry.isDirectory(); + let isFile = entry.isFile(); + + // Follow symlink targets so linked skills are discoverable. + if (entry.isSymbolicLink()) { + const entryStat = await stat(fullPath); + isDirectory = entryStat.isDirectory(); + isFile = entryStat.isFile(); } + + if (isDirectory) { + // Recursively search subdirectories. + await findSkillFiles( + fullPath, + rootPath, + skills, + errors, + source, + visitedRealPaths, + ); + } else if (isFile && entry.name.toUpperCase() === "SKILL.MD") { + // Found a SKILL.MD file + try { + const skill = await parseSkillFile(fullPath, rootPath, source); + if (skill) { + skills.push(skill); + } + } catch (error) { + errors.push({ + path: fullPath, + message: error instanceof Error ? error.message : String(error), + }); + } + } + } catch (error) { + errors.push({ + path: fullPath, + message: `Failed to inspect path: ${error instanceof Error ? error.message : String(error)}`, + }); } } } catch (error) { diff --git a/src/tests/agent/skills-discovery.test.ts b/src/tests/agent/skills-discovery.test.ts new file mode 100644 index 0000000..f797ba2 --- /dev/null +++ b/src/tests/agent/skills-discovery.test.ts @@ -0,0 +1,111 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { + existsSync, + mkdirSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { join } from "node:path"; +import { discoverSkills } from "../../agent/skills"; + +describe.skipIf(process.platform === "win32")( + "skills discovery with symlinks", + () => { + const testDir = join(process.cwd(), ".test-skills-discovery"); + const projectSkillsDir = join(testDir, ".skills"); + const originalCwd = process.cwd(); + + const writeSkill = (skillDir: string, skillName: string) => { + mkdirSync(skillDir, { recursive: true }); + writeFileSync( + join(skillDir, "SKILL.md"), + `---\nname: ${skillName}\ndescription: ${skillName} description\n---\n\n# ${skillName}\n`, + ); + }; + + beforeEach(() => { + mkdirSync(testDir, { recursive: true }); + process.chdir(testDir); + }); + + afterEach(() => { + process.chdir(originalCwd); + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true, force: true }); + } + }); + + test("discovers skills from symlinked directories", async () => { + mkdirSync(projectSkillsDir, { recursive: true }); + + const externalSkillDir = join(testDir, "external-skill"); + writeSkill(externalSkillDir, "Linked Skill"); + + symlinkSync( + externalSkillDir, + join(projectSkillsDir, "linked-skill"), + "dir", + ); + + const result = await discoverSkills(projectSkillsDir, undefined, { + skipBundled: true, + sources: ["project"], + }); + + expect(result.errors).toHaveLength(0); + expect(result.skills.some((skill) => skill.id === "linked-skill")).toBe( + true, + ); + }); + + test("handles symlink cycles without hanging and still discovers siblings", async () => { + mkdirSync(projectSkillsDir, { recursive: true }); + writeSkill(join(projectSkillsDir, "good-skill"), "Good Skill"); + + const cycleDir = join(projectSkillsDir, "cycle"); + mkdirSync(cycleDir, { recursive: true }); + symlinkSync("..", join(cycleDir, "loop"), "dir"); + + const result = (await Promise.race([ + discoverSkills(projectSkillsDir, undefined, { + skipBundled: true, + sources: ["project"], + }), + new Promise((_, reject) => { + setTimeout( + () => reject(new Error("skills discovery timed out")), + 2000, + ); + }), + ])) as Awaited>; + + expect(result.skills.some((skill) => skill.id === "good-skill")).toBe( + true, + ); + }); + + test("continues discovery when a dangling symlink cannot be inspected", async () => { + mkdirSync(projectSkillsDir, { recursive: true }); + writeSkill(join(projectSkillsDir, "healthy-skill"), "Healthy Skill"); + + symlinkSync( + join(projectSkillsDir, "missing-target"), + join(projectSkillsDir, "broken-link"), + "dir", + ); + + const result = await discoverSkills(projectSkillsDir, undefined, { + skipBundled: true, + sources: ["project"], + }); + + expect(result.skills.some((skill) => skill.id === "healthy-skill")).toBe( + true, + ); + expect( + result.errors.some((error) => error.path.includes("broken-link")), + ).toBe(true); + }); + }, +);