ci: add a gate for references/ links in skills
The path fix has no regression guard: nothing in CI resolves references/ links, so all 18 were broken while CI stayed green. validate-artifact- paths.js is scoped to spec/plan/todo artifacts and says in its own header that it is not a general markdown path linter. Add validate-reference-links.js, which resolves every `references/*.md` link in skills/*/SKILL.md against 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 stays narrow on purpose. Skills legitimately name paths that do not exist yet -- tasks/todo.md, PERF.md, docs/ideas/[idea-name].md -- and a general markdown linter would fail the build on them. A test pins that. Proven against the pre-fix tree: 18 error(s), exit 1, matching the 18 links fixed in the previous commit. After the fix: 0 error(s), exit 0. 7 unit tests cover the regression itself, colocated references/, markdown-link syntax, a renamed target, multiple violations in one skill, and the non-reference paths that must be ignored. Wired into the validate-skills job, alongside the other skill-content checks. Known limitation: fenced code blocks are not stripped, so a SKILL.md that documents the anti-pattern inside a fence would be flagged. Nothing does today. Sharing stripFencedCodeBlocks looks right once #444 lands. Refs #468 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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/<name>/, 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\/<file>\.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/<name>/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/);
|
||||
});
|
||||
@@ -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/<file>.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 = /(?<![A-Za-z0-9._/-])((?:\.\.\/)*references\/[A-Za-z0-9._-]+\.md)/g;
|
||||
|
||||
function findViolations(skillDir, skillFile) {
|
||||
const violations = [];
|
||||
const lines = fs.readFileSync(skillFile, 'utf8').split(/\r?\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 });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
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/<file>.md`, not `references/<file>.md`.');
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
main();
|
||||
Reference in New Issue
Block a user