diff --git a/src/skills/__tests__/skills.test.ts b/src/skills/__tests__/skills.test.ts index 3a1ca7b..2583010 100644 --- a/src/skills/__tests__/skills.test.ts +++ b/src/skills/__tests__/skills.test.ts @@ -5,7 +5,7 @@ import * as path from "node:path" import { HeadlessSession } from "../../engine/loop.js" import type { ChatMessage, LlmClient, LlmRequest, LlmResponse } from "../../engine/types.js" import { createReadOnlyHeadlessExecutor } from "../../tools/executor.js" -import { discoverSkills } from "../registry.js" +import { discoverSkills, type SkillRegistry } from "../registry.js" import { runSkillsCli } from "../cli.js" import { readEnabledSkillRoots } from "../store.js" @@ -307,6 +307,106 @@ async function testMockSessionActivatesCapturedSkillWithinMode(): Promise } } +async function discoverEnabled( + root: string, + skills: string, +): Promise { + return discoverSkills(root, { + enabledRoots: [skills], + includeUserRoots: false, + }) +} + +async function testSnapshotOverflowPath(): Promise { + const root = await fixtureRoot() + try { + const skills = path.join(root, "skills") + const extra: Record = {} + for (let index = 0; index < 9; index++) { + extra[`data-${index}.txt`] = "x".repeat(60 * 1024) + } + const dir = await writeSkill(skills, "big-skill", "big-skill", "B", extra) + const registry = await discoverEnabled(root, skills) + assert.equal(registry.get("big-skill"), undefined) + const problem = registry.problems.find((item) => + item.message.includes("512 KiB")) + assert.ok(problem) + assert.equal(problem.path, dir) + } finally { + await fs.rm(root, { recursive: true, force: true }) + } +} + +async function testSnapshotSymlinkPath(): Promise { + const root = await fixtureRoot() + try { + const skills = path.join(root, "skills") + const dir = await writeSkill(skills, "link-skill", "link-skill", "B") + await fs.symlink(path.join(dir, "SKILL.md"), path.join(dir, "l.md")) + const registry = await discoverEnabled(root, skills) + assert.equal(registry.get("link-skill"), undefined) + const problem = registry.problems.find((item) => + item.message.includes("symlinks are not allowed")) + assert.ok(problem) + assert.equal(problem.path, dir) + } finally { + await fs.rm(root, { recursive: true, force: true }) + } +} + +async function testManifestErrorPath(): Promise { + const root = await fixtureRoot() + try { + const skills = path.join(root, "skills") + const dir = await writeSkill(skills, "bad-skill", "bad-skill", "B") + await fs.writeFile(path.join(dir, "SKILL.md"), "No frontmatter") + const registry = await discoverEnabled(root, skills) + assert.equal(registry.get("bad-skill"), undefined) + const problem = registry.problems.find((item) => + item.message.includes("frontmatter")) + assert.ok(problem) + assert.equal(problem.path, path.join(dir, "SKILL.md")) + } finally { + await fs.rm(root, { recursive: true, force: true }) + } +} + +async function testOversizedManifestPath(): Promise { + const root = await fixtureRoot() + try { + const skills = path.join(root, "skills") + const dir = await writeSkill(skills, "fat-skill", "fat-skill", "B") + await fs.writeFile( + path.join(dir, "SKILL.md"), + `---\nname: fat-skill\ndescription: x\n---\n${"y".repeat(70_000)}\n`, + ) + const registry = await discoverEnabled(root, skills) + assert.equal(registry.get("fat-skill"), undefined) + const problem = registry.problems.find((item) => + item.message.includes("64 KiB")) + assert.ok(problem) + assert.equal(problem.path, path.join(dir, "SKILL.md")) + } finally { + await fs.rm(root, { recursive: true, force: true }) + } +} + +async function testDirectoryMismatchPath(): Promise { + const root = await fixtureRoot() + try { + const skills = path.join(root, "skills") + const dir = await writeSkill(skills, "wrong-dir", "right-name", "B") + const registry = await discoverEnabled(root, skills) + assert.equal(registry.get("right-name"), undefined) + const problem = registry.problems.find((item) => + item.message.includes("directory name must match")) + assert.ok(problem) + assert.equal(problem.path, dir) + } finally { + await fs.rm(root, { recursive: true, force: true }) + } +} + const tests: Array<[string, () => Promise]> = [ ["valid and invalid manifests plus bounded references", testValidInvalidManifestAndBoundedReference], ["duplicate precedence and explicitly enabled roots", testDuplicatePrecedenceAndEnabledRoots], @@ -317,6 +417,11 @@ const tests: Array<[string, () => Promise]> = [ ["local imports are offline and idempotent", testImportIsOfflineAndIdempotent], ["enable records a root for discovery", testEnablePersistsAnExplicitRoot], ["mocked session activates a captured skill without widening mode tools", testMockSessionActivatesCapturedSkillWithinMode], + ["snapshot overflow path", testSnapshotOverflowPath], + ["snapshot symlink path", testSnapshotSymlinkPath], + ["manifest error path", testManifestErrorPath], + ["oversized manifest path", testOversizedManifestPath], + ["directory mismatch path", testDirectoryMismatchPath], ] async function main(): Promise { diff --git a/src/skills/registry.ts b/src/skills/registry.ts index 95c5bbf..7e62df2 100644 --- a/src/skills/registry.ts +++ b/src/skills/registry.ts @@ -248,6 +248,22 @@ export function defaultSkillRoots(workspaceRoot: string, options: DiscoverSkillO return roots } +function snapshotProblemPath( + skillDir: string, + manifestPath: string, + message: string, +): string { + // snapshotSkillFiles reports oversized or binary companions as + // diagnostics, so a thrown size or binary error is SKILL.md itself. + if ( + message.includes("64 KiB") + || message.includes("binary files are unsupported") + ) { + return manifestPath + } + return skillDir +} + /** Discover once and capture every skill file so a running session cannot see mid-turn edits. */ export async function discoverSkills(workspaceRoot: string, options: DiscoverSkillOptions = {}): Promise { const problems: SkillProblem[] = [] @@ -282,8 +298,10 @@ export async function discoverSkills(workspaceRoot: string, options: DiscoverSki break rootLoop } const manifestPath = path.join(skillDir, "SKILL.md") + let snapshotTaken = false try { const files = await snapshotSkillFiles(skillDir, rootReal, (problem) => problems.push(problem)) + snapshotTaken = true const raw = files.get("SKILL.md") if (raw === undefined) throw new Error("SKILL.md is missing from the captured skill snapshot") parseSkillManifest(raw) @@ -295,7 +313,13 @@ export async function discoverSkills(workspaceRoot: string, options: DiscoverSki const pinnedManifest = files.get("SKILL.md") if (pinnedManifest === undefined) throw new Error("SKILL.md was not captured") const pinned = parseSkillManifest(pinnedManifest) - if (path.basename(skillDir) !== pinned.name) throw new Error(`directory name must match manifest name '${pinned.name}'`) + if (path.basename(skillDir) !== pinned.name) { + problems.push({ + path: skillDir, + message: `directory name must match manifest name '${pinned.name}'`, + }) + continue + } const digest = skillFilesDigest(files) const item: SkillSnapshot = Object.freeze({ name: pinned.name, @@ -314,7 +338,15 @@ export async function discoverSkills(workspaceRoot: string, options: DiscoverSki referenceFiles.set(item.name, new Map(files)) registryBytes += snapshotBytes } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "ENOENT") problems.push({ path: manifestPath, message: error instanceof Error ? error.message : String(error) }) + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + const message = error instanceof Error + ? error.message + : String(error) + const target = snapshotTaken + ? manifestPath + : snapshotProblemPath(skillDir, manifestPath, message) + problems.push({ path: target, message }) + } } } }