From d64c9265052488405115fe4ca7e55cee55ab1138 Mon Sep 17 00:00:00 2001 From: Alexander Sullivan Date: Thu, 24 Sep 2026 20:25:43 -0400 Subject: [PATCH 1/2] skills get README --- .claude/rules/prompt-skill-sync.md | 12 +- .../scripts/check-skill-publishability.mjs | 241 ++++++++------ .../check-skill-publishability.test.mjs | 298 ++++++++++++++++++ .claude/scripts/fixture-repository.mjs | 183 +++++++++++ .claude/scripts/plugin-manifests.mjs | 47 +-- .claude/scripts/plugin-manifests.test.mjs | 189 +++++------ .claude/skills/audit-docs/README.md | 25 ++ .claude/skills/audit-pr/README.md | 25 ++ .claude/skills/audit-quality/README.md | 22 ++ .claude/skills/check-skills/SKILL.md | 2 +- .../README.md | 26 ++ CLAUDE.md | 2 +- package-lock.json | 30 +- 13 files changed, 858 insertions(+), 244 deletions(-) create mode 100644 .claude/scripts/check-skill-publishability.test.mjs create mode 100644 .claude/scripts/fixture-repository.mjs create mode 100644 .claude/skills/audit-docs/README.md create mode 100644 .claude/skills/audit-pr/README.md create mode 100644 .claude/skills/audit-quality/README.md create mode 100644 .claude/skills/typescript-code-and-test-standards/README.md diff --git a/.claude/rules/prompt-skill-sync.md b/.claude/rules/prompt-skill-sync.md index 44f4431..341926e 100644 --- a/.claude/rules/prompt-skill-sync.md +++ b/.claude/rules/prompt-skill-sync.md @@ -1,12 +1,13 @@ --- paths: - - '.github/prompts/*.prompt.md' + - '.claude-plugin/marketplace.json' + - '.claude/skills/*/.claude-plugin/plugin.json' + - '.claude/skills/*/README.md' - '.claude/skills/*/SKILL.md' - - '.claude/skills/*/references/*.md' - '.claude/skills/*/agents/*.md' - '.claude/skills/*/assets/*.md' - - '.claude/skills/*/.claude-plugin/plugin.json' - - '.claude-plugin/marketplace.json' + - '.claude/skills/*/references/*.md' + - '.github/prompts/*.prompt.md' --- # Published skills and their prompt halves @@ -68,9 +69,10 @@ Consequences to know before editing a skill, its manifest, or the marketplace: - **The skill's manifest and `agents/` travel with every install.** `npx skills` copies every file except `metadata.json` and the `.git`, `__pycache__`, and `__pypackages__` directories, and `gh skill` copies every file in the tree, so a recipient's copy carries `.claude-plugin/` and loads as `@skills-dir` in their repository too. - **No manifest carries a `version`.** Claude Code keys a marketplace install on it, so a fixed value freezes every recipient on the copy they first installed. Left out, the version is the commit the plugin came from, and a push to `main` reaches marketplace installs the way it reaches `npx skills`. VS Code ignores the field and pulls the repository instead. - **A marketplace entry carries only `name`, `source`, and `description`.** VS Code reads `name`, `description`, `version`, and `source` from an entry and drops the rest, so a component declared there would exist in Claude Code alone, and `npx skills` skips any path without the leading `./`. The entry repeats the manifest's `description` because that is the copy VS Code shows. For the same reason the marketplace sets no `metadata.pluginRoot`, which VS Code applies to `./` sources and Claude Code does not. +- **Every listed skill carries a `README.md`, and it travels too.** VS Code renders `/README.md` as the plugin's page, under exactly that name and with no fallback to `SKILL.md`, so a skill without one shows an empty page. Every installer copies it with the skill, so it names nothing outside the skill directory, links nothing relative (VS Code renders it with no base address), and invokes the skill by name rather than by one host's command form. - **One manifest per skill, one marketplace per repository.** VS Code reads `.plugin/plugin.json`, or a root `plugin.json` declaring the Agent Plugins `$schema`, ahead of `.claude-plugin/plugin.json`, and that format finds skills only under `skills/`, which would leave the directory's own `SKILL.md` unloaded. The Copilot CLI reads `.plugin/plugin.json`, any root `plugin.json`, and `.github/plugin/plugin.json` first. For marketplaces, VS Code and the Copilot CLI try `marketplace.json`, `.plugin/marketplace.json`, and `.github/plugin/marketplace.json` before `.claude-plugin/marketplace.json`, and the first one found is the whole catalogue. -[`plugin-manifests.mjs`](../scripts/plugin-manifests.mjs), which `make -f .claude/Makefile check-skills` runs, holds every manifest to these rules: it parses, its `name` matches the directory, it sets no `version`, no competing manifest sits beside it, and every path in an `agents` key starts with `./`, stays inside the skill directory, and resolves. It requires the marketplace to list every skill that is not internal and nothing else, each entry carrying exactly the three keys above, with a `description` equal to its manifest's. That comparison is why a listed skill needs a manifest even though the marketplace route does not. [`plugin-manifests.test.mjs`](../scripts/plugin-manifests.test.mjs) covers each of these rules, and `make -f .claude/Makefile test-scripts` runs it. +[`plugin-manifests.mjs`](../scripts/plugin-manifests.mjs), which `make -f .claude/Makefile check-skills` runs, holds every manifest to these rules: it parses, its `name` matches the directory, it sets no `version`, no competing manifest sits beside it, and every path in an `agents` key starts with `./`, stays inside the skill directory, and resolves. It requires the marketplace to list every skill that is not internal and nothing else, each entry carrying exactly the three keys above, with a `description` equal to its manifest's and a `README.md` beside the skill. That comparison is why a listed skill needs a manifest even though the marketplace route does not. [`plugin-manifests.test.mjs`](../scripts/plugin-manifests.test.mjs) covers each of these rules, and `make -f .claude/Makefile test-scripts` runs it. ## A published skill stays reachable by name diff --git a/.claude/scripts/check-skill-publishability.mjs b/.claude/scripts/check-skill-publishability.mjs index 5cdbed0..6843755 100644 --- a/.claude/scripts/check-skill-publishability.mjs +++ b/.claude/scripts/check-skill-publishability.mjs @@ -10,26 +10,22 @@ // What is left here is what a machine can decide. // // Run with no arguments. Reports every failure, then exits 1 if there were any. -import { existsSync, readFileSync, readdirSync, statSync } from 'fs'; -import { dirname, join, resolve } from 'path'; -import { fileURLToPath } from 'url'; +import { existsSync, readFileSync, readdirSync } from 'fs'; +import { join, resolve } from 'path'; import { MARKETPLACE_MANIFEST, PLUGIN_MANIFEST, SHADOWING_MARKETPLACES, checkPlugins } from './plugin-manifests.mjs'; -const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..', '..'); +const REPO_ROOT = resolve(import.meta.dirname, '..', '..'); const PROMPT_DIR = join(REPO_ROOT, '.github', 'prompts'); const SKILL_DIR = join(REPO_ROOT, '.claude', 'skills'); /** * Skills published for use outside this repository. Only these are held to the agnosticism - * bar, because a skill written for this repository alone may name this repository's paths. + * bar, because a skill written for this repository alone may name this repository's paths. Here, + * membership reports a skill as `published` and subjects it to {@link checkInvocable}. * * Every other skill is either `installable`, meaning an installer offers it without holding it * to that bar, or `internal`, meaning `metadata.internal: true` keeps it out of `npx skills` - * discovery. Naming the middle state is the point: a skill that is nothing in particular drifts - * into being offered to strangers with nobody having decided that it should be. - * - * The licence rules below apply to all three, because `gh skill` reads no visibility field and - * offers an internal skill as readily as a published one. + * discovery. */ const PUBLISHED = ['audit-docs', 'audit-pr', 'typescript-code-and-test-standards']; @@ -40,23 +36,13 @@ const MAX_BODY_LINES = 500; const MAX_DESCRIPTION = 1024; /** - * A prompt is one file a reader scrolls in a chat pane, with no `references/` to move detail into, - * so its budget is the whole of what it can say. `audit-docs` is held tighter than the other two - * because its subject is narrower. Growth past a budget is a signal to condense, not to raise it: - * restatements of one rule across sections, and worked examples following the rule they - * illustrate, are what a prompt loses first, and never a rule, a checklist item, or a category. - * - * The budget counts characters because a line here is a paragraph. Markdown lint rule `MD013` is - * off repository-wide and Prettier leaves prose unwrapped, so one line in these files runs to - * seventeen hundred characters. Counting lines charges a document for its blank lines and its - * headings, and lets it pay by deleting them: the same eighteen sections cost 36 lines as `###` - * headings and nothing at all inline, while the text is identical either way. Characters do not - * move when a document is reformatted, so only cutting what a prompt says brings the number down. - * - * `wc -c` is the hand-check. It counts bytes where this counts UTF-16 code units, so it reads a - * dozen or so high on a file carrying emoji, which is far inside the headroom each budget leaves. + * The character budget for one prompt file, which a reader scrolls whole with no `references/` to + * move detail into. Characters rather than lines, because a line in these files is a paragraph. + * `wc -c` counts bytes rather than UTF-16 code units, so it reads slightly high on non-ASCII text. */ const MAX_PROMPT_CHARS = 52_000; + +/** Budgets overriding {@link MAX_PROMPT_CHARS}; `audit-docs` is held tighter because its subject is narrower. */ const MAX_PROMPT_CHARS_BY_FILE = { 'audit-docs.prompt.md': 36_000 }; /** @@ -98,19 +84,16 @@ function frontmatterValue(frontmatter, key) { return match[1].trim().replace(/^['"]|['"]$/g, ''); } -/** Whether a nested `metadata: internal: true` is set, which hides the skill from installers. */ +/** Returns whether a nested `metadata: internal: true` is set, which keeps the skill out of `npx skills` discovery. */ function isInternal(frontmatter) { return /^metadata:\s*$[\s\S]*?^\s+internal:[ \t]*true\s*$/m.test(frontmatter); } /** - * Frontmatter keys whose value is an unquoted plain scalar containing a colon followed by a - * space, which YAML forbids. - * - * This is checked rather than parsed because the script carries no dependencies, and it is - * checked at all because a regex reader like the one above happily returns a value that a - * real YAML parser refuses to produce. A skill whose frontmatter does not parse cannot be - * loaded by a host that uses a parser, and nothing else here would notice. + * Returns the frontmatter keys whose value is an unquoted plain scalar containing a colon followed + * by a space, which YAML forbids. Matched by pattern rather than parsed, since the script has no + * dependencies: {@link frontmatterValue} accepts such a value, but a host with a YAML parser cannot + * load the skill. */ function unparseableScalars(frontmatter) { return frontmatter @@ -121,7 +104,7 @@ function unparseableScalars(frontmatter) { } /** - * Every bundled file a Markdown body points at, whether as a link or as a code span. + * Returns every bundled file a Markdown body points at, whether as a link or as a code span. * * Only bundle directories count. These bodies also carry illustrative links such as * `../src/config.py`, which demonstrate the citation format rather than pointing at anything, @@ -141,7 +124,27 @@ function referencedPaths(body) { return [...found].filter(Boolean); } -/** Checks one skill directory against the specification, and against isolation when published. */ +/** Returns which of the three states a skill is in, reading its SKILL.md only the first time. */ +function state(name) { + if (!states.has(name)) { + states.set(name, readState(name)); + } + + return states.get(name); +} + +/** Reads a skill's state from its frontmatter and the `PUBLISHED` list. */ +function readState(name) { + const parts = split(readFileSync(join(SKILL_DIR, name, 'SKILL.md'), 'utf8')); + + if (parts && isInternal(parts.frontmatter)) { + return 'internal'; + } + + return PUBLISHED.includes(name) ? 'published' : 'installable'; +} + +/** Checks one skill directory against the specification, the licence and invocability policies, and, unless it is internal, isolation. */ function checkSkill(name) { const skillPath = join(SKILL_DIR, name, 'SKILL.md'); const label = `.claude/skills/${name}/SKILL.md`; @@ -152,8 +155,7 @@ function checkSkill(name) { return; } - const text = readFileSync(skillPath, 'utf8'); - const parts = split(text); + const parts = split(readFileSync(skillPath, 'utf8')); if (!parts) { fail(label, 'no frontmatter block'); @@ -161,8 +163,24 @@ function checkSkill(name) { return; } - const declared = frontmatterValue(parts.frontmatter, 'name'); - const description = frontmatterValue(parts.frontmatter, 'description'); + checkFrontmatter(name, parts.frontmatter, label); + checkBody(name, parts.body, label); + checkLicence(name, parts.frontmatter, label); + checkInvocable(name, parts.frontmatter); + checkIsolation(name, parts.frontmatter); +} + +/** + * Checks the frontmatter keys the specification constrains: a `name` matching the directory in the + * allowed format and length, a `description` within its limit, and every value parsing as YAML. + * + * @param {string} name Directory name of the skill under `.claude/skills/`. + * @param {string} frontmatter The skill's frontmatter block. + * @param {string} label Repository-relative path of the skill's SKILL.md, for reporting. + */ +function checkFrontmatter(name, frontmatter, label) { + const declared = frontmatterValue(frontmatter, 'name'); + const description = frontmatterValue(frontmatter, 'description'); if (declared !== name) { fail(label, `frontmatter name "${declared}" does not match the directory name "${name}"`); @@ -182,39 +200,66 @@ function checkSkill(name) { fail(label, `description is ${description.length} characters, over the ${MAX_DESCRIPTION} allowed`); } - const bodyLines = parts.body.split('\n').length; + for (const key of unparseableScalars(frontmatter)) { + fail(label, `\`${key}\` is an unquoted scalar containing ": ", so the frontmatter does not parse as YAML`); + } +} + +/** + * Checks the body of a SKILL.md: its length against the specification's cap, and every bundled file + * it points at resolving inside the skill directory. + * + * @param {string} name Directory name of the skill under `.claude/skills/`. + * @param {string} body The Markdown beneath the frontmatter. + * @param {string} label Repository-relative path of the skill's SKILL.md, for reporting. + */ +function checkBody(name, body, label) { + // A trailing newline ends the last line rather than starting another. + const bodyLines = body.replace(/\n$/, '').split('\n').length; if (bodyLines > MAX_BODY_LINES) { fail(label, `body is ${bodyLines} lines, over ${MAX_BODY_LINES}; move detail into references/`); } - for (const key of unparseableScalars(parts.frontmatter)) { - fail(label, `\`${key}\` is an unquoted scalar containing ": ", so the frontmatter does not parse as YAML`); - } - - for (const target of referencedPaths(parts.body)) { + for (const target of referencedPaths(body)) { if (!existsSync(join(SKILL_DIR, name, target))) { fail(label, `references "${target}", which does not exist in the skill directory`); } } +} +/** + * Checks that a skill carries its licence twice over: a `license` key in its frontmatter and a + * `LICENSE.txt` beside it. + * + * @param {string} name Directory name of the skill under `.claude/skills/`. + * @param {string} frontmatter The skill's frontmatter block. + * @param {string} label Repository-relative path of the skill's SKILL.md, for reporting. + */ +function checkLicence(name, frontmatter, label) { // `metadata.internal` buys no exemption here. `gh skill` reads no visibility field, so it // lists and installs every skill in this directory, and a copied directory is the whole of // what its recipient gets. - if (!frontmatterValue(parts.frontmatter, 'license')) { + if (!frontmatterValue(frontmatter, 'license')) { fail(label, 'an installer can offer any skill here, so it needs a license key'); } if (!existsSync(join(SKILL_DIR, name, 'LICENSE.txt'))) { fail(label, 'an installer can offer any skill here, so it needs a LICENSE.txt beside it'); } +} - checkInvocable(name, parts.frontmatter); - - // An internal skill names this repository's own prompt files on purpose, so the isolation - // rule below, which exists to keep a recipient from following a path they will not have, - // is the one thing it is exempt from. - if (isInternal(parts.frontmatter)) { +/** + * Checks that no file travelling with a skill names a prompt file, which a recipient of the copied + * directory will not have. + * + * @param {string} name Directory name of the skill under `.claude/skills/`. + * @param {string} frontmatter The skill's frontmatter block. + */ +function checkIsolation(name, frontmatter) { + // An internal skill names this repository's own prompt files on purpose, so it is exempt from + // this rule. + if (isInternal(frontmatter)) { return; } @@ -258,7 +303,7 @@ function checkInvocable(name, frontmatter) { } /** - * Every file inside a skill that travels with it and could name a path: top-level Markdown, every + * Returns every file inside a skill that travels with it and could name a path: top-level Markdown, every * file one level deep in each bundle directory, plus the plugin manifest. The manifest carries a * `description`, so it can name a prompt file exactly as a body can, and it ships in the copied * directory either way. @@ -272,7 +317,9 @@ function skillFiles(name) { continue; } - files.push(...readdirSync(join(root, dir)).map((file) => `${dir}/${file}`)); + const entries = readdirSync(join(root, dir), { withFileTypes: true }); + + files.push(...entries.filter((entry) => entry.isFile()).map((entry) => `${dir}/${entry.name}`)); } if (existsSync(join(root, PLUGIN_MANIFEST))) { @@ -282,7 +329,7 @@ function skillFiles(name) { return files; } -/** Checks that a prompt still works as the only file someone holds. */ +/** Checks a prompt against its character budget, and that it still works as the only file someone holds. */ function checkPrompt(file) { const label = `.github/prompts/${file}`; const text = readFileSync(join(PROMPT_DIR, file), 'utf8'); @@ -326,8 +373,50 @@ function checkPrompt(file) { } } +/** + * Prints an `ok` line for every skill and for the marketplace where nothing failed, then either every + * failure, exiting 1, or the totals by state. + * + * @param {string[]} skills Directory names under `.claude/skills/`. + * @param {string[]} prompts File names under `.github/prompts/`. + */ +function report(skills, prompts) { + for (const name of skills) { + if (!failures.some((entry) => entry.file.includes(`/skills/${name}/`))) { + console.log(`ok ${name.padEnd(36)} ${state(name)}`); + } + } + + if (!failures.some((entry) => [MARKETPLACE_MANIFEST, ...SHADOWING_MARKETPLACES].includes(entry.file))) { + console.log(`ok ${MARKETPLACE_MANIFEST.padEnd(36)} marketplace`); + } + + if (failures.length > 0) { + console.error(''); + + for (const { file, message } of failures) { + console.error(`FAIL ${file}: ${message}`); + } + + console.error(`\n${failures.length} problem(s).`); + process.exit(1); + } + + const byState = Object.groupBy(skills, state); + const published = byState.published?.length ?? 0; + const installable = byState.installable?.length ?? 0; + const internal = byState.internal?.length ?? 0; + + console.log( + `\nChecked ${skills.length} skill(s) and ${prompts.length} prompt(s): ` + + `${published} published, ${installable} installable, ${internal} internal.`, + ); +} + const skills = existsSync(SKILL_DIR) - ? readdirSync(SKILL_DIR).filter((entry) => statSync(join(SKILL_DIR, entry)).isDirectory()) + ? readdirSync(SKILL_DIR, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) : []; const prompts = existsSync(PROMPT_DIR) ? readdirSync(PROMPT_DIR).filter((file) => file.endsWith('.prompt.md')) : []; @@ -347,42 +436,4 @@ const offered = skills.filter((name) => existsSync(join(SKILL_DIR, name, 'SKILL. failures.push(...checkPlugins(skills, offered)); -/** Which of the three states a skill is in. */ -function state(name) { - if (!states.has(name)) { - const parts = split(readFileSync(join(SKILL_DIR, name, 'SKILL.md'), 'utf8')); - const internal = parts && isInternal(parts.frontmatter); - - states.set(name, internal ? 'internal' : PUBLISHED.includes(name) ? 'published' : 'installable'); - } - - return states.get(name); -} - -for (const name of skills) { - if (!failures.some((entry) => entry.file.includes(`/skills/${name}/`))) { - console.log(`ok ${name.padEnd(36)} ${state(name)}`); - } -} - -if (!failures.some((entry) => [MARKETPLACE_MANIFEST, ...SHADOWING_MARKETPLACES].includes(entry.file))) { - console.log(`ok ${MARKETPLACE_MANIFEST.padEnd(36)} marketplace`); -} - -if (failures.length > 0) { - console.error(''); - - for (const { file, message } of failures) { - console.error(`FAIL ${file}: ${message}`); - } - - console.error(`\n${failures.length} problem(s).`); - process.exit(1); -} - -const counts = skills.reduce((tally, name) => ({ ...tally, [state(name)]: (tally[state(name)] ?? 0) + 1 }), {}); - -console.log( - `\nChecked ${skills.length} skill(s) and ${prompts.length} prompt(s): ` + - `${counts.published ?? 0} published, ${counts.installable ?? 0} installable, ${counts.internal ?? 0} internal.`, -); +report(skills, prompts); diff --git a/.claude/scripts/check-skill-publishability.test.mjs b/.claude/scripts/check-skill-publishability.test.mjs new file mode 100644 index 0000000..2e8dcb8 --- /dev/null +++ b/.claude/scripts/check-skill-publishability.test.mjs @@ -0,0 +1,298 @@ +// Tests for the skill and prompt rules in `check-skill-publishability.mjs`, run against a fixture +// repository from `fixture-repository.mjs`. Run with `make -f .claude/Makefile test-scripts`. +import assert from 'assert/strict'; +import { unlinkSync } from 'fs'; +// `node:test` has no unprefixed name, unlike the other built-in modules imported here. +import { afterEach, beforeEach, describe, it } from 'node:test'; +import { join } from 'path'; +import { + ALPHA_FIELDS, + ALPHA_MANIFEST, + ALPHA_README, + ALPHA_SKILL, + BETA_FIELDS, + MARKETPLACE, + createFixtureRepository, + marketplace, + skillFile, +} from './fixture-repository.mjs'; + +/** Returns the text of a prompt file with valid frontmatter and `body` beneath it. */ +function promptFile(body) { + return `---\ndescription: Alpha prompt.\n---\n\n${body}`; +} + +/** Returns the text of a prompt file exactly `length` characters long. */ +function promptOfLength(length) { + return promptFile('x'.repeat(length - promptFile('').length)); +} + +let fixture; + +beforeEach(() => { + fixture = createFixtureRepository(); +}); + +afterEach(() => fixture.remove()); + +describe('skill checks', () => { + it('passes the fixture and totals its skills by state', () => { + const { status, output } = fixture.check(); + + assert.equal(status, 0, output); + assert.match(output, /ok {3}alpha +installable/); + assert.match(output, /ok {3}beta +internal/); + assert.match(output, /Checked 2 skill\(s\) and 0 prompt\(s\): 0 published, 1 installable, 1 internal\./); + }); + + it('reports a skill named in the published list as published', () => { + fixture.addOfferedSkill('audit-pr'); + + const { status, output } = fixture.check(); + + assert.equal(status, 0, output); + assert.match(output, /ok {3}audit-pr +published/); + assert.match(output, /1 published, 1 installable, 1 internal\./); + }); + + for (const { label, contents, message } of [ + { label: 'no frontmatter block', contents: '# Alpha\n', message: 'no frontmatter block' }, + { + label: 'a name unlike its directory', + contents: skillFile({ ...ALPHA_FIELDS, name: 'other' }), + message: 'does not match the directory name "alpha"', + }, + { + label: 'no description', + contents: skillFile({ ...ALPHA_FIELDS, description: undefined }), + message: 'no description, which is how', + }, + { + label: 'a description of 1025 characters', + contents: skillFile({ ...ALPHA_FIELDS, description: 'x'.repeat(1025) }), + message: 'over the 1024 allowed', + }, + { + label: 'an unquoted description containing ": "', + contents: skillFile({ ...ALPHA_FIELDS, description: 'Audit: everything' }), + message: '`description` is an unquoted scalar', + }, + { + label: 'no license key', + contents: skillFile({ ...ALPHA_FIELDS, license: undefined }), + message: 'so it needs a license key', + }, + { + // The blank line after the frontmatter is the body's first line, so this body has 501. + label: 'a body of 501 lines', + contents: skillFile(ALPHA_FIELDS, 'line\n'.repeat(500)), + message: 'body is 501 lines', + }, + { + label: 'a link to a bundled file that does not exist', + contents: skillFile(ALPHA_FIELDS, '[guide](references/gone.md)\n'), + message: 'references "references/gone.md"', + }, + { + label: 'a link with a fragment to a bundled file that does not exist', + contents: skillFile(ALPHA_FIELDS, '[guide](references/gone.md#part)\n'), + message: 'references "references/gone.md"', + }, + { + label: 'a code span naming a bundled file that does not exist', + contents: skillFile(ALPHA_FIELDS, 'Open `references/gone.md`.\n'), + message: 'references "references/gone.md"', + }, + ]) { + it(`reports a SKILL.md with ${label}`, () => { + fixture.write(ALPHA_SKILL, contents); + + fixture.assertFailsOnce(message); + }); + } + + for (const { label, contents } of [ + { + label: 'a quoted description containing ": "', + contents: skillFile({ ...ALPHA_FIELDS, description: '"Audit: everything"' }), + }, + { + label: 'a description of exactly 1024 characters', + contents: skillFile({ ...ALPHA_FIELDS, description: 'x'.repeat(1024) }), + }, + { + // The blank line after the frontmatter is the body's first line, so this body has 500. + label: 'a body of exactly 500 lines', + contents: skillFile(ALPHA_FIELDS, 'line\n'.repeat(499)), + }, + { + label: 'a link and a code span naming a bundled file that exists', + contents: skillFile(ALPHA_FIELDS, '[helper](agents/helper.md) and `agents/helper.md`\n'), + }, + { + label: '`user-invocable: false` on a skill that is not published', + contents: skillFile({ ...ALPHA_FIELDS, 'user-invocable': 'false' }), + }, + ]) { + it(`accepts a SKILL.md with ${label}`, () => { + fixture.write(ALPHA_SKILL, contents); + + const { status, output } = fixture.check(); + + assert.equal(status, 0, output); + }); + } + + for (const { label, name, message } of [ + { label: 'characters outside the allowed set', name: 'Bad_Name', message: 'must be lowercase alphanumeric' }, + { label: '65 characters', name: 'a'.repeat(65), message: 'over the 64 the specification allows' }, + ]) { + it(`reports a skill name with ${label}`, () => { + fixture.addInternalSkill(name); + + fixture.assertFailsOnce(message); + }); + } + + it('accepts a skill name of exactly 64 characters', () => { + fixture.addInternalSkill('a'.repeat(64)); + + assert.equal(fixture.check().status, 0); + }); + + it('reports a skill directory with no SKILL.md', () => { + fixture.write('.claude/skills/empty/notes.txt', 'notes'); + + fixture.assertFailsOnce('.claude/skills/empty/SKILL.md: no SKILL.md'); + }); + + it('reports a skill with no LICENSE.txt, and prints no ok line for it', () => { + unlinkSync(join(fixture.root, '.claude/skills/alpha/LICENSE.txt')); + + const output = fixture.assertFailsOnce('so it needs a LICENSE.txt beside it'); + + assert.doesNotMatch(output, /ok {3}alpha /); + }); + + for (const { label, path, contents, message } of [ + { + label: 'its README naming a prompt file', + path: ALPHA_README, + contents: '# alpha\n\nPairs with `alpha.prompt.md`.\n', + message: 'README.md: names a prompt file', + }, + { + label: 'its README naming the prompts directory', + path: ALPHA_README, + contents: '# alpha\n\nSee .github/prompts/ for the other half.\n', + message: 'README.md: names a prompt file', + }, + { + label: 'a bundled agent naming a prompt file', + path: '.claude/skills/alpha/agents/helper.md', + contents: '# Helper\n\nSee `alpha.prompt.md`.\n', + message: 'agents/helper.md: names a prompt file', + }, + ]) { + it(`reports a skill that is not internal with ${label}`, () => { + fixture.write(path, contents); + + fixture.assertFailsOnce(message); + }); + } + + it('reports a plugin manifest whose description names a prompt file', () => { + const description = 'Pairs with alpha.prompt.md.'; + const entry = { name: 'alpha', source: './.claude/skills/alpha', description }; + + fixture.write(ALPHA_MANIFEST, { name: 'alpha', description }); + fixture.write(MARKETPLACE, marketplace({ plugins: [entry] })); + + fixture.assertFailsOnce('.claude-plugin/plugin.json: names a prompt file'); + }); + + it('lets an internal skill name a prompt file', () => { + const beta = skillFile(BETA_FIELDS, 'Edit `.github/prompts/beta.prompt.md` alongside this file.\n'); + + fixture.write('.claude/skills/beta/SKILL.md', beta); + + assert.equal(fixture.check().status, 0); + }); + + it('skips a directory nested in a bundle directory', () => { + fixture.write('.claude/skills/alpha/references/nested/deep.md', '# Deep\n'); + + const { status, output } = fixture.check(); + + assert.equal(status, 0, output); + }); + + for (const { label, fields, message } of [ + { label: '`user-invocable: false`', fields: { 'user-invocable': 'false' }, message: 'user-invocable: false' }, + { label: '`user-invocable: no`', fields: { 'user-invocable': 'no' }, message: 'user-invocable: false' }, + { label: '`user-invocable: False`', fields: { 'user-invocable': 'False' }, message: 'user-invocable: false' }, + { label: '`paths:`', fields: { paths: "['src/**']" }, message: 'may not carry `paths:`' }, + ]) { + it(`reports a published skill carrying ${label}`, () => { + fixture.addOfferedSkill('audit-pr', fields); + + fixture.assertFailsOnce(message); + }); + } +}); + +describe('prompt checks', () => { + it('passes a prompt whose links are an illustration and an in-page anchor', () => { + fixture.write( + '.github/prompts/alpha.prompt.md', + promptFile('Cite as [config](../src/config.py). See [rules](#rules).\n'), + ); + + const { status, output } = fixture.check(); + + assert.equal(status, 0, output); + assert.match(output, /and 1 prompt\(s\)/); + }); + + for (const { file, budget } of [ + { file: 'alpha.prompt.md', budget: 52_000 }, + { file: 'audit-docs.prompt.md', budget: 36_000 }, + ]) { + it(`accepts ${file} at exactly its budget of ${budget} characters`, () => { + fixture.write(`.github/prompts/${file}`, promptOfLength(budget)); + + assert.equal(fixture.check().status, 0); + }); + + it(`reports ${file} one character over its budget of ${budget}`, () => { + fixture.write(`.github/prompts/${file}`, promptOfLength(budget + 1)); + + fixture.assertFailsOnce(`budget of ${budget}`); + }); + } + + for (const { label, contents, message } of [ + { + label: 'with a link to a real file', + contents: promptFile('[catalogue](../../.claude-plugin/marketplace.json)\n'), + message: 'a real file that will not travel', + }, + { + label: 'naming a bundle directory', + contents: promptFile('Open `references/guide.md`.\n'), + message: 'names "references/"', + }, + { + label: 'naming the skill half', + contents: promptFile('Read SKILL.md first.\n'), + message: 'names the skill half', + }, + { label: 'with no frontmatter block', contents: '# Alpha\n', message: 'alpha.prompt.md: no frontmatter block' }, + ]) { + it(`reports a prompt ${label}`, () => { + fixture.write('.github/prompts/alpha.prompt.md', contents); + + fixture.assertFailsOnce(message); + }); + } +}); diff --git a/.claude/scripts/fixture-repository.mjs b/.claude/scripts/fixture-repository.mjs new file mode 100644 index 0000000..42538ee --- /dev/null +++ b/.claude/scripts/fixture-repository.mjs @@ -0,0 +1,183 @@ +// Builds a throwaway repository for the tests of `check-skill-publishability.mjs` and +// `plugin-manifests.mjs`. Both scripts are copied into a temporary directory beside a skills tree +// that passes every rule, so a test changes one file and runs the checker there as its own process, +// exactly as `make -f .claude/Makefile check-skills` runs it. +import assert from 'assert/strict'; +import { spawnSync } from 'child_process'; +import { copyFileSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs'; +import { tmpdir } from 'os'; +import { dirname, join } from 'path'; + +/** The scripts copied into every fixture, the first being the entry point the checker runs from. */ +const SCRIPTS = ['check-skill-publishability.mjs', 'plugin-manifests.mjs']; + +/** The `metadata` value that marks a skill internal, nesting `internal: true` beneath the key. */ +const INTERNAL_METADATA = '\n internal: true'; + +/** Returns the frontmatter a fixture skill named `name` passes every rule with. */ +function passingFields(name) { + return { name, description: `${name} skill.`, license: 'MIT' }; +} + +/** Returns the marketplace entry offering the fixture skill named `name`. */ +function entryFor(name) { + return { name, source: `./.claude/skills/${name}`, description: `${name} plugin.` }; +} + +/** The frontmatter of `alpha`, the fixture's installable skill, for a case to override one key of. */ +export const ALPHA_FIELDS = passingFields('alpha'); + +/** The frontmatter of `beta`, the fixture's internal skill. */ +export const BETA_FIELDS = { ...passingFields('beta'), metadata: INTERNAL_METADATA }; + +/** The description `alpha` carries in both its manifest and its marketplace entry. */ +export const ALPHA_DESCRIPTION = entryFor('alpha').description; + +/** Repository-relative path of the SKILL.md for `alpha`. */ +export const ALPHA_SKILL = '.claude/skills/alpha/SKILL.md'; + +/** Repository-relative path of `alpha`'s plugin manifest. */ +export const ALPHA_MANIFEST = '.claude/skills/alpha/.claude-plugin/plugin.json'; + +/** Repository-relative path of `alpha`'s README, the page VS Code shows for the plugin. */ +export const ALPHA_README = '.claude/skills/alpha/README.md'; + +/** Repository-relative path of the marketplace. */ +export const MARKETPLACE = '.claude-plugin/marketplace.json'; + +/** + * A fixture repository and what a test does with it. + * + * @typedef {object} FixtureRepository + * @property {string} root Absolute path of the temporary directory holding the repository. + * @property {(path: string, contents: string | object) => void} write Writes `contents` to the + * repository-relative `path`, creating its directory. Anything but a string is written as JSON. + * @property {(name: string, fields?: Record) => void} addOfferedSkill + * Adds a skill that passes every rule an offered skill meets, with a licence, a README, a plugin + * manifest, and a marketplace entry, and with `fields` overriding its frontmatter. + * @property {(name: string, fields?: Record) => void} addInternalSkill + * Adds an internal skill with a licence, and with `fields` overriding its frontmatter. + * @property {() => { status: number | null, output: string }} check Runs the checker and returns its + * exit status and everything it printed. + * @property {(...messages: string[]) => string} assertFailsOnce Runs the checker once and asserts + * that it exits 1, prints each of `messages` exactly once, and reports no other problem. Returns + * everything it printed. + * @property {() => void} remove Deletes the repository. + */ + +/** + * Returns the text of a SKILL.md whose frontmatter holds one `key: value` line per entry of `fields` + * that is not `undefined`, in order, followed by `body`. + * + * @param {Record} fields Frontmatter keys and their values, written as given. + * @param {string} [body] The Markdown beneath the frontmatter. + * @returns {string} The file's contents. + */ +export function skillFile(fields, body = '# Skill\n') { + const lines = Object.entries(fields) + .filter(([, value]) => value !== undefined) + .map(([key, value]) => `${key}: ${value}`); + + return `---\n${lines.join('\n')}\n---\n\n${body}`; +} + +/** + * Returns the marketplace the fixture starts from, with any top-level keys in `overrides` applied. + * + * @param {Record} [overrides] Top-level keys replacing the fixture's own. + * @returns {Record} The marketplace object. + */ +export function marketplace(overrides = {}) { + return { name: 'fixture', owner: { name: 'Fixture' }, plugins: [entryFor('alpha')], ...overrides }; +} + +/** Writes `contents` to `path` under `root`, creating its directory, and anything but a string as JSON. */ +function writeFile(root, path, contents) { + mkdirSync(dirname(join(root, path)), { recursive: true }); + writeFileSync(join(root, path), typeof contents === 'string' ? contents : JSON.stringify(contents)); +} + +/** Runs the checker copied into `root` and returns its exit status and everything it printed. */ +function runChecker(root) { + const result = spawnSync(process.execPath, [join(root, '.claude/scripts', SCRIPTS[0])], { encoding: 'utf8' }); + + return { status: result.status, output: `${result.stdout}${result.stderr}` }; +} + +/** + * Runs the checker in `root` and asserts that it exits 1, prints each of `messages` exactly once, + * and reports as many problems as there are messages. + * + * @param {string} root Absolute path of the fixture repository. + * @param {string[]} messages Text each expected failure contains. + * @returns {string} Everything the checker printed. + */ +function assertFailsOnceIn(root, messages) { + const { status, output } = runChecker(root); + + assert.equal(status, 1, output); + assert.ok( + output.includes(`\n${messages.length} problem(s).`), + `expected ${messages.length} problem(s) in:\n${output}`, + ); + + for (const message of messages) { + const occurrences = output.split(message).length - 1; + + assert.equal(occurrences, 1, `expected "${message}" exactly once in:\n${output}`); + } + + return output; +} + +/** + * Creates a repository holding one offered skill, `alpha`, which also bundles one agent, and one + * internal skill, `beta`, plus a marketplace listing `alpha`. It holds no prompt files, and the + * checker passes it as created. + * + * @returns {FixtureRepository} The repository and the operations on it. + */ +export function createFixtureRepository() { + const root = mkdtempSync(join(tmpdir(), 'check-skills-')); + const offered = []; + const write = (path, contents) => writeFile(root, path, contents); + + const addOfferedSkill = (name, fields = {}) => { + const entry = entryFor(name); + + write(`.claude/skills/${name}/SKILL.md`, skillFile({ ...passingFields(name), ...fields })); + write(`.claude/skills/${name}/LICENSE.txt`, 'MIT'); + write(`.claude/skills/${name}/README.md`, `# ${name}\n`); + write(`.claude/skills/${name}/.claude-plugin/plugin.json`, { name, description: entry.description }); + offered.push(entry); + write(MARKETPLACE, marketplace({ plugins: offered })); + }; + + const addInternalSkill = (name, fields = {}) => { + write( + `.claude/skills/${name}/SKILL.md`, + skillFile({ ...passingFields(name), metadata: INTERNAL_METADATA, ...fields }), + ); + write(`.claude/skills/${name}/LICENSE.txt`, 'MIT'); + }; + + mkdirSync(join(root, '.claude/scripts'), { recursive: true }); + + for (const script of SCRIPTS) { + copyFileSync(join(import.meta.dirname, script), join(root, '.claude/scripts', script)); + } + + addOfferedSkill('alpha'); + write('.claude/skills/alpha/agents/helper.md', '# Helper\n'); + addInternalSkill('beta'); + + return { + root, + write, + addOfferedSkill, + addInternalSkill, + check: () => runChecker(root), + assertFailsOnce: (...messages) => assertFailsOnceIn(root, messages), + remove: () => rmSync(root, { recursive: true, force: true }), + }; +} diff --git a/.claude/scripts/plugin-manifests.mjs b/.claude/scripts/plugin-manifests.mjs index 8a9e839..9be07d9 100644 --- a/.claude/scripts/plugin-manifests.mjs +++ b/.claude/scripts/plugin-manifests.mjs @@ -4,11 +4,10 @@ // These rules follow what VS Code, Claude Code, the Copilot CLI, and `npx skills` read, not the // Agent Skills specification, so they change when one of those hosts does. // `check-skill-publishability.mjs` runs them and reports the result. -import { existsSync, readFileSync, realpathSync } from 'fs'; -import { dirname, isAbsolute, join, relative, resolve, sep } from 'path'; -import { fileURLToPath } from 'url'; +import { existsSync, readFileSync, readdirSync, realpathSync } from 'fs'; +import { isAbsolute, join, relative, resolve, sep } from 'path'; -const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..', '..'); +const REPO_ROOT = resolve(import.meta.dirname, '..', '..'); const SKILL_DIR = join(REPO_ROOT, '.claude', 'skills'); /** @@ -16,9 +15,6 @@ const SKILL_DIR = join(REPO_ROOT, '.claude', 'skills'); * files in `agents/` register as agents a run can delegate to instead of sitting there as unread * text. A marketplace install needs no manifest to find `agents/`, but every skill the marketplace * lists carries one, because its `description` is what the entry is checked against. - * - * Every bundled procedure is still written to be followed by opening its file, which needs no - * manifest and no host support. */ export const PLUGIN_MANIFEST = join('.claude-plugin', 'plugin.json'); @@ -34,6 +30,12 @@ const COMPETING_PLUGIN_MANIFESTS = [ join('.github', 'plugin', 'plugin.json'), ]; +/** + * The file VS Code renders as a plugin's page, read from the plugin root under exactly this name. + * It has no fallback to `SKILL.md` or to the description, so a plugin without one shows an empty page. + */ +const PLUGIN_README = 'README.md'; + /** * The catalogue that offers each skill as a plugin. It sits at the repository root because VS Code * looks for a marketplace nowhere else, and this is the one path under it that VS Code, Claude @@ -187,8 +189,8 @@ function checkAgentPaths(name, agents, label, fail) { } /** - * Whether `path` is `root` or sits beneath it. Both are compared as given, so pass real paths to - * rule out a symbolic link leading out of `root`. + * Returns whether `path` is `root` or sits beneath it. Both are compared as given, so pass real + * paths to rule out a symbolic link leading out of `root`. * * @param {string} path Absolute path to test. * @param {string} root Absolute path of the directory it must stay within. @@ -196,20 +198,15 @@ function checkAgentPaths(name, agents, label, fail) { */ function isInside(path, root) { const fromRoot = relative(root, path); + const leavesRoot = fromRoot === '..' || fromRoot.startsWith(`..${sep}`) || isAbsolute(fromRoot); - return fromRoot === '' || (fromRoot !== '..' && !fromRoot.startsWith(`..${sep}`) && !isAbsolute(fromRoot)); + return !leavesRoot; } /** * Checks that the marketplace offers exactly the skills an installer may offer, each as a plugin * rooted at its own skill directory. * - * A plugin rooted there is read the same way by every host: VS Code and Claude Code load the root - * `SKILL.md` as its one skill, and `npx skills` still finds the directory through its ordinary - * `.claude/skills/` scan. What the entry may say is narrow because the readers disagree about the - * rest. `npx skills` skips a path that does not start with `./`, and VS Code shows the entry's - * `description` rather than the manifest's, so the two copies are held equal. - * * @param {string[]} offered The skills an installer may offer. * @param {Map | null | undefined>} manifests Each skill's manifest, * as `checkPluginManifest` returned it. @@ -271,6 +268,12 @@ function checkMarketplace(offered, manifests, fail) { /** * Checks one marketplace entry against the skill directory it names. * + * A plugin rooted there is read the same way by every host: VS Code and Claude Code load the root + * `SKILL.md` as its one skill, and `npx skills` still finds the directory through its ordinary + * `.claude/skills/` scan. What the entry may say is narrow because the readers disagree about the + * rest. `npx skills` skips a path that does not start with `./`, and VS Code shows the entry's + * `description` rather than the manifest's, so the two copies are held equal. + * * @param {Record} entry One element of the marketplace's `plugins` array. * @param {string[]} offered Skills an installer may offer, which are the only names allowed. * @param {Map | null | undefined>} manifests Each skill's manifest. @@ -297,10 +300,18 @@ function checkMarketplaceEntry(entry, offered, manifests, fail) { ); } - if (typeof entry.description !== 'string' || !entry.description) { + const hasDescription = typeof entry.description === 'string' && entry.description !== ''; + + if (!hasDescription) { fail(label, `"${name}" has no description, which is the text VS Code lists it by`); } + // Matched against the listing rather than tested with `existsSync`, because a case-insensitive + // file system also finds `readme.md`, which GitHub does not serve under the name VS Code requests. + if (!readdirSync(join(SKILL_DIR, name)).includes(PLUGIN_README)) { + fail(label, `"${name}" has no ${PLUGIN_README}, so its VS Code plugin page is empty`); + } + const manifest = manifests.get(name); if (manifest === undefined) { @@ -310,7 +321,7 @@ function checkMarketplaceEntry(entry, offered, manifests, fail) { } // `checkPluginManifest` has already reported a manifest that could not be read. - if (manifest && entry.description !== manifest.description) { + if (manifest && hasDescription && entry.description !== manifest.description) { fail(label, `"${name}" description differs from its ${PLUGIN_MANIFEST}; VS Code shows the entry's copy`); } } diff --git a/.claude/scripts/plugin-manifests.test.mjs b/.claude/scripts/plugin-manifests.test.mjs index ab52373..b1cb93e 100644 --- a/.claude/scripts/plugin-manifests.test.mjs +++ b/.claude/scripts/plugin-manifests.test.mjs @@ -1,91 +1,31 @@ -// Tests for the plugin rules in `plugin-manifests.mjs`, run through the checker that reports them. -// -// Each case builds a small repository in a temporary directory, copies both scripts into it, and -// runs the checker there as its own process, so the rules are exercised against real files exactly -// as `make -f .claude/Makefile check-skills` runs them. Run with `make -f .claude/Makefile test-scripts`. +// Tests for the plugin rules in `plugin-manifests.mjs`, run through the checker that reports them +// against a fixture repository from `fixture-repository.mjs`. Run with +// `make -f .claude/Makefile test-scripts`. import assert from 'assert/strict'; -import { spawnSync } from 'child_process'; -import { copyFileSync, mkdirSync, mkdtempSync, rmSync, unlinkSync, writeFileSync } from 'fs'; +import { renameSync, symlinkSync, unlinkSync } from 'fs'; // `node:test` has no unprefixed name, unlike the other built-in modules imported here. import { afterEach, beforeEach, describe, it } from 'node:test'; -import { tmpdir } from 'os'; -import { dirname, join } from 'path'; -import { fileURLToPath } from 'url'; - -const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url)); -const ALPHA_DESCRIPTION = 'Alpha plugin.'; -const ALPHA_MANIFEST = '.claude/skills/alpha/.claude-plugin/plugin.json'; -const MARKETPLACE = '.claude-plugin/marketplace.json'; - -let root; - -/** Writes `contents` to `path` under the fixture repository, creating its directory. */ -function write(path, contents) { - mkdirSync(dirname(join(root, path)), { recursive: true }); - writeFileSync(join(root, path), typeof contents === 'string' ? contents : JSON.stringify(contents)); -} - -/** - * Builds a repository holding one installable skill, `alpha`, with a manifest and one agent, and - * one internal skill, `beta`, with neither, plus a marketplace listing `alpha` alone. - */ -function buildRepository() { - root = mkdtempSync(join(tmpdir(), 'plugin-manifests-')); - - for (const script of ['check-skill-publishability.mjs', 'plugin-manifests.mjs']) { - mkdirSync(join(root, '.claude/scripts'), { recursive: true }); - copyFileSync(join(SCRIPT_DIR, script), join(root, '.claude/scripts', script)); - } - - write( - '.claude/skills/alpha/SKILL.md', - '---\nname: alpha\ndescription: Alpha skill.\nlicense: MIT\n---\n\n# Alpha\n', - ); - write('.claude/skills/alpha/LICENSE.txt', 'MIT'); - write('.claude/skills/alpha/agents/helper.md', '# Helper\n'); - write(ALPHA_MANIFEST, { name: 'alpha', description: ALPHA_DESCRIPTION }); - write( - '.claude/skills/beta/SKILL.md', - '---\nname: beta\ndescription: Beta skill.\nlicense: MIT\nmetadata:\n internal: true\n---\n\n# Beta\n', - ); - write('.claude/skills/beta/LICENSE.txt', 'MIT'); - write(MARKETPLACE, marketplace()); -} - -/** The marketplace the fixture starts from, with any top-level keys in `overrides` applied. */ -function marketplace(overrides = {}) { - return { - name: 'fixture', - owner: { name: 'Fixture' }, - plugins: [{ name: 'alpha', source: './.claude/skills/alpha', description: ALPHA_DESCRIPTION }], - ...overrides, - }; -} - -/** Runs the checker against the fixture and returns its exit status and everything it printed. */ -function check() { - const result = spawnSync(process.execPath, [join(root, '.claude/scripts/check-skill-publishability.mjs')], { - encoding: 'utf8', - }); - - return { status: result.status, output: `${result.stdout}${result.stderr}` }; -} - -/** Asserts that the checker exits 1 and prints a failure containing `message` exactly once. */ -function assertFailsOnce(message) { - const { status, output } = check(); - - assert.equal(status, 1, output); - assert.equal(output.split(message).length - 1, 1, output); -} - -beforeEach(buildRepository); +import { join } from 'path'; +import { + ALPHA_DESCRIPTION, + ALPHA_MANIFEST, + ALPHA_README, + MARKETPLACE, + createFixtureRepository, + marketplace, +} from './fixture-repository.mjs'; + +let fixture; + +beforeEach(() => { + fixture = createFixtureRepository(); +}); -afterEach(() => rmSync(root, { recursive: true, force: true })); +afterEach(() => fixture.remove()); describe('plugin manifest checks', () => { it('passes a skill whose manifest and marketplace entry agree', () => { - const { status, output } = check(); + const { status, output } = fixture.check(); assert.equal(status, 0, output); assert.match(output, /ok {3}\.claude-plugin\/marketplace\.json/); @@ -112,38 +52,52 @@ describe('plugin manifest checks', () => { }, ]) { it(`reports a manifest carrying ${label}`, () => { - write(ALPHA_MANIFEST, { name: 'alpha', description: ALPHA_DESCRIPTION, ...manifest }); + fixture.write(ALPHA_MANIFEST, { name: 'alpha', description: ALPHA_DESCRIPTION, ...manifest }); - assertFailsOnce(message); + fixture.assertFailsOnce(message); }); } - it('accepts an agents path pointing at a directory inside the skill', () => { - write(ALPHA_MANIFEST, { name: 'alpha', description: ALPHA_DESCRIPTION, agents: './agents' }); + it('reports an agents path whose symbolic link leads out of the skill', () => { + symlinkSync( + join(fixture.root, '.claude/skills/beta'), + join(fixture.root, '.claude/skills/alpha/agents/escape'), + ); + fixture.write(ALPHA_MANIFEST, { name: 'alpha', description: ALPHA_DESCRIPTION, agents: './agents/escape' }); - assert.equal(check().status, 0); + fixture.assertFailsOnce('outside the skill'); }); - it('reports a second manifest that a host reads first', () => { - write('.claude/skills/alpha/plugin.json', { name: 'alpha' }); + it('accepts an agents path pointing at a directory inside the skill', () => { + fixture.write(ALPHA_MANIFEST, { name: 'alpha', description: ALPHA_DESCRIPTION, agents: './agents' }); - assertFailsOnce('a host reads this ahead of'); + assert.equal(fixture.check().status, 0); }); + for (const competing of ['plugin.json', '.plugin/plugin.json', '.github/plugin/plugin.json']) { + it(`reports a second manifest at ${competing}, which a host reads first`, () => { + fixture.write(`.claude/skills/alpha/${competing}`, { name: 'alpha' }); + + fixture.assertFailsOnce('a host reads this ahead of'); + }); + } + for (const { label, contents, message } of [ { label: 'does not parse', contents: '{', message: 'does not parse as JSON' }, { label: 'is not an object', contents: 'null', message: 'is not a JSON object' }, ]) { - it(`reports a manifest that ${label} without stopping the run`, () => { - write(ALPHA_MANIFEST, contents); + it(`reports a manifest that ${label}, and goes on to check the marketplace`, () => { + fixture.write(ALPHA_MANIFEST, contents); + fixture.write(MARKETPLACE, marketplace({ metadata: { pluginRoot: './plugins' } })); - assertFailsOnce(message); + fixture.assertFailsOnce(message, 'sets `metadata.pluginRoot`'); }); } }); describe('plugin marketplace checks', () => { for (const { label, overrides, message } of [ + { label: 'no name', overrides: { name: undefined }, message: 'needs a `name` and an `owner.name`' }, { label: 'no owner', overrides: { owner: undefined }, message: 'needs a `name` and an `owner.name`' }, { label: 'a plugin root', @@ -154,9 +108,9 @@ describe('plugin marketplace checks', () => { { label: 'no entry for an offered skill', overrides: { plugins: [] }, message: 'does not list "alpha"' }, ]) { it(`reports a marketplace with ${label}`, () => { - write(MARKETPLACE, marketplace(overrides)); + fixture.write(MARKETPLACE, marketplace(overrides)); - assertFailsOnce(message); + fixture.assertFailsOnce(message); }); } @@ -177,44 +131,61 @@ describe('plugin marketplace checks', () => { it(`reports an entry with ${label}`, () => { const alpha = { name: 'alpha', source: './.claude/skills/alpha', description: ALPHA_DESCRIPTION, ...entry }; - write(MARKETPLACE, marketplace({ plugins: [alpha] })); + fixture.write(MARKETPLACE, marketplace({ plugins: [alpha] })); - assertFailsOnce(message); + fixture.assertFailsOnce(message); }); } it('reports an entry for an internal skill', () => { const beta = { name: 'beta', source: './.claude/skills/beta', description: 'Beta plugin.' }; - write(MARKETPLACE, marketplace({ plugins: [...marketplace().plugins, beta] })); + fixture.write(MARKETPLACE, marketplace({ plugins: [...marketplace().plugins, beta] })); - assertFailsOnce('lists "beta", which is not a skill an installer may offer'); + fixture.assertFailsOnce('lists "beta", which is not a skill an installer may offer'); }); it('reports a repeated entry once, and its other failures once', () => { const wrong = { name: 'alpha', source: './alpha', description: ALPHA_DESCRIPTION }; - write(MARKETPLACE, marketplace({ plugins: [wrong, wrong] })); + fixture.write(MARKETPLACE, marketplace({ plugins: [wrong, wrong] })); - assertFailsOnce('lists "alpha" more than once'); - assertFailsOnce('must have source'); + fixture.assertFailsOnce('lists "alpha" more than once', 'must have source'); }); - it('reports a listed skill that has no manifest to compare against', () => { - unlinkSync(join(root, ALPHA_MANIFEST)); + it('reports a listed skill with no README.md', () => { + unlinkSync(join(fixture.root, ALPHA_README)); + + fixture.assertFailsOnce('has no README.md'); + }); + + // Only a case-insensitive file system, such as macOS's default, tells this case apart from an + // `existsSync` check; on a case-sensitive one both approaches report the renamed file. + it('reports a README whose name differs from README.md only in case', () => { + renameSync(join(fixture.root, ALPHA_README), join(fixture.root, '.claude/skills/alpha/readme.md')); - assertFailsOnce('has no .claude-plugin/plugin.json'); + fixture.assertFailsOnce('has no README.md'); }); - it('reports a marketplace file that hosts read before this one', () => { - write('.github/plugin/marketplace.json', { plugins: [] }); + it('reports a listed skill that has no manifest to compare against', () => { + unlinkSync(join(fixture.root, ALPHA_MANIFEST)); - assertFailsOnce('so it hides that catalogue'); + fixture.assertFailsOnce('has no .claude-plugin/plugin.json'); }); + for (const shadowing of ['marketplace.json', '.plugin/marketplace.json', '.github/plugin/marketplace.json']) { + it(`reports a marketplace file at ${shadowing}, which hosts read first, and prints no ok line`, () => { + fixture.write(shadowing, { plugins: [] }); + + const output = fixture.assertFailsOnce('so it hides that catalogue'); + + assert.doesNotMatch(output, /ok {3}\.claude-plugin\/marketplace\.json/); + }); + } + it('reports a missing marketplace', () => { - unlinkSync(join(root, MARKETPLACE)); + unlinkSync(join(fixture.root, MARKETPLACE)); - assertFailsOnce('missing, so no skill is installable'); + fixture.assertFailsOnce('missing, so no skill is installable'); }); }); diff --git a/.claude/skills/audit-docs/README.md b/.claude/skills/audit-docs/README.md new file mode 100644 index 0000000..c5ced7f --- /dev/null +++ b/.claude/skills/audit-docs/README.md @@ -0,0 +1,25 @@ +# audit-docs + +Audits a project's documentation against its code and corrects what has drifted, grounding every claim in a file opened during the run. It edits documentation only, never code behaviour. + +## What it does + +- Brings the documentation in line with the code, the active pull request, or uncommitted changes. +- Corrects statements the code contradicts, and writes documentation for a component that has none. +- Audits the documentation comments and file headers in the code it covers, documenting any public symbol that lacks one. +- Holds every page to two readers: a newcomer meeting the system for the first time, and someone who already works in it. +- Lists each claim it could not verify under "Unverified" in its report, rather than writing it into the documentation. + +## Usage + +Invoke the skill by name, for example `/audit-docs`, optionally followed by the paths or area to audit. Without one, it works from the active pull request, then uncommitted changes, then the component the surrounding task concerns, and only then the whole documentation set. + +- `/audit-docs` +- `/audit-docs docs/api` +- "Check the setup guide against the code" + +It ships subagents and reference material that it opens only when a run needs them. + +## Licence + +MIT. The full text is in `LICENSE.txt`. diff --git a/.claude/skills/audit-pr/README.md b/.claude/skills/audit-pr/README.md new file mode 100644 index 0000000..9862fc9 --- /dev/null +++ b/.claude/skills/audit-pr/README.md @@ -0,0 +1,25 @@ +# audit-pr + +Reviews a pull request, or a working branch's diff, across eighteen categories from correctness and security to cost and accessibility, and produces findings a person can verify and paste into the pull request. It reports findings rather than editing files. + +## What it does + +- Checks that the change does what its title, description, and linked ticket say. +- Enters only the categories the diff triggers, chosen through a triage table. +- Quotes the changed line behind every finding, with any credential value redacted. +- Tries to refute each finding before publishing it, and drops the ones that do not survive. +- Gives every finding a severity, says what the change does well, and ends with a verdict. + +## Usage + +Invoke the skill by name, for example `/audit-pr`, optionally followed by a pull request number or a branch. Without one, it reviews the active pull request, then uncommitted changes, then the branch's own commits. With no change to review, it says so and stops rather than auditing the whole codebase. + +- `/audit-pr` +- `/audit-pr 42` +- "Review the changes on this branch before I merge" + +It ships a refutation subagent, reference material, and a summary template that it opens only when a run needs them. + +## Licence + +MIT. The full text is in `LICENSE.txt`. diff --git a/.claude/skills/audit-quality/README.md b/.claude/skills/audit-quality/README.md new file mode 100644 index 0000000..d4dfefe --- /dev/null +++ b/.claude/skills/audit-quality/README.md @@ -0,0 +1,22 @@ +# audit-quality + +Audits code as it stands, rather than a change to it, across thirteen categories including architecture, security, privacy, testing, dependencies and supply chain, and operating cost. Every finding rests on a file opened during the run and names the symbol it concerns. + +## What it does + +- Resolves its scope first, states which rule decided it, and audits only what that scope selects. +- Gives every finding a severity: blocking, should fix, suggestion, or positive. +- Tries to refute each finding before reporting it, and drops the ones that do not survive. +- Applies changes only when invoked in a mode that allows edits, in small batches, running the project's full validation after each one. + +## Usage + +Invoke the skill by name, for example `/audit-quality`, optionally followed by paths, categories, or `all`. Without one, it audits the active pull request, then uncommitted changes, then the component the surrounding task concerns, and the whole repository only when nothing narrower applies. + +- `/audit-quality` +- `/audit-quality src/payments` +- `/audit-quality security` + +## Licence + +MIT. The full text is in `LICENSE.txt`. diff --git a/.claude/skills/check-skills/SKILL.md b/.claude/skills/check-skills/SKILL.md index e4887f3..840ba95 100644 --- a/.claude/skills/check-skills/SKILL.md +++ b/.claude/skills/check-skills/SKILL.md @@ -19,7 +19,7 @@ Three audits ship twice: `.github/prompts/.prompt.md` for an agent that re make -f .claude/Makefile check-skills ``` -It decides everything a machine can: `name` matching the directory, `description` within its character limit, a body under 500 lines, a licence on every skill, every bundled path resolving, no skill naming a prompt, no prompt naming a file that will not travel with it, and the plugin marketplace listing every skill that is not internal, each entry matching that skill's `.claude-plugin/plugin.json` and no manifest carrying a `version`. +It decides everything a machine can: `name` matching the directory, `description` within its character limit, a body under 500 lines, a licence on every skill, every bundled path resolving, no skill naming a prompt, no prompt naming a file that will not travel with it, and the plugin marketplace listing every skill that is not internal, each entry matching that skill's `.claude-plugin/plugin.json`, each listed skill carrying a `README.md`, and no manifest carrying a `version`. Exit 0 means the mechanical rules hold. It does **not** mean the two halves still agree. diff --git a/.claude/skills/typescript-code-and-test-standards/README.md b/.claude/skills/typescript-code-and-test-standards/README.md new file mode 100644 index 0000000..fc6c9e7 --- /dev/null +++ b/.claude/skills/typescript-code-and-test-standards/README.md @@ -0,0 +1,26 @@ +# typescript-code-and-test-standards + +TypeScript and JavaScript standards that formatters and linters cannot catch, applied while code is written or reviewed. It reads the project's own formatter, linter, compiler, and test-runner configuration first, and never overrides what that tooling is configured to check. + +## What it covers + +- Comment discipline, and a documentation block on every exported symbol. +- Naming, readability, and language-level defects such as an unhandled promise. +- Structure measured rather than sensed: file length, interface size, directory shape, parameter counts, and repeated logic. +- Reuse of what a dependency or the platform already provides. +- Logic changes shipping with tests, one colocated test per source file, and a mocking policy whose default is not to mock. + +It applies to `.ts`, `.tsx`, `.js`, `.jsx`, `.mjs`, `.cjs`, `.mts`, and `.cts` files. + +## Usage + +An agent picks the skill up when writing or reviewing one of those files. Invoke it by name to apply it on request, for example `/typescript-code-and-test-standards`. + +- "Review this module against the TypeScript standards" +- "Add tests for the parser without mocking the file system" + +It also ships review subagents, reference material, and templates for adopting the standards in a project. + +## Licence + +MIT. The full text is in `LICENSE.txt`. diff --git a/CLAUDE.md b/CLAUDE.md index 8e7cce3..e3b0c3a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,6 +60,6 @@ The conventions live in two layers. The generic set (comment discipline, JSDoc, - Each [`.github/prompts/`](.github/prompts/readme.md) file ships twice: as a single prompt file and as a skill directory. The two carry the **same objective, not the same bytes**, because only the skill can bundle `references/`, `agents/`, and `assets/`. After editing either half, run `make -f .claude/Makefile check-skills` and hand both halves to the `prompt-skill-sync` subagent (see [`prompt-skill-sync.md`](.claude/rules/prompt-skill-sync.md)). Each half is downloaded alone: the prompt names nothing beside it, the skill names nothing outside itself, and neither names a sibling audit or this repository. - Skills: `/audit-docs`, `/audit-pr`, and `/audit-quality` (the paired audits); `/write-tests` (repo procedure for authoring a test); `/check-skills` (validate the skills and their prompt halves); and `/typescript-code-and-test-standards` (the codebase-agnostic conventions, which `code-style.md` also loads you into on TypeScript and JavaScript files). Plus the built-in `/code-review` and `/security-review`. - Skills carry one of **three states**, which `make -f .claude/Makefile check-skills` prints and enforces. **Published** (`audit-docs`, `audit-pr`, `typescript-code-and-test-standards`) are used outside this repository, so they stay codebase-agnostic and, apart from the TypeScript one, language-agnostic. **Installable** (`audit-quality`) can be offered by an installer but is not held to that bar. **Internal** (`check-skills`, `write-tests`) set `metadata: internal: true`, which hides them from `npx skills` discovery but not from `gh skill`, which reads no visibility field and offers all six. The rule tying it together: every skill carries a `license` key and a `LICENSE.txt`, because a copied directory is all the recipient gets and the state cannot be relied on to stop the copy. Nothing is vendored here; a third-party skill is fetched on demand with `npx skills add / --skill `. -- The published and installable skills are also **agent plugins**. [`.claude-plugin/marketplace.json`](.claude-plugin/marketplace.json) lists each one with its own directory as the plugin root, and VS Code and Claude Code load its root `SKILL.md` as the plugin's skill. `check-skills` keeps the entries and each skill's `.claude-plugin/plugin.json` in step, including the rule that neither carries a `version`; the reasons are in [`prompt-skill-sync.md`](.claude/rules/prompt-skill-sync.md). +- The published and installable skills are also **agent plugins**. [`.claude-plugin/marketplace.json`](.claude-plugin/marketplace.json) lists each one with its own directory as the plugin root, and VS Code and Claude Code load its root `SKILL.md` as the plugin's skill. `check-skills` keeps the entries and each skill's `.claude-plugin/plugin.json` in step, including the rule that neither carries a `version`, and requires a `README.md` in each skill, which is the plugin's VS Code page; the reasons are in [`prompt-skill-sync.md`](.claude/rules/prompt-skill-sync.md). - Subagents: `validator` runs the local quality gates in its own context and returns a verdict instead of several thousand lines; `prompt-skill-sync` judges whether a published audit's two halves still aim at the same outcome, repairs a divergence, and returns a verdict instead of two long files. - Hooks ([`.claude/hooks/`](.claude/hooks/validate-gate.mts)): `markdown-audit-reminder` restates the doc-authoring rules whenever you edit a markdown file; `prompt-skill-sync` names the counterpart when you edit either half of a published audit; `validate-gate` tracks which gates have run and blocks the first attempt to finish while any are outstanding. diff --git a/package-lock.json b/package-lock.json index bad37b7..34a39cd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5139,9 +5139,9 @@ } }, "node_modules/@napi-rs/wasm-runtime": { - "version": "1.2.3", - "resolved": "https://registry.npmjs.org/@napi-rs/wasm-runtime/-/wasm-runtime-1.2.3.tgz", - "integrity": "sha512-UMduMbqO5s5zF2NkNacMT/yK5Y5QiKvWr2+50bzIIxFDwVJ2h49b+oyjaCGPhJxd2/gC2x39EHv/gHVuu36x2Q==", + "version": "1.2.4", + "resolved": "https://registry.npmjs.org/@napi-rs/wasm-runtime/-/wasm-runtime-1.2.4.tgz", + "integrity": "sha512-AJxoUD2/15ESHbvpcyjU274nsAPLuOtPHCk0vKJM5pj//Fg/B1FXNWjPnXTT9PymCYYiHo4zPj0ZomXBKhoy7g==", "dev": true, "license": "MIT", "optional": true, @@ -8799,9 +8799,9 @@ "license": "MIT" }, "node_modules/baseline-browser-mapping": { - "version": "2.11.21", - "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.11.21.tgz", - "integrity": "sha512-uh8vpY/1/YyFkunIDFH/12p7/7VdPKA1hejMVEbdkEaWnUz0Hesvx5EbiU6XxjyHZIOju+ZMbQJkRh+es3/spQ==", + "version": "2.11.22", + "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.11.22.tgz", + "integrity": "sha512-pWc4w51fBFd7mav43/zKRC+RI6f4yfzQoVlfvE8dECePyfkn1bzLp01Fj0QACcyCZyFhiEMyD2qScfKRWgWibA==", "license": "Apache-2.0", "bin": { "baseline-browser-mapping": "dist/cli.cjs" @@ -10025,9 +10025,9 @@ } }, "node_modules/electron-to-chromium": { - "version": "1.5.425", - "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.425.tgz", - "integrity": "sha512-QvPtl41EUOnuT1HBvMKgxXRIaHNcagBPs50u7VULzhZXaGfqTbZyE16LQsctZ/RQHlGu+FOWeDTR4mY6YbeF1g==", + "version": "1.5.427", + "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.427.tgz", + "integrity": "sha512-n14zb3FdsChZ2BNobqNHAJMcP3ifFv4paox2LvCrfVAQcqGiSURgbJl+PfMpHVCNFkStnNc+RRVtPBTVW5PDgw==", "license": "ISC" }, "node_modules/emittery": { @@ -14832,9 +14832,9 @@ "license": "MIT" }, "node_modules/nanoid": { - "version": "3.3.18", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz", - "integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==", + "version": "3.3.19", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.19.tgz", + "integrity": "sha512-Y2tUNy4ouw6tq5oDSKeQYGOyhkUBhNOcGV/02KC+6kd9eDGqdZd++mjMiIDilrBYvjEnCYvVtsuHCuP+okSfug==", "funding": [ { "type": "github", @@ -17480,9 +17480,9 @@ } }, "node_modules/update-browserslist-db": { - "version": "1.3.2", - "resolved": "https://registry.npmjs.org/update-browserslist-db/-/update-browserslist-db-1.3.2.tgz", - "integrity": "sha512-UQ+MSxlhRm1bzjhU+DcuXfjFO1FzNtqhK5+9Yvlp90ItDLk5vT932A0rFu619nf7RVS+Y/VeaUW1jaRDqZ8VJw==", + "version": "1.3.3", + "resolved": "https://registry.npmjs.org/update-browserslist-db/-/update-browserslist-db-1.3.3.tgz", + "integrity": "sha512-pJ2sYawQS0R/WI928Gj5GlPhTGzbMelq0+4INtSYNDV9ErKJcX6xjGWkoG/VnB3dpUm00zALaqkrUD77pO5TDQ==", "funding": [ { "type": "opencollective", From 55c66c192912f963882703c7525c7c3dda140e25 Mon Sep 17 00:00:00 2001 From: Alexander Sullivan Date: Thu, 24 Sep 2026 21:18:08 -0400 Subject: [PATCH 2/2] update --- .claude/rules/prompt-skill-sync.md | 6 +- .../scripts/check-skill-publishability.mjs | 40 +++++---- .../check-skill-publishability.test.mjs | 83 ++++++++++++++++++- .claude/scripts/file-system.mjs | 42 ++++++++++ .claude/scripts/file-system.test.mjs | 65 +++++++++++++++ .claude/scripts/fixture-repository.mjs | 2 +- .claude/scripts/plugin-manifests.mjs | 43 +++++----- .claude/scripts/plugin-manifests.test.mjs | 27 +++++- .claude/skills/check-skills/SKILL.md | 2 +- .github/copilot-instructions.md | 2 +- CLAUDE.md | 2 +- 11 files changed, 270 insertions(+), 44 deletions(-) create mode 100644 .claude/scripts/file-system.mjs create mode 100644 .claude/scripts/file-system.test.mjs diff --git a/.claude/rules/prompt-skill-sync.md b/.claude/rules/prompt-skill-sync.md index 341926e..a4070c5 100644 --- a/.claude/rules/prompt-skill-sync.md +++ b/.claude/rules/prompt-skill-sync.md @@ -53,7 +53,7 @@ Whichever half someone takes is the only thing they get. Four rules follow. An illustrative link, such as `[config.py](../src/config.py)` inside an example teaching the citation format, is not a real link and is allowed. The test is whether the target exists here: if it does, the author linked to something real and it will break. -`make -f .claude/Makefile check-skills` enforces every rule in this section, plus the specification itself: `name` matching the directory, `description` within its character limit, a body under 500 lines, a licence on every skill, and every bundled path resolving. It also gives each prompt a character budget: 36,000 for [`audit-docs.prompt.md`](../../.github/prompts/audit-docs.prompt.md), whose subject is narrower, and 52,000 for the other two. A skill can move detail into `references/`; a prompt is one file a reader scrolls, so its budget is the whole of what it can say. **Growth past a budget is a signal to condense, never to raise it.** What a prompt loses first is a rule restated across sections and a worked example following the rule it illustrates, and never a rule, a checklist item, or a category. +`make -f .claude/Makefile check-skills` enforces every rule in this section, plus the specification itself: `name` matching the directory, `description` within its character limit, a body under 500 lines, a licence on every skill, and every bundled path resolving inside the skill directory. It also gives each prompt a character budget: 36,000 for [`audit-docs.prompt.md`](../../.github/prompts/audit-docs.prompt.md), whose subject is narrower, and 52,000 for the other two. A skill can move detail into `references/`; a prompt is one file a reader scrolls, so its budget is the whole of what it can say. **Growth past a budget is a signal to condense, never to raise it.** What a prompt loses first is a rule restated across sections and a worked example following the rule it illustrates, and never a rule, a checklist item, or a category. **Characters, because a line here is a paragraph.** `MD013` is off repository-wide and Prettier leaves prose unwrapped, so a single line runs to seventeen hundred characters. A line budget charges a prompt for its blank lines and its headings and lets it pay by deleting them, which makes the document harder to read while the number improves and nothing is condensed at all. Characters do not move when a file is reformatted, so the only way down is to cut what the prompt says. It is deliberately **not** part of `npm run validate`, because the repository must build, test, and lint with no agent tooling present. @@ -69,10 +69,10 @@ Consequences to know before editing a skill, its manifest, or the marketplace: - **The skill's manifest and `agents/` travel with every install.** `npx skills` copies every file except `metadata.json` and the `.git`, `__pycache__`, and `__pypackages__` directories, and `gh skill` copies every file in the tree, so a recipient's copy carries `.claude-plugin/` and loads as `@skills-dir` in their repository too. - **No manifest carries a `version`.** Claude Code keys a marketplace install on it, so a fixed value freezes every recipient on the copy they first installed. Left out, the version is the commit the plugin came from, and a push to `main` reaches marketplace installs the way it reaches `npx skills`. VS Code ignores the field and pulls the repository instead. - **A marketplace entry carries only `name`, `source`, and `description`.** VS Code reads `name`, `description`, `version`, and `source` from an entry and drops the rest, so a component declared there would exist in Claude Code alone, and `npx skills` skips any path without the leading `./`. The entry repeats the manifest's `description` because that is the copy VS Code shows. For the same reason the marketplace sets no `metadata.pluginRoot`, which VS Code applies to `./` sources and Claude Code does not. -- **Every listed skill carries a `README.md`, and it travels too.** VS Code renders `/README.md` as the plugin's page, under exactly that name and with no fallback to `SKILL.md`, so a skill without one shows an empty page. Every installer copies it with the skill, so it names nothing outside the skill directory, links nothing relative (VS Code renders it with no base address), and invokes the skill by name rather than by one host's command form. +- **Every listed skill carries a `README.md`, and it travels too.** VS Code renders `/README.md` as the plugin's page, under exactly that name and with no fallback to `SKILL.md`, so a skill without one shows an empty page. It is a regular file rather than a symbolic link, because GitHub serves a link as the text of the path it points to, which `gh skill` installs in the file's place and VS Code can show before install. Every installer copies it with the skill, so it names nothing outside the skill directory and invokes the skill by name rather than by one host's command form. It also links nothing relative, naming a file beside it such as `LICENSE.txt` in a code span instead. This overrides the clickable-link rule in [`docs-authoring.md`](docs-authoring.md), because the plugin page VS Code opens from its Agent Plugins view strips a relative link's target and leaves link-coloured text that opens nothing. - **One manifest per skill, one marketplace per repository.** VS Code reads `.plugin/plugin.json`, or a root `plugin.json` declaring the Agent Plugins `$schema`, ahead of `.claude-plugin/plugin.json`, and that format finds skills only under `skills/`, which would leave the directory's own `SKILL.md` unloaded. The Copilot CLI reads `.plugin/plugin.json`, any root `plugin.json`, and `.github/plugin/plugin.json` first. For marketplaces, VS Code and the Copilot CLI try `marketplace.json`, `.plugin/marketplace.json`, and `.github/plugin/marketplace.json` before `.claude-plugin/marketplace.json`, and the first one found is the whole catalogue. -[`plugin-manifests.mjs`](../scripts/plugin-manifests.mjs), which `make -f .claude/Makefile check-skills` runs, holds every manifest to these rules: it parses, its `name` matches the directory, it sets no `version`, no competing manifest sits beside it, and every path in an `agents` key starts with `./`, stays inside the skill directory, and resolves. It requires the marketplace to list every skill that is not internal and nothing else, each entry carrying exactly the three keys above, with a `description` equal to its manifest's and a `README.md` beside the skill. That comparison is why a listed skill needs a manifest even though the marketplace route does not. [`plugin-manifests.test.mjs`](../scripts/plugin-manifests.test.mjs) covers each of these rules, and `make -f .claude/Makefile test-scripts` runs it. +[`plugin-manifests.mjs`](../scripts/plugin-manifests.mjs), which `make -f .claude/Makefile check-skills` runs, holds every manifest to these rules: it parses, its `name` matches the directory, it sets no `version`, no competing manifest sits beside it, and every path in an `agents` key starts with `./`, stays inside the skill directory, and resolves. It requires the marketplace to list every skill that is not internal and nothing else, each entry carrying exactly the three keys above, with a `description` equal to its manifest's. That comparison is why a listed skill needs a manifest even though the marketplace route does not. It also requires each listed skill to carry a `README.md` that is a regular file. [`plugin-manifests.test.mjs`](../scripts/plugin-manifests.test.mjs) covers each of these rules, and `make -f .claude/Makefile test-scripts` runs it. ## A published skill stays reachable by name diff --git a/.claude/scripts/check-skill-publishability.mjs b/.claude/scripts/check-skill-publishability.mjs index 6843755..0964aa9 100644 --- a/.claude/scripts/check-skill-publishability.mjs +++ b/.claude/scripts/check-skill-publishability.mjs @@ -10,8 +10,9 @@ // What is left here is what a machine can decide. // // Run with no arguments. Reports every failure, then exits 1 if there were any. -import { existsSync, readFileSync, readdirSync } from 'fs'; +import { existsSync, readFileSync, readdirSync, realpathSync } from 'fs'; import { join, resolve } from 'path'; +import { isDirectory, isFile, isInside } from './file-system.mjs'; import { MARKETPLACE_MANIFEST, PLUGIN_MANIFEST, SHADOWING_MARKETPLACES, checkPlugins } from './plugin-manifests.mjs'; const REPO_ROOT = resolve(import.meta.dirname, '..', '..'); @@ -149,7 +150,7 @@ function checkSkill(name) { const skillPath = join(SKILL_DIR, name, 'SKILL.md'); const label = `.claude/skills/${name}/SKILL.md`; - if (!existsSync(skillPath)) { + if (!isFile(skillPath)) { fail(label, 'no SKILL.md'); return; @@ -221,9 +222,15 @@ function checkBody(name, body, label) { fail(label, `body is ${bodyLines} lines, over ${MAX_BODY_LINES}; move detail into references/`); } + const skillRoot = join(SKILL_DIR, name); + for (const target of referencedPaths(body)) { - if (!existsSync(join(SKILL_DIR, name, target))) { + const resolved = resolve(skillRoot, target); + + if (!existsSync(resolved)) { fail(label, `references "${target}", which does not exist in the skill directory`); + } else if (!isInside(realpathSync(resolved), realpathSync(skillRoot))) { + fail(label, `references "${target}", which resolves outside the skill directory, so no install copies it`); } } } @@ -244,7 +251,7 @@ function checkLicence(name, frontmatter, label) { fail(label, 'an installer can offer any skill here, so it needs a license key'); } - if (!existsSync(join(SKILL_DIR, name, 'LICENSE.txt'))) { + if (!isFile(join(SKILL_DIR, name, 'LICENSE.txt'))) { fail(label, 'an installer can offer any skill here, so it needs a LICENSE.txt beside it'); } } @@ -307,22 +314,25 @@ function checkInvocable(name, frontmatter) { * file one level deep in each bundle directory, plus the plugin manifest. The manifest carries a * `description`, so it can name a prompt file exactly as a body can, and it ships in the copied * directory either way. + * + * A symbolic link is followed, so a linked file is scanned as the file it resolves to, and an entry + * that is not a file, such as a directory or a link to nothing, is skipped. */ function skillFiles(name) { const root = join(SKILL_DIR, name); - const files = readdirSync(root).filter((file) => file.endsWith('.md')); + const files = readdirSync(root).filter((file) => file.endsWith('.md') && isFile(join(root, file))); for (const dir of BUNDLE_DIRS) { - if (!existsSync(join(root, dir))) { + if (!isDirectory(join(root, dir))) { continue; } - const entries = readdirSync(join(root, dir), { withFileTypes: true }); + const bundled = readdirSync(join(root, dir)).filter((file) => isFile(join(root, dir, file))); - files.push(...entries.filter((entry) => entry.isFile()).map((entry) => `${dir}/${entry.name}`)); + files.push(...bundled.map((file) => `${dir}/${file}`)); } - if (existsSync(join(root, PLUGIN_MANIFEST))) { + if (isFile(join(root, PLUGIN_MANIFEST))) { files.push(PLUGIN_MANIFEST); } @@ -413,13 +423,13 @@ function report(skills, prompts) { ); } -const skills = existsSync(SKILL_DIR) - ? readdirSync(SKILL_DIR, { withFileTypes: true }) - .filter((entry) => entry.isDirectory()) - .map((entry) => entry.name) +const skills = isDirectory(SKILL_DIR) + ? readdirSync(SKILL_DIR).filter((entry) => isDirectory(join(SKILL_DIR, entry))) : []; -const prompts = existsSync(PROMPT_DIR) ? readdirSync(PROMPT_DIR).filter((file) => file.endsWith('.prompt.md')) : []; +const prompts = isDirectory(PROMPT_DIR) + ? readdirSync(PROMPT_DIR).filter((file) => file.endsWith('.prompt.md') && isFile(join(PROMPT_DIR, file))) + : []; if (skills.length === 0 && prompts.length === 0) { console.log('No skills or prompts found. Nothing to check.'); @@ -432,7 +442,7 @@ prompts.forEach(checkPrompt); // Only an internal skill is withheld from the marketplace. Every other one is already offered by // `npx skills`, so leaving it out there would make the two catalogues disagree without anyone // deciding they should. -const offered = skills.filter((name) => existsSync(join(SKILL_DIR, name, 'SKILL.md')) && state(name) !== 'internal'); +const offered = skills.filter((name) => isFile(join(SKILL_DIR, name, 'SKILL.md')) && state(name) !== 'internal'); failures.push(...checkPlugins(skills, offered)); diff --git a/.claude/scripts/check-skill-publishability.test.mjs b/.claude/scripts/check-skill-publishability.test.mjs index 2e8dcb8..812cccc 100644 --- a/.claude/scripts/check-skill-publishability.test.mjs +++ b/.claude/scripts/check-skill-publishability.test.mjs @@ -1,7 +1,7 @@ // Tests for the skill and prompt rules in `check-skill-publishability.mjs`, run against a fixture // repository from `fixture-repository.mjs`. Run with `make -f .claude/Makefile test-scripts`. import assert from 'assert/strict'; -import { unlinkSync } from 'fs'; +import { mkdirSync, symlinkSync, unlinkSync, writeFileSync } from 'fs'; // `node:test` has no unprefixed name, unlike the other built-in modules imported here. import { afterEach, beforeEach, describe, it } from 'node:test'; import { join } from 'path'; @@ -103,6 +103,16 @@ describe('skill checks', () => { contents: skillFile(ALPHA_FIELDS, 'Open `references/gone.md`.\n'), message: 'references "references/gone.md"', }, + { + label: 'a link climbing out of the skill to a file that exists', + contents: skillFile(ALPHA_FIELDS, '[beta](agents/../../beta/SKILL.md)\n'), + message: 'references "agents/../../beta/SKILL.md", which resolves outside the skill directory', + }, + { + label: 'a code span climbing out of the skill to a file that exists', + contents: skillFile(ALPHA_FIELDS, 'Compare `agents/../../beta/SKILL.md`.\n'), + message: 'references "agents/../../beta/SKILL.md", which resolves outside the skill directory', + }, ]) { it(`reports a SKILL.md with ${label}`, () => { fixture.write(ALPHA_SKILL, contents); @@ -160,6 +170,40 @@ describe('skill checks', () => { assert.equal(fixture.check().status, 0); }); + it('reports a link to a bundled symbolic link that leads out of the skill', () => { + fixture.write('shared/guide.md', '# Guide\n'); + symlinkSync(join(fixture.root, 'shared/guide.md'), join(fixture.root, '.claude/skills/alpha/agents/guide.md')); + fixture.write(ALPHA_SKILL, skillFile(ALPHA_FIELDS, '[guide](agents/guide.md)\n')); + + fixture.assertFailsOnce('references "agents/guide.md", which resolves outside the skill directory'); + }); + + it('checks a skill directory reached through a symbolic link', () => { + fixture.write('shared/gamma/SKILL.md', skillFile({ ...BETA_FIELDS, name: 'gamma' })); + fixture.write('shared/gamma/LICENSE.txt', 'MIT'); + symlinkSync(join(fixture.root, 'shared/gamma'), join(fixture.root, '.claude/skills/gamma')); + + const { status, output } = fixture.check(); + + assert.equal(status, 0, output); + assert.match(output, /ok {3}gamma +internal/); + }); + + it('reports a skill directory whose SKILL.md is a directory', () => { + mkdirSync(join(fixture.root, '.claude/skills/empty/SKILL.md'), { recursive: true }); + + fixture.assertFailsOnce('.claude/skills/empty/SKILL.md: no SKILL.md'); + }); + + it('reports a skill whose LICENSE.txt is a directory', () => { + const licence = join(fixture.root, '.claude/skills/alpha/LICENSE.txt'); + + unlinkSync(licence); + mkdirSync(licence); + + fixture.assertFailsOnce('so it needs a LICENSE.txt beside it'); + }); + it('reports a skill directory with no SKILL.md', () => { fixture.write('.claude/skills/empty/notes.txt', 'notes'); @@ -219,6 +263,43 @@ describe('skill checks', () => { assert.equal(fixture.check().status, 0); }); + it('reports a bundled symbolic link whose target names a prompt file', () => { + fixture.write('shared/linked.md', 'See `alpha.prompt.md`.\n'); + symlinkSync( + join(fixture.root, 'shared/linked.md'), + join(fixture.root, '.claude/skills/alpha/agents/linked.md'), + ); + + fixture.assertFailsOnce('agents/linked.md: names a prompt file'); + }); + + for (const { label, create } of [ + { + label: 'a directory named like Markdown at the top of a skill', + create: (at) => mkdirSync(at('.claude/skills/alpha/notes.md')), + }, + { + label: 'a symbolic link to nothing at the top of a skill', + create: (at) => symlinkSync(at('gone.md'), at('.claude/skills/alpha/notes.md')), + }, + { + label: 'a file where a bundle directory belongs', + create: (at) => writeFileSync(at('.claude/skills/alpha/assets'), 'x'), + }, + { + label: 'a directory named like a prompt file', + create: (at) => mkdirSync(at('.github/prompts/alpha.prompt.md'), { recursive: true }), + }, + ]) { + it(`skips ${label}`, () => { + create((path) => join(fixture.root, path)); + + const { status, output } = fixture.check(); + + assert.equal(status, 0, output); + }); + } + it('skips a directory nested in a bundle directory', () => { fixture.write('.claude/skills/alpha/references/nested/deep.md', '# Deep\n'); diff --git a/.claude/scripts/file-system.mjs b/.claude/scripts/file-system.mjs new file mode 100644 index 0000000..be497e4 --- /dev/null +++ b/.claude/scripts/file-system.mjs @@ -0,0 +1,42 @@ +// File-system predicates shared by the skill checks. `isFile` and `isDirectory` follow a symbolic +// link to what it points at, because `npx skills` copies a linked file's contents in its place, so +// a check reads what a recipient receives. `isInside` compares paths as given. +import { statSync } from 'fs'; +import { isAbsolute, relative, sep } from 'path'; + +/** + * Returns whether `path` is a regular file once any symbolic link is followed. A missing path or a + * link to nothing is not a file. + * + * @param {string} path Absolute path to test. + * @returns {boolean} True when `path` resolves to a regular file. + */ +export function isFile(path) { + return statSync(path, { throwIfNoEntry: false })?.isFile() ?? false; +} + +/** + * Returns whether `path` is a directory once any symbolic link is followed. A missing path or a + * link to nothing is not a directory. + * + * @param {string} path Absolute path to test. + * @returns {boolean} True when `path` resolves to a directory. + */ +export function isDirectory(path) { + return statSync(path, { throwIfNoEntry: false })?.isDirectory() ?? false; +} + +/** + * Returns whether `path` is `root` or sits beneath it. Both are compared as given, so pass real + * paths to rule out a symbolic link leading out of `root`. + * + * @param {string} path Absolute path to test. + * @param {string} root Absolute path of the directory it must stay within. + * @returns {boolean} True when `path` does not leave `root`. + */ +export function isInside(path, root) { + const fromRoot = relative(root, path); + const leavesRoot = fromRoot === '..' || fromRoot.startsWith(`..${sep}`) || isAbsolute(fromRoot); + + return !leavesRoot; +} diff --git a/.claude/scripts/file-system.test.mjs b/.claude/scripts/file-system.test.mjs new file mode 100644 index 0000000..a54eca7 --- /dev/null +++ b/.claude/scripts/file-system.test.mjs @@ -0,0 +1,65 @@ +// Tests for the file-system predicates in `file-system.mjs`, against real files, directories, and +// symbolic links in a temporary directory. Run with `make -f .claude/Makefile test-scripts`. +import assert from 'assert/strict'; +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'fs'; +// `node:test` has no unprefixed name, unlike the other built-in modules imported here. +import { afterEach, beforeEach, describe, it } from 'node:test'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { isDirectory, isFile, isInside } from './file-system.mjs'; + +let root; + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'file-system-')); + writeFileSync(join(root, 'file.md'), '# File\n'); + mkdirSync(join(root, 'folder')); + symlinkSync(join(root, 'file.md'), join(root, 'link-to-file')); + symlinkSync(join(root, 'folder'), join(root, 'link-to-folder')); + symlinkSync(join(root, 'gone.md'), join(root, 'link-to-nothing')); +}); + +afterEach(() => rmSync(root, { recursive: true, force: true })); + +describe('isFile', () => { + for (const { entry, expected } of [ + { entry: 'file.md', expected: true }, + { entry: 'link-to-file', expected: true }, + { entry: 'folder', expected: false }, + { entry: 'link-to-folder', expected: false }, + { entry: 'link-to-nothing', expected: false }, + { entry: 'missing.md', expected: false }, + ]) { + it(`returns ${expected} for ${entry}`, () => { + assert.equal(isFile(join(root, entry)), expected); + }); + } +}); + +describe('isDirectory', () => { + for (const { entry, expected } of [ + { entry: 'folder', expected: true }, + { entry: 'link-to-folder', expected: true }, + { entry: 'file.md', expected: false }, + { entry: 'link-to-nothing', expected: false }, + { entry: 'missing', expected: false }, + ]) { + it(`returns ${expected} for ${entry}`, () => { + assert.equal(isDirectory(join(root, entry)), expected); + }); + } +}); + +describe('isInside', () => { + for (const { label, path, expected } of [ + { label: 'the root itself', path: '/skills/alpha', expected: true }, + { label: 'a path beneath the root', path: '/skills/alpha/agents/helper.md', expected: true }, + { label: 'a child whose name starts with two dots', path: '/skills/alpha/..cache/x', expected: true }, + { label: 'a sibling of the root', path: '/skills/beta/SKILL.md', expected: false }, + { label: 'the parent of the root', path: '/skills', expected: false }, + ]) { + it(`returns ${expected} for ${label}`, () => { + assert.equal(isInside(path, '/skills/alpha'), expected); + }); + } +}); diff --git a/.claude/scripts/fixture-repository.mjs b/.claude/scripts/fixture-repository.mjs index 42538ee..9d813ea 100644 --- a/.claude/scripts/fixture-repository.mjs +++ b/.claude/scripts/fixture-repository.mjs @@ -9,7 +9,7 @@ import { tmpdir } from 'os'; import { dirname, join } from 'path'; /** The scripts copied into every fixture, the first being the entry point the checker runs from. */ -const SCRIPTS = ['check-skill-publishability.mjs', 'plugin-manifests.mjs']; +const SCRIPTS = ['check-skill-publishability.mjs', 'plugin-manifests.mjs', 'file-system.mjs']; /** The `metadata` value that marks a skill internal, nesting `internal: true` beneath the key. */ const INTERNAL_METADATA = '\n internal: true'; diff --git a/.claude/scripts/plugin-manifests.mjs b/.claude/scripts/plugin-manifests.mjs index 9be07d9..1c68bd5 100644 --- a/.claude/scripts/plugin-manifests.mjs +++ b/.claude/scripts/plugin-manifests.mjs @@ -5,7 +5,8 @@ // Agent Skills specification, so they change when one of those hosts does. // `check-skill-publishability.mjs` runs them and reports the result. import { existsSync, readFileSync, readdirSync, realpathSync } from 'fs'; -import { isAbsolute, join, relative, resolve, sep } from 'path'; +import { join, resolve } from 'path'; +import { isInside } from './file-system.mjs'; const REPO_ROOT = resolve(import.meta.dirname, '..', '..'); const SKILL_DIR = join(REPO_ROOT, '.claude', 'skills'); @@ -188,21 +189,6 @@ function checkAgentPaths(name, agents, label, fail) { } } -/** - * Returns whether `path` is `root` or sits beneath it. Both are compared as given, so pass real - * paths to rule out a symbolic link leading out of `root`. - * - * @param {string} path Absolute path to test. - * @param {string} root Absolute path of the directory it must stay within. - * @returns {boolean} True when `path` does not leave `root`. - */ -function isInside(path, root) { - const fromRoot = relative(root, path); - const leavesRoot = fromRoot === '..' || fromRoot.startsWith(`..${sep}`) || isAbsolute(fromRoot); - - return !leavesRoot; -} - /** * Checks that the marketplace offers exactly the skills an installer may offer, each as a plugin * rooted at its own skill directory. @@ -265,6 +251,25 @@ function checkMarketplace(offered, manifests, fail) { } } +/** + * Returns whether a skill directory holds a {@link PLUGIN_README} that is a regular file, under + * exactly that name. + * + * The name is matched against the listing rather than tested with `existsSync`, because a + * case-insensitive file system also finds `readme.md`, which GitHub does not serve under the name + * VS Code requests. The entry is not followed if it is a symbolic link: GitHub serves a link as the + * text of the path it points to, which VS Code can show before install and which `gh skill` + * installs in the file's place. + * + * @param {string} name Directory name of the skill under `.claude/skills/`. + * @returns {boolean} True when the README is a regular file. + */ +function hasRegularReadme(name) { + const entries = readdirSync(join(SKILL_DIR, name), { withFileTypes: true }); + + return entries.find((entry) => entry.name === PLUGIN_README)?.isFile() ?? false; +} + /** * Checks one marketplace entry against the skill directory it names. * @@ -306,10 +311,8 @@ function checkMarketplaceEntry(entry, offered, manifests, fail) { fail(label, `"${name}" has no description, which is the text VS Code lists it by`); } - // Matched against the listing rather than tested with `existsSync`, because a case-insensitive - // file system also finds `readme.md`, which GitHub does not serve under the name VS Code requests. - if (!readdirSync(join(SKILL_DIR, name)).includes(PLUGIN_README)) { - fail(label, `"${name}" has no ${PLUGIN_README}, so its VS Code plugin page is empty`); + if (!hasRegularReadme(name)) { + fail(label, `"${name}" has no ${PLUGIN_README} that is a regular file, so its VS Code plugin page is empty`); } const manifest = manifests.get(name); diff --git a/.claude/scripts/plugin-manifests.test.mjs b/.claude/scripts/plugin-manifests.test.mjs index b1cb93e..492073b 100644 --- a/.claude/scripts/plugin-manifests.test.mjs +++ b/.claude/scripts/plugin-manifests.test.mjs @@ -2,7 +2,7 @@ // against a fixture repository from `fixture-repository.mjs`. Run with // `make -f .claude/Makefile test-scripts`. import assert from 'assert/strict'; -import { renameSync, symlinkSync, unlinkSync } from 'fs'; +import { mkdirSync, renameSync, symlinkSync, unlinkSync } from 'fs'; // `node:test` has no unprefixed name, unlike the other built-in modules imported here. import { afterEach, beforeEach, describe, it } from 'node:test'; import { join } from 'path'; @@ -153,6 +153,31 @@ describe('plugin marketplace checks', () => { fixture.assertFailsOnce('lists "alpha" more than once', 'must have source'); }); + for (const { label, replace } of [ + { label: 'a directory', replace: (readme) => mkdirSync(readme) }, + { + label: 'a symbolic link to a file inside the skill', + replace: (readme) => symlinkSync(join(fixture.root, '.claude/skills/alpha/SKILL.md'), readme), + }, + { + label: 'a symbolic link to a file outside the skill', + replace: (readme) => symlinkSync(join(fixture.root, '.claude/skills/beta/SKILL.md'), readme), + }, + { + label: 'a symbolic link to nothing', + replace: (readme) => symlinkSync(join(fixture.root, 'gone.md'), readme), + }, + ]) { + it(`reports a README.md that is ${label}`, () => { + const readme = join(fixture.root, ALPHA_README); + + unlinkSync(readme); + replace(readme); + + fixture.assertFailsOnce('has no README.md that is a regular file'); + }); + } + it('reports a listed skill with no README.md', () => { unlinkSync(join(fixture.root, ALPHA_README)); diff --git a/.claude/skills/check-skills/SKILL.md b/.claude/skills/check-skills/SKILL.md index 840ba95..9d95b7b 100644 --- a/.claude/skills/check-skills/SKILL.md +++ b/.claude/skills/check-skills/SKILL.md @@ -19,7 +19,7 @@ Three audits ship twice: `.github/prompts/.prompt.md` for an agent that re make -f .claude/Makefile check-skills ``` -It decides everything a machine can: `name` matching the directory, `description` within its character limit, a body under 500 lines, a licence on every skill, every bundled path resolving, no skill naming a prompt, no prompt naming a file that will not travel with it, and the plugin marketplace listing every skill that is not internal, each entry matching that skill's `.claude-plugin/plugin.json`, each listed skill carrying a `README.md`, and no manifest carrying a `version`. +It decides everything a machine can: `name` matching the directory, `description` within its character limit, a body under 500 lines, a licence on every skill, every bundled path resolving inside the skill directory, no skill naming a prompt, no prompt naming a file that will not travel with it, and the plugin marketplace listing every skill that is not internal, each entry matching that skill's `.claude-plugin/plugin.json`, each listed skill carrying a `README.md`, and no manifest carrying a `version`. Exit 0 means the mechanical rules hold. It does **not** mean the two halves still agree. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 889fe5f..8962d70 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -257,4 +257,4 @@ When writing or editing any Markdown, the full rules are in [`audit-docs.prompt. - **No em-dashes or en-dashes**: replace each with a comma, parenthesis, colon, separate sentence, or a spaced hyphen, including existing ones in any file you edit - **Canadian English** for prose you write or change (colour, behaviour, standardize), never for code identifiers, config keys, or package names - **No subjective adjectives** (important, robust, seamless). State the fact that would earn the adjective -- Every file reference is a clickable Markdown link to a **file**, never a bare filename or a directory, and every Mermaid diagram carries both `accTitle` and `accDescr` +- Every file reference is a clickable Markdown link to a **file**, never a bare filename or a directory, and every Mermaid diagram carries both `accTitle` and `accDescr`. The exception is a skill's `README.md`, which names a file beside it in a code span, because the plugin page VS Code opens from its Agent Plugins view strips a relative link's target diff --git a/CLAUDE.md b/CLAUDE.md index e3b0c3a..02aa303 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,6 +60,6 @@ The conventions live in two layers. The generic set (comment discipline, JSDoc, - Each [`.github/prompts/`](.github/prompts/readme.md) file ships twice: as a single prompt file and as a skill directory. The two carry the **same objective, not the same bytes**, because only the skill can bundle `references/`, `agents/`, and `assets/`. After editing either half, run `make -f .claude/Makefile check-skills` and hand both halves to the `prompt-skill-sync` subagent (see [`prompt-skill-sync.md`](.claude/rules/prompt-skill-sync.md)). Each half is downloaded alone: the prompt names nothing beside it, the skill names nothing outside itself, and neither names a sibling audit or this repository. - Skills: `/audit-docs`, `/audit-pr`, and `/audit-quality` (the paired audits); `/write-tests` (repo procedure for authoring a test); `/check-skills` (validate the skills and their prompt halves); and `/typescript-code-and-test-standards` (the codebase-agnostic conventions, which `code-style.md` also loads you into on TypeScript and JavaScript files). Plus the built-in `/code-review` and `/security-review`. - Skills carry one of **three states**, which `make -f .claude/Makefile check-skills` prints and enforces. **Published** (`audit-docs`, `audit-pr`, `typescript-code-and-test-standards`) are used outside this repository, so they stay codebase-agnostic and, apart from the TypeScript one, language-agnostic. **Installable** (`audit-quality`) can be offered by an installer but is not held to that bar. **Internal** (`check-skills`, `write-tests`) set `metadata: internal: true`, which hides them from `npx skills` discovery but not from `gh skill`, which reads no visibility field and offers all six. The rule tying it together: every skill carries a `license` key and a `LICENSE.txt`, because a copied directory is all the recipient gets and the state cannot be relied on to stop the copy. Nothing is vendored here; a third-party skill is fetched on demand with `npx skills add / --skill `. -- The published and installable skills are also **agent plugins**. [`.claude-plugin/marketplace.json`](.claude-plugin/marketplace.json) lists each one with its own directory as the plugin root, and VS Code and Claude Code load its root `SKILL.md` as the plugin's skill. `check-skills` keeps the entries and each skill's `.claude-plugin/plugin.json` in step, including the rule that neither carries a `version`, and requires a `README.md` in each skill, which is the plugin's VS Code page; the reasons are in [`prompt-skill-sync.md`](.claude/rules/prompt-skill-sync.md). +- The published and installable skills are also **agent plugins**. [`.claude-plugin/marketplace.json`](.claude-plugin/marketplace.json) lists each one with its own directory as the plugin root, and VS Code and Claude Code load its root `SKILL.md` as the plugin's skill. `check-skills` keeps the entries and each skill's `.claude-plugin/plugin.json` in step, including the rule that neither carries a `version`, and requires each listed skill to carry a `README.md`, which VS Code shows as the plugin's page; the reasons are in [`prompt-skill-sync.md`](.claude/rules/prompt-skill-sync.md). - Subagents: `validator` runs the local quality gates in its own context and returns a verdict instead of several thousand lines; `prompt-skill-sync` judges whether a published audit's two halves still aim at the same outcome, repairs a divergence, and returns a verdict instead of two long files. - Hooks ([`.claude/hooks/`](.claude/hooks/validate-gate.mts)): `markdown-audit-reminder` restates the doc-authoring rules whenever you edit a markdown file; `prompt-skill-sync` names the counterpart when you edit either half of a published audit; `validate-gate` tracks which gates have run and blocks the first attempt to finish while any are outstanding.