fix(impeccable): global-scope install with agents, pin fallback, output-read failure check

make plugin never installed impeccable. The 3.2.0 pin had rotted upstream
(the CLI fetches its skill dist at install time; that release's zip is
gone), the --scope=project staging moved the skill dir alone and dropped
the 4 impeccable-* subagents, and /impeccable init was never announced.

Step 8d now installs at --scope=global straight through the
~/.claude/{skills,agents} symlinks into the repo (both paths gitignored),
guards on those symlinks existing, keeps a profile-parked copy parked,
falls back to @latest on a pin failure with a bump-the-lock warning, and
prints the per-project init hint. update-all.sh mirrors the shape.

Found while probing: with a copy already installed a rotted pin exits 0
("Could not check for skill updates ... left unchanged"), byte-identical
on disk to an up-to-date rerun, so imp_install reads the installer output
instead of trusting the exit code. Harness 4/4 in a sandbox HOME with the
real installer.

plugins.lock.json: impeccable 3.2.0 -> 4.1.0 (CLI only). link.sh drops
impeccable from EXTERNAL_SKILLS. lib/design-gate.md section 5: suggest-only
/impeccable init check when a frontend project has no PRODUCT.md.
This commit is contained in:
bastien
2026-09-22 03:20:25 +00:00
parent 7c05f75eab
commit cc2f246e65
7 changed files with 221 additions and 61 deletions
+8 -6
View File
@@ -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
+37
View File
@@ -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
+93 -33
View File
@@ -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 <cwd>/.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 ""
+27
View File
@@ -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
+4 -1
View File
@@ -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
+2 -2
View File
@@ -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."
}
}
+50 -19
View File
@@ -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