diff --git a/.claude/skills/pre-push-gate/SKILL.md b/.claude/skills/pre-push-gate/SKILL.md index e24e7efe..f17d574a 100644 --- a/.claude/skills/pre-push-gate/SKILL.md +++ b/.claude/skills/pre-push-gate/SKILL.md @@ -23,8 +23,12 @@ npm run format # auto-fix: core/, scripts/, the shared surface, every clien npm run local:gate # several minutes ``` -`local:gate` is `validate` → `verify:skills:cli` → `coverage` → `verify:build-gate` → -`verify:bundle-externals` → `smoke` → `smoke:web:firefox` → `local:storybook`. +The stages it runs, in order, are listed in +[`docs/quality-gate.md`](../../../docs/quality-gate.md) — deliberately in one +place only. That list drifted apart across three copies while this PR was in +review, which is the argument for not making a fourth. `npm run local:gate` +prints each stage as it starts, so the running command is the other reliable +answer. It is a **strict superset** of GitHub CI (which additionally runs `npm install`, and runs `coverage` as a parallel job). So the direction that matters holds: diff --git a/README.md b/README.md index b7671ed6..4bcb4c10 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ npm run coverage # the per-file ≥90% gate (lines/statements/functions/bran npm run local:gate # MANDATORY before pushing — a strict superset of GitHub CI ``` -`npm run local:gate` is `validate` → `verify:skills:cli` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → `smoke:web:firefox` → `local:storybook`. See [Testing and the quality gate](./docs/quality-gate.md) for what each stage covers and why two of them are local-only, and [`AGENTS.md`](./AGENTS.md) for the testing rules themselves. +`npm run local:gate` chains every check below, plus the smokes and the Storybook tests. [Testing and the quality gate](./docs/quality-gate.md) owns the stage list and says what each one covers and why two are local-only; [`AGENTS.md`](./AGENTS.md) holds the testing rules themselves. ## Contributing — `AGENTS.md`, `CLAUDE.md`, and the skills diff --git a/scripts/verify-skills.main.test.mjs b/scripts/verify-skills.main.test.mjs index cdd114ba..74b766a9 100644 --- a/scripts/verify-skills.main.test.mjs +++ b/scripts/verify-skills.main.test.mjs @@ -24,7 +24,7 @@ import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import { checkWiring } from "./verify-skills.mjs"; +import { checkWiring, ciRunsUnconditionally } from "./verify-skills.mjs"; const SCRIPT = path.join( path.dirname(fileURLToPath(import.meta.url)), @@ -236,7 +236,7 @@ test("checkWiring catches the authoritative validator dropped from CI", () => { WIRED_SCRIPTS, "jobs:\n build:\n steps:\n - run: npm run validate\n", ).join(), - /has no step that runs/, + /has no unconditional step that runs/, ); }); @@ -275,7 +275,7 @@ test("checkWiring is not satisfied by a mention outside an executable step", () for (const workflow of mentions) { assert.match( checkWiring(WIRED_SCRIPTS, workflow).join(), - /has no step that runs/, + /has no unconditional step that runs/, `should not count: ${workflow.split("\n")[0]}`, ); } @@ -286,3 +286,59 @@ test("checkWiring accepts the command inside a multi-line run step", () => { "jobs:\n build:\n steps:\n - run: |\n npm run validate\n npm run verify:skills:cli\n"; assert.deepEqual(checkWiring(WIRED_SCRIPTS, workflow), []); }); + +test("ciRunsUnconditionally requires a step that runs on every push", () => { + const C = "npm run verify:skills:cli"; + const wired = + "jobs:\n build:\n steps:\n - run: npm run verify:skills:cli\n"; + assert.equal(ciRunsUnconditionally(wired, C), true); + + // A multi-line `run:` block is the obvious way to over-correct; it counts. + assert.equal( + ciRunsUnconditionally( + "jobs:\n build:\n steps:\n - run: |\n npm run validate\n npm run verify:skills:cli\n", + C, + ), + true, + ); +}); + +test("ciRunsUnconditionally rejects a step that only runs sometimes", () => { + const C = "npm run verify:skills:cli"; + // Both are syntactically `run:` steps, so an executable-position check alone + // reports them as wired while PR CI validates nothing. + const releaseOnly = + "jobs:\n publish:\n if: github.event_name == 'release'\n steps:\n - run: npm run verify:skills:cli\n"; + const stepGated = + "jobs:\n build:\n steps:\n - if: false\n run: npm run verify:skills:cli\n"; + assert.equal(ciRunsUnconditionally(releaseOnly, C), false); + assert.equal(ciRunsUnconditionally(stepGated, C), false); +}); + +test("ciRunsUnconditionally is not satisfied by an unrelated or absent command", () => { + const C = "npm run verify:skills:cli"; + // `verify:skills` must not satisfy a search for `verify:skills:cli`. + assert.equal( + ciRunsUnconditionally( + "jobs:\n build:\n steps:\n - run: npm run verify:skills\n", + C, + ), + false, + ); + assert.equal(ciRunsUnconditionally("not: yaml: at: all:", C), false); + assert.equal(ciRunsUnconditionally("on: push\n", C), false); +}); + +test("the repository's workflow runs the validator unconditionally", () => { + const repoRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "..", + ); + assert.equal( + ciRunsUnconditionally( + readFileSync(path.join(repoRoot, ".github/workflows/main.yml"), "utf8"), + "npm run verify:skills:cli", + ), + true, + ); +}); diff --git a/scripts/verify-skills.mjs b/scripts/verify-skills.mjs index 126e3519..636f6c95 100755 --- a/scripts/verify-skills.mjs +++ b/scripts/verify-skills.mjs @@ -32,7 +32,7 @@ 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 { extractExecutableRegions } from "./lib/workflow-gate.mjs"; +import { parse as parseYaml } from "yaml"; import { compareVersions, parseClaudeVersion, @@ -60,6 +60,46 @@ export function skillDirs( .sort(); } +/** + * Whether the workflow has a step that runs `command` on every push. + * + * Deliberately conservative: a job or step carrying **any** `if:` is skipped. + * This guard cannot evaluate an Actions expression, and the property being + * asserted is "CI validates the skills on a PR" — which a conditional step does + * not establish. Over-strictness costs a false failure that is obvious to fix; + * under-strictness costs a guard that reports green while nothing runs. + * + * @param {string} workflowText + * @param {string} command + * @returns {boolean} + */ +export function ciRunsUnconditionally(workflowText, command) { + let doc; + try { + doc = parseYaml(workflowText); + } catch { + return false; + } + const jobs = doc?.jobs; + if (jobs === null || typeof jobs !== "object") return false; + + for (const job of Object.values(jobs)) { + if (job === null || typeof job !== "object") continue; + if ("if" in job) continue; + const steps = Array.isArray(job.steps) ? job.steps : []; + for (const step of steps) { + if (step === null || typeof step !== "object") continue; + if ("if" in step) continue; + // A plain substring is enough and cannot over-match: the command is a + // full `npm run