diff --git a/.gitignore b/.gitignore index a5e13a8..abe976b 100644 --- a/.gitignore +++ b/.gitignore @@ -64,8 +64,16 @@ skills/ios-sync skills/design-motion-principles skills/emil-design-eng skills/frontend-design + +# Impeccable — NOT a symlink: `impeccable skills install --scope=global` +# writes the skill dir (and its ~15 MB engine binary) straight in through the +# ~/.claude/skills symlink. Machine-owned, regenerated by make plugin/update. skills/impeccable +# …and the 4 subagents the same installer drops through ~/.claude/agents. +# agents/ is a tracked directory, so these need naming explicitly. +agents/impeccable-*.md + # External skills installed via `npx skills add` — auto-created by link.sh skills/darwin-skill @@ -169,12 +177,6 @@ skills-external/emil-design-eng/ # layout and the content is sha256-verified against 21st.dev's manifest. skills-external/21st-*/ -# Impeccable — machine-owned dist produced by `npx impeccable skills install` -# (install-plugins.sh Step 8d, update-all.sh), pinned in plugins.lock.json. -# Not vendored: the installer owns the layout and rewrites it on update -# (ctx7 pattern). Symlinked into skills/ by link.sh. -skills-external/impeccable/ - # npx `skills add` project-scope artifacts — darwin-skill copies itself into # the repo's .agents/ and writes skills-lock.json at root. Our own agents live # in agents/ (no dot) and stay tracked. Anchored to root so only the dotted diff --git a/CHANGELOG.md b/CHANGELOG.md index b839431..afca61b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,12 @@ Format follows [Keep a Changelog](https://keepachangelog.com/). `profile set|upload`) — publishing puts a component on a public listing. That tier rather than `ask`, per LRN-153. +- `lib/design-gate.md` §5: a suggest-only check, same shape as the §4 + animation-library one. When impeccable is active and the frontend project + has no `PRODUCT.md` at its root, the gate proposes `/impeccable init` once + and never runs it itself (it interviews the user). Skipped for single + component reviews and non-UI work. + ### Changed - **Design gate: `magic` → the `21st` CLI in the required-manual slot.** `design.profile`'s `GATE-BLOCK` now lists `21st` (CLI channel) and @@ -119,6 +125,23 @@ Format follows [Keep a Changelog](https://keepachangelog.com/). `bypassPermissions`). Adding a restriction stays allowed; removing one does not. No instruction clears these. +- **impeccable installs at `--scope=global`, subagents included, and the + pin fails safe.** `install-plugins.sh` Step 8d no longer stages a + `--scope=project` install in a tmpdir and moves the skill directory alone. + The installer writes through the `~/.claude/skills` and `~/.claude/agents` + symlinks straight into the repo: `skills/impeccable` plus the four + `agents/impeccable-*.md`, both gitignored and machine-owned, which is what + the manual `--scope=global` command already did. The step refuses to run + before `make link` has created those symlinks (an install before them + materializes real directories that `link.sh` then refuses to replace), + keeps a profile-parked copy parked, reports the skill version and agent + count, and prints the per-project `/impeccable init` hint. A pinned + install that fails falls back to `impeccable@latest` with a warning to + bump `plugins.lock.json`. `update-all.sh` follows the same shape. + `plugins.lock.json` pin 3.2.0 → 4.1.0 (the CLI only: the skill dist and + the engine binary have their own release tracks). `link.sh` drops + impeccable from `EXTERNAL_SKILLS`; `skills-external/impeccable/` is gone. + ### Security - **Ten secret-reader deny rules added**: `sed`, `awk`, `cut`, `tr`, `sort`, `uniq`, `diff`, `od`, `xxd`, `strings` against `.env*`. Six of @@ -160,6 +183,20 @@ Format follows [Keep a Changelog](https://keepachangelog.com/). holds under `defaultMode: default`, not under this config's `auto`. The paragraph now separates what is verified from what is not, and names `deny` as the only tier the classifier cannot lift. +- **`make plugin` never installed impeccable.** Three defects. The 3.2.0 + pin had rotted upstream: the CLI fetches its skill dist at install time and + that release's artifact is gone (`Download failed: invalid zip data`), + which the step reported as "run it yourself" on every run. The + project-scope staging dropped the four subagents the same install writes. + And `/impeccable init` was never announced. A fourth, found while probing + the fix: once a copy is already installed, a rotted pin exits 0 + (`Could not check for skill updates … Existing skills were left + unchanged`), byte-identical on disk to a genuine "Skills are up to date" + rerun, so `imp_install` now reads the installer output instead of trusting + the exit code or a version compare. Verified with the real installer in a + sandbox HOME: fresh install, rotted pin over a copy (fallback fires), same + pin rerun (no false warning), parked copy plus rotted pin (fallback, then + returned to `skills-disabled/`). ## [1.5.0] — 2026-09-13 diff --git a/install-plugins.sh b/install-plugins.sh index fa6e51c..c1b25d3 100644 --- a/install-plugins.sh +++ b/install-plugins.sh @@ -788,54 +788,114 @@ else fi echo "" -# ── Step 8d: Impeccable (design anti-pattern detector + skill) ── -# 45 deterministic detector rules (CLI `impeccable detect`, exit 0/2) + -# /impeccable skill (23 verbs). Machine-owned dist: the installer produces -# it, we stage it in a tmpdir then move it under skills-external/ -# (gitignored, ctx7 pattern) — never let the installer write through the -# ~/.claude/skills symlink into the tracked repo dir. -echo "── Step 8d: Impeccable — design anti-pattern detector ────" +# ── Step 8d: Impeccable (design detector + skill + subagents) ── +# 45 deterministic detector rules (`impeccable detect`, exit 0/2), the +# /impeccable skill (23 verbs) and 4 `impeccable-*` subagents. +# +# GLOBAL scope, no staging: the installer writes ~/.claude/skills/impeccable/ +# (skill + its self-contained engine binary) and ~/.claude/agents/ +# impeccable-*.md, and both of those are symlinks into this repo — so the +# global install IS the repo install. Machine-owned and gitignored on both +# sides. `--scope=project` was wrong twice over: it writes /.claude/, +# which serves only the directory it ran in, and the staged `mv` that +# followed it moved the skill alone, silently dropping the subagents. +# +# The pin rots. The CLI downloads its skill dist at install time and an older +# release's artifact eventually disappears (`impeccable@3.2.0` → "Download +# failed: invalid zip data", 2026-09-22) — which is what left `make plugin` +# telling the user to run the command by hand. So a pin failure falls back to +# @latest and says, loudly, that the lock needs bumping. +echo "── Step 8d: Impeccable — design detector, skill + agents ──" echo "" -IMP_DIR="$REPO/skills-external/impeccable" +IMP_SKILL_DIR="$HOME/.claude/skills/impeccable" +IMP_PARKED="$REPO/skills-disabled/impeccable" IMP_VER=$(pinned_version "impeccable") NODE_MAJOR=$(node -v 2>/dev/null | sed 's/^v//' | cut -d. -f1) -if [ -z "${NODE_MAJOR:-}" ] || [ "$NODE_MAJOR" -lt 24 ]; then - if [ -f "$IMP_DIR/SKILL.md" ]; then + +# One install attempt. $1 = "latest" or an exact version. On failure, IMP_FAIL +# holds the reason. The exit code alone is not enough: with a copy already in +# place, a rotted pin exits 0 ("Could not check for skill updates: invalid +# zip data … Existing skills were left unchanged"), exactly like a genuine +# up-to-date no-op ("Skills are up to date") — only the output tells them +# apart. Probed 2026-09-22 on 4.1.0 vs 3.2.0 in a sandbox HOME. +imp_install() { + local pkg="impeccable" out rc=0 + [ "$1" != "latest" ] && pkg="impeccable@$1" + out=$(npx -y "$pkg" skills install -y --providers=claude --scope=global \ + --no-hooks 2>&1) || rc=$? + IMP_FAIL=$(printf '%s\n' "$out" \ + | grep -E 'Download failed|Could not check for skill updates' \ + | head -1 || true) + if [ "$rc" -ne 0 ] && [ -z "$IMP_FAIL" ]; then + IMP_FAIL="installer exited $rc" + fi + [ -z "$IMP_FAIL" ] +} + +# Precondition: ~/.claude/{skills,agents} must already be link.sh's symlinks. +# Installing before they exist materializes real directories there, and +# link.sh then refuses to replace them ("is a real directory") — a worse +# failure than skipping, because it needs manual repair. +IMP_READY=true +for _imp_d in skills agents; do + if [ "$(readlink "$HOME/.claude/$_imp_d" 2>/dev/null || true)" != "$REPO/$_imp_d" ]; then + IMP_READY=false + fi +done + +if [ "$IMP_READY" != true ]; then + warn "impeccable: ~/.claude/skills and ~/.claude/agents are not this repo's symlinks yet" + warn " → run 'make link' first, then re-run 'make plugin'" +elif [ -z "${NODE_MAJOR:-}" ] || [ "$NODE_MAJOR" -lt 24 ]; then + if [ -f "$IMP_SKILL_DIR/SKILL.md" ] || [ -f "$IMP_PARKED/SKILL.md" ]; then ok "impeccable already present (update skipped — needs Node >= 24, found ${NODE_MAJOR:-none})" else warn "impeccable: needs Node >= 24 (found ${NODE_MAJOR:-none}) — skipped. Bump Node, then: make plugin" fi else - IMP_PKG="impeccable" + # A profile may hold impeccable parked in skills-disabled/. Install writes + # to the live slot, so remember the state and put the fresh copy back where + # it was — otherwise `make plugin` silently re-enables a disabled skill. + IMP_WAS_PARKED=false + [ -d "$IMP_PARKED" ] && IMP_WAS_PARKED=true + IMP_USED="" if [ "$IMP_VER" != "latest" ]; then - IMP_PKG="impeccable@${IMP_VER}" - info "Installing impeccable ${IMP_VER} (pinned in plugins.lock.json, staged)..." + info "Installing impeccable ${IMP_VER} (pinned in plugins.lock.json, global scope)..." + if imp_install "$IMP_VER"; then + IMP_USED="$IMP_VER" + else + warn "impeccable@${IMP_VER} did not install (${IMP_FAIL}) — that release's skill dist is gone upstream" + info "Falling back to impeccable@latest..." + if imp_install latest; then + IMP_USED="latest" + warn "installed @latest instead of the pin. Bump \"impeccable\".version in plugins.lock.json to the version this produced, so the next run is reproducible again." + fi + fi else info "Installing impeccable latest (consider pinning in plugins.lock.json)..." + imp_install latest && IMP_USED="latest" fi - IMP_STAGE=$(mktemp -d) - if (cd "$IMP_STAGE" && npx -y "$IMP_PKG" skills install -y --providers=claude --scope=project --no-hooks >/dev/null 2>&1); then - IMP_SRC=$(find "$IMP_STAGE" -type d -name impeccable -path "*skills*" 2>/dev/null | head -1) - if [ -n "$IMP_SRC" ] && [ -f "$IMP_SRC/SKILL.md" ]; then - rm -rf "$IMP_DIR" - mv "$IMP_SRC" "$IMP_DIR" - ok "impeccable synced to skills-external/ (CLI ${IMP_VER})" - else - warn "impeccable: installer ran but produced no skills/impeccable/SKILL.md — layout changed? Inspect: npx impeccable skills install" + + if [ -n "$IMP_USED" ] && [ -f "$IMP_SKILL_DIR/SKILL.md" ]; then + IMP_SKILL_VER=$(sed -n 's/^version:[[:space:]]*//p' "$IMP_SKILL_DIR/SKILL.md" | head -1) + # -L: ~/.claude/agents is a symlink, and find would otherwise stop on it. + IMP_AGENTS=$(find -L "$HOME/.claude/agents" -maxdepth 1 -name 'impeccable-*.md' 2>/dev/null | wc -l) + ok "impeccable installed (CLI ${IMP_USED}, skill ${IMP_SKILL_VER:-?}, ${IMP_AGENTS} agents)" + if [ "$IMP_AGENTS" -eq 0 ]; then + warn "no impeccable-* agent landed in agents/ — the skill's finish/document verbs dispatch to them" fi + if [ "$IMP_WAS_PARKED" = true ]; then + rm -rf "${IMP_PARKED:?}" + mv "$IMP_SKILL_DIR" "$IMP_PARKED" + info "impeccable was parked by a profile — refreshed copy returned to skills-disabled/" + fi + info "Per-project step, in the agent chat of each frontend project: /impeccable init" + info " (writes PRODUCT.md — the design context every impeccable verb reads)" + elif [ -f "$IMP_SKILL_DIR/SKILL.md" ] || [ -f "$IMP_PARKED/SKILL.md" ]; then + ok "impeccable already present (install failed: ${IMP_FAIL:-no SKILL.md written} — existing copy kept)" else - if [ -f "$IMP_DIR/SKILL.md" ]; then - ok "impeccable already present (installer failed — existing dist kept)" - else - warn "impeccable install failed — run manually: npx impeccable skills install -y --providers=claude --scope=project --no-hooks" - fi + warn "impeccable install failed (${IMP_FAIL:-no SKILL.md written}) — run manually: npx impeccable skills install -y --providers=claude --scope=global --no-hooks" fi - rm -rf "$IMP_STAGE" -fi -if [ -L "$HOME/.claude/skills/impeccable" ]; then - ok "impeccable symlink OK" -else - info "Symlinking — will be created by link.sh" fi echo "" diff --git a/lib/design-gate.md b/lib/design-gate.md index 34bec19..01b1ff6 100644 --- a/lib/design-gate.md +++ b/lib/design-gate.md @@ -139,6 +139,33 @@ count: toolchain check handles the skill; this step handles the lib. Don't conflate them when talking to the user. +### 5. Impeccable design context — suggest-only (one check, one line) + +Same class as §4: a PROJECT-side prerequisite, not a tool. `impeccable` +installs globally, but every one of its verbs reads a per-project `PRODUCT.md` +that only `/impeccable init` writes. Without it the skill runs on invented +context, which is worse than not running it — and nothing else in the process +says so, because init has to happen in the agent chat, not in an installer. + +**Fires when BOTH hold** — else stay silent: + +1. impeccable is active (`skills/impeccable` present, i.e. it did not trip §3). +2. The project has no `PRODUCT.md` at its root. + +Evaluate it on the same path as §4: after the toolchain resolves, never on the +INCOMPLETE stop path. One line, non-blocking: + + 🧭 impeccable has no project context here (no PRODUCT.md) — run `/impeccable init` first? (optional) + +**Rules:** + +- Non-blocking, and never run `init` unprompted: it interviews the user about + the product, so it needs their attention, not their absence. +- One line per session at most. A refusal is an answer; do not re-ask inside + the same task. +- Skip entirely for a review/audit of a single component and for any non-UI + work. This is for Build and design-system tiers. + ### Other toolchains The script defaults to the `design` profile. A task needing another profile's diff --git a/link.sh b/link.sh index 023f1e2..ec942ff 100644 --- a/link.sh +++ b/link.sh @@ -71,7 +71,10 @@ if [ -d "$GSTACK_SRC/browse/dist" ]; then fi fi -EXTERNAL_SKILLS=(emil-design-eng frontend-design design-motion-principles impeccable) +# impeccable is NOT here: its installer writes the skill straight into +# skills/ (and its agents into agents/) at --scope=global, so there is no +# skills-external/ copy to symlink. See install-plugins.sh Step 8d. +EXTERNAL_SKILLS=(emil-design-eng frontend-design design-motion-principles) for _ext_skill in "${EXTERNAL_SKILLS[@]}"; do if [ -d "$REPO/skills-external/$_ext_skill" ]; then if [ -L "$CLAUDE/skills/$_ext_skill" ] && [ "$(readlink "$CLAUDE/skills/$_ext_skill")" = "$REPO/skills-external/$_ext_skill" ]; then diff --git a/plugins.lock.json b/plugins.lock.json index c7a5a8f..636f7c5 100644 --- a/plugins.lock.json +++ b/plugins.lock.json @@ -45,7 +45,7 @@ }, "impeccable": { "source": "npm:impeccable", - "version": "3.2.0", - "note": "Design anti-pattern detector (45 deterministic rules, CLI 'impeccable detect', exit 0/2) + /impeccable skill (23 verbs) by pbakaus. Pin = CLI version; the skill dist has its own release track fetched by 'skills install'. Pinned for audit reproducibility (LRN-077 class: a rules update silently changes audit output). Requires Node >= 24 — install step skips gracefully below that. Machine-owned: synced to skills-external/impeccable/ (gitignored), symlinked by link.sh." + "version": "4.1.0", + "note": "Design anti-pattern detector (45 deterministic rules, CLI 'impeccable detect', exit 0/2) + /impeccable skill (23 verbs) + 4 impeccable-* subagents, by pbakaus. Pin = CLI version ONLY: the skill dist and the engine binary have their own release tracks, fetched by 'skills install' at install time, so this pin does not freeze audit output the way a semgrep pin does. It still gates the CLI deliberately (LRN-077 class). BEWARE: the pin rots — the CLI downloads its skill dist at install time and an older release's artifact disappears upstream (3.2.0 -> 'Download failed: invalid zip data', 2026-09-22, which left 'make plugin' printing a run-it-yourself warning). install-plugins.sh Step 8d and update-all.sh therefore fall back to @latest on a pin failure and warn to bump this version. Requires Node >= 24. Installed at --scope=global: lands in ~/.claude/skills/impeccable + ~/.claude/agents/impeccable-*.md, both symlinks into this repo, both gitignored." } } diff --git a/update-all.sh b/update-all.sh index 99d3a9a..8432110 100644 --- a/update-all.sh +++ b/update-all.sh @@ -379,11 +379,34 @@ else info "design-motion-principles not installed — skipping" fi -# ── Impeccable (design anti-pattern detector + skill) ── +# ── Impeccable (design detector + skill + subagents) ── +# Global scope: the installer writes through the ~/.claude/{skills,agents} +# symlinks straight into this repo (install-plugins.sh Step 8d explains why +# staging + project scope was wrong). The pin can rot upstream, so a pinned +# failure falls back to @latest rather than leaving the tool stale forever. +# +# One install attempt. $1 = "latest" or an exact version. On failure, IMP_FAIL +# holds the reason. Same helper as Step 8d: with a copy already in place a +# rotted pin exits 0 and says "Could not check for skill updates … left +# unchanged", so the exit code cannot tell it from an up-to-date no-op. +imp_install() { + local pkg="impeccable" out rc=0 + [ "$1" != "latest" ] && pkg="impeccable@$1" + out=$(npx -y "$pkg" skills install -y --providers=claude --scope=global \ + --no-hooks 2>&1) || rc=$? + IMP_FAIL=$(printf '%s\n' "$out" \ + | grep -E 'Download failed|Could not check for skill updates' \ + | head -1 || true) + if [ "$rc" -ne 0 ] && [ -z "$IMP_FAIL" ]; then + IMP_FAIL="installer exited $rc" + fi + [ -z "$IMP_FAIL" ] +} echo "" echo "── Updating impeccable..." -IMP_DIR="$REPO/skills-external/impeccable" -if [ ! -f "$IMP_DIR/SKILL.md" ]; then +IMP_SKILL_DIR="$HOME/.claude/skills/impeccable" +IMP_PARKED="$REPO/skills-disabled/impeccable" +if [ ! -f "$IMP_SKILL_DIR/SKILL.md" ] && [ ! -f "$IMP_PARKED/SKILL.md" ]; then info "impeccable not installed — skipping (run: make plugin)" else IMP_VER="" @@ -397,26 +420,34 @@ print(d.get('impeccable',{}).get('version','latest')) fi IMP_NODE=$(node -v 2>/dev/null | sed 's/^v//' | cut -d. -f1) if [ -z "${IMP_NODE:-}" ] || [ "$IMP_NODE" -lt 24 ]; then - info "impeccable update skipped — needs Node >= 24 (found ${IMP_NODE:-none}); existing dist kept" + info "impeccable update skipped — needs Node >= 24 (found ${IMP_NODE:-none}); existing copy kept" else - IMP_PKG="impeccable" - # Pin honored (LRN-077 class: a silent rules update changes audit - # output on unchanged code) — bump the pin deliberately, then update. - [ -n "$IMP_VER" ] && [ "$IMP_VER" != "latest" ] && IMP_PKG="impeccable@${IMP_VER}" - IMP_STAGE=$(mktemp -d) - if (cd "$IMP_STAGE" && npx -y "$IMP_PKG" skills install -y --providers=claude --scope=project --no-hooks >/dev/null 2>&1); then - IMP_SRC=$(find "$IMP_STAGE" -type d -name impeccable -path "*skills*" 2>/dev/null | head -1) - if [ -n "$IMP_SRC" ] && [ -f "$IMP_SRC/SKILL.md" ]; then - rm -rf "$IMP_DIR" - mv "$IMP_SRC" "$IMP_DIR" - ok "impeccable refreshed (CLI ${IMP_VER:-latest})" - else - warn "impeccable: installer produced no dist — existing kept" + IMP_WAS_PARKED=false + [ -d "$IMP_PARKED" ] && IMP_WAS_PARKED=true + # Pin honored (LRN-077 class: a silent rules update changes audit output + # on unchanged code) — bump the pin deliberately, then update. + IMP_PIN="${IMP_VER:-latest}" + IMP_OK=false + if imp_install "$IMP_PIN"; then + IMP_OK=true + elif [ "$IMP_PIN" != "latest" ]; then + warn "impeccable@${IMP_PIN} did not install (${IMP_FAIL}) — that release's skill dist is gone upstream; trying @latest" + if imp_install latest; then + IMP_OK=true + warn "refreshed from @latest, not the pin — bump \"impeccable\".version in plugins.lock.json" + fi + fi + if [ "$IMP_OK" = true ] && [ -f "$IMP_SKILL_DIR/SKILL.md" ]; then + IMP_SKILL_VER=$(sed -n 's/^version:[[:space:]]*//p' "$IMP_SKILL_DIR/SKILL.md" | head -1) + ok "impeccable refreshed (CLI ${IMP_VER:-latest}, skill ${IMP_SKILL_VER:-?})" + if [ "$IMP_WAS_PARKED" = true ]; then + rm -rf "${IMP_PARKED:?}" + mv "$IMP_SKILL_DIR" "$IMP_PARKED" + info "impeccable was parked by a profile — refreshed copy returned to skills-disabled/" fi else - warn "impeccable refresh failed — existing dist kept" + warn "impeccable refresh failed (${IMP_FAIL:-no SKILL.md written}) — existing copy kept" fi - rm -rf "$IMP_STAGE" fi fi