diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 6fa62863..518c9f26 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -33,11 +33,17 @@ The bump is part of the milestone's work, so it belongs on the develop branch and flows into `main` with everything else. ```sh -git checkout -b v2/chore/-bump-2-3-0 v2/main +# The branch name carries the version you are bumping TO, so read it first. +node -p "require('./package.json').version" # what is on v2/main now +git checkout -b v2/chore/-bump- v2/main npm version minor --no-git-tag-version # or major / patch; bump only, no tag -# PR → v2/main +node -p "require('./package.json').version" # confirm, then PR → v2/main ``` +⚠️ **Never copy a version out of this file.** It would be a version that has +already shipped by the time you read it, and following it would cut a release +branch named for the wrong release (Copilot). + ⚠️ **`--no-git-tag-version` is load-bearing.** A bare `npm version` also tags, and the tag would land on a `v2/main` commit — but the release must be cut from `main`, so the tag has to point at the merge commit there (step 3). Tagging here @@ -57,9 +63,14 @@ it behind, something went wrong. ## 3. Tag `origin/main` and draft the Release +Derive the tag from the version that just landed, rather than typing one — a +hard-coded tag is either already taken (so `git tag` aborts) or, worse, wrong: + ```sh git fetch origin main -git tag 2.3.0 origin/main && git push origin 2.3.0 +VERSION=$(git show origin/main:package.json | node -p "JSON.parse(require('fs').readFileSync(0)).version") +echo "$VERSION" # sanity-check before tagging +git tag "$VERSION" origin/main && git push origin "$VERSION" # then draft & publish a GitHub Release for that tag → triggers `publish` ``` @@ -69,8 +80,8 @@ divergent local `main` can quietly produce or replay local commits. Tagging `HEAD` there tags a commit that is not on `origin/main`, and `git push origin ` pushes only the tag — leaving a release whose commit was never published. -⚠️ **No `v` prefix.** This repo's release tags are bare `x.y.z` — `2.2.0`, -`2.1.0`, `2.0.0`. npm's own `tag-version-prefix` defaults to `v` and the repo +⚠️ **No `v` prefix.** This repo's release tags are bare `x.y.z` — which is why +the command above tags `$VERSION` and not `v$VERSION`. npm's own `tag-version-prefix` defaults to `v` and the repo sets no `.npmrc`, so a bare `npm version` would have produced a mismatched tag; tagging by hand is what keeps it right. (The workflow's assert step strips a leading `v` before comparing, so a `v`-prefixed tag would still publish — it diff --git a/scripts/verify-format-coverage.main.test.mjs b/scripts/verify-format-coverage.main.test.mjs index e10130f6..fd0d6f63 100644 --- a/scripts/verify-format-coverage.main.test.mjs +++ b/scripts/verify-format-coverage.main.test.mjs @@ -74,24 +74,26 @@ function runWithGuards(guards) { } } -const BOTH_SIBLINGS = ["verify:typecheck-coverage", "verify:dep-lockstep"]; +const ALL_SIBLINGS = [ + "verify:typecheck-coverage", + "verify:dep-lockstep", + "verify:skills", +]; -test("vouch: fails when `verify:dep-lockstep` is dropped from validate", () => { - const { status, out } = runWithGuards(["verify:typecheck-coverage"]); - assert.equal(status, 1, out); - assert.match(out, /no longer runs `verify:dep-lockstep`/); -}); +for (const dropped of ALL_SIBLINGS) { + test(`vouch: fails when \`${dropped}\` is dropped from validate`, () => { + const { status, out } = runWithGuards( + ALL_SIBLINGS.filter((g) => g !== dropped), + ); + assert.equal(status, 1, out); + assert.match(out, new RegExp("no longer runs `" + dropped + "`")); + }); +} -test("vouch: fails when `verify:typecheck-coverage` is dropped from validate", () => { - const { status, out } = runWithGuards(["verify:dep-lockstep"]); - assert.equal(status, 1, out); - assert.match(out, /no longer runs `verify:typecheck-coverage`/); -}); - -test("vouch: passes when both siblings are still wired", () => { +test("vouch: passes when every sibling is still wired", () => { // The run still fails afterwards — the fixture has no client manifests to // harvest globs from — so assert on the *reason*, not the exit status: no // sibling may be reported missing. - const { out } = runWithGuards(BOTH_SIBLINGS); + const { out } = runWithGuards(ALL_SIBLINGS); assert.doesNotMatch(out, /no longer runs/); }); diff --git a/scripts/verify-format-coverage.mjs b/scripts/verify-format-coverage.mjs index 20ea65db..c7abd3fa 100644 --- a/scripts/verify-format-coverage.mjs +++ b/scripts/verify-format-coverage.mjs @@ -166,15 +166,19 @@ function trackedSourceFiles() { return out.split("\n").filter(Boolean); } -// Vouch for the sibling guards: a guard can't detect being unrun itself, so the -// three form a cycle instead. This one checks BOTH others; each of them checks -// only this one. So dropping `verify:typecheck-coverage` or -// `verify:dep-lockstep` is caught here, and dropping *this* guard is caught by -// either of them. Only removing all three at once slips through. +// Vouch for the sibling guards: a guard can't detect being unrun itself, so they +// form a cycle instead. This one checks the others; each of them checks only +// this one. So dropping `verify:typecheck-coverage`, `verify:dep-lockstep` or +// `verify:skills` is caught here, and dropping *this* guard is caught by any of +// them. Only removing all of them at once slips through. const rootScripts = JSON.parse( readFileSync(path.join(repoRoot, "package.json"), "utf8"), ).scripts; -for (const sibling of ["verify:typecheck-coverage", "verify:dep-lockstep"]) { +for (const sibling of [ + "verify:typecheck-coverage", + "verify:dep-lockstep", + "verify:skills", +]) { if (rootReachesScript(rootScripts, sibling)) continue; console.error( `verify:format-coverage — the root \`validate\` no longer runs \`${sibling}\` (its sibling guard). Restore it.`, diff --git a/scripts/verify-skills.main.test.mjs b/scripts/verify-skills.main.test.mjs index 0bbf2c61..4ead997c 100644 --- a/scripts/verify-skills.main.test.mjs +++ b/scripts/verify-skills.main.test.mjs @@ -12,11 +12,20 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + cpSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { checkWiring } from "./verify-skills.mjs"; + const SCRIPT = path.join( path.dirname(fileURLToPath(import.meta.url)), "verify-skills.mjs", @@ -175,3 +184,78 @@ test("reports every offender in one pass rather than dying on the first", () => assert.match(out, /3 problem\(s\)/); rmSync(dir, { recursive: true, force: true }); }); + +// --- wiring vouch ----------------------------------------------------------- +// +// The gates cannot detect being unrun, so they vouch for each other — the same +// cycle the sibling guards use. `checkWiring` is the pure half of that, driven +// here with fixture inputs; the other half (`verify:format-coverage` noticing +// `verify:skills` gone from `validate`) lives in that guard. +// +// Three distinct links, each of which would otherwise disappear in silence: +// `verify:skills` reachable from `validate`, `verify:skills:cli` in +// `local:gate`, and `verify:skills:cli` in the workflow. + +const WIRED_SCRIPTS = { + validate: "npm run verify:format-coverage && npm run verify:skills", + "verify:format-coverage": "node scripts/verify-format-coverage.mjs", + "verify:skills": "node scripts/verify-skills.mjs", + "local:gate": + "npm run validate && npm run verify:skills:cli && npm run coverage", + "verify:skills:cli": "node scripts/verify-skills-cli.mjs", +}; +const WIRED_WORKFLOW = + "jobs:\n build:\n steps:\n - run: npm run verify:skills:cli\n"; + +test("checkWiring is silent when both gates are wired", () => { + assert.deepEqual(checkWiring(WIRED_SCRIPTS, WIRED_WORKFLOW), []); +}); + +test("checkWiring catches a sibling guard dropped from validate", () => { + const scripts = { ...WIRED_SCRIPTS, validate: "npm run verify:skills" }; + assert.match( + checkWiring(scripts, WIRED_WORKFLOW).join(), + /no longer runs `verify:format-coverage`/, + ); +}); + +test("checkWiring catches the authoritative validator dropped from local:gate", () => { + const scripts = { + ...WIRED_SCRIPTS, + "local:gate": "npm run validate && npm run coverage", + }; + assert.match( + checkWiring(scripts, WIRED_WORKFLOW).join(), + /local:gate` no longer runs `verify:skills:cli`/, + ); +}); + +test("checkWiring catches the authoritative validator dropped from CI", () => { + assert.match( + checkWiring( + WIRED_SCRIPTS, + "jobs:\n build:\n steps:\n - run: npm run validate\n", + ).join(), + /workflows\/main\.yml` no longer runs/, + ); +}); + +test("checkWiring reports every broken link at once", () => { + assert.equal(checkWiring({ validate: "", "local:gate": "" }, "").length, 3); +}); + +test("the repository as it stands is wired", () => { + // The live assertion: fixtures can drift from what the repo actually does. + const repoRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "..", + ); + const scripts = JSON.parse( + readFileSync(path.join(repoRoot, "package.json"), "utf8"), + ).scripts; + const workflow = readFileSync( + path.join(repoRoot, ".github/workflows/main.yml"), + "utf8", + ); + assert.deepEqual(checkWiring(scripts, workflow), []); +}); diff --git a/scripts/verify-skills.mjs b/scripts/verify-skills.mjs index 7151975e..fd4d44c9 100755 --- a/scripts/verify-skills.mjs +++ b/scripts/verify-skills.mjs @@ -31,6 +31,7 @@ import { spawnSync } from "node:child_process"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { claudeSpawnArgs, probeClaudeVersion } from "./lib/claude-cli.mjs"; +import { reachableScripts, rootReachesScript } from "./lib/npm-scripts.mjs"; import { compareVersions, parseClaudeVersion, @@ -58,6 +59,41 @@ export function skillDirs( .sort(); } +/** + * Assert the two skill gates are still WIRED, not merely present. + * + * A gate that stops being invoked fails silently and every fixture test stays + * green — which is the same failure the sibling guards' vouch cycle exists to + * prevent, so this joins it (Copilot). Two distinct links: + * + * - `verify:skills` (this file) must be reachable from the root `validate`. + * It cannot detect its own absence, so `verify:format-coverage` vouches for + * it and this vouches back — the established cycle. + * - `verify:skills:cli` must be in `local:gate` AND in the workflow. That is + * the step that actually runs the authoritative validator, and it is + * reachable from neither `validate` nor any test, so nothing else would + * notice it going missing. + */ +export function checkWiring(rootScripts, workflowText) { + const problems = []; + if (!rootReachesScript(rootScripts, "verify:format-coverage")) { + problems.push( + "the root `validate` no longer runs `verify:format-coverage` (a sibling guard). Restore it.", + ); + } + if (!reachableScripts(rootScripts, "local:gate").has("verify:skills:cli")) { + problems.push( + "`npm run local:gate` no longer runs `verify:skills:cli`, so the authoritative validator never runs locally. Restore it.", + ); + } + if (!/npm run verify:skills:cli/.test(workflowText)) { + problems.push( + "`.github/workflows/main.yml` no longer runs `npm run verify:skills:cli`, so the authoritative validator never runs in CI. Restore it.", + ); + } + return problems; +} + function main(argv = process.argv.slice(2)) { // An explicit directory is the seam the fixture tests drive; without it this // guard could only ever be exercised against the repo's own (green) skills, @@ -66,6 +102,20 @@ function main(argv = process.argv.slice(2)) { const SKILLS_DIR = override ? path.resolve(override) : DEFAULT_SKILLS_DIR; const failures = []; + // Only meaningful against the real repo; a fixture run has no wiring. + if (!override) { + const rootScripts = JSON.parse( + readFileSync(path.join(ROOT, "package.json"), "utf8"), + ).scripts; + const workflow = readFileSync( + path.join(ROOT, ".github", "workflows", "main.yml"), + "utf8", + ); + for (const problem of checkWiring(rootScripts, workflow)) { + failures.push(problem); + } + } + if (!existsSync(SKILLS_DIR)) { console.error( `verify:skills — ${path.relative(ROOT, SKILLS_DIR)} does not exist.`,