docs(lessons): record two patterns from PR #85's round 4

A config's local mode can prove nothing about the mode that ships:
repo: local collapses the clone prefix, cwd and repo root into one
directory, so a byte-identical entry: string worked locally for a
reason that exists only locally, through three review rounds.

Deleting a token from a shared artifact breaks whatever parses it,
silently: dropping --config killed the loop that gave the bundled
Vale styles release coverage, shrinking a derived path list with no
error and no failing test.

Kept separate from the adjacent "clean linter result" and "one signal,
two consumers" entries, which describe different failure modes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58

LESSONS.md

@@ -151,3 +151,11 @@ Vale's output fed two consumers with different contracts: the audit skills read
 ## 2026-08-08 — Measure a rule's false-positive rate at the severity you will ship it at
 
 `Kyberforge.VagueQualifier` was cherry-picked from `write-good` after being trialled as "low-noise against this repo's corpus" — but the trial ran at `level: warning`, where a false positive costs nothing because nobody ever sees it. Shipped at `error`, the same false positive costs a blocked commit and a permanent suppression comment. Re-measured at the severity it actually shipped at, the rule scored one marginal true positive and one unfixable false positive across 41 files (`caveman/SKILL.md` *quotes* filler words as its subject matter — a mention, not a use), and was deleted. Fix: trial conditions must match shipping conditions. A noise measurement taken where false positives are free does not transfer to a context where they are expensive, and "low-noise" is not a property of a rule alone — it is a property of the rule at a severity.
+
+## 2026-08-09 — Exercising a config's "local" mode proves nothing about the mode that ships
+
+The root `.pre-commit-hooks.yaml` shipped Vale hooks whose `entry:` carried a `--config <repo-relative-path>` argument. pre-commit prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]), *cmd[1:])`), so every later argument resolves against the *consuming* repo's root: each external consumer hard-failed with `E100 [--config] Runtime error ... does not exist`, and two of the three hooks ADR-0014 promised were unusable. The defect survived three review rounds of PR #85 and a green `pre-commit run --all-files` every time, because this repo consumes the same hooks through `repo: local`, where the clone prefix, the cwd, and the repo root are one directory — the byte-identical `entry:` string worked locally for a reason that exists only locally. Nothing under `tests/` exercised the manifest as a hook repo at all. The sharp part: the local run was not weaker evidence of the same thing, it was evidence of a different thing, and the two were indistinguishable by reading either file. Fix: when a config has a local mode whose resolution semantics differ from the shipped mode, test the shipped mode against a real consumer — `tests/test-vale-hooks-consumer.sh` stands up a `file://` clone of this repo and runs the hooks from it — and then delete the divergence rather than living with it: `vale-wrap.sh` now self-locates its config from `${BASH_SOURCE[0]}`, and the local and shipped `entry:` lines are identical, so the local run no longer exercises a path no consumer takes.
+
+## 2026-08-09 — Deleting a token from a shared artifact breaks whatever parses it, silently
+
+Dropping the `--config` argument from `.pre-commit-hooks.yaml` was the right fix, but `scripts/check-release-needed.sh` derived its release-relevant path list by scanning those same `entry:` lines for `--config` and taking the target's `dirname` — that parse was the only thing giving the bundled `.vale.ini` and its sibling `styles/` tree release coverage. With the token gone the loop simply never fired: no error, no failing test, no warning, just a path list that shrank from six entries to four and lost both `assets/vale/` trees. Consequence: a change to a Vale *rule* could land on `main` without demanding a release tag, leaving external consumers pinned to an old `rev:` with stale rules — the exact drift the gate exists to prevent. It surfaced only because the agent making the change reported it as a suspected side effect of its own edit, and was confirmed by diffing the derived path list before and after. Fix: before removing a token from an artifact more than one script reads, grep for everything that *parses* the artifact, not just everything that consumes its documented purpose. The smell to watch for is a loop that builds a list, where an empty or short list is indistinguishable from a correct one — assert on the expected members, so a derivation whose input vanished fails loudly instead of quietly covering less.

