Commit graph inspector/scripts/verify-skills-cli.test.mjs
Author SHA1 Message Date
cliffhall
c26995bf82 chore: address Copilot review round 16
`parseClaudeVersion` dropped SemVer suffixes, so an installed `2.1.250-beta.1`
parsed as `[2, 1, 250]` and compared EQUAL to the stable pin — a different
validator schema running locally while the log claimed an exact match, which is
the cross-machine disagreement the pin exists to prevent.

It now returns `{ parts, prerelease }`, because the two callers want different
things: `isPinnedVersion` rejects any prerelease (build metadata still matches,
per SemVer precedence), while the eval's availability probe only needs some
usable CLI. `formatClaudeVersion` keeps the suffix in the messages, so a
prerelease is reported as what it is rather than as the pin.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-29 04:40:48 -04:00
cliffhall
09b8f0e834 chore: address Copilot review round 7 (Windows .cmd shims)
Node has refused shell-free `.cmd`/`.bat` spawns since CVE-2024-27980, and an
npm-installed `claude` is a `.cmd` shim on Windows — so every shell-free spawn
of it reported ENOENT there, each in a quiet way: `verify:skills` skipped its
authoritative hand-off, `verify:skills:cli` downloaded the pinned package it
already had (and failed offline), and `skills:eval` said "not on PATH".

- New `scripts/lib/claude-cli.mjs` is the one place that decides how `claude` is
  spawned: `shell` on win32, arguments through the existing `winShellArgs`
  quoting so `cmd.exe` cannot re-parse them as syntax (#1939). Unit-tested with
  the platform injected, since a win32-only bug no runner here executes is
  exactly the kind that ships.
- `verify-skills`, `verify-skills-cli` and `skill-eval` all route their probes
  and spawns through it.
- skill-eval sends the prompt on **stdin** rather than in argv. `claude -p` reads
  a piped prompt (verified end to end), which sidesteps `cmd.exe` metacharacter
  re-parsing entirely rather than relying on quoting a model-visible string, and
  keeps the prompt out of the process table.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-29 02:32:30 -04:00
cliffhall
75ab1b069f chore: address Copilot review round 5
- verify-skills-cli: use a local CLI only when it matches the pin EXACTLY. A
  floor let a maintainer on a newer CLI validate against a different schema than
  CI's, so the same `local:gate` could disagree across machines — the failure a
  pin exists to prevent.
- verify-skills-cli: extract `runValidator({probe, spawn, log, error})` and
  cover the orchestration — validator accepts, validator rejects, spawn fails,
  and the three unusable-probe shapes. On a machine with a working CLI an actual
  run only ever walks the happy path.
- pr-flow: stop putting the `gh` token in curl's argv, where any local process
  can read it off the process table. Feed the header through `curl --config -`.
- docs/quality-gate: three places had drifted — the CI tier row, the
  `local:gate` chain, and the `test:scripts` inventory (which now names all four
  suites this PR adds, not two).

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-29 02:02:51 -04:00
cliffhall
d05e76d470 chore: address Copilot review round 4
- verify:skills:cli — the authoritative validator is now a GUARANTEED step in
  `local:gate` and CI, not a best-effort hand-off. It resolves the CLI itself
  (an installed one at 2.1.233+, else a pinned `npx -y`), so both tiers run the
  same script and cannot drift. `verify:skills` keeps skipping when the CLI is
  absent, which is right for `validate`: fast, offline, no Claude Code required.
- skill-manifest: the 1,536-char listing-entry cap covers `description` +
  `when_to_use` together; checking the description alone passed an over-cap
  entry. `when_to_use` is now parsed, validated and counted, with boundary tests
  at 1,536/1,537.
- verify-skills: report the budget as *this repo's contribution*. The real
  listing budget is shared with bundled skills and the contributor's own
  ~/.claude/skills, none of which is visible from here, so the number was
  claiming more than it measured.
- skill-eval: a negative case now means "no skill OF THIS REPO'S fired". It
  failed on any Skill invocation, so a contributor with an unrelated global
  skill got a false failure about their environment rather than these skills.
- docs/architecture + local-dev: the Node backend runner gets its
  `@inspector/core` alias from `clients/web/tsup.runner.config.ts`, not from
  Vite. The old text sent runner work to the wrong config.
- local-dev: the tarball does not ship "only each client's build/" — the
  allowlist also carries `clients/web/dist`, `clients/web/static` and the
  install helper. States the invariant that actually matters instead.
- issue-triage: the test for needing triage is "no board card". Adding "and no
  milestone" would skip the approved-but-unboarded case the same section then
  handles. Also corrected the `$M` note: the lookup is `--state all`, so closed
  issues DO have entries — `isopen` is about the invariants, not the lookup.
- issue-create: replaced "all five" with a per-version table. Steps 3 and 6
  (milestone, Priority) do not exist for v1, so the old list opened with a rule
  the same skill later tells you to violate.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-29 01:47:43 -04:00