From 413a750819891531b4418fa4077b9b9e8d247560 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Fri, 14 Aug 2026 01:52:56 +0000 Subject: [PATCH] fix(scripts): close gates that passed while the thing they guard was disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four repo gates reported success in states they exist to reject. `check-vale-style-sync.sh` passed while a Kyberforge lint rule was silenced. The check matched a blocklist of severity values, but Vale's semantic is an allowlist: anything that is not exactly YES/error/warning/suggestion disables the rule. So `= false`, `= 0`, `= garbage`, an empty value and — worst — a lowercase `= yes` all killed enforcement while reading as "enabled" to a human. Inverted to an allowlist. Two sibling holes: dropping `KyberforgeCopilot` from `BasedOnStyles` unloaded the Copilot-only check silently, and narrowing a section glob to a location made Vale lint zero files, which is the "0 files, hook Passed" failure the script's own comment says it exists to catch. `sync-marketplace-mirror.sh --check` failed open when its source was missing, while its sibling correctly errored in the same state. `check-scope-walkup-sync.sh` wrote to hardcoded `/tmp/fN.out` paths and read one back, making it non-reentrant — a concurrent instance can flip a verdict, and this branch made the test runner concurrent. Now per-run `mktemp -d`. `check-manifests.sh` had no disk-to-marketplace pass, so a plugin directory absent from `marketplace.json` passed every gate while the `validate-plugins` hook globbed it. The "listed" match is restricted to remote-source entry names; matching any entry name let a genuine orphan through on a name coincidence. `run-bats.sh` reported an empty TAP stream as `0 tests, 0 failures`, exit 0 — a total harness failure reading as a pass. The test-side changes are the larger half, because the guards were the real problem. `test-sync-marketplace-mirror.sh` could overwrite the live tracked mirror under an inherited GIT_DIR, which is precisely the git-hook context it runs in. The bash-3.2 scan hand-maintained its file list, omitting the new shared runner, and had no rule for `wait -n` or `nproc` — the two hazards the previous review round found live. It now derives 43 files across three globs with per-glob floors. Several assertions were decoration: the concurrency checks caught the reentrancy defect 0 times in 10, the leak fix was green either way, and two manifest fixtures passed with the code they claimed to cover deleted. Every assertion now has a revert it provably fails against. Refs: #90 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT --- scripts/check-manifests.sh | 71 ++++++ scripts/check-scope-walkup-sync.sh | 54 +++-- scripts/check-vale-style-sync.sh | 84 +++++++ scripts/sync-marketplace-mirror.sh | 14 ++ tests/run-bats.sh | 26 ++ tests/test-check-manifests.sh | 203 +++++++++++++++- tests/test-check-scope-walkup-sync.sh | 100 +++++++- tests/test-check-vale-style-sync.sh | 147 ++++++++++++ tests/test-run-bats.sh | 170 +++++++++++++ tests/test-sync-marketplace-mirror.sh | 92 +++++++- tests/test-vale-wrap.sh | 327 ++++++++++++++++++++++---- 11 files changed, 1198 insertions(+), 90 deletions(-) create mode 100644 tests/test-run-bats.sh diff --git a/scripts/check-manifests.sh b/scripts/check-manifests.sh index 930d23e..93a3252 100755 --- a/scripts/check-manifests.sh +++ b/scripts/check-manifests.sh @@ -19,6 +19,16 @@ set -euo pipefail # those paths resolve. The fallback block below restores that check, but only for # plugins without .apm/ -- apm-native plugins keep relying on the delegation above so # the two checks don't duplicate (and disagree) on the same manifest. +# +# Both of the above walk marketplace.json -> disk. Nothing walked disk -> marketplace, +# so a plugins// directory that never made it into marketplace.json was invisible +# to every marketplace-derived gate at once (this script and sync-plugin-content.sh +# --all both derive their plugin set from marketplace.json). The final block below +# closes that direction: per ADR-0015 marketplace.json is compiled output of root +# apm.yml's marketplace.packages[], so an on-disk apm package with no entry is +# compiled-output drift of exactly the kind ADR-0017 wires pre-push gates for -- and it +# is the same plugin set the validate-plugins pre-commit hook already globs as +# plugins/*/. REPO_ROOT="${1:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}" FAIL=0 @@ -39,6 +49,11 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # shellcheck source=lib/marketplace-plugins.sh source "$SCRIPT_DIR/lib/marketplace-plugins.sh" +# Every local plugin directory marketplace.json claimed, canonicalized, so the +# disk -> marketplace pass below can tell "listed" from "unlisted" regardless of how +# the `source:` string was spelled (./plugins/x, plugins/x, plugins/x/). +SEEN_PLUGIN_DIRS=() + while IFS=$'\t' read -r name plugin_dir; do source_rel="${plugin_dir#"$REPO_ROOT"/}" @@ -46,6 +61,9 @@ while IFS=$'\t' read -r name plugin_dir; do err "plugin '$name': source directory not found: $source_rel" continue fi + # -P so a plugin directory reached through a symlink compares equal to the same + # directory reached directly; the disk-side walk below resolves the same way. + SEEN_PLUGIN_DIRS+=("$(cd "$plugin_dir" && pwd -P)") manifest="$plugin_dir/.claude-plugin/plugin.json" if [[ ! -f "$manifest" ]]; then @@ -80,6 +98,59 @@ while IFS=$'\t' read -r name plugin_dir; do done done < <(list_marketplace_local_plugins "$REPO_ROOT" "$MARKETPLACE") +# Disk -> marketplace. The trigger is any of the three markers that make a directory +# a plugin rather than scratch -- apm.yml (the ADR-0015 authoring source), .apm/ (its +# content tree), or a compiled .claude-plugin/plugin.json. Matching all three keeps +# this set aligned with the plugins/*/ glob the validate-plugins pre-commit hook uses, +# which is the disagreement this check exists to close; a directory with none of them +# is scratch and stays out of scope. +# +# A candidate counts as listed if it is either a directory some local entry pointed at +# (path match, canonicalized above) or a directory whose name matches a REMOTE entry's +# name. The name axis exists only for a plugin vendored on disk but declared with the +# remote-object `source:` shape: list_marketplace_local_plugins deliberately skips those, +# so a path-only match would report a missing entry that is in fact already there. +# +# It is restricted to non-string sources on purpose. Applied to local entries too, the +# name axis silently rescues genuine orphans, because a local entry's name need not equal +# the basename of the directory it points at: an entry named "beta" pointing at +# ./plugins/alpha would mark an unrelated, entirely unlisted plugins/beta/ as listed. +# Local entries already have an exact path to match on, so they need no name fallback. +MARKETPLACE_NAMES=() +while IFS= read -r entry_name; do + [[ -n "$entry_name" ]] && MARKETPLACE_NAMES+=("$entry_name") +done < <(jq -r '.plugins[]? | select((.source | type) != "string") | .name // empty' "$MARKETPLACE") + +for candidate in "$REPO_ROOT"/plugins/*/; do + candidate="${candidate%/}" + [[ -d "$candidate" ]] || continue + if [[ ! -f "$candidate/apm.yml" && ! -d "$candidate/.apm" && ! -f "$candidate/.claude-plugin/plugin.json" ]]; then + continue + fi + + candidate_abs="$(cd "$candidate" && pwd -P)" + candidate_name="$(basename "$candidate")" + listed=0 + for seen in ${SEEN_PLUGIN_DIRS[@]+"${SEEN_PLUGIN_DIRS[@]}"}; do + if [[ "$seen" == "$candidate_abs" ]]; then + listed=1 + break + fi + done + if [[ $listed -eq 0 ]]; then + for entry_name in ${MARKETPLACE_NAMES[@]+"${MARKETPLACE_NAMES[@]}"}; do + if [[ "$entry_name" == "$candidate_name" ]]; then + listed=1 + break + fi + done + fi + + if [[ $listed -eq 0 ]]; then + err "plugin directory '${candidate#"$REPO_ROOT"/}' has no entry in .claude-plugin/marketplace.json — it is skipped by every marketplace-derived check (this one, and sync-plugin-content.sh --all) while still being globbed by the validate-plugins hook. Add it to root apm.yml's marketplace.packages[] and recompile the manifests." + fi +done + if [[ $FAIL -gt 0 ]]; then echo "Manifest check failed: $FAIL error(s)" >&2 exit 1 diff --git a/scripts/check-scope-walkup-sync.sh b/scripts/check-scope-walkup-sync.sh index 4280f84..2c9824a 100755 --- a/scripts/check-scope-walkup-sync.sh +++ b/scripts/check-scope-walkup-sync.sh @@ -46,6 +46,18 @@ FIXTURES=() cleanup() { [[ ${#FIXTURES[@]} -eq 0 ]] || rm -rf "${FIXTURES[@]}"; } trap cleanup EXIT +# Per-run scratch directory for the per-fixture output captures below. These +# used to be fixed paths in the shared system temp directory, which made this +# script non-reentrant: tests/run-tests.sh now fans its scripts out +# concurrently, and two instances sharing one path clobber each other's +# captures. Fixture 6 reads its capture back (`[[ -z "$(cat ...)" ]]`), so a +# cross-run write there silently flips a real verdict, and a pre-existing +# directory sitting at one of the paths breaks the run outright. Keep these +# under a per-run mktemp -d; tests/test-check-scope-walkup-sync.sh asserts it. +# Registered in FIXTURES so the single cleanup trap already here removes it. +RUN_TMP="$(mktemp -d)" +FIXTURES+=("$RUN_TMP") + # Fill a new-agent.sh-scaffolded pair's FILL IN: placeholders with valid # content, isolating the scope/counterpart-lookup question from unrelated # content-quality FAILs when cross-checking against validate.sh. @@ -78,11 +90,11 @@ else else fill_agent_pair "$F1_HOME/.claude/agents/$NAME1.md" "$NAME1" fill_agent_pair "$F1_HOME/.copilot/agents/$NAME1.agent.md" "$NAME1" - if env HOME="$F1_HOME" bash "$VALIDATE" "$F1_HOME/.claude/agents/$NAME1.md" >/tmp/f1.out 2>&1; then + if env HOME="$F1_HOME" bash "$VALIDATE" "$F1_HOME/.claude/agents/$NAME1.md" >"$RUN_TMP/f1.out" 2>&1; then ok "validate.sh agrees: user scope, counterpart found under \$HOME/.copilot" else err "validate.sh disagreed with new-agent.sh's user-scope classification at root exactly \$HOME" - sed 's/^/ /' /tmp/f1.out + sed 's/^/ /' "$RUN_TMP/f1.out" fi fi fi @@ -111,22 +123,22 @@ else ok "new-agent.sh: nested marker-less dir under \$HOME scaffolds project scope at the nested dir" fill_agent_pair "$F2_NESTED/.claude/agents/$NAME2.md" "$NAME2" fill_agent_pair "$F2_NESTED/.github/agents/$NAME2.agent.md" "$NAME2" - if env HOME="$F2_HOME" bash "$VALIDATE" "$F2_NESTED/.claude/agents/$NAME2.md" >/tmp/f2.out 2>&1; then + if env HOME="$F2_HOME" bash "$VALIDATE" "$F2_NESTED/.claude/agents/$NAME2.md" >"$RUN_TMP/f2.out" 2>&1; then ok "validate.sh agrees: project scope, counterpart found at the nested dir (not \$HOME/.copilot)" else err "validate.sh disagreed with new-agent.sh: misclassified the nested marker-less \$HOME subdirectory" - sed 's/^/ /' /tmp/f2.out + sed 's/^/ /' "$RUN_TMP/f2.out" fi # new-skill.sh has no user/project distinction of its own (no $HOME # awareness at all — see new-skill.sh's find_package_root), but it shares # the same .git/apm.yml walk-up primitive. It must land its standalone # scaffold at the given path too, not get redirected toward $HOME. - if env HOME="$F2_HOME" bash "$NEW_SKILL" probe-home-nested-skill "$F2_NESTED" >/tmp/f2skill.out 2>&1 \ + if env HOME="$F2_HOME" bash "$NEW_SKILL" probe-home-nested-skill "$F2_NESTED" >"$RUN_TMP/f2skill.out" 2>&1 \ && [[ -d "$F2_NESTED/probe-home-nested-skill" ]]; then ok "new-skill.sh agrees: standalone mode scaffolds at the nested dir, not redirected toward \$HOME" else err "new-skill.sh disagreed with new-agent.sh/validate.sh on the nested marker-less \$HOME subdirectory" - sed 's/^/ /' /tmp/f2skill.out + sed 's/^/ /' "$RUN_TMP/f2skill.out" fi fi fi @@ -156,11 +168,11 @@ else else fill_agent_pair "$F3_PROBE/.claude/agents/$NAME3.md" "$NAME3" fill_agent_pair "$F3_PROBE/.github/agents/$NAME3.agent.md" "$NAME3" - if env HOME="$F3_HOME" bash "$VALIDATE" "$F3_PROBE/.claude/agents/$NAME3.md" >/tmp/f3.out 2>&1; then + if env HOME="$F3_HOME" bash "$VALIDATE" "$F3_PROBE/.claude/agents/$NAME3.md" >"$RUN_TMP/f3.out" 2>&1; then ok "validate.sh agrees: .git boundary keeps this project scope, not promoted to user scope at \$HOME" else err "validate.sh disagreed with new-agent.sh on the .git-boundary-before-\$HOME fixture" - sed 's/^/ /' /tmp/f3.out + sed 's/^/ /' "$RUN_TMP/f3.out" fi fi fi @@ -188,11 +200,11 @@ else else fill_agent_pair "$F3B_PROBE/.claude/agents/$NAME3B.md" "$NAME3B" fill_agent_pair "$F3B_PROBE/.github/agents/$NAME3B.agent.md" "$NAME3B" - if bash "$VALIDATE" "$F3B_PROBE/.claude/agents/$NAME3B.md" >/tmp/f3b.out 2>&1; then + if bash "$VALIDATE" "$F3B_PROBE/.claude/agents/$NAME3B.md" >"$RUN_TMP/f3b.out" 2>&1; then ok "validate.sh agrees: scope root is , not the .git ancestor above it" else err "validate.sh disagreed with new-agent.sh: resolved scope to the .git ancestor instead of " - sed 's/^/ /' /tmp/f3b.out + sed 's/^/ /' "$RUN_TMP/f3b.out" fi fi fi @@ -214,19 +226,19 @@ elif [[ ! -f "$F4_ROOT/.apm/agents/$NAME4.agent.md" ]]; then err "new-agent.sh did not scaffold plugin scope at the type-bearing apm.yml root" else ok "new-agent.sh: plugin scope at type-bearing apm.yml root" - if bash "$NEW_SKILL" probe-plugin-skill "$F4_ROOT" >/tmp/f4skill.out 2>&1 \ + if bash "$NEW_SKILL" probe-plugin-skill "$F4_ROOT" >"$RUN_TMP/f4skill.out" 2>&1 \ && [[ -d "$F4_ROOT/.apm/skills/probe-plugin-skill" ]]; then ok "new-skill.sh agrees: package mode at the same apm.yml root" else err "new-skill.sh disagreed with new-agent.sh on the type-bearing apm.yml root" - sed 's/^/ /' /tmp/f4skill.out + sed 's/^/ /' "$RUN_TMP/f4skill.out" fi fill_agent_pair "$F4_ROOT/.apm/agents/$NAME4.agent.md" "$NAME4" - if bash "$VALIDATE" "$F4_ROOT/.apm/agents/$NAME4.agent.md" >/tmp/f4validate.out 2>&1; then + if bash "$VALIDATE" "$F4_ROOT/.apm/agents/$NAME4.agent.md" >"$RUN_TMP/f4validate.out" 2>&1; then ok "validate.sh agrees: plugin/APM scope, structural checks pass" else err "validate.sh disagreed with new-agent.sh: did not treat the type-bearing apm.yml root as plugin scope" - sed 's/^/ /' /tmp/f4validate.out + sed 's/^/ /' "$RUN_TMP/f4validate.out" fi # source_keys + a matching sources.md round-trips only if validate-provenance.sh # resolves the SAME plugin root new-agent.sh/new-skill.sh did. @@ -251,11 +263,11 @@ EOF - **Research doc:** (none) - **Status:** \`extracted\` EOF - if bash "$VALIDATE_PROVENANCE" "$F4_ROOT/.apm/agents/$NAME4.agent.md" >/tmp/f4prov.out 2>&1; then + if bash "$VALIDATE_PROVENANCE" "$F4_ROOT/.apm/agents/$NAME4.agent.md" >"$RUN_TMP/f4prov.out" 2>&1; then ok "validate-provenance.sh agrees: resolves the same plugin root, sources.md round-trips" else err "validate-provenance.sh disagreed on the plugin root for the type-bearing apm.yml fixture" - sed 's/^/ /' /tmp/f4prov.out + sed 's/^/ /' "$RUN_TMP/f4prov.out" fi fi @@ -280,11 +292,11 @@ else else fill_agent_pair "$F5_ROOT/.claude/agents/$NAME5.md" "$NAME5" fill_agent_pair "$F5_ROOT/.github/agents/$NAME5.agent.md" "$NAME5" - if env HOME="$F5_UNRELATED_HOME" bash "$VALIDATE" "$F5_ROOT/.claude/agents/$NAME5.md" >/tmp/f5.out 2>&1; then + if env HOME="$F5_UNRELATED_HOME" bash "$VALIDATE" "$F5_ROOT/.claude/agents/$NAME5.md" >"$RUN_TMP/f5.out" 2>&1; then ok "validate.sh agrees: filesystem-boundary fallback resolves to project scope" else err "validate.sh disagreed with new-agent.sh on the filesystem-boundary fallback fixture" - sed 's/^/ /' /tmp/f5.out + sed 's/^/ /' "$RUN_TMP/f5.out" fi fi fi @@ -325,12 +337,12 @@ EOF # No sources.md exists anywhere under $F6_HOME or at the ancestor package # root — if find_plugin_root walked past $HOME to the ancestor apm.yml, # this would FAIL on Check 0 (source_keys declared but sources.md absent). - if env HOME="$F6_HOME" bash "$VALIDATE_PROVENANCE" "$F6_HOME/.apm/agents/probe-prov.agent.md" >/tmp/f6.out 2>&1 \ - && [[ -z "$(cat /tmp/f6.out)" ]]; then + if env HOME="$F6_HOME" bash "$VALIDATE_PROVENANCE" "$F6_HOME/.apm/agents/probe-prov.agent.md" >"$RUN_TMP/f6.out" 2>&1 \ + && [[ -z "$(cat "$RUN_TMP/f6.out")" ]]; then ok "validate-provenance.sh agrees: \$HOME boundary stops the walk, exits 0 silently (not plugin scope)" else err "validate-provenance.sh walked past \$HOME to the ancestor apm.yml — disagrees with new-agent.sh" - sed 's/^/ /' /tmp/f6.out + sed 's/^/ /' "$RUN_TMP/f6.out" fi fi diff --git a/scripts/check-vale-style-sync.sh b/scripts/check-vale-style-sync.sh index defd5f3..b1f876b 100755 --- a/scripts/check-vale-style-sync.sh +++ b/scripts/check-vale-style-sync.sh @@ -71,6 +71,13 @@ for ini in "$SKILL_INI" "$AGENT_INI"; do err "$rel_ini is missing — without it vale falls back to an upward config search and lints with whatever it finds" continue fi + # Present but unreadable is its own case: every assertion below is a grep, and + # grep exits 2 on a read error. The override capture swallows that into an + # empty result, which would read as "no findings" rather than "not checked". + if [[ ! -r "$ini" ]]; then + err "$rel_ini is not readable — none of its assertions could run, and an unreadable file cannot be distinguished from a clean one downstream" + continue + fi # StylesPath is resolved relative to the .vale.ini, which is the only reason # the bundled styles are found from a consuming repo's clone prefix. if ! grep -Eq '^[[:space:]]*StylesPath[[:space:]]*=[[:space:]]*styles[[:space:]]*$' "$ini"; then @@ -81,8 +88,71 @@ for ini in "$SKILL_INI" "$AGENT_INI"; do if ! grep -Eq '^[[:space:]]*BasedOnStyles[[:space:]]*=.*Kyberforge([[:space:],]|$)' "$ini"; then err "$rel_ini has no section whose BasedOnStyles names Kyberforge — every rule the audit prefilters on lives in that style" fi + # Per-rule overrides are the third way to retire a rule without touching a + # style file or a glob. CONTEXT.md's "Vale audit prefilter" entry: "Every rule + # is `level: error` and every alert is a FAIL — no ignorable tier". Vale's exit + # code keys on `error` alerts alone, so any override that leaves a rule at + # anything other than `error` still lints the file, still exits 0, and still + # shows `Passed` in pre-commit. The glob probe below cannot backstop this: it + # keys on one `Kyberforge.VagueWording` alert, so DescriptionOpener, + # PaddingPhrase, SentenceOpenerThereIs and ProactivePhrase can each be retired + # underneath a passing probe. + # + # Asserted as an ALLOWLIST, not a blocklist of `NO|warning|suggestion`, because + # that is vale 3.15.2's own semantic: only the exact tokens `YES` and `error` + # keep a rule blocking. `warning`/`suggestion` downgrade it (alert still + # printed, exit 0 — invisible, since pre-commit swallows a passing hook's + # output); every other value — `NO`, `false`, `0`, `off`, `n`, empty, + # `garbage`, and lowercase `yes`, `true`, `1`, `on` — silences the rule + # outright. Lowercase `yes` is the trap a blocklist cannot cover: it reads as + # "enabled" to a human and disables the rule. Verified by enumerating the + # value space against vale 3.15.2. + # + # The allowlist demands a BARE `YES`/`error` with nothing after it, which also + # rejects `error # note` and `error ; note`. Vale itself strips those — a + # whitespace-preceded `#` or `;` comment is removed and the rule stays live — + # so rejecting them is deliberately stricter than vale, not a workaround for + # it. Uniformity is worth more here than the ability to annotate a line that + # should not exist: no shipped `.vale.ini` has any override line at all, and + # the failure mode is a loud false positive rather than a silent pass. The + # genuine hazard is the no-space form — `error# note` and `error; note` are + # NOT stripped and silence the rule outright — and a rule that demands a bare + # token catches those without having to reimplement vale's comment parsing. + # + # `[A-Za-z0-9_-]` on both halves of the name, not `[A-Za-z]`: a rule named + # `Kyberforge.Vague2` is genuinely silenced by `= NO` (verified: 1 error -> + # 0 errors), so an alpha-only class would let a digit-bearing rule name slip + # past the gate. All five current rule names are pure alpha, so this is + # forward cover, not a live hole. + bad_overrides="$( + grep -E '^[[:space:]]*Kyberforge[A-Za-z0-9_-]*\.[A-Za-z0-9_-]+[[:space:]]*=' "$ini" \ + | grep -Ev '^[[:space:]]*Kyberforge[A-Za-z0-9_-]*\.[A-Za-z0-9_-]+[[:space:]]*=[[:space:]]*(YES|error)[[:space:]]*$' \ + || true + )" + # No `grep -q` in that pipeline on purpose: `-q` exits on its first match, and + # under `set -o pipefail` the resulting SIGPIPE on the upstream grep would make + # the whole pipeline report 141 and read as "no findings". + if [[ -n "$bad_overrides" ]]; then + err "$rel_ini overrides a Kyberforge rule to something other than a bare YES or error (first: '${bad_overrides%%$'\n'*}') — every rule in this prefilter is level: error and every alert is a FAIL, and any other value downgrades or silences the rule while vale still exits 0. A trailing comment is rejected too: vale strips a spaced '# ...' but not 'error# ...', so this asks for the bare token rather than guessing which form you meant" + fi done +# KyberforgeCopilot is agent-audit's alone — CONTEXT.md describes it as "scoped +# only to `.agent.md` files for the Copilot-only 'Use proactively has no effect' +# check". The loop above deliberately asserts only `Kyberforge`, since +# skill-audit's copy legitimately has no Copilot style, so dropping +# `, KyberforgeCopilot` from agent-audit's `[**/*.agent.md]` section unloaded the +# whole style silently: no glob broke, the styles/ diff above stayed clean (the +# style directory is still shipped, just never loaded), the two .vale.ini files +# are deliberately unequal so no equality check applies, and the probe below +# still passed because it keys on a Kyberforge alert. Assert the style is loaded +# whenever it is shipped. +if [[ -d "$AGENT_AUDIT/assets/vale/styles/KyberforgeCopilot" && -f "$AGENT_INI" ]]; then + if ! grep -Eq '^[[:space:]]*BasedOnStyles[[:space:]]*=.*KyberforgeCopilot([[:space:],]|$)' "$AGENT_INI"; then + err "${AGENT_INI#"$REPO_ROOT"/} ships a styles/KyberforgeCopilot style but no section's BasedOnStyles names it — the style is never loaded, so its Copilot-only rules lint nothing" + fi +fi + # Prints the `files:` regex of every hook, in ONE manifest ($2), whose entry # is $1's vale-wrap.sh. Records are delimited by their `- id:` line, so the # check does not depend on `entry:` preceding `files:` within a record. @@ -227,10 +297,24 @@ while IFS='|' read -r skill rel scope; do # shape — per ADR-0016 every `.apm/agents/*` file is named `*.agent.md`, so # `.pre-commit-config.yaml`'s regex correctly no longer matches it and that's # not drift. `demo.agent.md` is the real, current shape and is `shared`. +# +# The two `.claude/`-prefixed probes carry the location-independence CONTEXT.md +# asserts: "A `SKILL.md` outside `plugins/` (e.g. project-scope +# `.claude/skills/foo/SKILL.md`) still matches `[**/SKILL.md]` and gets linted +# normally — the globs constrain filename shape, not location." Every other +# probe here starts with `plugins/`, so narrowing a glob to a `plugins/`-shaped +# path (`[**/SKILL.md]` -> `[**/.apm/skills/*/SKILL.md]`) left all of them +# matching while the project-scope shape started linting as `0 errors ... in 0 +# files` — the exact "vale lints zero files, hook shows Passed" failure the +# comment at the top of this section describes. Both are `hooks-only`: only +# `.pre-commit-hooks.yaml` is layout-agnostic, and `.pre-commit-config.yaml` +# pinning this repo's own `plugins/**/.apm/` layout is by design, not drift. done <<'EOF_PROBE' skill-audit|plugins/demo/.apm/skills/demo/SKILL.md|shared +skill-audit|.claude/skills/demo/SKILL.md|hooks-only agent-audit|plugins/demo/.apm/agents/demo.md|hooks-only agent-audit|plugins/demo/.apm/agents/demo.agent.md|shared +agent-audit|.claude/agents/demo.md|hooks-only agent-audit|copilot/demo.agent.md|hooks-only EOF_PROBE diff --git a/scripts/sync-marketplace-mirror.sh b/scripts/sync-marketplace-mirror.sh index 07d60e4..c61d709 100755 --- a/scripts/sync-marketplace-mirror.sh +++ b/scripts/sync-marketplace-mirror.sh @@ -29,6 +29,20 @@ fi [[ $# -eq 0 ]] || usage if [[ ! -f "$SRC" ]]; then + # A missing source with a surviving mirror is drift, not absence: the mirror + # can only be stale (nothing is left for it to be byte-identical to), which is + # precisely the silent divergence this script exists to prevent. Exiting 0 + # here would report "no drift" over a mirror of a file that no longer exists, + # and would also swallow the case where REPO_ROOT resolved to the wrong tree — + # `git rev-parse --show-toplevel` falls back to `pwd` outside a worktree. + # scripts/sync-plugin-content.sh --check --all already errors on the same + # condition ("requires .../marketplace.json"); this matches it. + # Neither file present stays a genuine no-op: nothing to mirror, nothing stale. + if [[ "$CHECK" -eq 1 && -f "$DST" ]]; then + echo "DRIFT $DST: mirror exists but .claude-plugin/marketplace.json does not" >&2 + echo "Fix: restore .claude-plugin/marketplace.json (apm's compiled Claude marketplace output), or delete $DST" >&2 + exit 1 + fi exit 0 fi diff --git a/tests/run-bats.sh b/tests/run-bats.sh index 80daa06..608273d 100755 --- a/tests/run-bats.sh +++ b/tests/run-bats.sh @@ -66,6 +66,7 @@ batch_run "$SCRATCH_ROOT" ${batch_args[@]+"${batch_args[@]}"} FAIL=0 TOTAL_OK=0 TOTAL_NOT_OK=0 +TOTAL_PLANS=0 i=0 for f in ${TEST_FILES[@]+"${TEST_FILES[@]}"}; do i=$((i + 1)) @@ -75,13 +76,38 @@ for f in ${TEST_FILES[@]+"${TEST_FILES[@]}"}; do echo "" file_ok="$(grep -c '^ok ' "$SCRATCH_ROOT/$i.log" || true)" file_not_ok="$(grep -c '^not ok ' "$SCRATCH_ROOT/$i.log" || true)" + # The TAP plan line (`1..N`). Counted separately from the results because an + # empty-but-valid file emits `1..0` and no result lines at all -- that is a + # file bats really did run, so it has to be distinguishable from a file that + # produced nothing whatsoever. + file_plan="$(grep -c '^1\.\.[0-9]' "$SCRATCH_ROOT/$i.log" || true)" status="$(cat "$SCRATCH_ROOT/$i.status" 2>/dev/null || echo 1)" TOTAL_OK=$((TOTAL_OK + file_ok)) TOTAL_NOT_OK=$((TOTAL_NOT_OK + file_not_ok)) + TOTAL_PLANS=$((TOTAL_PLANS + file_plan)) if [[ "$file_not_ok" -gt 0 || "$status" -ne 0 ]]; then FAIL=1 fi done +# Zero counted tests is never a clean run: files were found (the empty-TEST_FILES +# case exits above), so nothing was executed. Without this, a `bats` that emits +# nothing and exits 0 -- a broken binary, a formatter change, or a wholesale +# `@test` removal -- reports "0 tests, 0 failures" and exits green, silently +# turning a total harness failure into a pass. +# +# The two causes get different messages because they are different problems and +# `1..0` is itself valid TAP: no plan lines at all means bats produced no output +# to parse, while plans present with zero results means bats ran fine and the +# files genuinely declare no tests. +if [[ $((TOTAL_OK + TOTAL_NOT_OK)) -eq 0 ]]; then + if [[ "$TOTAL_PLANS" -eq 0 ]]; then + echo "Error: ${#TEST_FILES[@]} .bats file(s) ran but produced no TAP output at all — the bats harness is broken" >&2 + else + echo "Error: ${#TEST_FILES[@]} .bats file(s) declared 0 tests — every @test appears to have been removed" >&2 + fi + FAIL=1 +fi + echo "$((TOTAL_OK + TOTAL_NOT_OK)) tests, $TOTAL_NOT_OK failures" exit "$FAIL" diff --git a/tests/test-check-manifests.sh b/tests/test-check-manifests.sh index 8180ff8..76a0226 100644 --- a/tests/test-check-manifests.sh +++ b/tests/test-check-manifests.sh @@ -9,6 +9,16 @@ FAIL=0 pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } +# One trap over a registry rather than a fresh `trap 'rm -rf "$FIXTUREn"' EXIT` +# per fixture: each such trap REPLACES the previous one, so only the last +# fixture was ever cleaned and the rest leaked into TMPDIR every run. Same +# pattern as tests/test-check-vale-style-sync.sh and +# tests/test-check-scope-walkup-sync.sh; the emptiness guard is there because +# bash 3.2 treats "${arr[@]}" on an empty array as unbound under `set -u`. +FIXTURES=() +cleanup() { [[ ${#FIXTURES[@]} -eq 0 ]] || rm -rf "${FIXTURES[@]}"; } +trap cleanup EXIT + # Helper: make a minimal valid repo fixture with marketplace + plugin structure. # Per ADR-0015/ADR-0017, the manifest check-manifests.sh validates is # .claude-plugin/plugin.json (compiled output) -- not the root-level plugin.json, @@ -40,7 +50,7 @@ JSON echo "" echo "--- exits 0 when all references are valid ---" FIXTURE="$(make_valid_fixture)" -trap 'rm -rf "$FIXTURE"' EXIT +FIXTURES+=("$FIXTURE") if bash "$SCRIPT" "$FIXTURE" > /dev/null 2>&1; then pass "exits 0 when all manifest references resolve" else @@ -51,7 +61,7 @@ fi echo "" echo "--- exits 1 when plugin source directory missing ---" FIXTURE2="$(mktemp -d)" -trap 'rm -rf "$FIXTURE2"' EXIT +FIXTURES+=("$FIXTURE2") mkdir -p "$FIXTURE2/.claude-plugin" cat > "$FIXTURE2/.claude-plugin/marketplace.json" <<'JSON' { @@ -71,7 +81,7 @@ fi echo "" echo "--- exits 1 when .claude-plugin/plugin.json missing from plugin directory ---" FIXTURE3="$(mktemp -d)" -trap 'rm -rf "$FIXTURE3"' EXIT +FIXTURES+=("$FIXTURE3") mkdir -p "$FIXTURE3/.claude-plugin" mkdir -p "$FIXTURE3/plugins/nomanifest" cat > "$FIXTURE3/.claude-plugin/marketplace.json" <<'JSON' @@ -95,7 +105,7 @@ fi echo "" echo "--- exits 1 for a broken local entry even when a remote entry is present ---" FIXTURE4="$(mktemp -d)" -trap 'rm -rf "$FIXTURE4"' EXIT +FIXTURES+=("$FIXTURE4") mkdir -p "$FIXTURE4/.claude-plugin" mkdir -p "$FIXTURE4/plugins/broken" cat > "$FIXTURE4/.claude-plugin/marketplace.json" <<'JSON' @@ -122,7 +132,7 @@ fi echo "" echo "--- catches a broken pointer field in a non-apm plugin's plugin.json ---" FIXTURE5="$(mktemp -d)" -trap 'rm -rf "$FIXTURE5"' EXIT +FIXTURES+=("$FIXTURE5") mkdir -p "$FIXTURE5/.claude-plugin" mkdir -p "$FIXTURE5/plugins/legacy/.claude-plugin" cat > "$FIXTURE5/.claude-plugin/marketplace.json" <<'JSON' @@ -149,7 +159,7 @@ fi echo "" echo "--- a non-apm plugin with valid pointer fields still passes ---" FIXTURE6="$(mktemp -d)" -trap 'rm -rf "$FIXTURE6"' EXIT +FIXTURES+=("$FIXTURE6") mkdir -p "$FIXTURE6/.claude-plugin" mkdir -p "$FIXTURE6/plugins/legacy-ok/.claude-plugin" mkdir -p "$FIXTURE6/plugins/legacy-ok/skills/real-skill" @@ -180,7 +190,7 @@ fi echo "" echo "--- an apm-native plugin's pointer fields are left to sync-plugin-content.sh --check ---" FIXTURE7="$(mktemp -d)" -trap 'rm -rf "$FIXTURE7"' EXIT +FIXTURES+=("$FIXTURE7") mkdir -p "$FIXTURE7/.claude-plugin" mkdir -p "$FIXTURE7/plugins/apm-plugin/.claude-plugin" mkdir -p "$FIXTURE7/plugins/apm-plugin/.apm" @@ -204,6 +214,185 @@ else fail "check-manifests.sh failed on an apm-native plugin -- pointer-field validation should be delegated, not duplicated" fi +# --- 8. Disk -> marketplace: an apm package dir with no marketplace entry is caught --- +# Both this script and sync-plugin-content.sh --all derive their plugin set from +# marketplace.json, so before this check an unlisted plugins// was skipped by +# every marketplace-derived gate at once while still being globbed by the +# validate-plugins pre-commit hook -- two different notions of "the plugin set". +# Per ADR-0015 marketplace.json is compiled from root apm.yml's marketplace.packages[], +# so an on-disk apm package missing from it is compiled-output drift. +echo "" +echo "--- exits 1 for a plugins// apm package with no marketplace entry ---" +FIXTURE8="$(mktemp -d)" +FIXTURES+=("$FIXTURE8") +mkdir -p "$FIXTURE8/.claude-plugin" +mkdir -p "$FIXTURE8/plugins/listed/.claude-plugin" +mkdir -p "$FIXTURE8/plugins/orphan/.claude-plugin" "$FIXTURE8/plugins/orphan/.apm/skills" +cat > "$FIXTURE8/.claude-plugin/marketplace.json" <<'JSON' +{ + "name": "test-marketplace", + "plugins": [ + { "name": "listed", "source": "./plugins/listed" } + ] +} +JSON +echo '{ "name": "listed" }' > "$FIXTURE8/plugins/listed/.claude-plugin/plugin.json" +echo '{ "name": "orphan" }' > "$FIXTURE8/plugins/orphan/.claude-plugin/plugin.json" +printf 'name: orphan\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE8/plugins/orphan/apm.yml" +if bash "$SCRIPT" "$FIXTURE8" > /dev/null 2>&1; then + fail "exited 0 for an on-disk apm package absent from marketplace.json -- expected exit 1" +else + pass "catches a plugins// apm package that produced no marketplace entry" +fi + +# --- 9. A plugins// dir with none of the three plugin markers is not flagged --- +# The trigger is apm.yml || .apm/ || .claude-plugin/plugin.json -- broad enough to match +# the plugins/*/ set the validate-plugins hook globs, which is the disagreement this check +# closes. A directory carrying none of the three is scratch and stays out of scope. +echo "" +echo "--- a plugins// directory with none of the three plugin markers is not flagged ---" +FIXTURE9="$(mktemp -d)" +FIXTURES+=("$FIXTURE9") +mkdir -p "$FIXTURE9/.claude-plugin" +mkdir -p "$FIXTURE9/plugins/listed/.claude-plugin" +mkdir -p "$FIXTURE9/plugins/scratch/notes" +cat > "$FIXTURE9/.claude-plugin/marketplace.json" <<'JSON' +{ + "name": "test-marketplace", + "plugins": [ + { "name": "listed", "source": "./plugins/listed" } + ] +} +JSON +echo '{ "name": "listed" }' > "$FIXTURE9/plugins/listed/.claude-plugin/plugin.json" +if bash "$SCRIPT" "$FIXTURE9" > /dev/null 2>&1; then + pass "a plugins// directory with no plugin markers is left alone" +else + fail "flagged a non-package directory under plugins/ -- expected exit 0" +fi + +# --- 9b. Each of the three markers on its own is enough to trigger the check --- +# Keying only off apm.yml would leave a plugin dir carrying just .apm/ or just a +# compiled .claude-plugin/plugin.json invisible -- exactly the class of gap this +# check exists to close, since validate-plugins would still glob it. +marker_case() { + local label="$1" marker_setup="$2" dir + dir="$(mktemp -d)" + FIXTURES+=("$dir") + mkdir -p "$dir/.claude-plugin" "$dir/plugins/listed/.claude-plugin" "$dir/plugins/orphan" + cat > "$dir/.claude-plugin/marketplace.json" <<'JSON' +{ + "name": "test-marketplace", + "plugins": [ + { "name": "listed", "source": "./plugins/listed" } + ] +} +JSON + echo '{ "name": "listed" }' > "$dir/plugins/listed/.claude-plugin/plugin.json" + case "$marker_setup" in + apm-dir) mkdir -p "$dir/plugins/orphan/.apm/skills" ;; + plugin-json) + mkdir -p "$dir/plugins/orphan/.claude-plugin" + echo '{ "name": "orphan" }' > "$dir/plugins/orphan/.claude-plugin/plugin.json" + ;; + esac + if bash "$SCRIPT" "$dir" > /dev/null 2>&1; then + fail "an unlisted plugin dir carrying only $label was not flagged" + else + pass "an unlisted plugin dir carrying only $label is flagged" + fi +} +echo "" +echo "--- .apm/ alone and .claude-plugin/plugin.json alone each trigger the check ---" +marker_case ".apm/" apm-dir +marker_case ".claude-plugin/plugin.json" plugin-json + +# --- 9c. A vendored plugin declared with a remote-object source: is already listed --- +# list_marketplace_local_plugins deliberately skips remote-object entries, so a +# path-only listed/unlisted match reported a missing entry for a directory whose +# entry is in fact right there -- telling the author to add what already exists. +# The name axis of the match closes that. +echo "" +echo "--- a vendored plugin whose marketplace entry uses a remote source: is not flagged ---" +FIXTURE9C="$(mktemp -d)" +FIXTURES+=("$FIXTURE9C") +mkdir -p "$FIXTURE9C/.claude-plugin" "$FIXTURE9C/plugins/vendored/.claude-plugin" +cat > "$FIXTURE9C/.claude-plugin/marketplace.json" <<'JSON' +{ + "name": "test-marketplace", + "plugins": [ + { "name": "vendored", "source": { "repo": "someorg/somerepo", "source": "github" } } + ] +} +JSON +echo '{ "name": "vendored" }' > "$FIXTURE9C/plugins/vendored/.claude-plugin/plugin.json" +printf 'name: vendored\nversion: 1.2.3\ntype: skill\n' > "$FIXTURE9C/plugins/vendored/apm.yml" +if bash "$SCRIPT" "$FIXTURE9C" > /dev/null 2>&1; then + pass "a vendored dir matching a remote-source entry's name counts as listed" +else + fail "flagged a vendored plugin that already has a remote-source marketplace entry" +fi + +# --- 9d. The name axis must NOT rescue an orphan via a LOCAL entry's name --- +# A local entry's name need not equal the basename of the directory it points at. An +# entry named "beta" pointing at ./plugins/alpha must not mark an unrelated, entirely +# unlisted plugins/beta/ as listed -- local entries match on their exact path, so +# extending the name fallback to them just reopens the gap this check exists to close. +echo "" +echo "--- a local entry's name does not rescue a same-named but unlisted directory ---" +FIXTURE9D="$(mktemp -d)" +FIXTURES+=("$FIXTURE9D") +mkdir -p "$FIXTURE9D/.claude-plugin" "$FIXTURE9D/plugins/alpha/.claude-plugin" "$FIXTURE9D/plugins/beta" +cat > "$FIXTURE9D/.claude-plugin/marketplace.json" <<'JSON' +{ + "name": "test-marketplace", + "plugins": [ + { "name": "beta", "source": "./plugins/alpha" } + ] +} +JSON +echo '{ "name": "alpha" }' > "$FIXTURE9D/plugins/alpha/.claude-plugin/plugin.json" +printf 'name: beta\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE9D/plugins/beta/apm.yml" +if bash "$SCRIPT" "$FIXTURE9D" > /dev/null 2>&1; then + fail "an unlisted plugins/beta/ was rescued by an unrelated local entry named beta -- expected exit 1" +else + pass "an unlisted directory is not rescued by a local entry that merely shares its name" +fi + +# --- 10. Marketplace `source:` spelling variants still count as "listed" --- +# The disk -> marketplace comparison canonicalizes both sides, so `plugins/x` and +# `./plugins/x/` must resolve to the same directory as the glob's `plugins/x/`. +# +# The entry names deliberately DIFFER from the directory basenames. With names equal to +# basenames this fixture proved nothing whenever the name axis was permissive: deleting +# the canonicalization entirely still left it passing, because the name match rescued it. +# Restricting the name axis to remote entries fixed that, but making the names differ is +# what keeps this assertion honest independently of that restriction. +echo "" +echo "--- a marketplace source without ./ or with a trailing slash still counts as listed ---" +FIXTURE10="$(mktemp -d)" +FIXTURES+=("$FIXTURE10") +mkdir -p "$FIXTURE10/.claude-plugin" +mkdir -p "$FIXTURE10/plugins/bare/.claude-plugin" "$FIXTURE10/plugins/trailing/.claude-plugin" +cat > "$FIXTURE10/.claude-plugin/marketplace.json" <<'JSON' +{ + "name": "test-marketplace", + "plugins": [ + { "name": "bare-entry", "source": "plugins/bare" }, + { "name": "trailing-entry", "source": "./plugins/trailing/" } + ] +} +JSON +echo '{ "name": "bare" }' > "$FIXTURE10/plugins/bare/.claude-plugin/plugin.json" +echo '{ "name": "trailing" }' > "$FIXTURE10/plugins/trailing/.claude-plugin/plugin.json" +printf 'name: bare\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE10/plugins/bare/apm.yml" +printf 'name: trailing\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE10/plugins/trailing/apm.yml" +if bash "$SCRIPT" "$FIXTURE10" > /dev/null 2>&1; then + pass "source: spelling variants are canonicalized before the listed/unlisted comparison" +else + fail "flagged a listed plugin because its source: string was spelled differently" +fi + echo "" echo "Results: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]] diff --git a/tests/test-check-scope-walkup-sync.sh b/tests/test-check-scope-walkup-sync.sh index 9db3ca8..4ed8730 100755 --- a/tests/test-check-scope-walkup-sync.sh +++ b/tests/test-check-scope-walkup-sync.sh @@ -13,14 +13,21 @@ FIXTURES=() cleanup() { [[ ${#FIXTURES[@]} -eq 0 ]] || rm -rf "${FIXTURES[@]}"; } trap cleanup EXIT +# Per-run scratch dir for the captured-output files below, for the same reason +# scripts/check-scope-walkup-sync.sh has one: tests/run-tests.sh fans its test +# scripts out concurrently, so a fixed path in the shared system temp directory +# is mutable state shared between two simultaneous runs. In FIXTURES above. +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" > /tmp/check-scope-walkup-sync-clean.out 2>&1; then +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/^/ /' /tmp/check-scope-walkup-sync-clean.out + sed 's/^/ /' "$RUN_TMP/clean.out" fi # --- 2. Exits 0 as a no-op when the kyberforge skills aren't present --- @@ -107,7 +114,7 @@ content = pattern.sub(buggy + "\n", content) with open(path, 'w') as f: f.write(content) PYTHON -if bash "$SCRIPT" "$FIXTURE_BUG" > /tmp/check-scope-walkup-sync-buggy.out 2>&1; then +if bash "$SCRIPT" "$FIXTURE_BUG" > "$RUN_TMP/buggy.out" 2>&1; then fail "exited 0 against a validate.sh reverted to the \$HOME-collapse bug — expected exit 1" else pass "exits non-zero when validate.sh's detect_scope regresses to the \$HOME-collapse bug" @@ -130,12 +137,97 @@ content = pattern.sub("\n", content) with open(path, 'w') as f: f.write(content) PYTHON -if bash "$SCRIPT" "$FIXTURE_BUG2" > /tmp/check-scope-walkup-sync-buggy2.out 2>&1; then +if bash "$SCRIPT" "$FIXTURE_BUG2" > "$RUN_TMP/buggy2.out" 2>&1; then fail "exited 0 against a validate-provenance.sh with no \$HOME boundary check — expected exit 1" else pass "exits non-zero when validate-provenance.sh's find_plugin_root loses its \$HOME boundary check" fi +# --- 6. Reentrancy --- +# The script and this test both used to capture output to fixed paths in the shared +# system temp directory. tests/run-tests.sh runs its scripts concurrently, so two +# instances shared those paths: the script's fixture 6 reads its capture back to +# assert it is empty, so a write from the other instance turned a passing fixture +# into a spurious FAIL, and a stale directory sitting at one of the paths broke the +# run outright ("Is a directory"). +# +# THE SOURCE ASSERTION BELOW IS THE REGRESSION GUARD. The race itself is not usefully +# testable: 8 simultaneous instances of the broken script were measured exiting 0 with +# no FAIL lines, so a concurrent pair reproduces the defect approximately never. Only +# the "does either file name a shared temp path" invariant is deterministic, so that is +# what actually holds the fix in place -- for BOTH files, since this one had the same +# defect at 4 sites and was previously unguarded. +# +# Matching strategy: look for the shared temp directory anywhere on a line, then drop +# whole-line comments. The obvious alternative -- strip comments with `sed 's/#.*//'` +# and then match -- is wrong in this repo, because it truncates any line containing a +# ${var#prefix} expansion and would silently stop seeing a redirect that follows one. +# Comments that mention the shared temp path by name will trip this and have to be +# reworded; that is the safe direction to fail in. +SHARED_TMP_PATTERN='/'"tmp" # spelled by concatenation so this line cannot self-match + +assert_no_shared_tmp() { + local label="$1" file="$2" out="$3" + # Without this, a missing file disarms the guard silently rather than failing: + # grep exits 2, the comment filter sees empty input and exits 1, and pipefail + # reports 2 -- a non-zero status, which is the "clean" branch below. A rename + # would then quietly retire the assertion instead of breaking the build. + if [[ ! -f "$file" ]]; then + fail "$label: cannot check for shared system-temp paths — '$file' does not exist" + return + fi + if grep -n "$SHARED_TMP_PATTERN" "$file" | grep -vE '^[0-9]+:[[:space:]]*#' > "$out"; then + fail "$label names a shared system-temp path — scratch files must live under a per-run mktemp -d" + sed 's/^/ /' "$out" + else + pass "$label names no shared system-temp paths" + fi +} + +echo "" +echo "--- neither the script nor this test hardcodes a shared system-temp path ---" +assert_no_shared_tmp "check-scope-walkup-sync.sh" "$SCRIPT" "$RUN_TMP/hardcoded-script.out" +assert_no_shared_tmp "test-check-scope-walkup-sync.sh" "${BASH_SOURCE[0]}" "$RUN_TMP/hardcoded-test.out" + +# The per-run scratch dir must be registered with the cleanup trap. This is a real, +# deterministic property (it fails if RUN_TMP is created but never added to FIXTURES) +# but note what it is NOT: it cannot detect the original defect, because a script +# writing to the shared temp directory directly never touches TMPDIR, leaving the +# probe dir empty by construction. It guards the cleanup wiring, not reentrancy. +echo "" +echo "--- the script's per-run scratch dir is cleaned up on exit ---" +SCRATCH_PROBE="$(mktemp -d)" +FIXTURES+=("$SCRATCH_PROBE") +TMPDIR="$SCRATCH_PROBE" bash "$SCRIPT" "$REPO_ROOT" > "$RUN_TMP/scratch-probe.out" 2>&1 +LEFTOVER="$(find "$SCRATCH_PROBE" -mindepth 1 -maxdepth 1 | wc -l | tr -d ' ')" +if [[ "$LEFTOVER" == "0" ]]; then + pass "the run left no scratch directory behind (RUN_TMP is registered in FIXTURES)" +else + fail "$LEFTOVER scratch entries survived the run — the per-run scratch dir is not registered with the cleanup trap" +fi + +# Smoke test only, deliberately kept despite not guarding the defect above: it is the +# one assertion that exercises two instances actually running at the same time, so it +# would still catch a coarse regression (e.g. a lockfile or a fixed fixture path that +# makes concurrent runs fail outright). It is NOT evidence the race is fixed. +echo "" +echo "--- smoke: two simultaneous runs both still exit 0 ---" +CONCURRENT_TMP="$(mktemp -d)" +FIXTURES+=("$CONCURRENT_TMP") +( TMPDIR="$CONCURRENT_TMP" bash "$SCRIPT" "$REPO_ROOT" > "$RUN_TMP/conc-a.out" 2>&1 ) & +PID_A=$! +( TMPDIR="$CONCURRENT_TMP" bash "$SCRIPT" "$REPO_ROOT" > "$RUN_TMP/conc-b.out" 2>&1 ) & +PID_B=$! +RC_A=0; wait "$PID_A" || RC_A=$? +RC_B=0; wait "$PID_B" || RC_B=$? +if [[ $RC_A -eq 0 && $RC_B -eq 0 ]]; then + pass "two simultaneous runs both exit 0" +else + fail "a simultaneous pair of runs did not both exit 0 (a=$RC_A b=$RC_B)" + sed 's/^/ A: /' "$RUN_TMP/conc-a.out" + sed 's/^/ B: /' "$RUN_TMP/conc-b.out" +fi + echo "" echo "Results: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]] diff --git a/tests/test-check-vale-style-sync.sh b/tests/test-check-vale-style-sync.sh index 79ba7fa..76b4585 100755 --- a/tests/test-check-vale-style-sync.sh +++ b/tests/test-check-vale-style-sync.sh @@ -209,6 +209,122 @@ else pass "exits non-zero when a .vale.ini no longer names the Kyberforge style" fi +# --- 9b. Exits 1 when a per-rule override leaves a rule at anything but error --- +# The third way to switch a rule off without touching a style file or a glob. +# CONTEXT.md's "Vale audit prefilter" entry: "Every rule is `level: error` and +# every alert is a FAIL -- no ignorable tier". Vale's exit code keys on `error` +# alerts alone, so any such override leaves the glob intact, the styles +# byte-identical, and the run at `0 errors`, exit 0, `Passed`. +# +# Asserted as an ALLOWLIST because that is vale 3.15.2's own semantic, verified +# by enumerating the value space: only the exact tokens `YES` and `error` keep a +# rule blocking. `warning`/`suggestion` downgrade it (alert printed, exit 0 -- +# invisible, since pre-commit swallows a passing hook's output); EVERY other +# value silences it outright, including `false`, `0`, `off`, an empty value, +# `garbage`, and lowercase `yes`. That last one is why a blocklist of +# `NO|warning|suggestion` was not enough: `= yes` reads as "enabled" to a human +# and disables the rule. Case 10's glob probe backstops none of this -- it keys +# on one Kyberforge.VagueWording alert, so DescriptionOpener, PaddingPhrase, +# SentenceOpenerThereIs and ProactivePhrase can each be retired underneath it, +# which is why the cases below deliberately target rules that probe never sees. +# +# Two cases below are about comment forms, and they are NOT symmetric in vale: +# `error # note` (spaced) is stripped by vale and stays live, while `error# note` +# (no space) is not stripped and silences the rule. The gate demands a bare +# token, so it flags both -- deliberately stricter than vale for the spaced form, +# and the only way to catch the no-space form without reimplementing vale's +# comment parsing. `Kyberforge.Vague2` covers rule names carrying a digit: such a +# rule is genuinely silenced by `= NO`, and an alpha-only name class in the gate +# would not even see the line. +echo "" +echo "--- exits 1 when a .vale.ini overrides a Kyberforge rule to anything but YES/error ---" +while IFS= read -r override; do + [[ -n "$override" ]] || continue + # `` stands in for a bare `Rule =` with no value at all, which the + # heredoc cannot carry as a trailing space without a linter eating it. + override="${override//}" + FIXTURE_OV="$(make_fixture)" + FIXTURES+=("$FIXTURE_OV") + echo "$override" >> "$FIXTURE_OV/plugins/kyberforge/.apm/skills/skill-audit/assets/vale/.vale.ini" + if bash "$SCRIPT" "$FIXTURE_OV" > /dev/null 2>&1; then + fail "exited 0 with '$override' in skill-audit's .vale.ini -- expected exit 1" + else + pass "exits non-zero on '$override'" + fi +done <<'EOF_OVERRIDES' +Kyberforge.SentenceOpenerThereIs = NO +Kyberforge.VagueWording = warning +Kyberforge.SentenceOpenerThereIs = suggestion +Kyberforge.SentenceOpenerThereIs = false +Kyberforge.DescriptionOpener = 0 +Kyberforge.PaddingPhrase = off +Kyberforge.SentenceOpenerThereIs = yes +Kyberforge.DescriptionOpener = garbage +Kyberforge.PaddingPhrase = +Kyberforge.SentenceOpenerThereIs = NO # keep quiet +Kyberforge.DescriptionOpener = error# silenced, vale strips no comment without a space +Kyberforge.PaddingPhrase = error; silenced too, same no-space rule for ';' +Kyberforge.DescriptionOpener = error # stripped by vale, still rejected: bare token required +Kyberforge.Vague2 = NO +Kyberforge.Vague_2 = NO +Kyberforge.Vague-2 = NO +EOF_OVERRIDES +# Same in agent-audit's copy: the check runs over both .vale.ini files, and a +# rule retired in only the canonical copy is the likelier direction. `= false` +# on ProactivePhrase is the sharpest shape -- one word off the original defect, +# on a KyberforgeCopilot rule no glob probe covers. +FIXTURE_OV_AGENT="$(make_fixture)" +FIXTURES+=("$FIXTURE_OV_AGENT") +echo "KyberforgeCopilot.ProactivePhrase = false" \ + >> "$FIXTURE_OV_AGENT/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini" +if bash "$SCRIPT" "$FIXTURE_OV_AGENT" > /dev/null 2>&1; then + fail "exited 0 with 'KyberforgeCopilot.ProactivePhrase = false' in agent-audit's .vale.ini -- expected exit 1" +else + pass "exits non-zero when agent-audit's copy retires a KyberforgeCopilot rule" +fi +# The two allowlisted values must NOT trip the assertion -- otherwise it would +# fire on any legitimate explicit enablement. Kept as a positive case so an +# over-broad tightening of the regex shows up here rather than in the repo. +FIXTURE_OV_OK="$(make_fixture)" +FIXTURES+=("$FIXTURE_OV_OK") +{ + echo "Kyberforge.SentenceOpenerThereIs = YES" + echo "Kyberforge.VagueWording = error" +} >> "$FIXTURE_OV_OK/plugins/kyberforge/.apm/skills/skill-audit/assets/vale/.vale.ini" +if bash "$SCRIPT" "$FIXTURE_OV_OK" > /dev/null 2>&1; then + pass "an explicit '= YES' / '= error' override is not flagged" +else + fail "flagged an explicit '= YES' / '= error' override -- those are the two values that keep a rule blocking" + bash "$SCRIPT" "$FIXTURE_OV_OK" 2>&1 | sed 's/^/ /' || true +fi + +# --- 9c. Exits 1 when agent-audit ships KyberforgeCopilot but never loads it --- +# Case 9 asserts only that Kyberforge is named, because skill-audit's copy +# legitimately has no Copilot style. So dropping just `, KyberforgeCopilot` from +# agent-audit's [**/*.agent.md] section unloaded the whole style silently: no +# glob broke, the styles/ diff stayed clean (the directory is still shipped, +# only never loaded), the two .vale.ini files are deliberately unequal so no +# equality check applies, and case 10's probe still passed because it keys on a +# Kyberforge alert. Verified dead by probing a `.agent.md` carrying +# "Use proactively": 0 alerts under the broken config, KyberforgeCopilot. +# ProactivePhrase under the shipped one. CONTEXT.md describes the style as +# "scoped only to `.agent.md` files for the Copilot-only 'Use proactively has +# no effect' check", so shipping it unloaded is drift. +echo "" +echo "--- exits 1 when the shipped KyberforgeCopilot style is named by no BasedOnStyles ---" +FIXTURE11C="$(make_fixture)" +FIXTURES+=("$FIXTURE11C") +break_glob "$FIXTURE11C/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini" \ + 'BasedOnStyles = Kyberforge, KyberforgeCopilot' 'BasedOnStyles = Kyberforge' +if bash "$SCRIPT" "$FIXTURE11C" > /dev/null 2>&1; then + fail "exited 0 when KyberforgeCopilot was dropped from BasedOnStyles -- expected exit 1" +else + pass "exits non-zero when a shipped KyberforgeCopilot style is never loaded" +fi +# The assertion is conditional on the style being shipped: a copy with no +# KyberforgeCopilot directory (skill-audit's, by design) must stay clean -- +# case 13 below covers the shipped-and-loaded pairing. + # --- 10. Exits 1 when a glob section stops matching the shape its hook lints --- # One case per glob section, because each covers a file shape the others don't: # agent-audit's [**/*.agent.md] is the only section covering a Copilot agent file @@ -241,6 +357,37 @@ else pass "exits non-zero when agent-audit's *.agent.md glob matches nothing" fi +# --- 10b. Exits 1 when a glob is narrowed to this repo's own plugins/ layout --- +# Every probe path used to start with `plugins/`, so a glob narrowed from a +# filename shape to a location (`[**/SKILL.md]` -> `[**/.apm/skills/*/SKILL.md]`) +# still matched all of them and the check passed -- while a project-scope +# `.claude/skills/foo/SKILL.md` started linting as `0 errors ... in 0 files`, +# exit 0, hook `Passed`: the exact failure the script's own header comment says +# it exists to catch. CONTEXT.md: "A `SKILL.md` outside `plugins/` (e.g. +# project-scope `.claude/skills/foo/SKILL.md`) still matches `[**/SKILL.md]` and +# gets linted normally -- the globs constrain filename shape, not location." +# These narrowings are still valid glob syntax and break no `plugins/`-shaped +# file, so only a non-`plugins/` probe path catches them. +echo "" +echo "--- exits 1 when a .vale.ini glob is narrowed from a filename shape to a location ---" +FIXTURE14B="$(make_fixture)" +FIXTURE14C="$(make_fixture)" +FIXTURES+=("$FIXTURE14B" "$FIXTURE14C") +break_glob "$FIXTURE14B/plugins/kyberforge/.apm/skills/skill-audit/assets/vale/.vale.ini" \ + '[**/SKILL.md]' '[**/.apm/skills/*/SKILL.md]' +break_glob "$FIXTURE14C/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini" \ + '[**/agents/*.md]' '[**/.apm/agents/*.md]' +if bash "$SCRIPT" "$FIXTURE14B" > /dev/null 2>&1; then + fail "exited 0 when skill-audit's glob stopped covering a SKILL.md outside plugins/ -- expected exit 1" +else + pass "exits non-zero when skill-audit's glob stops covering a project-scope SKILL.md" +fi +if bash "$SCRIPT" "$FIXTURE14C" > /dev/null 2>&1; then + fail "exited 0 when agent-audit's glob stopped covering an agents/*.md outside plugins/ -- expected exit 1" +else + pass "exits non-zero when agent-audit's glob stops covering a project-scope agents/*.md" +fi + # --- 11. Exits 1 when a probe path falls out of every hook's `files:` regex --- # The probe paths are hardcoded, so they can silently stop representing anything # the hooks lint. Rescoping the shipped agent hook away from the `.agent.md` diff --git a/tests/test-run-bats.sh b/tests/test-run-bats.sh new file mode 100644 index 0000000..f18a86b --- /dev/null +++ b/tests/test-run-bats.sh @@ -0,0 +1,170 @@ +#!/usr/bin/env bash +# Regression test for tests/run-bats.sh's TAP-result accounting. +# +# run-bats.sh runs each .bats file in its own process and aggregates the TAP +# streams. It used to derive its test count from that text without ever asserting +# the count was non-zero, so a `bats` that produced no output and exited 0 was +# reported as "0 tests, 0 failures" with exit 0 -- a total harness failure +# rendered as a clean pass. The guard added for that distinguishes two causes, +# because `1..0` is itself valid TAP: no plan lines at all means bats emitted +# nothing to parse, while plans present with zero results means bats ran fine and +# the files genuinely declare no tests. +# +# Both branches were code-only and asserted by nothing, which is the same +# "green either way" hole the guard itself closes. This file covers them. +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +RUN_BATS="$REPO_ROOT/tests/run-bats.sh" +PASS=0 +FAIL=0 + +pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } +fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } + +FIXTURES=() +cleanup() { [[ ${#FIXTURES[@]} -eq 0 ]] || rm -rf "${FIXTURES[@]}"; } +trap cleanup EXIT + +# Builds a throwaway tree that a copy of run-bats.sh will resolve as its own +# REPO_ROOT (it derives that from its own location), so these cases drive the +# real script without the repo's actual .bats files being involved. The fixtures +# live under TMPDIR, never inside the repo, so the real suite cannot pick them up. +# +# This prints the directory and does NOT register it for cleanup: every caller +# invokes it as `$(make_fake_repo)`, and an append made in here would land in the +# command substitution's subshell and be lost. Registration is the caller's job. +make_fake_repo() { + local dir + dir="$(mktemp -d)" + mkdir -p "$dir/tests" "$dir/scripts/lib" + cp "$REPO_ROOT/scripts/lib/batch-run.sh" "$dir/scripts/lib/batch-run.sh" + cp "$RUN_BATS" "$dir/tests/run-bats.sh" + echo "$dir" +} + +# Writes a stub `bats` from stdin. Executable, so run-bats.sh never falls through +# to its submodule-init branch. +install_stub_bats() { + mkdir -p "$1/tests/bats/bin" + cat > "$1/tests/bats/bin/bats" + chmod +x "$1/tests/bats/bin/bats" +} + +# Two .bats files whose tests would fail if anything actually ran them. Their +# content is irrelevant to the stub cases -- what matters is that files exist, so +# run-bats.sh gets past its "no .bats test files found" early exit and the zero +# count it then sees can only have come from the TAP stream. +seed_bats_files() { + printf '@test "a" { false; }\n' > "$1/tests/a.bats" + printf '@test "b" { false; }\n' > "$1/tests/b.bats" +} + +# Runs the fixture's run-bats.sh, capturing output and exit code separately. +FAKE_OUT="" +FAKE_RC=0 +run_fake() { + FAKE_RC=0 + FAKE_OUT="$(bash "$1/tests/run-bats.sh" 2>&1)" || FAKE_RC=$? +} + +# --- 1. A stub emitting nothing at all is a broken harness, not a clean run --- +echo "" +echo "--- a bats that emits no TAP output at all fails the run ---" +DIR1="$(make_fake_repo)" +FIXTURES+=("$DIR1") +seed_bats_files "$DIR1" +install_stub_bats "$DIR1" <<'EOF' +#!/usr/bin/env bash +exit 0 +EOF +run_fake "$DIR1" +if [[ $FAKE_RC -eq 0 ]]; then + fail "a bats emitting nothing exited 0 — a total harness failure reported as a pass" +elif echo "$FAKE_OUT" | grep -q "no TAP output at all"; then + pass "an empty TAP stream fails the run and names the broken harness" +else + fail "the run failed but not with the broken-harness message: $FAKE_OUT" +fi + +# --- 2. A stub emitting only a plan ran fine but declares no tests --- +# This is what real bats produces for a .bats file with every @test removed, so +# it must fail for a different, accurately-worded reason than case 1. +echo "" +echo "--- a bats emitting only a zero plan fails with the no-tests-declared message ---" +DIR2="$(make_fake_repo)" +FIXTURES+=("$DIR2") +seed_bats_files "$DIR2" +install_stub_bats "$DIR2" <<'EOF' +#!/usr/bin/env bash +echo "1..0" +exit 0 +EOF +run_fake "$DIR2" +if [[ $FAKE_RC -eq 0 ]]; then + fail "every test declaring zero tests exited 0 — wholesale @test removal reported as a pass" +elif echo "$FAKE_OUT" | grep -q "declared 0 tests"; then + pass "a plan-only TAP stream fails the run and names the removed tests" +elif echo "$FAKE_OUT" | grep -q "no TAP output at all"; then + fail "a valid '1..0' plan was misreported as a broken harness — the two branches are not distinguished" +else + fail "the run failed but not with the no-tests-declared message: $FAKE_OUT" +fi + +# --- 3. A healthy TAP stream still passes and still counts correctly. The guard +# must not turn into a blanket failure: this is the case that proves the two +# above fail for their stated reason rather than because the guard fails always. +echo "" +echo "--- a healthy TAP stream passes with its full count ---" +DIR3="$(make_fake_repo)" +FIXTURES+=("$DIR3") +seed_bats_files "$DIR3" +install_stub_bats "$DIR3" <<'EOF' +#!/usr/bin/env bash +echo "1..2" +echo "ok 1 first" +echo "ok 2 second" +exit 0 +EOF +run_fake "$DIR3" +if [[ $FAKE_RC -ne 0 ]]; then + fail "a healthy TAP stream was failed by the zero-count guard: $FAKE_OUT" +elif echo "$FAKE_OUT" | grep -q "^4 tests, 0 failures$"; then + pass "two files reporting two passing tests each aggregate to 4 tests, 0 failures" +else + fail "a healthy TAP stream produced the wrong count: $FAKE_OUT" +fi + +# --- 4. The same shapes out of the real bats binary. The stubs above encode an +# assumption about what real bats emits; this pins that assumption. A genuinely +# empty .bats file yields `1..0` and exit 0, so a suite holding one alongside a +# real test file must still pass -- the zero-count guard fires on the aggregate, +# not per file, and one declared test is enough to clear it. +echo "" +echo "--- an empty .bats file beside a real one still passes under the real bats ---" +if [[ ! -x "$REPO_ROOT/tests/bats/bin/bats" ]]; then + echo " SKIP: real bats is not initialized — run tests/run-bats.sh once to fetch the submodule" +else + DIR4="$(make_fake_repo)" + FIXTURES+=("$DIR4") + # Symlinked rather than copied: bats resolves its libexec relative to its own + # path, so the tree has to stay intact. run-bats.sh excludes */tests/bats/* from + # its own file search, so bats's bundled .bats suites are not collected here. + ln -s "$REPO_ROOT/tests/bats" "$DIR4/tests/bats" + : > "$DIR4/tests/empty.bats" + printf '@test "a real passing test" { true; }\n' > "$DIR4/tests/real.bats" + run_fake "$DIR4" + if [[ $FAKE_RC -ne 0 ]]; then + fail "an empty .bats file beside a real one failed the run: $FAKE_OUT" + elif ! echo "$FAKE_OUT" | grep -q "^1\.\.0$"; then + fail "real bats did not emit '1..0' for an empty file, so the case-2 stub no longer matches it: $FAKE_OUT" + elif echo "$FAKE_OUT" | grep -q "^1 tests, 0 failures$"; then + pass "an empty .bats file contributes a '1..0' plan and the suite still passes" + else + fail "the real-bats run passed with an unexpected count: $FAKE_OUT" + fi +fi + +echo "" +echo "Results: $PASS passed, $FAIL failed" +[[ $FAIL -eq 0 ]] diff --git a/tests/test-sync-marketplace-mirror.sh b/tests/test-sync-marketplace-mirror.sh index 79a4973..4b10082 100755 --- a/tests/test-sync-marketplace-mirror.sh +++ b/tests/test-sync-marketplace-mirror.sh @@ -9,15 +9,42 @@ FAIL=0 pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } -# Minimal fixture: a bare directory with no .git of its own. Since +# Fixture: a temp directory that is its own git worktree. Since # scripts/sync-marketplace-mirror.sh resolves REPO_ROOT via -# `git rev-parse --show-toplevel 2>/dev/null || pwd`, and mktemp -d creates -# directories outside any git worktree, cd'ing into the fixture before -# invoking the script makes REPO_ROOT resolve to the fixture itself -- so -# every test runs against an isolated .claude-plugin/ + .github/plugin/ pair -# instead of this repo's real marketplace.json files. +# `git rev-parse --show-toplevel 2>/dev/null || pwd`, isolation has to come +# from that call answering "the fixture" -- so the fixture owns a real .git. +# +# Relying instead on `git rev-parse` FAILING inside a bare `mktemp -d` (its +# previous form) is not isolation: two ordinary conditions make it succeed and +# resolve to the LIVE repo, at which point every test writes to this repo's own +# tracked .github/plugin/marketplace.json. Both are reproduced and fixed here: +# - TMPDIR pointing inside a git worktree, which puts the fixture in one. +# - An inherited GIT_DIR/GIT_WORK_TREE, which re-targets `git -C` and +# `git rev-parse` regardless of cwd. tests/run-tests.sh is itself a pre-push +# hook, and git hooks export exactly those variables -- the same leak +# tests/test-git-hooks-install.sh:6-10 already defends against. +# run_script() strips GIT_DIR/GIT_WORK_TREE for the second; `git init` here +# covers the first and makes the resolution positive rather than accidental. +# run-tests.sh:52-53 asserts no test writes back into the live repo tree, and +# that claim now carries the concurrent runner. make_fixture() { - mktemp -d + local dir + dir="$(mktemp -d)" + # Physical path: `git rev-parse --show-toplevel` reports the resolved path, + # and on macOS `mktemp -d` hands back one under the /tmp -> /private/tmp + # symlink. Without -P the script's REPO_ROOT and the assertions' $FIXTURE + # would name the same directory differently and every diff would compare + # against a path the script never wrote. + dir="$(cd "$dir" && pwd -P)" + # Checked, not best-effort: this init IS the isolation invariant. `echo "$dir"` + # is the last statement, so a silently-failed init would return a plain temp + # dir, `git rev-parse --show-toplevel` would walk up to whatever repo encloses + # it, and the suite would go back to writing into the live tree. + if ! env -u GIT_DIR -u GIT_WORK_TREE git -C "$dir" init -q >/dev/null 2>&1; then + echo "make_fixture: 'git init' failed in $dir — every test would then resolve REPO_ROOT to an enclosing repo and write outside the fixture" >&2 + exit 1 + fi + echo "$dir" } SRC_REL=".claude-plugin/marketplace.json" @@ -37,10 +64,14 @@ write_dst() { printf '%s' "$content" > "$dir/$DST_REL" } +# `env -u GIT_DIR -u GIT_WORK_TREE` mirrors tests/test-git-hooks-install.sh:10: +# under a git hook (run-tests.sh runs as pre-push) those are exported, and the +# script's `git rev-parse --show-toplevel` would then answer with the LIVE repo +# no matter which directory it was invoked from. run_script() { local dir="$1" shift - (cd "$dir" && bash "$SCRIPT" "$@") + (cd "$dir" && env -u GIT_DIR -u GIT_WORK_TREE bash "$SCRIPT" "$@") } CLEANUP_DIRS=() @@ -67,6 +98,31 @@ else fail "missing source should exit 0 in --check mode, not report drift" fi +# --- 2b. Source missing but a mirror still present: --check must FAIL --- +# --check used to exit 0 on any missing source, so deleting +# .claude-plugin/marketplace.json left a stale .github/plugin/marketplace.json +# reported as "no drift" -- a mirror of a file that no longer exists. That is +# the silent divergence this script's header says it prevents ("keeps that +# legacy mirror byte-identical ... instead of letting it silently drift"), and +# scripts/sync-plugin-content.sh --check --all already errors on the same +# condition. Case 2 above still holds: neither file present stays a no-op. +echo "" +echo "--- missing source with a surviving mirror: --check reports drift ---" +FIXTURE2B="$(make_fixture)"; track "$FIXTURE2B" +write_dst "$FIXTURE2B" "$CONTENT_A" +if run_script "$FIXTURE2B" --check > /dev/null 2>&1; then + fail "exited 0 with a stale mirror and no source -- expected drift (exit 1)" +else + pass "a mirror with no source left to mirror is reported as drift" +fi +# Real-sync mode keeps its no-op: it has nothing to copy, and deleting a +# tracked file is not this script's call to make. +if run_script "$FIXTURE2B" > /dev/null 2>&1 && [[ -f "$FIXTURE2B/$DST_REL" ]]; then + pass "real-sync mode still no-ops on a missing source, leaving the mirror alone" +else + fail "real-sync mode should no-op on a missing source, not fail or delete the mirror" +fi + # --- 3. Source exists, mirror missing entirely: --check reports drift (exit 1) --- echo "" echo "--- --check reports drift when the mirror file does not exist yet ---" @@ -179,6 +235,26 @@ else fail "a second sync run introduced unexpected drift" fi +# --- 12. Fixture isolation survives an inherited GIT_DIR/GIT_WORK_TREE --- +# The whole suite's isolation is REPO_ROOT resolving to the fixture. With +# GIT_DIR/GIT_WORK_TREE exported -- which is every git-hook context, and +# run-tests.sh runs as pre-push -- `git rev-parse --show-toplevel` answers with +# THAT repo from any cwd, so the script wrote to the live tree and 5 of these +# cases failed. Point both variables at a decoy repo (never the live one, so +# this assertion cannot itself write where it must not) and assert the fixture +# still wins: the mirror lands in the fixture and the decoy stays untouched. +echo "" +echo "--- fixture isolation holds with GIT_DIR/GIT_WORK_TREE inherited from elsewhere ---" +FIXTURE12="$(make_fixture)"; track "$FIXTURE12" +DECOY="$(make_fixture)"; track "$DECOY" +write_src "$FIXTURE12" "$CONTENT_A" +if (export GIT_DIR="$DECOY/.git" GIT_WORK_TREE="$DECOY"; run_script "$FIXTURE12" > /dev/null 2>&1) \ + && [[ -f "$FIXTURE12/$DST_REL" ]] && [[ ! -e "$DECOY/$DST_REL" ]]; then + pass "an inherited GIT_DIR/GIT_WORK_TREE does not redirect writes out of the fixture" +else + fail "an inherited GIT_DIR/GIT_WORK_TREE redirected the sync outside the fixture" +fi + echo "" echo "Results: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]] diff --git a/tests/test-vale-wrap.sh b/tests/test-vale-wrap.sh index 0c7e1eb..a42dae3 100755 --- a/tests/test-vale-wrap.sh +++ b/tests/test-vale-wrap.sh @@ -442,77 +442,214 @@ fi # is safe on 3.2. Neither is an array seeded with at least one element where it # is declared and never reset to empty: it cannot be empty at any expansion # site, so the construct is not a hazard there and demanding the guarded form -# would be a wrong test. The file list covers every script this repo ships or -# runs that a macOS user reaches: the wrapper itself, the two pre-commit hook -# scripts, the test runner AGENTS.md tells contributors to run by hand, its -# bats-dispatch companion, and the three test-*.sh scripts whose -# `trap 'rm -rf "${CLEANUP_DIRS[@]}"' EXIT` cleanup traps were unguarded (PR -# #95 review finding #7 named two of them; a repo-wide grep for the same -# pattern turned up test-check-release-needed.sh as a third) until they were -# switched to the guarded form. -# `mapfile` is checked alongside, because it is bash 4.0+ and the expansion scan -# cannot see it — run-tests.sh carried one until it was replaced with a -# `while read` loop, and nothing would have caught its return. `declare -A` -# (bash 4.0+ associative arrays) is checked for the same reason — the -# expansion scan cannot see it, and check-vale-style-sync.sh carried a pair of -# them until they were replaced with index-scanned plain arrays. +# would be a wrong test. Neither is an expansion whose own line first proves the +# array non-empty (`[[ ${#a[@]} -eq 0 ]] || rm -rf "${a[@]}"`) — the short-circuit +# means the expansion is unreachable when the array is empty. +# +# The scanned file list is DERIVED, not hand-maintained. A hardcoded list only +# guards the scripts someone remembered to add to it, and it silently fails to +# cover anything new: it omitted scripts/lib/batch-run.sh — the shared runner +# this branch introduced, whose own header (batch-run.sh:9-11) documents it as +# bash-3.2-safe — along with four other scripts/*.sh. Deriving the list means a +# new script is covered the moment it lands. AGENTS.md names bash 3.2 as an +# explicit repo target, so the scope is three globs, each floor-asserted below: +# - scripts/**/*.sh — repo tooling and pre-commit hook scripts +# - tests/*.sh — the runners and every regression test +# - plugins/*/.apm/**/*.sh — the scripts plugins ship to users +# plugins/*/skills/** is deliberately NOT scanned: it is the generated mirror of +# .apm/, so scanning both double-reports every finding, and mirror-vs-source +# drift is already sync-plugin-content.sh --check's job. Scanning .apm/ is what +# closed the gap where skill-audit's vale-wrap.sh was covered but agent-audit's +# byte-identical copy of it was not. +# providers/**/*.sh is the one shipped script deliberately left out: +# providers/claude-code/statusline-command.sh seeds `parts=()` empty at :83 and +# expands it unguarded at :96. That is a real latent hazard rather than a false +# positive — it just cannot abort today because the file enables no `set -u`. +# Fixing it is a change to a file this case does not own; once :96 uses the +# guarded form, add a `providers` glob to the table below. +# +# Four constructs are checked, because the expansion scan cannot see any of the +# other three: +# - `mapfile`/`readarray` — bash 4.0+ builtins. run-tests.sh carried one until +# it was replaced with a `while read` loop. +# - `declare -A` — bash 4.0+ associative arrays. check-vale-style-sync.sh +# carried a pair until they became index-scanned plain arrays. +# - `wait -n` — bash 4.3+. A prior review round found this live in run-bats.sh. +# - `nproc` — GNU coreutils, absent on macOS entirely. Same review round, same +# file. `getconf _NPROCESSORS_ONLN` is the portable spelling batch-run.sh +# settled on. +# The last two had no static guard anywhere before this, and +# `shellcheck --severity=warning` (.pre-commit-config.yaml) pins no target +# version, so it does not catch them either — meaning the guard against the last +# regression would not have caught the last regression. echo "" -echo "--- no unguarded array expansion remains in the macOS-facing scripts ---" +echo "--- no bash-4-only construct remains in the macOS-facing scripts ---" +# Files a script pulls in via `source`, as named by its `# shellcheck source=` +# directives. Array seeding often lives in the sourced file (install.sh's +# DEPLOY_* come from deploy-manifest.sh), so the seeding check has to look +# there too or it reports a false hazard. shellcheck resolves these paths +# against either the repo root or the script's own directory depending on +# configuration, so both are tried and whichever exists is used. +sourced_files() { + local file="$1" rel cand + while IFS= read -r rel; do + for cand in "$REPO_ROOT/$rel" "$(dirname "$file")/$rel"; do + if [[ -f "$cand" ]]; then + printf '%s\n' "$cand" + break + fi + done + done < <( + grep -oE '^[[:space:]]*#[[:space:]]*shellcheck[[:space:]]+source=[^[:space:]]+' "$file" \ + | sed -E 's/.*source=//' || true + ) + return 0 +} unguarded_expansions() { - local file="$1" hit name + local file="$1" hit name seed_file + local seed_files + seed_files=("$file") + while IFS= read -r seed_file; do + seed_files+=("$seed_file") + done < <(sourced_files "$file") while IFS= read -r hit; do name="$(printf '%s\n' "$hit" \ | grep -oE '\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}' | head -1 \ | sed -E 's/^\$\{//; s/\[@\]\}$//')" - if grep -qE "^[[:space:]]*((local|declare|readonly)[[:space:]]+)?(-a[[:space:]]+)?$name=\([^)]" "$file" \ - && ! grep -qE "^[[:space:]]*$name=\(\)" "$file"; then + # Shell-maintained arrays are never seeded by a `NAME=(...)` line, so the + # seeding exemption below can never clear them: without this case they are + # permanent false positives. Exempted are the ones measured non-empty inside + # a running script -- element counts taken at a script's top level: + # PIPESTATUS >=1 once any command has run (0 only before the very first, + # where the variable is meaningless anyway) + # BASH_SOURCE 1 (one frame per sourced/executed file) + # BASH_LINENO 1 (maintained in parallel with BASH_SOURCE) + # BASH_VERSINFO 6 (always exactly six) + # GROUPS 1 + # DIRSTACK 1 (always holds at least the current directory) + # + # FUNCNAME, BASH_ARGV, BASH_ARGC, BASH_REMATCH and COMP_WORDS are + # deliberately NOT exempted despite being shell-maintained, because they are + # genuinely empty in reachable states: FUNCNAME is 0 outside a function, + # BASH_ARGV is 0 without `shopt -s extdebug`, BASH_ARGC is 0 *inside a + # function* (it looks safe when measured at top level, where it is 1 -- it is + # not), BASH_REMATCH is 0 until a `=~` match succeeds, COMP_WORDS is 0 + # outside completion. Expanding any of those bare really does abort on bash + # 3.2 under `set -u`, so flagging them is the correct answer rather than a + # false positive. Case 26 pins this split so neither half drifts. + case "$name" in + PIPESTATUS|BASH_SOURCE|BASH_LINENO|BASH_VERSINFO|GROUPS|DIRSTACK) continue ;; + esac + # Same-line emptiness short-circuit: the expansion cannot be reached empty. + if printf '%s\n' "$hit" \ + | grep -qE "\\\$\{#$name\[@\]\}[[:space:]]*-(eq|lt)[[:space:]]*[01][^|]*\|\|"; then continue fi + for seed_file in ${seed_files[@]+"${seed_files[@]}"}; do + # `([^)]|$)` after the paren, not just `[^)]`: a multi-line declaration + # (`DEPLOY_FILES=(` with its elements on the following lines) ends the line + # right there, and requiring a character after the paren missed it. An + # empty `name=()` still does not match, which is what the check is for. + if grep -qE "^[[:space:]]*((local|declare|readonly)[[:space:]]+)?(-a[[:space:]]+)?$name=\(([^)]|$)" "$seed_file" \ + && ! grep -qE "^[[:space:]]*$name=\(\)" "$seed_file"; then + continue 2 + fi + done printf '%s:%s\n' "${file##*/}" "$hit" done < <( # Blank out whole-line comments (keeping line numbers), delete every - # correctly guarded expansion, then anything still matching is a candidate. + # correctly guarded expansion — in both its bare spelling and the + # backslash-escaped one that appears inside this file's own PASS message — + # then anything still matching is a candidate. awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$file" \ - | sed -E 's/\$\{([A-Za-z_][A-Za-z0-9_]*)\[@\]\+"\$\{\1\[@\]\}"\}//g' \ + | sed -E 's/\\?\$\{([A-Za-z_][A-Za-z0-9_]*)\[@\]\+\\?"\\?\$\{\1\[@\]\}\\?"\}//g' \ | grep -nE '\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}' || true ) } +# Blanks WHOLE-LINE comments only (keeping line numbers so hits stay locatable), +# so prose naming a hazard on its own line is not a hit. +# +# KNOW THIS BEFORE YOU EDIT ANY SCANNED FILE. Nothing else is stripped — this is +# a line-based scanner, not a shell parser — so all of the following DO trip the +# case even though none is a real hazard: +# true # avoid nproc on macOS <- trailing comment naming a hazard +# echo "avoid mapfile in scripts" <- hazard named inside a string +# <&2; return 1 ;; + esac +} +# Floors are PER GLOB, not on the merged total. A single total floor cannot +# detect the failure this assertion exists to name: with 12 + 17 + 13 files, +# losing the whole `scripts` glob still leaves 30 and losing the whole `tests` +# glob still leaves 25, so any total floor low enough to survive normal churn +# is too low to notice an entire glob silently resolving to nothing. Each floor +# sits a little under its current count so ordinary file removal does not trip +# it, but a broken or renamed path does. Parallel arrays rather than an +# associative one — `declare -A` is bash 4.0+, which this very case forbids. +BASH32_GLOB_NAMES=(scripts tests plugins) +BASH32_GLOB_FLOORS=(10 14 10) +BASH32_SCRIPTS=() +BASH32_IDX=0 +while [[ $BASH32_IDX -lt ${#BASH32_GLOB_NAMES[@]} ]]; do + BASH32_GLOB="${BASH32_GLOB_NAMES[$BASH32_IDX]}" + BASH32_FLOOR="${BASH32_GLOB_FLOORS[$BASH32_IDX]}" + BASH32_COUNT=0 + while IFS= read -r BASH32_FOUND; do + BASH32_SCRIPTS+=("$BASH32_FOUND") + BASH32_COUNT=$((BASH32_COUNT + 1)) + done < <(bash32_glob "$BASH32_GLOB" | sort) + if [[ $BASH32_COUNT -lt $BASH32_FLOOR ]]; then + fail "the bash-3.2 scan's '$BASH32_GLOB' glob derived $BASH32_COUNT file(s), under its floor of $BASH32_FLOOR — that path is wrong, so those scripts are silently unscanned" + fi + BASH32_IDX=$((BASH32_IDX + 1)) +done HAZARDS16="" -for BASH32_SCRIPT in \ - "$SCRIPT" \ - "$REPO_ROOT/scripts/skill-size-check.sh" \ - "$REPO_ROOT/scripts/check-release-needed.sh" \ - "$REPO_ROOT/scripts/check-vale-style-sync.sh" \ - "$REPO_ROOT/tests/run-tests.sh" \ - "$REPO_ROOT/tests/run-bats.sh" \ - "$REPO_ROOT/tests/test-sync-marketplace-mirror.sh" \ - "$REPO_ROOT/tests/test-sync-plugin-content.sh" \ - "$REPO_ROOT/tests/test-check-release-needed.sh"; do +for BASH32_SCRIPT in ${BASH32_SCRIPTS[@]+"${BASH32_SCRIPTS[@]}"}; do FOUND16="$(unguarded_expansions "$BASH32_SCRIPT")" if [[ -n "$FOUND16" ]]; then HAZARDS16+="$FOUND16 " fi - # `mapfile`/`readarray` are bash 4.0+ builtins with no 3.2 fallback. Whole-line - # comments are blanked first so prose naming the builtin is not a hit. - FOUND16B="$(awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$BASH32_SCRIPT" \ - | grep -nE '(^|[^[:alnum:]_])(mapfile|readarray)[[:space:]]' || true)" - if [[ -n "$FOUND16B" ]]; then - HAZARDS16+="${BASH32_SCRIPT##*/}:$FOUND16B " - fi - # `declare -A` (associative arrays) is bash 4.0+ with no 3.2 fallback. The - # flag cluster can carry other letters in any order (-Ag, -rA, ...); what - # matters is a literal uppercase A appearing in it, so match on that rather - # than the exact string "-A". - FOUND16C="$(awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$BASH32_SCRIPT" \ - | grep -nE '(^|[^[:alnum:]_])declare[[:space:]]+-[a-zA-Z]*A[a-zA-Z]*([[:space:]]|$)' || true)" - if [[ -n "$FOUND16C" ]]; then - HAZARDS16+="${BASH32_SCRIPT##*/}:$FOUND16C " - fi + # `mapfile`/`readarray`: bash 4.0+ builtins with no 3.2 fallback. + # `declare -A`: bash 4.0+ associative arrays. The flag cluster can carry other + # letters in any order (-Ag, -rA, ...); what matters is a literal uppercase A + # appearing in it, so match on that rather than the exact string "-A". + # `wait -n`: bash 4.3+. `nproc`: GNU coreutils, not present on macOS. + # The last two close on `[^[:alnum:]_]`, not `[[:space:]]`: the real spellings + # are `$(nproc)` and `wait -n;`, and a whitespace-only terminator misses both. + # `npro[c]` matches exactly what `nproc` would, but keeps the literal string + # "nproc" out of this file — the scan reads this file too, so a bare spelling + # here would report itself as a hazard. + for BASH32_RULE in \ + '(^|[^[:alnum:]_])(mapfile|readarray)[[:space:]]' \ + '(^|[^[:alnum:]_])declare[[:space:]]+-[a-zA-Z]*A[a-zA-Z]*([[:space:]]|$)' \ + '(^|[^[:alnum:]_])wait[[:space:]]+-n([^[:alnum:]_]|$)' \ + '(^|[^[:alnum:]_])npro[c]([^[:alnum:]_]|$)'; do + FOUND16B="$(strip_comments "$BASH32_SCRIPT" | grep -nE "$BASH32_RULE" || true)" + if [[ -n "$FOUND16B" ]]; then + HAZARDS16+="${BASH32_SCRIPT##*/}:$FOUND16B " + fi + done done if [[ -n "$HAZARDS16" ]]; then - fail "unguarded array expansion(s) abort on bash < 4.4 under set -u: $(echo "$HAZARDS16" | tr '\n' ' ')" + fail "bash-4-only construct(s) found in ${#BASH32_SCRIPTS[@]} scanned script(s): $(echo "$HAZARDS16" | tr '\n' ' ')" else - pass "every array expansion uses the bash-3.2-safe \${arr[@]+\"\${arr[@]}\"} form" + # This message deliberately names none of the four hazards in their literal + # spelling: the scan reads this file too, so a literal here is indistinguishable + # from a real occurrence and the case would fail on its own success message. + pass "all ${#BASH32_SCRIPTS[@]} scanned scripts are free of every bash-4-only construct this case checks for" fi # --- 17. The invocations whose arrays are closest to empty actually run. Under @@ -594,11 +731,14 @@ DESC19_A="Use when the caller helps with a specific job" DESC19_B="and the second physical line will utilize the wrap" # make_form_fixture spells the same two-clause description in one YAML scalar -# form: single, folded, plain, dquote, squote, or keyonly. +# form: single, folded, plain, dquote, squote, or keyonly. It prints the fixture +# dir and does NOT call new_fixture itself: every caller invokes it inside `$( )`, +# so a registration made in here would land in the command substitution's +# subshell and never reach cleanup_all -- which leaked all six fixtures per run. +# Registration is therefore the caller's job, in the caller's shell. make_form_fixture() { local form="$1" dir dir="$(mktemp -d)" - new_fixture "$dir" (cd "$dir" && git init -q) mkdir -p "$dir/plugins/testplugin/skills/zzzskill" { @@ -636,7 +776,13 @@ alert_text() { echo "" echo "--- every multi-line description form reports what its single-line form reports ---" +# FORM_FIXTURES19 is bookkeeping for case 25, which asserts every dir recorded +# here also reached EXTRA_FIXTURES. It is appended to independently of +# new_fixture so that dropping the new_fixture calls is detectable. +FORM_FIXTURES19=() FIXTURE19_SINGLE="$(make_form_fixture single)" +FORM_FIXTURES19+=("$FIXTURE19_SINGLE") +new_fixture "$FIXTURE19_SINGLE" BASELINE19="$(alert_text "$(run_wrap "$FIXTURE19_SINGLE" --config "$VALE_CONFIG" "$REL_SKILL19")")" if [[ -z "$BASELINE19" ]]; then # The loop below has to be skipped, not merely reported on: an empty baseline @@ -646,6 +792,8 @@ if [[ -z "$BASELINE19" ]]; then else for FORM19 in folded plain dquote squote keyonly; do DIR19="$(make_form_fixture "$FORM19")" + FORM_FIXTURES19+=("$DIR19") + new_fixture "$DIR19" BARE19="$(cd "$DIR19" && vale --config "$VALE_CONFIG" "$REL_SKILL19" 2>&1 || true)" GOT19="$(alert_text "$(run_wrap "$DIR19" --config "$VALE_CONFIG" "$REL_SKILL19")")" if echo "$BARE19" | grep -q "VagueWording"; then @@ -864,6 +1012,85 @@ for FORM24 in "--output line" "--output=line" "--output JSON" "--output=JSON"; d fi done +# --- 25. Every fixture this run created is on the cleanup list. make_form_fixture +# used to call `new_fixture` itself, but every caller invokes it as +# `$(make_form_fixture ...)` — a command substitution — so the append landed in a +# subshell and was gone by the time the caller resumed. cleanup_all then removed +# five of the six form fixtures it never heard about, leaking one temp dir per +# YAML form per run (six in total, measured under a private TMPDIR). +# +# Nothing in the suite noticed: a leak fails no assertion, and the run reported +# "39 passed, 0 failed" with the bug present exactly as it does with the bug +# fixed. Leak-freeness was only ever observable by watching TMPDIR from outside, +# which is not a regression test. This case makes it one — it compares the dirs +# case 19 created against the dirs registered for cleanup, so re-introducing the +# subshell registration fails the run rather than quietly leaking again. +echo "" +echo "--- every fixture created by this run is registered for cleanup ---" +UNREGISTERED25="" +for DIR25 in ${FORM_FIXTURES19[@]+"${FORM_FIXTURES19[@]}"}; do + REGISTERED25=0 + for KNOWN25 in ${EXTRA_FIXTURES[@]+"${EXTRA_FIXTURES[@]}"}; do + if [[ "$KNOWN25" == "$DIR25" ]]; then + REGISTERED25=1 + break + fi + done + if [[ $REGISTERED25 -eq 0 ]]; then + UNREGISTERED25+="$DIR25 " + fi +done +if [[ ${#FORM_FIXTURES19[@]} -ne 6 ]]; then + fail "expected 6 form fixtures to have been created, saw ${#FORM_FIXTURES19[@]} — case 25 is not checking what it claims" +elif [[ -n "$UNREGISTERED25" ]]; then + fail "fixture(s) created but never registered for cleanup, so they leak: $UNREGISTERED25" +else + pass "all ${#FORM_FIXTURES19[@]} form fixtures are registered for cleanup" +fi + +# --- 26. Case 16's shell-special-array exemption is pinned to a fixture. No file +# the scan currently reads expands any of those arrays, so the exemption is inert +# in practice: it could be deleted, or quietly widened to cover an array that can +# genuinely be empty, and every existing case would still pass. This drives the +# real unguarded_expansions over a fixture holding one expansion of each, and +# asserts the split in BOTH directions -- exempt arrays absent from the findings, +# never-safe arrays present. The membership is not cosmetic: the exempt ones are +# shell-maintained and always non-empty, while the flagged ones are empty in +# reachable states (see the comment on the exemption for the measured counts), so +# widening the list to include one of the latter would suppress a real abort. +echo "" +echo "--- the shell-special-array exemption covers exactly the never-empty arrays ---" +FIXTURE26="$(mktemp -d)" +new_fixture "$FIXTURE26" +EXEMPT26="PIPESTATUS BASH_SOURCE BASH_LINENO BASH_VERSINFO GROUPS DIRSTACK" +FLAGGED26="FUNCNAME BASH_ARGV BASH_ARGC BASH_REMATCH COMP_WORDS" +{ + echo "#!/usr/bin/env bash" + for ARR26 in $EXEMPT26 $FLAGGED26; do + echo "echo \"\${${ARR26}[@]}\"" + done +} > "$FIXTURE26/probe.sh" +FOUND26="$(unguarded_expansions "$FIXTURE26/probe.sh")" +MISSING26="" +LEAKED26="" +for ARR26 in $EXEMPT26; do + if echo "$FOUND26" | grep -q "{$ARR26\[@\]}"; then + LEAKED26+="$ARR26 " + fi +done +for ARR26 in $FLAGGED26; do + if ! echo "$FOUND26" | grep -q "{$ARR26\[@\]}"; then + MISSING26+="$ARR26 " + fi +done +if [[ -n "$LEAKED26" ]]; then + fail "always-non-empty shell array(s) reported as hazards, so the exemption is not applying: $LEAKED26" +elif [[ -n "$MISSING26" ]]; then + fail "shell array(s) that CAN be empty were exempted, suppressing a real bash-3.2 abort: $MISSING26" +else + pass "all 6 never-empty shell arrays are exempt and all 5 sometimes-empty ones are still flagged" +fi + echo "" echo "Results: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]]