plugins/kyberforge/.claude-plugin/plugin.json

@@ -8,5 +8,5 @@
   "keywords": [],
   "license": "MIT",
   "name": "kyberforge",
-  "version": "1.2.6"
+  "version": "1.2.7"
 }

plugins/kyberforge/plugin.json

@@ -13,5 +13,5 @@
   "skills": [
     "skills/"
   ],
-  "version": "1.2.6"
+  "version": "1.2.7"
 }

plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh

@@ -34,6 +34,11 @@ set -euo pipefail
 
 cwd="$(pwd -P)"
 
+# Every array below is expanded as `${arr[@]+"${arr[@]}"}`: bash before 4.4 —
+# including the 3.2 that macOS still ships as /bin/bash — treats `"${arr[@]}"`
+# on an empty array as an unbound variable under `set -u`. No expansion site is
+# reachable while empty on today's control flow, so this is insurance against a
+# later edit breaking that invariant, not a live fix.
 vale_args=()
 path_args=()
 config_next=false
@@ -93,7 +98,7 @@ fi
 if [[ ${#path_args[@]} -eq 0 ]]; then
   # Nothing to flatten. Hand off directly, with stdin closed so vale doesn't
   # block waiting on a pipe that will never carry content.
-  exec vale "${vale_args[@]}" < /dev/null
+  exec vale ${vale_args[@]+"${vale_args[@]}"} < /dev/null
 fi
 
 # `realpath -m` would be the obvious normalizer, but `-m` (canonicalize-missing)
@@ -178,7 +183,7 @@ mirror="$tmpdir$cwd"
 mkdir -p "$mirror"
 
 argv_paths=()
-for arg in "${path_args[@]}"; do
+for arg in ${path_args[@]+"${path_args[@]}"}; do
   if [[ "$arg" == /* ]]; then
     dest="$tmpdir$arg"
   else
@@ -220,4 +225,4 @@ for arg in "${path_args[@]}"; do
 done
 
 cd "$mirror"
-vale "${vale_args[@]}" "${argv_paths[@]}"
+vale ${vale_args[@]+"${vale_args[@]}"} ${argv_paths[@]+"${argv_paths[@]}"}

plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh

@@ -34,6 +34,11 @@ set -euo pipefail
 
 cwd="$(pwd -P)"
 
+# Every array below is expanded as `${arr[@]+"${arr[@]}"}`: bash before 4.4 —
+# including the 3.2 that macOS still ships as /bin/bash — treats `"${arr[@]}"`
+# on an empty array as an unbound variable under `set -u`. No expansion site is
+# reachable while empty on today's control flow, so this is insurance against a
+# later edit breaking that invariant, not a live fix.
 vale_args=()
 path_args=()
 config_next=false
@@ -93,7 +98,7 @@ fi
 if [[ ${#path_args[@]} -eq 0 ]]; then
   # Nothing to flatten. Hand off directly, with stdin closed so vale doesn't
   # block waiting on a pipe that will never carry content.
-  exec vale "${vale_args[@]}" < /dev/null
+  exec vale ${vale_args[@]+"${vale_args[@]}"} < /dev/null
 fi
 
 # `realpath -m` would be the obvious normalizer, but `-m` (canonicalize-missing)
@@ -178,7 +183,7 @@ mirror="$tmpdir$cwd"
 mkdir -p "$mirror"
 
 argv_paths=()
-for arg in "${path_args[@]}"; do
+for arg in ${path_args[@]+"${path_args[@]}"}; do
   if [[ "$arg" == /* ]]; then
     dest="$tmpdir$arg"
   else
@@ -220,4 +225,4 @@ for arg in "${path_args[@]}"; do
 done
 
 cd "$mirror"
-vale "${vale_args[@]}" "${argv_paths[@]}"
+vale ${vale_args[@]+"${vale_args[@]}"} ${argv_paths[@]+"${argv_paths[@]}"}

scripts/check-release-needed.sh

@@ -30,6 +30,16 @@ if [[ ! -f "$HOOKS_MANIFEST" ]]; then
   exit 0
 fi
 
+# Only vX.Y.Z release tags count as a baseline — an incidental checkpoint or
+# experiment tag reachable from HEAD must not shift the diff baseline.
+LAST_TAG="$(git describe --tags --abbrev=0 --match 'v[0-9]*.[0-9]*.[0-9]*' 2>/dev/null || true)"
+
+if [[ -z "$LAST_TAG" ]]; then
+  echo "FAIL: no release tag exists yet, but .pre-commit-hooks.yaml already exposes hooks to external consumers." >&2
+  echo "  Fix: cut the first release tag (e.g. v1.0.0) before this lands on main." >&2
+  exit 1
+fi
+
 # Derive release-relevant paths from .pre-commit-hooks.yaml's own entry: lines
 # instead of hand-maintaining a parallel list — the manifest is the single
 # source of truth for what external consumers actually pull at a pinned rev,
@@ -46,26 +56,64 @@ fi
 # from inventing paths: a bundle root of "." is skipped, because a script in a
 # top-level directory (scripts/skill-size-check.sh) would derive the repo's own
 # shared assets/, which no hook owns and whose churn must not demand a release;
-# and the directory is added only when it exists, since a hook that bundles
+# and the assets/ directory is added only where it is known to exist, since a
-# nothing must not contribute a pathspec matching nothing.
+# hook that bundles nothing must not contribute a pathspec matching nothing.
 RELEASE_PATHS=("$HOOKS_MANIFEST")
-while IFS= read -r entry; do
-  read -ra tokens <<< "$entry"
-  [[ ${#tokens[@]} -eq 0 ]] && continue
-  RELEASE_PATHS+=("${tokens[0]}")
-  bundle_root="$(dirname "$(dirname "${tokens[0]}")")"
-  if [[ "$bundle_root" != "." && -d "$bundle_root/assets" ]]; then
-    RELEASE_PATHS+=("$bundle_root/assets")
-  fi
-done < <(sed -n 's/^[[:space:]]*entry:[[:space:]]*//p' "$HOOKS_MANIFEST")
 
-# Only vX.Y.Z release tags count as a baseline — an incidental checkpoint or
+add_release_path() {
-# experiment tag reachable from HEAD must not shift the diff baseline.
+  local candidate="$1" existing
-LAST_TAG="$(git describe --tags --abbrev=0 --match 'v[0-9]*.[0-9]*.[0-9]*' 2>/dev/null || true)"
+  for existing in "${RELEASE_PATHS[@]}"; do
+    [[ "$existing" == "$candidate" ]] && return 0
+  done
+  RELEASE_PATHS+=("$candidate")
+}
 
-if [[ -z "$LAST_TAG" ]]; then
+# $1 selects where the "does this hook bundle an assets/ tree?" guard looks:
-  echo "FAIL: no release tag exists yet, but .pre-commit-hooks.yaml already exposes hooks to external consumers." >&2
+# "worktree" probes the filesystem, anything else is a rev whose tree is probed
-  echo "  Fix: cut the first release tag (e.g. v1.0.0) before this lands on main." >&2
+# with git plumbing. Reading entry lines from stdin keeps one derivation for
+# both the tagged manifest and the current one.
+collect_release_paths() {
+  local scope="$1" entry bundle_root
+  local -a tokens
+  while IFS= read -r entry; do
+    read -ra tokens <<< "$entry"
+    [[ ${#tokens[@]} -eq 0 ]] && continue
+    add_release_path "${tokens[0]}"
+    bundle_root="$(dirname "$(dirname "${tokens[0]}")")"
+    [[ "$bundle_root" == "." ]] && continue
+    if [[ "$scope" == "worktree" ]]; then
+      [[ -d "$bundle_root/assets" ]] && add_release_path "$bundle_root/assets"
+    else
+      git cat-file -e "$scope:$bundle_root/assets" 2>/dev/null && add_release_path "$bundle_root/assets"
+    fi
+  done
+  return 0
+}
+
+# The worktree alone is not enough: a path is release-relevant if it was part of
+# the contract at $LAST_TAG *or* is part of it at HEAD, so both trees have to be
+# derived and unioned. Deriving only from the worktree meant that deleting a
+# hook's entire assets/ tree made the `-d` guard drop the path from the pathspec
+# altogether, and the deletion — which breaks every consumer at the next rev —
+# diffed clean. The two manifests can genuinely disagree (an entry added,
+# removed, or renamed since the tag), and the union is the conservative side of
+# that disagreement: a path the tag exposed and HEAD no longer does is a removal
+# consumers must be told about, and a path only HEAD exposes is new contract
+# surface they cannot reach without a new tag. The union never over-fires on its
+# own, either — any manifest edit that makes the two disagree already changes
+# $HOOKS_MANIFEST, which is itself a release-relevant path.
+collect_release_paths worktree < <(sed -n 's/^[[:space:]]*entry:[[:space:]]*//p' "$HOOKS_MANIFEST")
+
+# A missing manifest at the tag is legitimate (the manifest was added since) but
+# is indistinguishable from an unreadable tagged tree by its exit status alone,
+# so the tag's root tree is verified separately. An absent tree object — a
+# shallow clone, a truncated fetch — fails closed exactly like a `git diff`
+# failure does, rather than silently degrading to worktree-only derivation.
+if MANIFEST_AT_TAG="$(git cat-file -p "$LAST_TAG:$HOOKS_MANIFEST" 2>/dev/null)"; then
+  collect_release_paths "$LAST_TAG" < <(printf '%s\n' "$MANIFEST_AT_TAG" | sed -n 's/^[[:space:]]*entry:[[:space:]]*//p')
+elif ! git cat-file -e "$LAST_TAG^{tree}" 2>/dev/null; then
+  echo "FAIL: could not read the tree at $LAST_TAG to determine which paths that release exposed." >&2
+  echo "  Fix: ensure full tag history is available (e.g. git fetch --unshallow) and retry." >&2
   exit 1
 fi
 

tests/test-check-release-needed.sh

@@ -211,6 +211,63 @@ else
   fail "invented a bogus assets/ path for a hook script with no bundled tree"
 fi
 
+# --- 12. Deleting a hook's entire bundled assets/ tree is release-relevant ---
+# The worktree-only derivation guarded the assets/ path on the directory still
+# existing, so wiping the whole tree removed the path from the pathspec instead
+# of diffing it: the single most consumer-breaking change possible diffed clean.
+# The path list therefore has to be unioned with what $LAST_TAG exposed.
+echo ""
+echo "--- exits 1 when a hook's entire bundled assets/ tree was deleted since the tag ---"
+FIXTURE12="$(make_tagged_fixture)"; track "$FIXTURE12"
+rm -rf "${FIXTURE12:?}/$HOOK_DIR/assets"
+(cd "$FIXTURE12" && git add -A && git commit -q -m "delete the whole bundled assets tree")
+OUT12=$(run_check "$FIXTURE12" "refs/heads/main" || true)
+if echo "$OUT12" | grep -q "assets/vale/.vale.ini"; then
+  pass "flags a wholesale deletion of a hook's bundled assets/ tree"
+else
+  fail "a hook's entire bundled assets/ tree vanished since the tag without demanding a release"
+fi
+
+# --- 13. A hook script deleted while its manifest entry survives is flagged ---
+# Characterisation test, not a bug fix: tokens[0] is added to the pathspec
+# unconditionally (no existence guard), so this case was already covered. It is
+# pinned here so the tagged-tree union can't accidentally introduce an existence
+# guard on tokens[0] and reopen the hole its assets/ sibling had.
+echo ""
+echo "--- exits 1 when a hook script was deleted but its manifest entry remains ---"
+FIXTURE13="$(make_tagged_fixture)"; track "$FIXTURE13"
+rm -f "$FIXTURE13/$HOOK_DIR/scripts/vale-wrap.sh"
+(cd "$FIXTURE13" && git add -A && git commit -q -m "delete a hook script, keep its manifest entry")
+OUT13=$(run_check "$FIXTURE13" "refs/heads/main" || true)
+if echo "$OUT13" | grep -q "vale-wrap.sh"; then
+  pass "flags a hook script deleted out from under a surviving manifest entry"
+else
+  fail "a manifest entry's script vanished since the tag without demanding a release"
+fi
+
+# --- 14. Retiring a whole hook names what the tag exposed, not just the manifest ---
+# Removing the entry and everything it shipped changes $HOOKS_MANIFEST, so the
+# gate fires either way — but a derivation that only reads the current manifest
+# can no longer name the retired script or its assets, and the failure message
+# understates the breakage to consumers pinned at the old rev. The tagged
+# manifest is what makes those paths reportable.
+echo ""
+echo "--- names the retired hook's own paths when an entry and its files are removed together ---"
+FIXTURE14="$(make_tagged_fixture)"; track "$FIXTURE14"
+cat > "$FIXTURE14/.pre-commit-hooks.yaml" <<'EOF'
+-   id: fake-size-check
+    entry: scripts/skill-size-check.sh
+    language: script
+EOF
+rm -rf "${FIXTURE14:?}/$HOOK_DIR"
+(cd "$FIXTURE14" && git add -A && git commit -q -m "retire the vale hook entirely")
+OUT14=$(run_check "$FIXTURE14" "refs/heads/main" || true)
+if echo "$OUT14" | grep -q "vale-wrap.sh" && echo "$OUT14" | grep -q "assets/vale/.vale.ini"; then
+  pass "names the retired hook's script and bundled assets, not just the manifest edit"
+else
+  fail "reported only the manifest change and hid which shipped paths the retirement removed"
+fi
+
 echo ""
 echo "Results: $PASS passed, $FAIL failed"
 [[ $FAIL -eq 0 ]]

tests/test-vale-wrap.sh

@@ -425,6 +425,81 @@ else
   fail "a path with a space was dropped from the directory walk"
 fi
 
+# --- 16. No unguarded `"${arr[@]}"` expansion survives in the wrapper. bash
+# before 4.4 — including the 3.2 that macOS still ships as /bin/bash — treats
+# that form on an empty array as an unbound variable under `set -u` and aborts.
+# The portable form is `${arr[@]+"${arr[@]}"}`. This is a static check because
+# no bash 5 host can reproduce the abort at runtime: the construct is only fatal
+# on the older shell, so absence of the construct is the property to assert.
+# `${#arr[@]}` is deliberately not flagged — the count form is safe on 3.2.
+echo ""
+echo "--- no unguarded array expansion remains in vale-wrap.sh ---"
+unguarded_expansions() {
+  # Blank out whole-line comments (keeping line numbers), delete every correctly
+  # guarded expansion, then anything still matching is a real hazard.
+  awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$1" \
+    | sed -E 's/\$\{([A-Za-z_][A-Za-z0-9_]*)\[@\]\+"\$\{\1\[@\]\}"\}//g' \
+    | grep -nE '\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}' || true
+}
+HAZARDS16="$(unguarded_expansions "$SCRIPT")"
+if [[ -n "$HAZARDS16" ]]; then
+  fail "unguarded array expansion(s) abort on bash < 4.4 under set -u: $(echo "$HAZARDS16" | tr '\n' ' ')"
+else
+  pass "every array expansion uses the bash-3.2-safe \${arr[@]+\"\${arr[@]}\"} form"
+fi
+
+# --- 17. The invocations whose arrays are closest to empty actually run. Under
+# a bash older than 4.4 this is genuine macOS-shell coverage; on a modern bash it
+# degrades to a smoke test, so the pass message names the shell that really ran.
+# Point VALE_WRAP_TEST_BASH at a 3.2 build to get the real thing in CI.
+echo ""
+echo "--- degenerate invocations survive on the oldest available bash ---"
+OLD_BASH="bash"
+OLD_BASH_VER="$(bash -c 'echo "${BASH_VERSINFO[0]}.${BASH_VERSINFO[1]}"')"
+for CAND in "${VALE_WRAP_TEST_BASH:-}" bash-3.2 bash3 /bin/bash /usr/local/bin/bash; do
+  [[ -n "$CAND" ]] && command -v "$CAND" >/dev/null 2>&1 || continue
+  CAND_VER="$("$CAND" -c 'echo "${BASH_VERSINFO[0]}.${BASH_VERSINFO[1]}"' 2>/dev/null)" || continue
+  [[ -n "$CAND_VER" ]] || continue
+  if (( ${CAND_VER%.*} * 100 + ${CAND_VER#*.} < ${OLD_BASH_VER%.*} * 100 + ${OLD_BASH_VER#*.} )); then
+    OLD_BASH="$CAND"
+    OLD_BASH_VER="$CAND_VER"
+  fi
+done
+FIXTURE17="$(make_fixture 2)"
+mkdir -p "$FIXTURE17/emptydir"
+trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12" "$STUB13" "$FIXTURE14" "$FIXTURE17"' EXIT
+# Zero args, flags with no path, and a directory that walks to nothing are the
+# three shapes that leave vale_args/path_args/argv_paths at their emptiest.
+OUT17=""
+for ARGS17 in "" "--config $VALE_CONFIG" "--config $VALE_CONFIG emptydir"; do
+  # shellcheck disable=SC2086 # deliberate word splitting of the argv fixture
+  OUT17+="$( (cd "$FIXTURE17" && "$OLD_BASH" "$SCRIPT" $ARGS17 </dev/null 2>&1) || true)"
+done
+if echo "$OUT17" | grep -q "unbound variable"; then
+  fail "aborted with 'unbound variable' on bash $OLD_BASH_VER — the bug this test guards against"
+else
+  pass "degenerate invocations run clean under bash $OLD_BASH_VER ($OLD_BASH)"
+fi
+
+# --- 18. The guarded expansion must keep argv word boundaries intact. Dropping
+# the quotes (`${arr[@]}`) also silences the unbound-variable abort, so it is the
+# tempting wrong fix — and it splits any path containing a space into two bogus
+# arguments. Case 15 covers spaces found by the directory walk; this covers a
+# space in the path argument itself, which is what argv_paths expands.
+echo ""
+echo "--- a path argument containing a space survives the guarded expansion ---"
+FIXTURE18="$(make_fixture 2)"
+trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12" "$STUB13" "$FIXTURE14" "$FIXTURE17" "$FIXTURE18"' EXIT
+SPACED18="$FIXTURE18/plugins/testplugin/skills/zzz skill dir"
+mkdir -p "$SPACED18"
+mv "$FIXTURE18/plugins/testplugin/skills/zzzskill/SKILL.md" "$SPACED18/SKILL.md"
+OUT18=$(run_wrap "$FIXTURE18" --config "$VALE_CONFIG" "plugins/testplugin/skills/zzz skill dir/SKILL.md")
+if echo "$OUT18" | grep -q "zzz skill dir/SKILL.md" && echo "$OUT18" | grep -q "VagueWording"; then
+  pass "a path argument with a space is passed to vale as one word"
+else
+  fail "a path argument with a space was split by the array expansion: $OUT18"
+fi
+
 echo ""
 echo "Results: $PASS passed, $FAIL failed"
 [[ $FAIL -eq 0 ]]