test(skill): cover the `skill:sync` CLI and split the adversarial fixture into named cases

[IgorShevchik]

Background

Follow-up from #346, raised independently by the QA, engineering and CTO reviewers on that PR. Neither item is a defect today; both are gaps that will cost someone an afternoon later.

1. scripts/sync-skill-manifest.mjs has no test of its own

test/utils/skill-manifest.spec.ts imports buildManifest directly — deliberately, because an earlier draft combined module and CLI and importing it rewrote the manifest at import time, making a mutation test pass against a broken file. That split is right, but it leaves the CLI wrapper entirely unexercised:

• the --check branch and its exit code
• the "already up to date" no-op branch
• the writeFile path
• the ENOENT-narrowed read catch

CI never runs either script (ci.yml runs pnpm run test run only), so an inverted comparison or a wrong exit code would ship and only surface when a human ran pnpm run skill:sync by hand and squinted at the output. The guarantee is not lost — the vitest check enforces manifest freshness through a different path — but the file AGENTS.md tells a contributor to run has nothing standing behind it.

Blocker to solve first: the CLI resolves repoRoot from its own location, so it can only ever operate on the real repository. Testing it means accepting a root (an argument, or an env var) so a fixture can be pointed at a temp directory. That is a deliberate API decision, which is why it is filed rather than bolted on:

// today
const repoRoot = join(dirname(fileURLToPath(import.meta.url)), '..')

Once that exists, a small spec using the same mkdtemp pattern already in the suite can spawnSync the CLI and assert exit code and file contents for all three paths.

2. The adversarial fixture is one it() doing about twenty things

discovers packages and refuses the shapes it cannot represent now walks sequentially through: multi-package discovery, package sort order, nested-path joining, quoted scalars (single and double), YAML escapes, trailing comments, whole-value comments, block scalars with indicators, duplicate description, duplicate name, quoted name, name-vs-directory mismatch, empty description, missing description, control characters, path traversal, unportable package name, install-time collisions, symlinked files, symlinked directories, and the zero-package case.

Every one of those is mutation-verified, so the coverage is real. The problem is diagnosis: a failure on step 3 hides steps 4 through 21 for that CI run, and the reporter names the test, not the property. Splitting into it.each over named cases would give the same coverage with a failure message that says which shape broke.

The obvious cost is fixture setup — twenty mkdtemp calls instead of one. A shared beforeAll-built root with per-case SKILL.md writes keeps it to one, at the price of some ordering coupling. Worth thinking about rather than doing reflexively, hence a separate issue.

Not in scope

Several checks in that spec only ever run against the real, clean tree — ships no empty reference file, the four OR'd clauses of the relative-link check. Proving those can fire needs synthetic markdown fixtures rather than scanning skillDir in place. Same shape of work as item 2; fold it in if item 2 gets done.