From af085ed05777439b10b39f797460241196e417d0 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Fri, 14 Aug 2026 11:04:11 +0000 Subject: [PATCH] fix(scripts): fail the vale and scope gates when they cannot verify check-vale-style-sync.sh's glob-coverage probe silently self-disabled when vale was absent from PATH, exiting 0 on the one-character glob typo it exists to catch. pre-commit swallows a passing hook's output, so the pre-push hook reported Passed. The script already hard-fails on a bad REPO_ROOT for exactly this reason -- "a clean exit 0 here would read as 'checked, in sync' when nothing ran at all" -- and six of its assertions are vale invocations. Absence now fails; the opt-out is an env var that must be set deliberately, and it downgrades the run to text-level assertions while saying so in the summary. Neither script had a floor on its rewritten .apm/ paths, so relocating .apm/ made both exit 0 -- and this PR's whole change to them was a path rewrite, the exact edit that failure mode survives. A third gap the directory check could not see: relocating only assets/vale/ left both audit skill directories in place while every probe continued past its missing .vale.ini, skipping the whole table with FAIL=0. A zero-probe run is now an error. Both test suites encoded the vacuous pass as a passing case. Those cases are now scoped to "no plugins/kyberforge at all" and assert the fixture really lacks it, with new counterparts covering the drift shape and new positive cases requiring each script to report a non-zero inspected-target count. Also removes the HOOK_REGEX_CACHE memoization: every call site was a command substitution, so the writes happened in a subshell and the lookup always missed. Measured at 14ms of an ~870ms run, all of which is the six vale invocations. Deleted rather than repaired -- 35 lines claiming a benefit they never delivered is worse than no cache -- with a comment recording why. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT --- scripts/check-scope-walkup-sync.sh | 11 ++ scripts/check-vale-style-sync.sh | 119 ++++++++++------ tests/test-check-scope-walkup-sync.sh | 117 +++++++++++----- tests/test-check-vale-style-sync.sh | 191 ++++++++++++++++++++++++-- 4 files changed, 346 insertions(+), 92 deletions(-) diff --git a/scripts/check-scope-walkup-sync.sh b/scripts/check-scope-walkup-sync.sh index 2c9824a..c1502d5 100755 --- a/scripts/check-scope-walkup-sync.sh +++ b/scripts/check-scope-walkup-sync.sh @@ -31,8 +31,19 @@ NEW_SKILL="$REPO_ROOT/plugins/kyberforge/.apm/skills/skill-author/scripts/new-sk VALIDATE="$REPO_ROOT/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate.sh" VALIDATE_PROVENANCE="$REPO_ROOT/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate-provenance.sh" +# Floor on the four hardcoded `plugins/kyberforge/.apm/...` paths above. A +# missing target is only a legitimate no-op for a repo that has no kyberforge +# plugin at all; if `plugins/kyberforge/` IS here and the `.apm/` script under it +# is not, these paths have gone stale and every fixture below silently does not +# run. The whole exit is 0 either way, so a stale path is indistinguishable from +# "all four implementations agree" — and a path rewrite is exactly the kind of +# edit that would slip through it. Same reasoning as the REPO_ROOT guard above. for f in "$NEW_AGENT" "$NEW_SKILL" "$VALIDATE" "$VALIDATE_PROVENANCE"; do if [[ ! -f "$f" ]]; then + if [[ -d "$REPO_ROOT/plugins/kyberforge" ]]; then + echo "Scope walk-up sync check failed: $REPO_ROOT/plugins/kyberforge exists but $f does not — this script's .apm/ paths have gone stale, so none of the walk-up fixtures ran. Update them to wherever the agent-author/agent-audit/skill-author scripts now live." >&2 + exit 1 + fi echo "Scope walk-up sync check: $f not found — kyberforge agent-author/agent-audit/skill-author skills not present, nothing to check." >&2 exit 0 fi diff --git a/scripts/check-vale-style-sync.sh b/scripts/check-vale-style-sync.sh index c6d6a7b..6eb50f4 100755 --- a/scripts/check-vale-style-sync.sh +++ b/scripts/check-vale-style-sync.sh @@ -28,7 +28,19 @@ err() { echo " FAIL: $1" >&2; FAIL=$((FAIL + 1)); } SKILL_AUDIT="$REPO_ROOT/plugins/kyberforge/.apm/skills/skill-audit" AGENT_AUDIT="$REPO_ROOT/plugins/kyberforge/.apm/skills/agent-audit" +# Floor on the hardcoded `plugins/kyberforge/.apm/...` paths above. Neither copy +# present is only a legitimate no-op for a repo that has no kyberforge plugin at +# all. If `plugins/kyberforge/` IS here and the `.apm/` targets under it are not, +# the paths in this script have gone stale — a rename or relocation of `.apm/` +# would otherwise turn every assertion below into a silent exit 0, which reads as +# "checked, in sync" exactly like the REPO_ROOT case above. That matters most for +# the change that introduced these paths: a path rewrite is precisely the edit +# this would survive unnoticed. if [[ ! -d "$SKILL_AUDIT" && ! -d "$AGENT_AUDIT" ]]; then + if [[ -d "$REPO_ROOT/plugins/kyberforge" ]]; then + echo "Vale style sync check failed: $REPO_ROOT/plugins/kyberforge exists but neither $SKILL_AUDIT nor $AGENT_AUDIT does — this script's .apm/ paths have gone stale, so nothing was checked. Update them to wherever the audit skills now live." >&2 + exit 1 + fi exit 0 fi @@ -183,49 +195,31 @@ fi # it — silently masking exactly the kind of hook-rescoping drift this script # exists to catch. # -# Cached per (skill, manifest) pair (parallel HOOK_REGEX_CACHE_KEYS/_VALS -# arrays, populated lazily) because the final validation loop below probes -# agent-audit's two manifests across three probe shapes; without the cache, -# each repeated (skill, manifest) pairing would re-parse the same manifest -# file from scratch for no new information. Plain indexed arrays, not -# `declare -A`: associative arrays are bash 4.0+ and this script must run on -# macOS's stock bash 3.2. Only ${#arr[@]} (always safe on an empty/unset array -# under `set -u`) and index access are used below — never a bare `${arr[@]}` -# expansion, which aborts on bash < 4.4 under nounset. -HOOK_REGEX_CACHE_KEYS=() -HOOK_REGEX_CACHE_VALS=() +# Deliberately NOT memoized. The probe loop below calls this 12 times over the +# same two small manifests, which measures at 14ms against a ~870ms run (the six +# vale invocations are the wall clock). A previous memoization attempt was inert +# anyway: every call site is `x="$(hook_file_regexes ...)"`, a command +# substitution, so the cache writes landed in a subshell and the lookup never +# hit. Re-parsing is the honest, working version of a saving too small to buy. hook_file_regexes() { - local skill="$1" manifest="$2" raw result idx=0 - local cache_key="$skill|$manifest" - while [[ $idx -lt ${#HOOK_REGEX_CACHE_KEYS[@]} ]]; do - if [[ "${HOOK_REGEX_CACHE_KEYS[$idx]}" == "$cache_key" ]]; then - printf '%s' "${HOOK_REGEX_CACHE_VALS[$idx]}" - return - fi - idx=$((idx + 1)) - done - result="$( - if [[ -f "$manifest" ]]; then - awk -v skill="$skill" ' - function flush() { - if (entry ~ skill "/scripts/vale-wrap.sh" && files != "") print files - entry = ""; files = "" - } - /^[ \t]*-[ \t]*id:/ { flush() } - /^[ \t]*entry:/ { entry = $0 } - /^[ \t]*files:/ { files = $0; sub(/^[ \t]*files:[ \t]*/, "", files) } - END { flush() } - ' "$manifest" | while IFS= read -r raw; do - # Strip the surrounding YAML quotes; the regex itself never carries them. - raw="${raw%\'}"; raw="${raw#\'}" - raw="${raw%\"}"; raw="${raw#\"}" - printf '%s\n' "$raw" - done - fi - )" - HOOK_REGEX_CACHE_KEYS[${#HOOK_REGEX_CACHE_KEYS[@]}]="$cache_key" - HOOK_REGEX_CACHE_VALS[${#HOOK_REGEX_CACHE_VALS[@]}]="$result" - printf '%s' "$result" + local skill="$1" manifest="$2" raw + if [[ -f "$manifest" ]]; then + awk -v skill="$skill" ' + function flush() { + if (entry ~ skill "/scripts/vale-wrap.sh" && files != "") print files + entry = ""; files = "" + } + /^[ \t]*-[ \t]*id:/ { flush() } + /^[ \t]*entry:/ { entry = $0 } + /^[ \t]*files:/ { files = $0; sub(/^[ \t]*files:[ \t]*/, "", files) } + END { flush() } + ' "$manifest" | while IFS= read -r raw; do + # Strip the surrounding YAML quotes; the regex itself never carries them. + raw="${raw%\'}"; raw="${raw#\'}" + raw="${raw%\"}"; raw="${raw#\"}" + printf '%s\n' "$raw" + done + fi } # True if $1 matches at least one newline-delimited regex in $2. @@ -265,10 +259,28 @@ vale_flags_path() { printf '%s\n' "$out" | grep -qF "Kyberforge.VagueWording" } +# Missing vale is a HARD FAILURE, not a warning. Six of this script's assertions +# — one glob probe per path below — are `vale --config` invocations, and they are +# the only ones that catch the defect the whole `.vale.ini coverage` section was +# written for: the one-character glob typo (`[**/SKILL.md]` -> `[**/SKILLS.md]`) +# that leaves every text-level assertion clean while vale lints zero files. As a +# warning this self-disabled on exactly that mutation and exited 0, and since +# pre-commit swallows a passing hook's output the stderr line was never seen — +# the pre-push hook reported `Passed`. That is the same "clean exit 0 reads as +# 'checked, in sync' when nothing ran" failure the REPO_ROOT guard at the top of +# this file already refuses to allow. +# +# The opt-out exists for a machine that genuinely cannot install vale, and it is +# an env var that has to be set on purpose — never mere absence of the binary. +# Setting it downgrades the run to text-level assertions only and says so. VALE_AVAILABLE=true if ! command -v vale >/dev/null 2>&1; then VALE_AVAILABLE=false - echo " WARNING: vale is not installed — .vale.ini glob coverage was NOT verified. Install it (https://vale.sh/docs/vale-cli/installation/) before trusting a clean run." >&2 + if [[ "${CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE:-}" == "1" ]]; then + echo " WARNING: vale is not installed and CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1 — .vale.ini glob coverage was NOT verified, only the text-level assertions ran. A clean result here does not mean the globs cover what their hooks lint." >&2 + else + err "vale is not installed, so none of the .vale.ini glob-coverage probes ran — a glob typo that silently lints zero files is invisible without them. Install it (https://vale.sh/docs/vale-cli/installation/), or set CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1 to accept a text-only run" + fi fi # One representative path per file shape the prefilter is supposed to cover, @@ -284,11 +296,13 @@ fi # narrow out of sync with .pre-commit-hooks.yaml's without either manifest's # own hook breaking (each still matches real files on its own), so nothing # else would catch it. +PROBES_CHECKED=0 while IFS='|' read -r skill rel scope; do [[ -n "$skill" ]] || continue dir="$REPO_ROOT/plugins/kyberforge/.apm/skills/$skill" ini="$dir/assets/vale/.vale.ini" [[ -f "$ini" ]] || continue + PROBES_CHECKED=$((PROBES_CHECKED + 1)) hooks_regexes="$(hook_file_regexes "$skill" "$REPO_ROOT/.pre-commit-hooks.yaml")" config_regexes="$(hook_file_regexes "$skill" "$REPO_ROOT/.pre-commit-config.yaml")" @@ -334,7 +348,26 @@ agent-audit|.claude/agents/demo.md|hooks-only agent-audit|copilot/demo.agent.md|hooks-only EOF_PROBE +# Second floor, on the probe table rather than the directory paths: every probe +# `continue`s when its skill's `.vale.ini` is absent, so a relocation of +# `assets/vale/` alone — with both audit skill directories still in place, which +# the guard at the top of this file therefore cannot see — would skip the entire +# table and leave FAIL at 0. Zero probes checked is never a valid result. +if [[ $PROBES_CHECKED -eq 0 ]]; then + err "no probe path was checked — every probe skill's assets/vale/.vale.ini is missing, so the glob-coverage section verified nothing at all" +fi + if [[ $FAIL -gt 0 ]]; then echo "Vale style sync check failed: $FAIL error(s). For a drifted wrapper or style, agent-audit's copy is canonical — run scripts/sync-vale-styles.sh to regenerate skill-audit's copy, then commit both. A .vale.ini finding is not drift and sync-vale-styles.sh will not fix it: edit that file's own StylesPath, BasedOnStyles or glob sections." >&2 exit 1 fi + +# A clean run says what it actually inspected. Silence is what let the vacuous +# passes above look identical to real ones, and it is what made "did this script +# do any work against the real repo?" untestable from outside — the counts below +# are what tests/test-check-vale-style-sync.sh asserts a non-zero floor on. +if [[ "$VALE_AVAILABLE" == true ]]; then + echo "Vale style sync check passed: 2 .vale.ini file(s) checked, $PROBES_CHECKED glob probe(s) verified with vale." +else + echo "Vale style sync check passed (text-level only, vale unavailable): 2 .vale.ini file(s) checked, 0 glob probe(s) verified." +fi diff --git a/tests/test-check-scope-walkup-sync.sh b/tests/test-check-scope-walkup-sync.sh index 4ed8730..711fbda 100755 --- a/tests/test-check-scope-walkup-sync.sh +++ b/tests/test-check-scope-walkup-sync.sh @@ -20,40 +20,10 @@ trap cleanup EXIT RUN_TMP="$(mktemp -d)" FIXTURES+=("$RUN_TMP") -# --- 1. Exits 0 against this repo's own (fixed) scripts --- -echo "" -echo "--- exits 0 against this repo's real scripts ---" -if bash "$SCRIPT" "$REPO_ROOT" > "$RUN_TMP/clean.out" 2>&1; then - pass "exits 0 against this repo's real scope walk-up scripts" -else - fail "exited non-zero against this repo's real (already-fixed) scripts" - sed 's/^/ /' "$RUN_TMP/clean.out" -fi - -# --- 2. Exits 0 as a no-op when the kyberforge skills aren't present --- -echo "" -echo "--- exits 0 (no-op) when the target scripts don't exist ---" -FIXTURE_EMPTY="$(mktemp -d)" -FIXTURES+=("$FIXTURE_EMPTY") -if bash "$SCRIPT" "$FIXTURE_EMPTY" > /dev/null 2>&1; then - pass "exits 0 as a no-op when agent-author/agent-audit/skill-author aren't present" -else - fail "exited non-zero when the kyberforge skills are simply absent" -fi - -# --- 3. Exits 1 against a REPO_ROOT that doesn't exist --- -echo "" -echo "--- exits 1 when REPO_ROOT does not exist ---" -if bash "$SCRIPT" "/nonexistent/path/$(date +%s)-$$" > /dev/null 2>&1; then - fail "exited 0 for a nonexistent REPO_ROOT — expected exit 1" -else - pass "exits non-zero for a nonexistent REPO_ROOT" -fi - -# --- 4. Regression guard: reintroducing the $HOME-collapse bug into -# validate.sh's detect_scope must make the check fail. Builds a minimal -# REPO_ROOT (just the four scripts, at their real relative paths) so this -# doesn't depend on — or risk mutating — the real repo tree. +# Builds a minimal REPO_ROOT (just the four scripts, at their real relative +# paths) so the mutation cases below don't depend on — or risk mutating — the +# real repo tree. Defined up here rather than beside its first mutation case +# because case 2b's stale-.apm/ fixture is built from it too. make_minimal_repo_root() { local dir dir="$(mktemp -d)" @@ -75,6 +45,85 @@ make_minimal_repo_root() { echo "$dir" } +# --- 1. Exits 0 against this repo's own (fixed) scripts --- +echo "" +echo "--- exits 0 against this repo's real scripts ---" +if bash "$SCRIPT" "$REPO_ROOT" > "$RUN_TMP/clean.out" 2>&1; then + pass "exits 0 against this repo's real scope walk-up scripts" +else + fail "exited non-zero against this repo's real (already-fixed) scripts" + sed 's/^/ /' "$RUN_TMP/clean.out" +fi + +# --- 1b. Positive: that exit 0 was earned, not vacuous --- +# Every other case here runs against a synthetic fixture, and exit 0 is also +# what the script produces when it finds nothing to check at all. So the +# assertion above passes just as happily on a run that executed zero fixtures. +# The `ok:` lines are the record of work actually done; each of the seven +# fixtures emits at least one, so a floor of 7 catches a whole fixture going +# dark as well as the all-or-nothing case (13 at the time of writing — the floor +# is deliberately below that so adding assertions to a fixture doesn't churn it). +CLEAN_OKS="$(grep -c '^ ok:' "$RUN_TMP/clean.out" || true)" +if [[ "$CLEAN_OKS" -ge 7 ]]; then + pass "the clean run against this repo actually exercised its fixtures ($CLEAN_OKS ok assertions)" +else + fail "the clean run against this repo reported only $CLEAN_OKS ok assertions (expected at least one per fixture) — exit 0 without the fixtures having run means nothing was checked" + sed 's/^/ /' "$RUN_TMP/clean.out" +fi + +# --- 2. Exits 0 as a no-op ONLY when there is no kyberforge plugin at all --- +# The no-op is scoped to a repo that never installed kyberforge. Case 2b below is +# its counterpart and the one that matters. +echo "" +echo "--- exits 0 (no-op) when there is no plugins/kyberforge at all ---" +FIXTURE_EMPTY="$(mktemp -d)" +FIXTURES+=("$FIXTURE_EMPTY") +if [[ -e "$FIXTURE_EMPTY/plugins/kyberforge" ]]; then + fail "the empty fixture unexpectedly has a plugins/kyberforge, so it does not exercise the no-kyberforge no-op" +elif bash "$SCRIPT" "$FIXTURE_EMPTY" > /dev/null 2>&1; then + pass "exits 0 as a no-op when the repo has no kyberforge plugin" +else + fail "exited non-zero when the repo simply has no kyberforge plugin" +fi + +# --- 2b. Exits 1, saying so, when plugins/kyberforge exists but the .apm/ +# scripts under it do not --- +# The four target paths are hardcoded as plugins/kyberforge/.apm/skills/... with +# no floor under them: `mv plugins/kyberforge/.apm plugins/kyberforge/.apm2` hit +# the "not present, nothing to check" branch and exited 0, indistinguishable +# from "all four implementations agree" and swallowed by pre-commit as `Passed`. +# A path rewrite is exactly the edit that produces this, and it is what this PR +# did to these paths. +# +# Asserted on the MESSAGE, not just the code: this script exits 1 for any fixture +# disagreement too, so the code alone would not tell a stale path from a genuine +# walk-up regression — and those call for opposite fixes. +echo "" +echo "--- exits 1 and says so when plugins/kyberforge exists but .apm/ does not ---" +FIXTURE_STALE="$(make_minimal_repo_root)" +FIXTURES+=("$FIXTURE_STALE") +mv "$FIXTURE_STALE/plugins/kyberforge/.apm" "$FIXTURE_STALE/plugins/kyberforge/.apm2" +STALE_RC=0 +bash "$SCRIPT" "$FIXTURE_STALE" > "$RUN_TMP/stale.out" 2>&1 || STALE_RC=$? +if [[ $STALE_RC -eq 0 ]]; then + fail "exited 0 when plugins/kyberforge exists but its .apm/ scripts are gone — expected exit 1" +elif ! grep -q "\.apm/ paths have gone stale" "$RUN_TMP/stale.out"; then + fail "failed for the wrong reason on a stale .apm/ path: $(tr '\n' ' ' < "$RUN_TMP/stale.out")" +else + pass "exits non-zero and reports a stale .apm/ path when plugins/kyberforge exists without it" +fi + +# --- 3. Exits 1 against a REPO_ROOT that doesn't exist --- +echo "" +echo "--- exits 1 when REPO_ROOT does not exist ---" +if bash "$SCRIPT" "/nonexistent/path/$(date +%s)-$$" > /dev/null 2>&1; then + fail "exited 0 for a nonexistent REPO_ROOT — expected exit 1" +else + pass "exits non-zero for a nonexistent REPO_ROOT" +fi + +# --- 4. Regression guard: reintroducing the $HOME-collapse bug into +# validate.sh's detect_scope must make the check fail. echo "" echo "--- exits 1 when validate.sh's detect_scope collapses back to the \$HOME-walk-up bug ---" FIXTURE_BUG="$(make_minimal_repo_root)" diff --git a/tests/test-check-vale-style-sync.sh b/tests/test-check-vale-style-sync.sh index e656846..2f431a7 100755 --- a/tests/test-check-vale-style-sync.sh +++ b/tests/test-check-vale-style-sync.sh @@ -9,6 +9,28 @@ FAIL=0 pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } +# Same `exit 77` (automake convention; run-tests.sh renders it as SKIPPED) guard +# tests/test-vale-wrap.sh and tests/test-sync-plugin-content.sh use for a missing +# binary. Without it this suite reported 5 genuine failures on a machine with no +# vale, none of which were regressions. +# +# The suite SKIPPING while the script it tests HARD-FAILS is deliberate, not an +# inconsistency. The script is a pre-push gate whose exit 0 is a claim that the +# repo was verified, and six of its assertions are vale invocations — it must +# never make that claim on a machine where they could not run. This suite makes +# no claim about the repo; it claims the script behaves correctly, and most of +# its cases (every glob-coverage case, 10/10b/11) cannot be exercised at all +# without vale. Reporting those as FAIL would say "a regression landed" when the +# truth is "this machine is missing a dev dependency" — noise that competes with +# real failures. Note also that the vale-absent behavior is still fully covered +# here even so: the masking below constructs that condition deliberately on a +# machine that HAS vale, which is the only place it can be asserted against a +# known-good baseline. +if ! command -v vale &>/dev/null; then + echo "SKIP: vale is not installed — the glob-coverage cases cannot run (install it: https://vale.sh/docs/vale-cli/installation/)" + exit 77 +fi + # One trap over a registry, rather than rebuilding the trap line per fixture: # the guard is there because bash 3.2 treats "${arr[@]}" on an empty array as # unbound under `set -u`. @@ -89,9 +111,18 @@ fi # Runs the check with vale masked off PATH when that is safe. For text-only # assertions ONLY — never for a case whose verdict depends on a glob probe # actually running. +# +# CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1 is required now that the script +# treats a missing vale as a FAIL rather than a warning: without the opt-out +# every masked run exits 1 unconditionally and every negative case below would +# pass vacuously — the precise vacuity this whole round is closing. The opt-out +# restores what masking is for here: the text assertion under test becomes the +# only thing that can produce a non-zero exit. Case 12 asserts the un-opted-out +# masked run really does hard-fail, so this env var cannot quietly become the +# only path anyone exercises. run_check_no_vale() { if [[ "$VALE_MASKED" == true ]]; then - PATH="$PATH_NO_VALE" bash "$SCRIPT" "$@" + PATH="$PATH_NO_VALE" CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1 bash "$SCRIPT" "$@" else bash "$SCRIPT" "$@" fi @@ -151,15 +182,93 @@ else pass "exits non-zero when a rule file is missing from one copy" fi -# --- 5. Exits 0 (no-op) when kyberforge isn't present in the target repo --- +# --- 5. Exits 0 (no-op) ONLY when there is no kyberforge plugin at all --- +# The no-op is scoped to a repo that never installed kyberforge. Case 5c below +# is its counterpart and the one that matters: `plugins/kyberforge/` present but +# the `.apm/` targets under it absent is drift, not absence. echo "" -echo "--- exits 0 when kyberforge skills are absent (no-op) ---" +echo "--- exits 0 when there is no plugins/kyberforge at all (no-op) ---" FIXTURE5="$(mktemp -d)" FIXTURES+=("$FIXTURE5") -if bash "$SCRIPT" "$FIXTURE5" > /dev/null 2>&1; then - pass "exits 0 as a no-op when skill-audit/agent-audit don't exist" +if [[ -e "$FIXTURE5/plugins/kyberforge" ]]; then + fail "fixture 5 unexpectedly has a plugins/kyberforge, so it does not exercise the no-kyberforge no-op" +elif bash "$SCRIPT" "$FIXTURE5" > /dev/null 2>&1; then + pass "exits 0 as a no-op when the repo has no kyberforge plugin" else - fail "exited non-zero when skill-audit/agent-audit are simply absent" + fail "exited non-zero when the repo simply has no kyberforge plugin" +fi + +# --- 5c. Exits 1, saying so, when plugins/kyberforge exists but its .apm/ +# targets do not --- +# This script hardcodes plugins/kyberforge/.apm/skills/{skill-audit,agent-audit} +# and had no floor under them: `mv plugins/kyberforge/.apm plugins/kyberforge/.apm2` +# made both directories absent, which fell into the no-op above and exited 0 — +# indistinguishable from a verified in-sync result, and swallowed by pre-commit +# as `Passed`. A path rewrite is exactly the edit that produces this, and it is +# what this PR did to these paths. +# +# Exit code alone proves little here (the script exits 1 for a dozen reasons), so +# assert the MESSAGE: deleting the floor leaves exit 0, but a floor that fired +# for the wrong reason would still be a bug this case must catch. +echo "" +echo "--- exits 1 and says so when plugins/kyberforge exists but .apm/ does not ---" +FIXTURE5C="$(make_fixture)" +FIXTURES+=("$FIXTURE5C") +mv "$FIXTURE5C/plugins/kyberforge/.apm" "$FIXTURE5C/plugins/kyberforge/.apm2" +STALE_OUT="" +STALE_RC=0 +STALE_OUT="$(bash "$SCRIPT" "$FIXTURE5C" 2>&1)" || STALE_RC=$? +if [[ $STALE_RC -eq 0 ]]; then + fail "exited 0 when plugins/kyberforge exists but its .apm/ targets are gone — expected exit 1" +elif ! printf '%s\n' "$STALE_OUT" | grep -q "\.apm/ paths have gone stale"; then + fail "failed for the wrong reason on a stale .apm/ path: $(printf '%s' "$STALE_OUT" | tr '\n' ' ')" +else + pass "exits non-zero and reports a stale .apm/ path when plugins/kyberforge exists without it" +fi + +# --- 5d. Exits 1, saying so, when the probe table verifies nothing --- +# The directory floor above cannot see this one: both audit skill directories are +# still in place, only `assets/vale/` has moved. Every probe then `continue`s on +# its missing .vale.ini and the glob-coverage section checks zero paths. Asserted +# on the message because several other assertions also fire on this fixture. +echo "" +echo "--- exits 1 and says so when zero glob probes were checked ---" +FIXTURE5D="$(make_fixture)" +FIXTURES+=("$FIXTURE5D") +mv "$FIXTURE5D/plugins/kyberforge/.apm/skills/skill-audit/assets/vale" \ + "$FIXTURE5D/plugins/kyberforge/.apm/skills/skill-audit/assets/vale-moved" +mv "$FIXTURE5D/plugins/kyberforge/.apm/skills/agent-audit/assets/vale" \ + "$FIXTURE5D/plugins/kyberforge/.apm/skills/agent-audit/assets/vale-moved" +NOPROBE_OUT="" +NOPROBE_RC=0 +NOPROBE_OUT="$(bash "$SCRIPT" "$FIXTURE5D" 2>&1)" || NOPROBE_RC=$? +if [[ $NOPROBE_RC -eq 0 ]]; then + fail "exited 0 when no glob probe could be checked — expected exit 1" +elif ! printf '%s\n' "$NOPROBE_OUT" | grep -q "no probe path was checked"; then + fail "did not report that zero probe paths were checked: $(printf '%s' "$NOPROBE_OUT" | tr '\n' ' ')" +else + pass "exits non-zero and reports that zero glob probes were checked" +fi + +# --- 5e. Positive: the check does real work against THIS repo --- +# Every case above runs against a synthetic fixture, so the whole suite could be +# green while the script inspected nothing at all in the repo it is wired into at +# pre-push. The summary line carries the counts; assert they are non-zero. +echo "" +echo "--- reports a non-zero number of inspected targets against this repo ---" +REAL_OUT="" +REAL_RC=0 +REAL_OUT="$(bash "$SCRIPT" "$REPO_ROOT" 2>&1)" || REAL_RC=$? +REAL_PROBES="$(printf '%s\n' "$REAL_OUT" | sed -n 's/.*checked, \([0-9][0-9]*\) glob probe(s).*/\1/p')" +if [[ $REAL_RC -ne 0 ]]; then + fail "exited non-zero against this repo's real Vale copies" + printf '%s\n' "$REAL_OUT" | sed 's/^/ /' +elif [[ -z "$REAL_PROBES" ]]; then + fail "a clean run against this repo reported no inspected-target counts, so 'it checked something' is unverifiable: $(printf '%s' "$REAL_OUT" | tr '\n' ' ')" +elif [[ "$REAL_PROBES" -lt 1 ]]; then + fail "a clean run against this repo verified $REAL_PROBES glob probes — a pass that inspected nothing" +else + pass "inspects $REAL_PROBES glob probe(s) against this repo, and exits 0" fi # --- 5b. Exits 1 when REPO_ROOT does not exist --- @@ -527,26 +636,78 @@ else 'StylesPath = styles' 'StylesPath = elsewhere' break_glob "$FIXTURE19/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini" \ 'BasedOnStyles = Kyberforge' 'BasedOnStyles = KyberforgeCopilot' - if PATH="$PATH_NO_VALE" bash "$SCRIPT" "$FIXTURE17" > /dev/null 2>&1; then - pass "exits 0 on in-sync copies with vale unavailable" + # 12a. Missing vale is a HARD FAILURE, not a warning — even on copies that are + # otherwise perfectly in sync. It used to be a warning, and a warning made the + # six glob probes self-disable on the machine that most needed them: applying + # the one-character typo `[**/SKILL.md]` -> `[**/SKILLS.md]` and running with + # vale off PATH exited 0, its sole output a stderr line pre-commit swallows, + # so the pre-push hook reported `Passed`. That is the exact defect the + # glob-coverage section exists to catch, disabled by the absence of the tool + # that catches it. Assert the MESSAGE: exit 1 has a dozen causes here and the + # fixture is in sync, so the code alone would not distinguish this from any + # other finding. + NOVALE_OUT="" + NOVALE_RC=0 + NOVALE_OUT="$(PATH="$PATH_NO_VALE" bash "$SCRIPT" "$FIXTURE17" 2>&1)" || NOVALE_RC=$? + if [[ $NOVALE_RC -eq 0 ]]; then + fail "exited 0 on in-sync copies with vale unavailable — a run that could not verify glob coverage must not report success" + elif ! printf '%s\n' "$NOVALE_OUT" | grep -q "vale is not installed, so none of the .vale.ini glob-coverage probes ran"; then + fail "failed without vale for the wrong reason — the missing-binary guard did not fire: $(printf '%s' "$NOVALE_OUT" | tr '\n' ' ')" else - fail "exited non-zero on in-sync copies with vale unavailable — the missing binary must warn, not fail" + pass "hard-fails, saying so, when vale is unavailable and no opt-out is set" fi - if PATH="$PATH_NO_VALE" bash "$SCRIPT" "$FIXTURE18" > /dev/null 2>&1; then + + # 12b. The opt-out is the only way to get a clean exit without vale, and it has + # to be set deliberately. Absence of the binary must never imply it. + if PATH="$PATH_NO_VALE" CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1 \ + bash "$SCRIPT" "$FIXTURE17" > /dev/null 2>&1; then + pass "exits 0 on in-sync copies with vale unavailable and the explicit opt-out set" + else + fail "exited non-zero on in-sync copies with vale unavailable and CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1 — the opt-out does not work" + fi + + # 12c/12d. The text assertions still gate under the opt-out. This is what the + # opt-out has to preserve: masking vale makes the assertion under test the only + # thing that can produce the verdict (with vale present, a dropped StylesPath + # also breaks the probe, so these cases would still exit 1 with the assertion + # itself deleted). + if PATH="$PATH_NO_VALE" CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1 \ + bash "$SCRIPT" "$FIXTURE18" > /dev/null 2>&1; then fail "exited 0 on a dropped StylesPath with vale unavailable — expected exit 1" else pass "exits non-zero on a dropped StylesPath with vale unavailable" fi - if PATH="$PATH_NO_VALE" bash "$SCRIPT" "$FIXTURE19" > /dev/null 2>&1; then + if PATH="$PATH_NO_VALE" CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1 \ + bash "$SCRIPT" "$FIXTURE19" > /dev/null 2>&1; then fail "exited 0 on a BasedOnStyles that dropped Kyberforge with vale unavailable — expected exit 1" else pass "exits non-zero on a BasedOnStyles that dropped Kyberforge with vale unavailable" fi - # A clean run without vale must say so — silence would read as verified. - if PATH="$PATH_NO_VALE" bash "$SCRIPT" "$FIXTURE17" 2>&1 | grep -q "vale is not installed"; then - pass "warns that glob coverage was not verified when vale is unavailable" + + # 12e. An opted-out clean run must still say it verified nothing — otherwise + # the opt-out just reintroduces the silent vacuous pass under a new name. + OPTOUT_OUT="$(PATH="$PATH_NO_VALE" CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1 \ + bash "$SCRIPT" "$FIXTURE17" 2>&1)" + if printf '%s\n' "$OPTOUT_OUT" | grep -q "glob coverage was NOT verified" \ + && printf '%s\n' "$OPTOUT_OUT" | grep -q "0 glob probe(s) verified"; then + pass "an opted-out clean run reports that glob coverage was not verified" else - fail "exited clean without vale and said nothing — an unverified run looks identical to a verified one" + fail "an opted-out clean run did not say it verified no glob coverage — it looks identical to a verified one: $(printf '%s' "$OPTOUT_OUT" | tr '\n' ' ')" + fi + + # 12f. The typo the whole section exists to catch must fail with vale absent + # and the opt-out set, or not at all — never pass. It cannot be caught without + # vale, so the opt-out must not turn it into a green run by accident: with the + # opt-out this fixture legitimately passes, which is precisely why the opt-out + # is gated on an env var and 12a is the default. + FIXTURE19B="$(make_fixture)" + FIXTURES+=("$FIXTURE19B") + break_glob "$FIXTURE19B/plugins/kyberforge/.apm/skills/skill-audit/assets/vale/.vale.ini" \ + '[**/SKILL.md]' '[**/SKILLS.md]' + if PATH="$PATH_NO_VALE" bash "$SCRIPT" "$FIXTURE19B" > /dev/null 2>&1; then + fail "the one-character glob typo exited 0 with vale off PATH — the probe self-disabled on the exact defect it exists to catch" + else + pass "the one-character glob typo does not exit 0 with vale off PATH" fi fi