diff --git a/scripts/verify-skills.main.test.mjs b/scripts/verify-skills.main.test.mjs index 333b8571..6ec67382 100644 --- a/scripts/verify-skills.main.test.mjs +++ b/scripts/verify-skills.main.test.mjs @@ -28,6 +28,7 @@ import { checkWiring, ciRunsUnconditionally, runsCommand, + scriptChainRuns, } from "./verify-skills.mjs"; const SCRIPT = path.join( @@ -408,3 +409,77 @@ test("ciRunsUnconditionally requires the workflow to fire on ordinary changes", ); assert.equal(ciRunsUnconditionally(step, C), false); }); + +test("scriptChainRuns follows the chain to a real invocation", () => { + const T = "verify:skills:cli"; + assert.equal( + scriptChainRuns( + { "local:gate": `npm run validate && npm run ${T}` }, + "local:gate", + T, + ), + true, + ); + // Through an intermediate script, with or without flags on the way. + assert.equal( + scriptChainRuns( + { "local:gate": "npm run validate", validate: `npm run ${T}` }, + "local:gate", + T, + ), + true, + ); + assert.equal( + scriptChainRuns( + { "local:gate": "npm run validate --silent", validate: `npm run ${T}` }, + "local:gate", + T, + ), + true, + ); +}); + +test("scriptChainRuns is not satisfied by a mention or a longer name", () => { + // `reachableScripts` extracts any `npm run …` substring, so both of these + // passed the old check while the validator never ran. + const T = "verify:skills:cli"; + assert.equal( + scriptChainRuns( + { "local:gate": `echo npm run ${T} && npm run coverage` }, + "local:gate", + T, + ), + false, + ); + assert.equal( + scriptChainRuns({ "local:gate": `npm run ${T}:disabled` }, "local:gate", T), + false, + ); + assert.equal( + scriptChainRuns({ "local:gate": "npm run coverage" }, "local:gate", T), + false, + ); + assert.equal(scriptChainRuns({}, "local:gate", T), false); +}); + +test("scriptChainRuns terminates on a cyclic script graph", () => { + assert.equal( + scriptChainRuns( + { "local:gate": "npm run a", a: "npm run b", b: "npm run a" }, + "local:gate", + "verify:skills:cli", + ), + false, + ); +}); + +test("checkWiring rejects a local:gate that only mentions the validator", () => { + const scripts = { + ...WIRED_SCRIPTS, + "local:gate": "echo npm run verify:skills:cli && npm run coverage", + }; + assert.match( + checkWiring(scripts, WIRED_WORKFLOW).join(), + /local:gate` no longer runs `verify:skills:cli`/, + ); +}); diff --git a/scripts/verify-skills.mjs b/scripts/verify-skills.mjs index d9fd693b..64da9515 100755 --- a/scripts/verify-skills.mjs +++ b/scripts/verify-skills.mjs @@ -31,7 +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 { rootReachesScript } from "./lib/npm-scripts.mjs"; import { parse as parseYaml } from "yaml"; import { formatClaudeVersion, @@ -82,6 +82,44 @@ export function runsCommand(script, command) { .some((segment) => segment === command); } +/** + * Whether `npm run ` reaches an invocation of `npm run `. + * + * `reachableScripts` extracts every `npm run …` **substring**, so a gate such as + * `echo npm run verify:skills:cli && npm run coverage` satisfies it while the + * validator never runs (Copilot) — the same over-match the CI half had, from the + * other side. This walks the chain instead, splitting each script on the + * separators that begin a new command and requiring an exact invocation, the + * way `runsCommand` does. + * + * A segment naming some OTHER script is followed, with or without trailing + * flags: `npm run coverage --silent` still runs `coverage`. Only the target + * itself is matched exactly, since these gate scripts take no arguments. + * + * @param {Record} scripts The root manifest's `scripts`. + * @param {string} entry + * @param {string} target + */ +export function scriptChainRuns(scripts, entry, target) { + const seen = new Set(); + const queue = [entry]; + while (queue.length > 0) { + const name = queue.shift(); + if (seen.has(name)) continue; + seen.add(name); + const body = scripts?.[name]; + if (typeof body !== "string") continue; + for (const segment of body.split(/[\n;|&]+/).map((s) => s.trim())) { + if (segment === `npm run ${target}`) return true; + const tokens = segment.split(/\s+/); + if (tokens[0] === "npm" && tokens[1] === "run" && tokens[2]) { + queue.push(tokens[2]); + } + } + } + return false; +} + /** * Whether a workflow's `on:` includes an event that fires for ordinary changes. * @@ -182,7 +220,7 @@ export function checkWiring(rootScripts, workflowText) { "the root `validate` no longer runs `verify:format-coverage` (a sibling guard). Restore it.", ); } - if (!reachableScripts(rootScripts, "local:gate").has("verify:skills:cli")) { + if (!scriptChainRuns(rootScripts, "local:gate", "verify:skills:cli")) { problems.push( "`npm run local:gate` no longer runs `verify:skills:cli`, so the authoritative validator never runs locally. Restore it.", );