diff --git a/.github/workflows/test-plugin-install.yml b/.github/workflows/test-plugin-install.yml index 73b69b2c..782b330e 100644 --- a/.github/workflows/test-plugin-install.yml +++ b/.github/workflows/test-plugin-install.yml @@ -34,6 +34,12 @@ jobs: - name: Run skill evals (trigger + routing) run: node scripts/run-evals.js --min-rank1 80 + - name: Validate references/ links in skills + run: node scripts/validate-reference-links.js + + - name: Test reference-link validator + run: node --test scripts/validate-reference-links-test.js + validate-commands: name: Validate command parity and description sync runs-on: ubuntu-latest diff --git a/scripts/validate-reference-links-test.js b/scripts/validate-reference-links-test.js new file mode 100644 index 00000000..8c8b147a --- /dev/null +++ b/scripts/validate-reference-links-test.js @@ -0,0 +1,153 @@ +#!/usr/bin/env node + +'use strict'; + +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); +const { afterEach, test } = require('node:test'); + +const VALIDATOR = path.join(__dirname, 'validate-reference-links.js'); +const sandboxes = []; + +function makeSandbox() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-skills-validate-reference-links-test-')); + const scriptsDir = path.join(root, 'scripts'); + fs.mkdirSync(scriptsDir, { recursive: true }); + fs.copyFileSync(VALIDATOR, path.join(scriptsDir, 'validate-reference-links.js')); + sandboxes.push(root); + return root; +} + +function writeFile(root, relativePath, content) { + const file = path.join(root, relativePath); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, content); +} + +function run(root) { + return spawnSync(process.execPath, [path.join(root, 'scripts', 'validate-reference-links.js')], { + cwd: root, + encoding: 'utf8', + }); +} + +afterEach(() => { + for (const root of sandboxes.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test('passes when a skill reaches the shared checklist two levels up', () => { + const root = makeSandbox(); + writeFile(root, 'references/definition-of-done.md', '# Definition of Done\n'); + writeFile( + root, + 'skills/using-agent-skills/SKILL.md', + 'See `../../references/definition-of-done.md`.\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('fails when a skill links the shared checklist as if it were colocated', () => { + // The regression: references/ lives at the repo root, but the link is + // resolved from skills//, so it points two levels too deep. + const root = makeSandbox(); + writeFile(root, 'references/definition-of-done.md', '# Definition of Done\n'); + writeFile( + root, + 'skills/using-agent-skills/SKILL.md', + 'See `references/definition-of-done.md`.\n' + ); + + const result = run(root); + + assert.equal(result.status, 1, result.stdout + result.stderr); + assert.match(result.stdout, /1 skills checked — 1 error\(s\) — FAILED/); + assert.match( + result.stdout, + /L1: references\/definition-of-done\.md — resolves to skills\/using-agent-skills\/references\/definition-of-done\.md/ + ); + assert.match(result.stdout, /use `\.\.\/\.\.\/references\/\.md`/); +}); + +test('checks markdown link syntax, not just backtick mentions', () => { + const root = makeSandbox(); + writeFile(root, 'references/definition-of-done.md', '# Definition of Done\n'); + writeFile(root, 'skills/using-agent-skills/SKILL.md', 'See [DoD](references/definition-of-done.md).\n'); + + const result = run(root); + + assert.equal(result.status, 1, result.stdout + result.stderr); + assert.match(result.stdout, /L1: references\/definition-of-done\.md/); +}); + +test('passes when a skill colocates its own references directory', () => { + // CLAUDE.md allows self-contained skills to keep references under + // skills//references/. Those links are correct as written. + const root = makeSandbox(); + writeFile(root, 'skills/dataviz/references/palette.md', '# Palette\n'); + writeFile(root, 'skills/dataviz/SKILL.md', 'See `references/palette.md`.\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('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'); + writeFile(root, 'skills/shipping-and-launch/SKILL.md', 'See `../../references/renamed.md`.\n'); + + const result = run(root); + + assert.equal(result.status, 1, result.stdout + result.stderr); + assert.match(result.stdout, /L1: \.\.\/\.\.\/references\/renamed\.md/); + assert.match(result.stdout, /1 skills checked — 1 error\(s\) — FAILED/); +}); + +test('ignores paths that are not references/ links', () => { + // Skills legitimately name artifacts the user has yet to create. Widening + // this validator into a general markdown linter would fail the build on them. + const root = makeSandbox(); + writeFile(root, 'references/definition-of-done.md', '# Definition of Done\n'); + writeFile( + root, + 'skills/planning-and-task-breakdown/SKILL.md', + [ + 'Save the task list to `tasks/todo.md` and the plan to `tasks/plan.md`.', + 'Record findings in `PERF.md` or `docs/ideas/[idea-name].md`.', + 'Related: `skills/incremental-implementation/SKILL.md`.', + 'See `../../references/definition-of-done.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('reports every unresolvable link, not just the first per skill', () => { + const root = makeSandbox(); + writeFile(root, 'references/security-checklist.md', '# Security\n'); + writeFile(root, 'references/performance-checklist.md', '# Performance\n'); + writeFile( + root, + 'skills/code-review-and-quality/SKILL.md', + ['See `references/security-checklist.md`.', 'And `references/performance-checklist.md`.', ''].join('\n') + ); + + const result = run(root); + + assert.equal(result.status, 1, result.stdout + result.stderr); + assert.match(result.stdout, /1 skills checked — 2 error\(s\) — FAILED/); +}); diff --git a/scripts/validate-reference-links.js b/scripts/validate-reference-links.js new file mode 100644 index 00000000..f460bf98 --- /dev/null +++ b/scripts/validate-reference-links.js @@ -0,0 +1,103 @@ +#!/usr/bin/env node +/** + * validate-reference-links.js + * + * Guards links from skills to the shared `references/` checklists. + * + * Those checklists live in the repo-root `references/` directory, but every + * SKILL.md used to link them as `references/.md` — a path relative to + * the skill's own directory, which is two levels below the root. All 18 links + * across 11 skills resolved to files that do not exist, in the repo and in + * every plugin-install layout (~/.claude/plugins/cache/..., ~/.codex/...). + * Agents that followed the guidance — for example using-agent-skills pointing + * at the Definition of Done — hit a file-not-found and stalled. + * + * Nothing else in CI catches this: validate-artifact-paths.js is scoped to + * spec/plan/todo artifacts and is explicitly not a general markdown linter. + * + * The rule enforced here: every `references/*.md` link in a SKILL.md must + * resolve to an existing file relative to that skill's own directory. This + * accepts both conventions in CLAUDE.md — shared checklists reached via + * `../../references/`, and a skill's own colocated `references/` directory. + * + * 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. + * + * Exit codes: 0 = all clear, 1 = one or more unresolvable links. + */ + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +const ROOT = path.resolve(__dirname, '..'); +const SKILLS_DIR = path.join(ROOT, 'skills'); + +// Matches a link to a references/ markdown file, with any number of leading +// `../` segments: `references/x.md`, `../../references/x.md`. Anchored on a +// 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 }); + } + } + }); + + return violations; +} + +function main() { + console.log('Checking references/ links in skills...\n'); + + if (!fs.existsSync(SKILLS_DIR)) { + console.log('No skills/ directory — nothing to check.'); + return; + } + + let checked = 0; + let errors = 0; + + const skillNames = fs.readdirSync(SKILLS_DIR).sort(); + for (const name of skillNames) { + const skillDir = path.join(SKILLS_DIR, name); + const skillFile = path.join(skillDir, 'SKILL.md'); + if (!fs.statSync(skillDir).isDirectory() || !fs.existsSync(skillFile)) continue; + + checked++; + const violations = findViolations(skillDir, skillFile); + + 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)); + console.log(` L${line}: ${link} — resolves to ${resolved}, which does not exist`); + errors++; + } + } + } + + const status = errors > 0 ? 'FAILED' : 'PASSED'; + 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('use `../../references/.md`, not `references/.md`.'); + process.exit(1); + } +} + +main();