diff --git a/scripts/validate-reference-links-test.js b/scripts/validate-reference-links-test.js index a720fec2..e9ded5ef 100644 --- a/scripts/validate-reference-links-test.js +++ b/scripts/validate-reference-links-test.js @@ -103,6 +103,93 @@ test('passes when a skill colocates its own references directory', () => { assert.match(result.stdout, /1 skills checked — 0 error\(s\) — PASSED/); }); +test('passes when a skill reference file reaches the shared checklist three levels up', () => { + // A file under skills//references/ sits one directory deeper than + // SKILL.md, so the shared checklists are three levels up from it, not two. + const root = makeSandbox(); + writeFile(root, 'references/security-checklist.md', '# Security\n'); + writeFile(root, 'skills/hardening/SKILL.md', 'See [patterns](references/patterns.md).\n'); + writeFile( + root, + 'skills/hardening/references/patterns.md', + 'Shared checklists live in `../../../references/security-checklist.md`.\n' + ); + + const result = run(root); + + assert.equal(result.status, 0, result.stdout + result.stderr); + assert.match(result.stdout, /✓ {2}skills\/hardening\/references\/patterns\.md/); + assert.match(result.stdout, /1 skills checked — 0 error\(s\) — PASSED/); +}); + +test('fails when a skill reference file links the shared checklist with the SKILL.md prefix', () => { + // The same off-by-a-level mistake this validator exists to catch, made from + // inside the skill's references/ directory: the link is resolved from the + // file that contains it, so `../../` stops at skills/. + const root = makeSandbox(); + writeFile(root, 'references/security-checklist.md', '# Security\n'); + writeFile(root, 'skills/hardening/SKILL.md', 'See [patterns](references/patterns.md).\n'); + writeFile( + root, + 'skills/hardening/references/patterns.md', + ['# Patterns', '', 'See `../../references/security-checklist.md`.', ''].join('\n') + ); + + const result = run(root); + + assert.equal(result.status, 1, result.stdout + result.stderr); + assert.match(result.stdout, /✓ {2}skills\/hardening\/SKILL\.md/); + assert.match(result.stdout, /✗ {2}skills\/hardening\/references\/patterns\.md/); + assert.match( + result.stdout, + /L3: \.\.\/\.\.\/references\/security-checklist\.md — resolves to skills\/references\/security-checklist\.md/ + ); + assert.match(result.stdout, /1 skills checked — 1 error\(s\) — FAILED/); + assert.match(result.stdout, /use `\.\.\/\.\.\/\.\.\/references\/\.md`/); +}); + +test('keeps the same narrow rule inside a skill reference file', () => { + // Same exemptions as SKILL.md: fenced examples, sibling files named without + // a references/ prefix, and artifacts that do not exist yet. + const root = makeSandbox(); + writeFile(root, 'references/security-checklist.md', '# Security\n'); + writeFile(root, 'skills/hardening/SKILL.md', 'See [patterns](references/patterns.md).\n'); + writeFile( + root, + 'skills/hardening/references/patterns.md', + [ + 'Record findings in `SECURITY.md` or `docs/threat-model.md`. See also `other-patterns.md`.', + '', + '```markdown', + '[wrong on purpose](../../references/security-checklist.md)', + '```', + '', + ].join('\n') + ); + + const result = run(root); + + assert.equal(result.status, 0, result.stdout + result.stderr); + assert.match(result.stdout, /1 skills checked — 0 error\(s\) — PASSED/); +}); + +test('counts errors from SKILL.md and its reference files together', () => { + const root = makeSandbox(); + writeFile(root, 'references/security-checklist.md', '# Security\n'); + writeFile(root, 'skills/hardening/SKILL.md', 'See `references/security-checklist.md`.\n'); + writeFile(root, 'skills/hardening/references/a.md', 'See `../../references/security-checklist.md`.\n'); + writeFile(root, 'skills/hardening/references/b.md', 'See `../../../references/security-checklist.md`.\n'); + writeFile(root, 'skills/hardening/references/notes.txt', 'See ../../references/security-checklist.md\n'); + + const result = run(root); + + assert.equal(result.status, 1, result.stdout + result.stderr); + assert.match(result.stdout, /✗ {2}skills\/hardening\/references\/a\.md/); + assert.match(result.stdout, /✓ {2}skills\/hardening\/references\/b\.md/); + assert.doesNotMatch(result.stdout, /notes\.txt/); + assert.match(result.stdout, /1 skills checked — 2 error\(s\) — FAILED/); +}); + test('fails when a link points at a checklist that no longer exists', () => { const root = makeSandbox(); writeFile(root, 'references/definition-of-done.md', '# Definition of Done\n'); diff --git a/scripts/validate-reference-links.js b/scripts/validate-reference-links.js index 3e69a08e..870a204a 100644 --- a/scripts/validate-reference-links.js +++ b/scripts/validate-reference-links.js @@ -20,10 +20,16 @@ * accepts both conventions in CLAUDE.md — shared checklists reached via * `../../references/`, and a skill's own colocated `references/` directory. * + * The markdown files inside a skill's own `references/` directory get the + * same check, with each link resolved from the file that contains it. They + * sit one directory deeper than SKILL.md, so from there the shared checklists + * are `../../../references/`: the same off-by-a-level mistake, one level down. + * * Scope is deliberately narrow: only `references/*.md` links, only SKILL.md - * files. It is not a general markdown path linter — skills legitimately - * mention paths that do not exist yet (`tasks/todo.md`, `PERF.md`, - * `docs/ideas/[idea-name].md`), and those must not fail the build. + * and `skills//references/*.md` files. It is not a general markdown + * path linter — skills legitimately mention paths that do not exist yet + * (`tasks/todo.md`, `PERF.md`, `docs/ideas/[idea-name].md`), and those must + * not fail the build. * * Fenced code blocks are exempt for the same reason. Text inside a fence is * an example, not a link an agent will follow — and this rule in particular @@ -48,16 +54,19 @@ const SKILLS_DIR = path.join(ROOT, 'skills'); // non-path character so `myreferences/x.md` does not match. const REFERENCE_LINK_RE = /(? { for (const match of line.matchAll(REFERENCE_LINK_RE)) { const link = match[1]; - if (!fs.existsSync(path.resolve(skillDir, link))) { - violations.push({ line: i + 1, link }); + const target = path.resolve(baseDir, link); + if (!fs.existsSync(target)) { + violations.push({ line: i + 1, link, target }); } } }); @@ -65,6 +74,21 @@ function findViolations(skillDir, skillFile) { return violations; } +// The markdown files directly inside a skill's own references/ directory. +function skillReferenceFiles(skillDir) { + const dir = path.join(skillDir, 'references'); + if (!fs.existsSync(dir) || !fs.statSync(dir).isDirectory()) return []; + return fs.readdirSync(dir) + .filter((name) => name.endsWith('.md')) + .sort() + .map((name) => path.join(dir, name)) + .filter((file) => fs.statSync(file).isFile()); +} + +function toPosix(file) { + return path.relative(ROOT, file).split(path.sep).join('/'); +} + function main() { console.log('Checking references/ links in skills...\n'); @@ -75,6 +99,7 @@ function main() { let checked = 0; let errors = 0; + let referenceFileErrors = 0; const skillNames = fs.readdirSync(SKILLS_DIR).sort(); for (const name of skillNames) { @@ -83,16 +108,19 @@ function main() { if (!fs.statSync(skillDir).isDirectory() || !fs.existsSync(skillFile)) continue; checked++; - const violations = findViolations(skillDir, skillFile); + for (const file of [skillFile, ...skillReferenceFiles(skillDir)]) { + const violations = findViolations(file); - if (violations.length === 0) { - console.log(` ✓ skills/${name}/SKILL.md`); - } else { - console.log(` ✗ skills/${name}/SKILL.md`); - for (const { line, link } of violations) { - const resolved = path.relative(ROOT, path.resolve(skillDir, link)).split(path.sep).join('/'); - console.log(` L${line}: ${link} — resolves to ${resolved}, which does not exist`); + if (violations.length === 0) { + console.log(` ✓ ${toPosix(file)}`); + continue; + } + + console.log(` ✗ ${toPosix(file)}`); + for (const { line, link, target } of violations) { + console.log(` L${line}: ${link} — resolves to ${toPosix(target)}, which does not exist`); errors++; + if (file !== skillFile) referenceFileErrors++; } } } @@ -101,9 +129,13 @@ function main() { console.log(`\n${checked} skills checked — ${errors} error(s) — ${status}`); if (errors > 0) { - console.log('\nLinks to references/ are resolved from the skill\'s own directory.'); - console.log('Shared checklists live in the repo-root references/, two levels up:'); + console.log('\nLinks to references/ are resolved from the directory of the file that contains them.'); + console.log('Shared checklists live in the repo-root references/, two levels up from a SKILL.md:'); console.log('use `../../references/.md`, not `references/.md`.'); + if (referenceFileErrors > 0) { + console.log('From a file inside skills//references/ they are three levels up:'); + console.log('use `../../../references/.md`.'); + } process.exit(1); } }