diff --git a/.claude/skills/local-dev/SKILL.md b/.claude/skills/local-dev/SKILL.md index 15854b99..33074c93 100644 --- a/.claude/skills/local-dev/SKILL.md +++ b/.claude/skills/local-dev/SKILL.md @@ -1,6 +1,6 @@ --- name: local-dev -description: Getting the Inspector running locally and adding dependencies to it. Which install to run and which script starts each client (web, cli, tui, launcher); why v2 is not an npm workspace; how the @inspector/core alias works; and where a new dependency must be declared — root vs client, dependencies vs devDependencies, and the bundler `external` lists a root package must also appear in. +description: Getting the Inspector running locally, and the reasoning behind its dependency rules. Which install to run and which script starts each client (web, cli, tui, launcher); why v2 is not an npm workspace; how the @inspector/core alias works; and why the placement rules in AGENTS.md are what they are — what each one is defending against, what it looked like when it was violated, and how to tell you have hit one. disable-model-invocation: false --- diff --git a/AGENTS.md b/AGENTS.md index 074d5576..f7af437a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,7 +16,7 @@ users invoke them by name. | Skill | Covers | How it loads | | --- | --- | --- | -| [`local-dev`](.claude/skills/local-dev/SKILL.md) | Install and run each client; the `@inspector/core` alias; **where a new dependency is declared** (root vs client, `dependencies` vs `devDependencies`, the bundler `external` lists, `overrides`) | Model-invoked | +| [`local-dev`](.claude/skills/local-dev/SKILL.md) | Install and run each client; the `@inspector/core` alias; and the **reasoning** behind Dependency placement below — what each rule defends against and how to tell you have hit one (the rules themselves stay here) | Model-invoked | | [`project-structure`](.claude/skills/project-structure/SKILL.md) | Which client owns which surface, what is in `core/`, where a new file belongs | Model-invoked (not user-invocable) | | [`testing`](.claude/skills/testing/SKILL.md) | Where a test file goes, which command runs it, the tiers, clearing the coverage gate, `renderWithMantine` | Model-invoked | | [`issue-create`](.claude/skills/issue-create/SKILL.md) | The five-step create flow: version label, type label, milestone, board card, Status + Priority | `/issue-create` | @@ -201,7 +201,7 @@ that from happening: overflows, and drops the least-invoked entries **first** — which are exactly the model-invoked skills that must fire on their own. `verify:skills` prints the current cost against the budget recorded in `scripts/lib/skill-manifest.mjs` - (1,173/4,000 characters as of this writing) and fails when it is exceeded. Raise + (1,199/4,000 characters as of this writing) and fails when it is exceeded. Raise the budget deliberately, or tighten a description; each entry is capped at 1,536 characters regardless, so **put the key use case first**. diff --git a/scripts/verify-skills.main.test.mjs b/scripts/verify-skills.main.test.mjs index 74b766a9..4d5e0e50 100644 --- a/scripts/verify-skills.main.test.mjs +++ b/scripts/verify-skills.main.test.mjs @@ -24,7 +24,11 @@ import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import { checkWiring, ciRunsUnconditionally } from "./verify-skills.mjs"; +import { + checkWiring, + ciRunsUnconditionally, + runsCommand, +} from "./verify-skills.mjs"; const SCRIPT = path.join( path.dirname(fileURLToPath(import.meta.url)), @@ -342,3 +346,41 @@ test("the repository's workflow runs the validator unconditionally", () => { true, ); }); + +test("runsCommand matches an exact invocation, not a substring", () => { + const C = "npm run verify:skills:cli"; + for (const script of [ + C, + `npm run validate && ${C}`, + `npm run validate\n${C}\n`, + `${C} ; npm run something`, + ]) { + assert.equal(runsCommand(script, C), true, script); + } +}); + +test("runsCommand rejects a longer script name or a mere mention", () => { + // Both would let the real validator be removed while the wiring guard stayed + // green — the counterexamples to the substring test this replaced. + const C = "npm run verify:skills:cli"; + for (const script of [ + "npm run verify:skills:cli:disabled", + "echo npm run verify:skills:cli", + "npm run verify:skills", + "npm run verify:skills:cli --silent", + "", + ]) { + assert.equal(runsCommand(script, C), false, script); + } +}); + +test("ciRunsUnconditionally inherits the exact-invocation rule", () => { + const C = "npm run verify:skills:cli"; + assert.equal( + ciRunsUnconditionally( + "jobs:\n build:\n steps:\n - run: npm run verify:skills:cli:disabled\n", + C, + ), + false, + ); +}); diff --git a/scripts/verify-skills.mjs b/scripts/verify-skills.mjs index 636f6c95..f298624c 100755 --- a/scripts/verify-skills.mjs +++ b/scripts/verify-skills.mjs @@ -60,6 +60,30 @@ export function skillDirs( .sort(); } +/** + * Whether a shell script invokes exactly `command`. + * + * A substring test is not enough, and my own justification for one was wrong: + * it accepts a LONGER script name (`npm run verify:skills:cli:disabled`) and a + * script merely named by another command (`echo npm run verify:skills:cli`), + * either of which would let the real validator be removed while this guard + * stayed green (Copilot). + * + * So the script is split on the separators that start a new command, and a + * segment must equal the invocation exactly. That is precise for the commands + * this guard checks, which take no arguments; a command that did would need + * prefix matching with an argument boundary instead. + * + * @param {string} script A step's `run:` block. + * @param {string} command e.g. `npm run verify:skills:cli`. + */ +export function runsCommand(script, command) { + return script + .split(/[\n;|&]+/) + .map((segment) => segment.trim()) + .some((segment) => segment === command); +} + /** * Whether the workflow has a step that runs `command` on every push. * @@ -90,10 +114,7 @@ export function ciRunsUnconditionally(workflowText, command) { 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