fix(skills): quality sweep — compile errors, stale facts, contradictions across 20 skills

[citypaul]

What

A four-agent adversarial review of all 28 pre-existing skills (event-sourcing was reviewed separately in #181), applying the same methodology that caught the non-compiling snippets there: extract every fenced TypeScript block and compile under tsc 5.7.2 --strict; verify factual claims against primary sources, npm registries, and installed CLIs; hunt internal contradictions and broken references. Every finding was independently verified with a compiler error or a source URL before being fixed — several suspicious-looking claims were positively confirmed correct and left alone (Stryker flags, RateLimit draft status, OWASP list, 12-factor names, codex/claude/gemini CLI flags, Vitest Browser Mode APIs).

Fixed — snippets that did not compile under strict TS (9 groups)

• testing: union member access without narrowing in the skill's core ✅ examples (now whole-value toEqual assertions, with a note on why); implicit-any params + await in a non-async it callback in the extraction example; Pick factory tip that required all picked fields (now Partial<Pick<…>>); a spy example that never triggered its own spy
• domain-driven-design: testing-by-layer union narrowing; aggregate-design's createOccasion (self-overwriting spread that silently undid the advertised .trim(), zero-arg call to a one-arg factory); domain-events' evolve could not typecheck against the discriminated-union state the skill itself mandates
• hexagonal-architecture: Record<PledgeResult['reason'], number> indexed a union member the success variant lacks (now Extract<…>)
• api-design: idempotency example used a result variable that was never defined (restored the dropped parse step)
• characterisation-tests: randomUUID mocked with a non-UUID literal (its return type is a 4-hyphen template literal)
• react-testing (legacy resource): implicit-any provider/render-helper params
• functional: the ❌ over-engineered compose example didn't compile and filtered strings by .active — now coherent, so its only flaw is the unnecessary abstraction

Fixed — stale or wrong facts

• Stryker mutates trim() by removal (the swap pair is trimStart()↔trimEnd())
• Zod 4 deprecated z.string().email()/uuid() → z.email()/z.uuid()
• vitest-compatible-jest-extended-snapshot does not exist on npm and Vitest has no "Jest compatibility mode"; approvals integration claim tightened
• getByDisplayValue is not a Vitest Browser Mode locator; getByPlaceholder naming difference flagged
• Deprecation header is now RFC 9745 with @-timestamp Structured-Field syntax, not an IETF draft with HTTP-date
• Stripe introduced /v2 in 2024 — "never made a v2" corrected
• 422 is "Unprocessable Content" (RFC 9110)
• npx skills check is undocumented and applies updates — guidance now points at npx skills update
• PlantUML mxgraph.* stencils are a docu.md-viewer extension official renderers reject — portability warning added, plus a "where will this be viewed?" rule in diagrams (GitHub renders only ```mermaid)
• Dead attribution link in test-design-reviewer (account + file moved); stale Gemini model example in double-check

Fixed — contradictions and broken references

• DDD's Occasion entity now matches every snippet that uses it (totalPledged/isFundingClosed/GiftIdea.status)
• cli-design's FakeLogger now implements the real Logger port (was silently recording categories as messages); two type assertions with false written justifications deleted (the code compiles without them); FORCE_COLOR precedence wording
• Optimistic-locking guidance aligned with the skills' own error-modeling rule
• "Disable OPTIONS" vs CORS-preflight conflict resolved
• Given/When/Then framing in find-gaps/story-splitting reworded to precondition → trigger → observable outcome — consistent with this repo's "BDD is not Gherkin" stance
• refactoring's "Mutations" priority-row disambiguated to data mutation; two "when NOT to refactor" bullets that literally forbade all refactoring reworded
• storyboard: /harden → /impeccable harden (no standalone skill exists), commit steps respect the commit-approval gate, pre-commit-hook claim hedged
• Three wrong relative resource paths (resources/… from files already inside resources/, and a cross-skill file referenced as if local)

Also included — installer fix (supersedes #183)

install-claude.sh still assumed the old skills CLI layout (symlinks into the universal ~/.agents/skills/ cache). Since skills CLI ~1.5, npx skills add copies each skill into ~/.claude/skills/<name> as a regular directory, tracked in ~/.agents/.skill-lock.json. The installer treated every regular directory as a pre-skills.sh leftover, so each run moved all CLI-managed skills aside as "legacy", reinstalled them, then warned they "won't be visible to non-Claude agents" — an endless move-and-reinstall cycle that silently loses skills if an install step fails after the move (and made a newly merged skill like event-sourcing from #181 look like the installer "wasn't updated"; verified with sandboxed-HOME installs that discovery itself is fine — all 29 skills + resources land).

migrate_legacy_skill_dirs and verify_skills_installed now consult the skills CLI lock file via a new skill_is_lock_managed helper: symlinks (old layout) and lock-tracked directories (new layout) are CLI-managed and left alone; only genuinely unmanaged directories are migrated or flagged. New test/install-claude-skill-layout.sh (written RED first against the old behaviour) covers all three layouts. Separate patch changeset included.

Testing

• All type-sensitive fixes re-verified under tsc --strict from the updated markdown
• ./test/run.sh — all pass

Changeset

Includes .changeset/skills-quality-improvements.md (patch).

🤖 Generated with Claude Code