Merge #588: validate reference links inside skills' own reference files (#585)

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/<name>/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\/<file>\.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');

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/<name>/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 = /(?<![A-Za-z0-9._/-])((?:\.\.\/)*references\/[A-Za-z0-9._-]+\.md)/g;
 
-function findViolations(skillDir, skillFile) {
+// A link is resolved from the directory of the file that contains it.
+function findViolations(file) {
   const violations = [];
+  const baseDir = path.dirname(file);
   // Share the linter's fence rules; blanked lines preserve diagnostic positions.
-  const lines = stripFencedCodeBlocks(fs.readFileSync(skillFile, 'utf8')).split('\n');
+  const lines = stripFencedCodeBlocks(fs.readFileSync(file, 'utf8')).split('\n');
 
   lines.forEach((line, i) => {
     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/<file>.md`, not `references/<file>.md`.');
+    if (referenceFileErrors > 0) {
+      console.log('From a file inside skills/<name>/references/ they are three levels up:');
+      console.log('use `../../../references/<file>.md`.');
+    }
     process.exit(1);
   }
 }