mirror of
https://github.com/addyosmani/agent-skills.git
synced 2026-10-26 15:05:04 +00:00
fix(validator): check #anchor fragments on references/ links
This commit is contained in:
1 parent
9d0c60d406
commit
a831506ed1
2 files changed
+182
-11
No files matched your search
@@ -366,3 +366,130 @@ test('tilde fence info strings may contain backticks', () => {
|
||||
assert.equal(result.status, 0, result.stdout + result.stderr);
|
||||
assert.match(result.stdout, /1 skills checked — 0 error\(s\) — PASSED/);
|
||||
});
|
||||
|
||||
// Anchors. A `#fragment` after a references/ link must match a heading in the
|
||||
// target file, slugged the way GitHub does it. Without this, renaming a heading
|
||||
// silently breaks every link to it while the file itself still exists.
|
||||
|
||||
function hardeningSkill(root, link) {
|
||||
writeFile(root, 'skills/hardening/SKILL.md', `See [patterns](${link}).\n`);
|
||||
}
|
||||
|
||||
test('passes when an anchor matches a heading in the target file', () => {
|
||||
const root = makeSandbox();
|
||||
hardeningSkill(root, 'references/patterns.md#cross-site-scripting-xss');
|
||||
writeFile(root, 'skills/hardening/references/patterns.md', '# Patterns\n\n## Cross-Site Scripting (XSS)\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 an anchor matches no heading in the target file', () => {
|
||||
// The file exists, so the existing check is satisfied; only the fragment is stale.
|
||||
const root = makeSandbox();
|
||||
hardeningSkill(root, 'references/patterns.md#injection');
|
||||
writeFile(root, 'skills/hardening/references/patterns.md', '# Patterns\n\n## Injection attacks\n');
|
||||
|
||||
const result = run(root);
|
||||
|
||||
assert.equal(result.status, 1, result.stdout + result.stderr);
|
||||
assert.match(
|
||||
result.stdout,
|
||||
/L1: references\/patterns\.md#injection — no heading in skills\/hardening\/references\/patterns\.md produces #injection/
|
||||
);
|
||||
assert.match(result.stdout, /1 skills checked — 1 error\(s\) — FAILED/);
|
||||
assert.match(result.stdout, /GitHub's heading slugs/);
|
||||
// The path was right, so the path hint would only mislead.
|
||||
assert.doesNotMatch(result.stdout, /use `\.\.\/\.\.\/references/);
|
||||
});
|
||||
|
||||
test('a repeated heading gets a -1 suffix, as on GitHub', () => {
|
||||
const root = makeSandbox();
|
||||
writeFile(root, 'skills/hardening/SKILL.md', [
|
||||
'See [first](references/patterns.md#example).',
|
||||
'See [second](references/patterns.md#example-1).',
|
||||
'See [third](references/patterns.md#example-2).',
|
||||
'',
|
||||
].join('\n'));
|
||||
writeFile(root, 'skills/hardening/references/patterns.md', '## Example\n\n## Example\n');
|
||||
|
||||
const result = run(root);
|
||||
|
||||
assert.equal(result.status, 1, result.stdout + result.stderr);
|
||||
assert.match(result.stdout, /L3: references\/patterns\.md#example-2 — no heading/);
|
||||
assert.doesNotMatch(result.stdout, /L[12]:/);
|
||||
assert.match(result.stdout, /1 skills checked — 1 error\(s\) — FAILED/);
|
||||
});
|
||||
|
||||
test('slugs come from the rendered heading text, not its markdown', () => {
|
||||
// Real headings in the pack carry emphasis, code spans and links.
|
||||
const root = makeSandbox();
|
||||
writeFile(root, 'skills/hardening/SKILL.md', [
|
||||
'See [a](references/patterns.md#why-this-is-not-a-ship-job).',
|
||||
'See [b](references/patterns.md#owasp-top-10).',
|
||||
'See [c](references/patterns.md#snake_case-names-stay).',
|
||||
'See [d](references/patterns.md#closing-hashes).',
|
||||
'',
|
||||
].join('\n'));
|
||||
writeFile(root, 'skills/hardening/references/patterns.md', [
|
||||
'### Why this is *not* a `/ship` job',
|
||||
'## [OWASP](https://owasp.org/Top10/) Top 10',
|
||||
'## snake_case names stay',
|
||||
'## Closing hashes ##',
|
||||
'',
|
||||
].join('\n'));
|
||||
|
||||
const result = run(root);
|
||||
|
||||
assert.equal(result.status, 0, result.stdout + result.stderr);
|
||||
});
|
||||
|
||||
test('a heading-like line inside a fenced block is not an anchor', () => {
|
||||
// Bash comments in a fenced example look like H1s.
|
||||
const root = makeSandbox();
|
||||
hardeningSkill(root, 'references/patterns.md#chrome-devtools');
|
||||
writeFile(root, 'skills/hardening/references/patterns.md', [
|
||||
'# Patterns',
|
||||
'',
|
||||
'```bash',
|
||||
'# Chrome DevTools',
|
||||
'```',
|
||||
'',
|
||||
].join('\n'));
|
||||
|
||||
const result = run(root);
|
||||
|
||||
assert.equal(result.status, 1, result.stdout + result.stderr);
|
||||
assert.match(result.stdout, /#chrome-devtools — no heading/);
|
||||
});
|
||||
|
||||
test('anchors resolve from a reference file to a shared checklist', () => {
|
||||
const root = makeSandbox();
|
||||
writeFile(root, 'references/security-checklist.md', '# Security\n\n## OWASP Top 10 Quick Reference\n');
|
||||
hardeningSkill(root, 'references/patterns.md');
|
||||
writeFile(root, 'skills/hardening/references/patterns.md', [
|
||||
'See `../../../references/security-checklist.md#owasp-top-10-quick-reference`.',
|
||||
'See `../../../references/security-checklist.md#destructive-path-operations`.',
|
||||
'',
|
||||
].join('\n'));
|
||||
|
||||
const result = run(root);
|
||||
|
||||
assert.equal(result.status, 1, result.stdout + result.stderr);
|
||||
assert.match(result.stdout, /✗ {2}skills\/hardening\/references\/patterns\.md/);
|
||||
assert.match(result.stdout, /L2: \.\.\/\.\.\/\.\.\/references\/security-checklist\.md#destructive-path-operations — no heading in references\/security-checklist\.md/);
|
||||
assert.doesNotMatch(result.stdout, /L1:/);
|
||||
});
|
||||
|
||||
test('a missing file is reported once, not again for its anchor', () => {
|
||||
const root = makeSandbox();
|
||||
hardeningSkill(root, 'references/renamed.md#injection');
|
||||
|
||||
const result = run(root);
|
||||
|
||||
assert.equal(result.status, 1, result.stdout + result.stderr);
|
||||
assert.match(result.stdout, /L1: references\/renamed\.md — resolves to skills\/hardening\/references\/renamed\.md, which does not exist/);
|
||||
assert.match(result.stdout, /1 skills checked — 1 error\(s\) — FAILED/);
|
||||
});
|
||||
@@ -25,6 +25,10 @@
|
||||
* 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.
|
||||
*
|
||||
* A `#fragment` after such a link must match a heading in the target file,
|
||||
* slugged the way GitHub renders anchors. The file existing is not enough:
|
||||
* renaming a heading breaks every link to it while the path still resolves.
|
||||
*
|
||||
* Scope is deliberately narrow: only `references/*.md` links, only SKILL.md
|
||||
* and `skills/<name>/references/*.md` files. It is not a general markdown
|
||||
* path linter — skills legitimately mention paths that do not exist yet
|
||||
@@ -51,8 +55,34 @@ 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;
|
||||
// non-path character so `myreferences/x.md` does not match. An optional
|
||||
// `#fragment` is captured separately.
|
||||
const REFERENCE_LINK_RE = /(?<![A-Za-z0-9._/-])((?:\.\.\/)*references\/[A-Za-z0-9._-]+\.md)(?:#([\p{L}\p{N}_-]+))?/gu;
|
||||
|
||||
// GitHub's heading anchor: the rendered text, lowercased, with everything but
|
||||
// letters, numbers, `_`, `-` and spaces dropped, then spaces turned to hyphens.
|
||||
function slugify(heading) {
|
||||
const text = heading
|
||||
.replace(/!?\[([^\]]*)\]\([^)]*\)/g, '$1') // links and images keep their text
|
||||
.replace(/<[^>]+>/g, ''); // inline HTML renders no text
|
||||
return text.toLowerCase().replace(/[^\p{L}\p{M}\p{N}_\- ]/gu, '').replace(/ /g, '-');
|
||||
}
|
||||
|
||||
// The anchors a file's ATX headings produce, outside fenced blocks. A repeated
|
||||
// slug gets `-1`, `-2`, ... as on GitHub.
|
||||
function headingAnchors(file) {
|
||||
const anchors = new Set();
|
||||
const seen = new Map();
|
||||
for (const line of stripFencedCodeBlocks(fs.readFileSync(file, 'utf8')).split('\n')) {
|
||||
const match = line.match(/^ {0,3}#{1,6}[ \t]+(.*?)(?:[ \t]+#+)?[ \t]*$/);
|
||||
if (!match) continue;
|
||||
const slug = slugify(match[1]);
|
||||
const count = seen.get(slug) || 0;
|
||||
anchors.add(count === 0 ? slug : `${slug}-${count}`);
|
||||
seen.set(slug, count + 1);
|
||||
}
|
||||
return anchors;
|
||||
}
|
||||
|
||||
// A link is resolved from the directory of the file that contains it.
|
||||
function findViolations(file) {
|
||||
@@ -63,10 +93,12 @@ function findViolations(file) {
|
||||
|
||||
lines.forEach((line, i) => {
|
||||
for (const match of line.matchAll(REFERENCE_LINK_RE)) {
|
||||
const link = match[1];
|
||||
const [, link, anchor] = match;
|
||||
const target = path.resolve(baseDir, link);
|
||||
if (!fs.existsSync(target)) {
|
||||
violations.push({ line: i + 1, link, target });
|
||||
} else if (anchor && !headingAnchors(target).has(anchor)) {
|
||||
violations.push({ line: i + 1, link, target, anchor });
|
||||
}
|
||||
}
|
||||
});
|
||||
@@ -100,6 +132,7 @@ function main() {
|
||||
let checked = 0;
|
||||
let errors = 0;
|
||||
let referenceFileErrors = 0;
|
||||
let anchorErrors = 0;
|
||||
|
||||
const skillNames = fs.readdirSync(SKILLS_DIR).sort();
|
||||
for (const name of skillNames) {
|
||||
@@ -117,9 +150,14 @@ function main() {
|
||||
}
|
||||
|
||||
console.log(` ✗ ${toPosix(file)}`);
|
||||
for (const { line, link, target } of violations) {
|
||||
console.log(` L${line}: ${link} — resolves to ${toPosix(target)}, which does not exist`);
|
||||
for (const { line, link, target, anchor } of violations) {
|
||||
errors++;
|
||||
if (anchor) {
|
||||
console.log(` L${line}: ${link}#${anchor} — no heading in ${toPosix(target)} produces #${anchor}`);
|
||||
anchorErrors++;
|
||||
continue;
|
||||
}
|
||||
console.log(` L${line}: ${link} — resolves to ${toPosix(target)}, which does not exist`);
|
||||
if (file !== skillFile) referenceFileErrors++;
|
||||
}
|
||||
}
|
||||
@@ -129,12 +167,18 @@ function main() {
|
||||
console.log(`\n${checked} skills checked — ${errors} error(s) — ${status}`);
|
||||
|
||||
if (errors > 0) {
|
||||
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`.');
|
||||
if (errors > anchorErrors) {
|
||||
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`.');
|
||||
}
|
||||
}
|
||||
if (anchorErrors > 0) {
|
||||
console.log("\nAnchors are matched against GitHub's heading slugs: the heading text lowercased,");
|
||||
console.log('punctuation dropped, spaces turned to hyphens, and -1, -2, ... for repeated headings.');
|
||||
}
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user