From 8c570e96596812e9cdba4fb4e6157319d1788185 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Sun, 9 Aug 2026 13:06:23 +0000 Subject: [PATCH] fix(lint): make the Vale prefilter work for external consumers pre-commit prefixes only entry[0] with the hook-repo clone path (cmd = (prefix.path(cmd[0]), *cmd[1:])), so the --config argument in .pre-commit-hooks.yaml resolved against the *consuming* repo's root and hard-failed every external run with E100. Two of the three hooks ADR-0014 promises were unusable. vale-wrap.sh now self-locates its config from BASH_SOURCE when no --config is supplied; an explicit --config still wins in all three argv forms and stays cwd-relative. Both manifests drop the argument and are kept byte-identical: the local repo: local config resolved --config correctly only because the consuming repo *was* this repo, and that divergence is why three review rounds missed the defect. Also in the wrapper: - replace GNU-only `realpath -m` with a portable abspath helper; -m is load-bearing (dest does not exist yet), so BSD realpath aborted the script under set -e on macOS - walk directory arguments instead of passing them through unflattened, which reported a clean 0-error run for files that fail when named explicitly - read/write with errors='surrogateescape' so one non-UTF-8 .md under a directory argument cannot abort the hook New test-vale-hooks-consumer.sh builds the hook repo from the working tree and points a file:// consumer at it, covering the manifest as a hook repo for the first time. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58 --- .pre-commit-config.yaml | 4 +- .pre-commit-hooks.yaml | 4 +- .../skills/agent-audit/scripts/vale-wrap.sh | 136 ++++++++++++------ .../skills/skill-audit/scripts/vale-wrap.sh | 136 ++++++++++++------ tests/run-tests.sh | 19 ++- tests/test-vale-hooks-consumer.sh | 127 ++++++++++++++++ tests/test-vale-wrap.sh | 84 ++++++++++- 7 files changed, 412 insertions(+), 98 deletions(-) create mode 100755 tests/test-vale-hooks-consumer.sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 375ae7a..165ea75 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -129,7 +129,7 @@ repos: stages: ['pre-commit'] name: Vale audit prefilter (SKILL.md) description: Run Vale against SKILL.md files as a deterministic prefilter for skill-audit, via skill-audit's own bundled copy - entry: plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh --config plugins/kyberforge/skills/skill-audit/assets/vale/.vale.ini + entry: plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh language: script files: '^plugins/[^/]+/skills/[^/]+/SKILL\.md$' pass_filenames: true @@ -138,7 +138,7 @@ repos: stages: ['pre-commit'] name: Vale audit prefilter (agent files) description: Run Vale against agent markdown files as a deterministic prefilter for agent-audit, via agent-audit's own bundled copy - entry: plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh --config plugins/kyberforge/skills/agent-audit/assets/vale/.vale.ini + entry: plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh language: script files: '^plugins/[^/]+/agents/[^/]+\.md$' pass_filenames: true diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index 4ad76d9..eee3986 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -1,14 +1,14 @@ - id: kyberforge-vale-audit-skill name: Kyberforge Vale prose audit (SKILL.md) description: Deterministic prose-pattern prefilter for kyberforge's skill-audit, via its own bundled Vale config/styles - entry: plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh --config plugins/kyberforge/skills/skill-audit/assets/vale/.vale.ini + entry: plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh language: script files: '(^|/)SKILL\.md$' - id: kyberforge-vale-audit-agent name: Kyberforge Vale prose audit (agent files) description: Deterministic prose-pattern prefilter for kyberforge's agent-audit, via its own bundled Vale config/styles - entry: plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh --config plugins/kyberforge/skills/agent-audit/assets/vale/.vale.ini + entry: plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh language: script files: '(^|/)agents/[^/]+\.md$|\.agent\.md$' diff --git a/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh b/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh index 918f32e..9e22cb8 100755 --- a/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh +++ b/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh @@ -9,26 +9,35 @@ set -euo pipefail # the real `vale` binary against the copies. Drop-in replacement for calling # `vale` directly: same args, same exit code. # -# "Same args" means relative paths — `--config` values and file arguments alike +# "Same args" means relative paths — `--config` values and path arguments alike # — resolve against the caller's current directory, exactly as bare `vale` # resolves them. (An earlier version resolved them against the repo root, an # invented convention that hard-errored on `--config ../../.vale.ini` from a # subdirectory and, worse, silently dropped file arguments that didn't happen to # resolve from the repo root — skipping the flattening this script exists for.) # -# Vale prints each file path exactly as it was handed to it, so the scratch tree -# mirrors the caller's absolute cwd: a relative file argument is passed through +# The one addition to bare `vale`'s argument handling: with no `--config` at +# all, this script's own sibling `assets/vale/.vale.ini` is used instead of +# vale's upward search. pre-commit prefixes only `entry[0]` with the hook-repo +# clone path, so a `--config` in `.pre-commit-hooks.yaml` would resolve against +# the *consuming* repo and hard-fail (E100) for every external consumer. The +# manifest therefore passes the script alone, and an explicit `--config` from +# any other caller still wins. +# +# Vale prints each path exactly as it was handed to it, so the scratch tree +# mirrors the caller's absolute cwd: a relative path argument is passed through # verbatim and resolves to its flattened copy, keeping the report byte-identical -# to bare `vale`'s. An absolute file argument inside the cwd is relativized to -# keep that property. Only an absolute path outside the cwd is rewritten to its -# scratch copy and so reports a scratch path — unavoidable, since a file can -# only be read from where it actually is. +# to bare `vale`'s. An absolute path inside the cwd is relativized to keep that +# property. Only an absolute path outside the cwd is rewritten to its scratch +# copy and so reports a scratch path — unavoidable, since a file can only be +# read from where it actually is. cwd="$(pwd -P)" vale_args=() -file_args=() +path_args=() config_next=false +config_given=false for arg in "$@"; do if [[ "$config_next" == true ]]; then config_next=false @@ -43,19 +52,24 @@ for arg in "$@"; do --config) vale_args+=("$arg") config_next=true + config_given=true continue ;; --config=/*) vale_args+=("$arg") + config_given=true continue ;; --config=*) vale_args+=("--config=$cwd/${arg#--config=}") + config_given=true continue ;; esac - # `-f` resolves relative paths against the caller's cwd, same as vale does. - if [[ "$arg" != -* && -f "$arg" ]]; then + # `-f`/`-d` resolve relative paths against the caller's cwd, same as vale does. + # A path that doesn't exist is left for vale to report on, exactly as bare + # vale would. + if [[ "$arg" != -* && ( -f "$arg" || -d "$arg" ) ]]; then # An absolute path inside the caller's cwd is relativized so the report cites # a path that resolves against the real tree. Left absolute, it would be # rewritten to its scratch copy and printed as `/tmp/tmp.XXXX/...` — a real @@ -63,54 +77,41 @@ for arg in "$@"; do # quoting it. Absolute paths outside the cwd have no relative form and keep # the scratch-path behaviour documented above. if [[ "$arg" == "$cwd"/* ]]; then - file_args+=("${arg#"$cwd"/}") + path_args+=("${arg#"$cwd"/}") else - file_args+=("$arg") + path_args+=("$arg") fi else vale_args+=("$arg") fi done -if [[ ${#file_args[@]} -eq 0 ]]; then +if [[ "$config_given" == false ]]; then + vale_args+=(--config "$(cd "$(dirname "${BASH_SOURCE[0]}")/../assets/vale" && pwd)/.vale.ini") +fi + +if [[ ${#path_args[@]} -eq 0 ]]; then # Nothing to flatten. Hand off directly, with stdin closed so vale doesn't # block waiting on a pipe that will never carry content. exec vale "${vale_args[@]}" < /dev/null fi -tmpdir="$(realpath -m "$(mktemp -d)")" -trap 'rm -rf "$tmpdir"' EXIT +# `realpath -m` would be the obvious normalizer, but `-m` (canonicalize-missing) +# is a GNU extension the BSD realpath on macOS doesn't have — and every dest +# below is a path that doesn't exist yet. python3 is already a hard dependency. +abspath() { + python3 -c 'import os, sys; print(os.path.abspath(sys.argv[1]))' "$1" +} -# Mirror of the caller's cwd inside the scratch tree; relative file arguments -# are resolved from here. -mirror="$tmpdir$cwd" -mkdir -p "$mirror" - -argv_files=() -for arg in "${file_args[@]}"; do - if [[ "$arg" == /* ]]; then - dest="$tmpdir$arg" - else - dest="$mirror/$arg" - fi - dest="$(realpath -m "$dest")" - # A file argument with enough leading `..` to climb past the mirror root would - # write outside the scratch dir. The real filesystem clamps such a path at - # `/`; the mirror can't, so refuse rather than scribble outside the sandbox. - case "$dest" in - "$tmpdir"/*) ;; - *) - echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2 - exit 2 - ;; - esac - mkdir -p "$(dirname "$dest")" - python3 - "$arg" "$dest" <<'PYTHON' +flatten() { + python3 - "$1" "$2" <<'PYTHON' import re import sys src, dest = sys.argv[1], sys.argv[2] -with open(src) as fh: +# surrogateescape keeps a non-UTF-8 file (reachable via a directory argument) +# a byte-for-byte round trip instead of aborting the whole run on a decode error. +with open(src, encoding='utf-8', errors='surrogateescape') as fh: content = fh.read() fm_match = re.match(r'^(---\n)(.*?\n)(---\n)', content, re.DOTALL) @@ -163,15 +164,60 @@ if fm_match: new_fm = fm[:start] + f'description: {flat_q}\n{pad}' + fm[end:] content = fm_match.group(1) + new_fm + fm_match.group(3) + content[fm_match.end():] -with open(dest, 'w') as fh: +with open(dest, 'w', encoding='utf-8', errors='surrogateescape') as fh: fh.write(content) PYTHON +} + +tmpdir="$(cd "$(mktemp -d)" && pwd -P)" +trap 'rm -rf "$tmpdir"' EXIT + +# Mirror of the caller's cwd inside the scratch tree; relative path arguments +# are resolved from here. +mirror="$tmpdir$cwd" +mkdir -p "$mirror" + +argv_paths=() +for arg in "${path_args[@]}"; do if [[ "$arg" == /* ]]; then - argv_files+=("$dest") + dest="$tmpdir$arg" else - argv_files+=("$arg") + dest="$mirror/$arg" + fi + dest="$(abspath "$dest")" + # A path argument with enough leading `..` to climb past the mirror root would + # write outside the scratch dir. The real filesystem clamps such a path at + # `/`; the mirror can't, so refuse rather than scribble outside the sandbox. + case "$dest" in + "$tmpdir"/*) ;; + *) + echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2 + exit 2 + ;; + esac + mkdir -p "$(dirname "$dest")" + if [[ -d "$arg" ]]; then + # A directory is mirrored whole — vale applies its own format filtering to + # the tree, so any file dropped here would be silently unlinted — and then + # every markdown file in the copy is flattened in place. `.git` is pruned: + # vale never lints it and copying it can dwarf the rest of the tree. + mkdir -p "$dest" + while IFS= read -r -d '' rel; do + mkdir -p "$dest/$(dirname "$rel")" + cp "$arg/$rel" "$dest/$rel" + done < <(cd "$arg" && find . -name .git -prune -o -type f -print0) + while IFS= read -r -d '' md; do + flatten "$md" "$md" + done < <(find "$dest" -type f -name '*.md' -print0) + else + flatten "$arg" "$dest" + fi + if [[ "$arg" == /* ]]; then + argv_paths+=("$dest") + else + argv_paths+=("$arg") fi done cd "$mirror" -vale "${vale_args[@]}" "${argv_files[@]}" +vale "${vale_args[@]}" "${argv_paths[@]}" diff --git a/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh b/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh index 918f32e..9e22cb8 100755 --- a/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh +++ b/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh @@ -9,26 +9,35 @@ set -euo pipefail # the real `vale` binary against the copies. Drop-in replacement for calling # `vale` directly: same args, same exit code. # -# "Same args" means relative paths — `--config` values and file arguments alike +# "Same args" means relative paths — `--config` values and path arguments alike # — resolve against the caller's current directory, exactly as bare `vale` # resolves them. (An earlier version resolved them against the repo root, an # invented convention that hard-errored on `--config ../../.vale.ini` from a # subdirectory and, worse, silently dropped file arguments that didn't happen to # resolve from the repo root — skipping the flattening this script exists for.) # -# Vale prints each file path exactly as it was handed to it, so the scratch tree -# mirrors the caller's absolute cwd: a relative file argument is passed through +# The one addition to bare `vale`'s argument handling: with no `--config` at +# all, this script's own sibling `assets/vale/.vale.ini` is used instead of +# vale's upward search. pre-commit prefixes only `entry[0]` with the hook-repo +# clone path, so a `--config` in `.pre-commit-hooks.yaml` would resolve against +# the *consuming* repo and hard-fail (E100) for every external consumer. The +# manifest therefore passes the script alone, and an explicit `--config` from +# any other caller still wins. +# +# Vale prints each path exactly as it was handed to it, so the scratch tree +# mirrors the caller's absolute cwd: a relative path argument is passed through # verbatim and resolves to its flattened copy, keeping the report byte-identical -# to bare `vale`'s. An absolute file argument inside the cwd is relativized to -# keep that property. Only an absolute path outside the cwd is rewritten to its -# scratch copy and so reports a scratch path — unavoidable, since a file can -# only be read from where it actually is. +# to bare `vale`'s. An absolute path inside the cwd is relativized to keep that +# property. Only an absolute path outside the cwd is rewritten to its scratch +# copy and so reports a scratch path — unavoidable, since a file can only be +# read from where it actually is. cwd="$(pwd -P)" vale_args=() -file_args=() +path_args=() config_next=false +config_given=false for arg in "$@"; do if [[ "$config_next" == true ]]; then config_next=false @@ -43,19 +52,24 @@ for arg in "$@"; do --config) vale_args+=("$arg") config_next=true + config_given=true continue ;; --config=/*) vale_args+=("$arg") + config_given=true continue ;; --config=*) vale_args+=("--config=$cwd/${arg#--config=}") + config_given=true continue ;; esac - # `-f` resolves relative paths against the caller's cwd, same as vale does. - if [[ "$arg" != -* && -f "$arg" ]]; then + # `-f`/`-d` resolve relative paths against the caller's cwd, same as vale does. + # A path that doesn't exist is left for vale to report on, exactly as bare + # vale would. + if [[ "$arg" != -* && ( -f "$arg" || -d "$arg" ) ]]; then # An absolute path inside the caller's cwd is relativized so the report cites # a path that resolves against the real tree. Left absolute, it would be # rewritten to its scratch copy and printed as `/tmp/tmp.XXXX/...` — a real @@ -63,54 +77,41 @@ for arg in "$@"; do # quoting it. Absolute paths outside the cwd have no relative form and keep # the scratch-path behaviour documented above. if [[ "$arg" == "$cwd"/* ]]; then - file_args+=("${arg#"$cwd"/}") + path_args+=("${arg#"$cwd"/}") else - file_args+=("$arg") + path_args+=("$arg") fi else vale_args+=("$arg") fi done -if [[ ${#file_args[@]} -eq 0 ]]; then +if [[ "$config_given" == false ]]; then + vale_args+=(--config "$(cd "$(dirname "${BASH_SOURCE[0]}")/../assets/vale" && pwd)/.vale.ini") +fi + +if [[ ${#path_args[@]} -eq 0 ]]; then # Nothing to flatten. Hand off directly, with stdin closed so vale doesn't # block waiting on a pipe that will never carry content. exec vale "${vale_args[@]}" < /dev/null fi -tmpdir="$(realpath -m "$(mktemp -d)")" -trap 'rm -rf "$tmpdir"' EXIT +# `realpath -m` would be the obvious normalizer, but `-m` (canonicalize-missing) +# is a GNU extension the BSD realpath on macOS doesn't have — and every dest +# below is a path that doesn't exist yet. python3 is already a hard dependency. +abspath() { + python3 -c 'import os, sys; print(os.path.abspath(sys.argv[1]))' "$1" +} -# Mirror of the caller's cwd inside the scratch tree; relative file arguments -# are resolved from here. -mirror="$tmpdir$cwd" -mkdir -p "$mirror" - -argv_files=() -for arg in "${file_args[@]}"; do - if [[ "$arg" == /* ]]; then - dest="$tmpdir$arg" - else - dest="$mirror/$arg" - fi - dest="$(realpath -m "$dest")" - # A file argument with enough leading `..` to climb past the mirror root would - # write outside the scratch dir. The real filesystem clamps such a path at - # `/`; the mirror can't, so refuse rather than scribble outside the sandbox. - case "$dest" in - "$tmpdir"/*) ;; - *) - echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2 - exit 2 - ;; - esac - mkdir -p "$(dirname "$dest")" - python3 - "$arg" "$dest" <<'PYTHON' +flatten() { + python3 - "$1" "$2" <<'PYTHON' import re import sys src, dest = sys.argv[1], sys.argv[2] -with open(src) as fh: +# surrogateescape keeps a non-UTF-8 file (reachable via a directory argument) +# a byte-for-byte round trip instead of aborting the whole run on a decode error. +with open(src, encoding='utf-8', errors='surrogateescape') as fh: content = fh.read() fm_match = re.match(r'^(---\n)(.*?\n)(---\n)', content, re.DOTALL) @@ -163,15 +164,60 @@ if fm_match: new_fm = fm[:start] + f'description: {flat_q}\n{pad}' + fm[end:] content = fm_match.group(1) + new_fm + fm_match.group(3) + content[fm_match.end():] -with open(dest, 'w') as fh: +with open(dest, 'w', encoding='utf-8', errors='surrogateescape') as fh: fh.write(content) PYTHON +} + +tmpdir="$(cd "$(mktemp -d)" && pwd -P)" +trap 'rm -rf "$tmpdir"' EXIT + +# Mirror of the caller's cwd inside the scratch tree; relative path arguments +# are resolved from here. +mirror="$tmpdir$cwd" +mkdir -p "$mirror" + +argv_paths=() +for arg in "${path_args[@]}"; do if [[ "$arg" == /* ]]; then - argv_files+=("$dest") + dest="$tmpdir$arg" else - argv_files+=("$arg") + dest="$mirror/$arg" + fi + dest="$(abspath "$dest")" + # A path argument with enough leading `..` to climb past the mirror root would + # write outside the scratch dir. The real filesystem clamps such a path at + # `/`; the mirror can't, so refuse rather than scribble outside the sandbox. + case "$dest" in + "$tmpdir"/*) ;; + *) + echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2 + exit 2 + ;; + esac + mkdir -p "$(dirname "$dest")" + if [[ -d "$arg" ]]; then + # A directory is mirrored whole — vale applies its own format filtering to + # the tree, so any file dropped here would be silently unlinted — and then + # every markdown file in the copy is flattened in place. `.git` is pruned: + # vale never lints it and copying it can dwarf the rest of the tree. + mkdir -p "$dest" + while IFS= read -r -d '' rel; do + mkdir -p "$dest/$(dirname "$rel")" + cp "$arg/$rel" "$dest/$rel" + done < <(cd "$arg" && find . -name .git -prune -o -type f -print0) + while IFS= read -r -d '' md; do + flatten "$md" "$md" + done < <(find "$dest" -type f -name '*.md' -print0) + else + flatten "$arg" "$dest" + fi + if [[ "$arg" == /* ]]; then + argv_paths+=("$dest") + else + argv_paths+=("$arg") fi done cd "$mirror" -vale "${vale_args[@]}" "${argv_files[@]}" +vale "${vale_args[@]}" "${argv_paths[@]}" diff --git a/tests/run-tests.sh b/tests/run-tests.sh index 51464ab..1f3c067 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -2,6 +2,9 @@ # Run all test-*.sh files in the repo (including plugins) and the bats suite. # Usage: bash tests/run-tests.sh [--bats-only] # +# A script exiting 77 (the automake convention) is reported as SKIPPED, not +# passed — a suite that can't run for lack of a binary must not read as green. +# # TEST_DIR — override root to search for test-*.sh (default: REPO_ROOT); used by tests. set -euo pipefail @@ -13,7 +16,9 @@ BATS_ONLY=false SEARCH_ROOT="${TEST_DIR:-$REPO_ROOT}" FAILED=() +SKIPPED=() PASSED=0 +SKIP_EXIT=77 run_bats() { if [[ -x "$BATS" ]]; then @@ -40,15 +45,25 @@ mapfile -t SCRIPTS < <( for script in "${SCRIPTS[@]}"; do rel="${script#"$SEARCH_ROOT/"}" echo "=== $rel ===" - if bash "$script"; then + rc=0 + bash "$script" || rc=$? + if [[ $rc -eq 0 ]]; then PASSED=$((PASSED + 1)) + elif [[ $rc -eq $SKIP_EXIT ]]; then + SKIPPED+=("$rel") else FAILED+=("$rel") fi echo "" done -echo "=== Summary: $PASSED passed, ${#FAILED[@]} failed ===" +echo "=== Summary: $PASSED passed, ${#SKIPPED[@]} skipped, ${#FAILED[@]} failed ===" +if [[ ${#SKIPPED[@]} -gt 0 ]]; then + echo "Skipped scripts:" + for s in "${SKIPPED[@]}"; do + echo " $s" + done +fi if [[ ${#FAILED[@]} -gt 0 ]]; then echo "Failed scripts:" for s in "${FAILED[@]}"; do diff --git a/tests/test-vale-hooks-consumer.sh b/tests/test-vale-hooks-consumer.sh new file mode 100755 index 0000000..ef05a51 --- /dev/null +++ b/tests/test-vale-hooks-consumer.sh @@ -0,0 +1,127 @@ +#!/usr/bin/env bash +# Integration test for .pre-commit-hooks.yaml as an EXTERNAL hook repo — the +# contract ADR-0014 exists to provide, and the one thing running pre-commit +# inside this repo can never exercise: `repo: local` makes pre-commit's clone +# prefix equal to the consuming repo's root, so a hook entry that only works +# because those two coincide passes here and hard-fails everywhere else. +# (It did: every argument after entry[0] resolves against the CONSUMING repo, +# so a `--config plugins/.../.vale.ini` argument gave external consumers +# `E100 [--config] Runtime error ... does not exist`, exit 2, on both Vale hooks.) +# +# The hook repo is built from the WORKING TREE, not from HEAD, so an uncommitted +# change to the manifest or the wrapper is what gets tested. +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +PASS=0 +FAIL=0 + +pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } +fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } + +for bin in pre-commit vale git; do + if ! command -v "$bin" &>/dev/null; then + echo "SKIP: $bin is not installed — cannot stand up a consumer repo" + exit 77 + fi +done + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +HOOK_REPO="$WORK/hookrepo" +CONSUMER="$WORK/consumer" +export PRE_COMMIT_HOME="$WORK/pc-home" + +mkdir -p "$HOOK_REPO/plugins/kyberforge/skills" "$HOOK_REPO/scripts" +cp "$REPO_ROOT/.pre-commit-hooks.yaml" "$HOOK_REPO/" +cp "$REPO_ROOT/scripts/skill-size-check.sh" "$HOOK_REPO/scripts/" +for skill in skill-audit agent-audit; do + mkdir -p "$HOOK_REPO/plugins/kyberforge/skills/$skill" + cp -R "$REPO_ROOT/plugins/kyberforge/skills/$skill/scripts" \ + "$REPO_ROOT/plugins/kyberforge/skills/$skill/assets" \ + "$HOOK_REPO/plugins/kyberforge/skills/$skill/" +done +git -C "$HOOK_REPO" init -q +git -C "$HOOK_REPO" add -A +git -C "$HOOK_REPO" -c user.email=test@example.invalid -c user.name=test commit -qm "hook repo" +HOOK_REV="$(git -C "$HOOK_REPO" rev-parse HEAD)" + +# The two Vale hooks scope by filename, so the consumer needs one file of each +# shape: a hook with nothing to match reports `Skipped` and proves nothing. +mkdir -p "$CONSUMER/skills/demo" "$CONSUMER/agents" +git -C "$CONSUMER" init -q +cat > "$CONSUMER/.pre-commit-config.yaml" < "$CONSUMER/skills/demo/SKILL.md" < + Use when the caller wants a demonstration skill $body across two + physical lines of one folded block scalar. +--- + +Body. +EOF + cat > "$CONSUMER/agents/demo.md" < + Use when the caller wants a demonstration agent $body across two + physical lines of one folded block scalar. +--- + +Body. +EOF + git -C "$CONSUMER" add -A +} + +run_hooks() { + (cd "$CONSUMER" && pre-commit run --all-files 2>&1) || true +} + +# --- 1. Both hooks resolve their config and actually gate on a bad file --- +echo "" +echo "--- both Vale hooks run and fail a bad file in an external consumer repo ---" +write_fixtures "that helps with and utilize things" +OUT_BAD="$(run_hooks)" +if echo "$OUT_BAD" | grep -q "does not exist"; then + fail "hooks hard-errored on a path resolved against the consumer repo (E100) — the bug this test guards against" + echo "$OUT_BAD" | sed 's/^/ /' +elif echo "$OUT_BAD" | grep -q "Skipped"; then + fail "a hook matched no files, so it proved nothing" + echo "$OUT_BAD" | sed 's/^/ /' +elif [[ "$(echo "$OUT_BAD" | grep -c "VagueWording")" -ge 2 ]]; then + pass "both hooks flatten and flag the folded description in a consumer repo" +else + fail "hooks did not flag both fixtures" + echo "$OUT_BAD" | sed 's/^/ /' +fi + +# --- 2. Clean files pass — the hooks gate, they don't just always fail --- +echo "" +echo "--- both Vale hooks pass clean files in an external consumer repo ---" +write_fixtures "of the packaged hook contract" +set +e +(cd "$CONSUMER" && pre-commit run --all-files > "$WORK/clean.log" 2>&1) +RC_CLEAN=$? +set -e +if [[ $RC_CLEAN -eq 0 ]]; then + pass "both hooks exit 0 on clean files" +else + fail "hooks failed on clean files (rc=$RC_CLEAN)" + sed 's/^/ /' "$WORK/clean.log" +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 c6f057a..9816125 100755 --- a/tests/test-vale-wrap.sh +++ b/tests/test-vale-wrap.sh @@ -19,8 +19,8 @@ pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } if ! command -v vale &>/dev/null; then - echo "vale is not installed — skipping (matches skill-audit/agent-audit's own fallback behavior)" - exit 0 + echo "SKIP: vale is not installed — skipping (matches skill-audit/agent-audit's own fallback behavior)" + exit 77 fi make_fixture() { @@ -345,6 +345,86 @@ else fail "wrapper altered output for a literal (|) block scalar description — should be left untouched" fi +# --- 12. With no --config at all, the wrapper falls back to its own sibling +# assets/vale/.vale.ini. `.pre-commit-hooks.yaml` relies on this: pre-commit +# prefixes only entry[0] with the hook-repo clone path, so a --config argument +# there resolves against the consuming repo and hard-errors (E100) for every +# external consumer. +echo "" +echo "--- defaults --config to the wrapper's own sibling assets/vale/.vale.ini ---" +FIXTURE12="$(make_fixture 2)" +trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12"' EXIT +OUT12=$(run_wrap "$FIXTURE12" plugins/testplugin/skills/zzzskill/SKILL.md) +if echo "$OUT12" | grep -q "VagueWording"; then + pass "a --config-less invocation uses the wrapper's bundled config" +else + fail "a --config-less invocation found no config — external pre-commit consumers get E100, the bug this test guards against" +fi + +# --- 13. No GNU-only `realpath -m`. macOS ships the BSD realpath, which has no +# -m (canonicalize-missing) — and every scratch destination is a path that does +# not exist yet, so a plain `realpath` exits 1 and set -e aborts the hook. +echo "" +echo "--- runs with a BSD realpath that has no -m option ---" +STUB13="$(mktemp -d)" +trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12" "$STUB13"' EXIT +REAL_REALPATH="$(command -v realpath || echo /bin/false)" +cat > "$STUB13/realpath" <&2 + exit 1 + ;; + esac +done +exec "$REAL_REALPATH" "\$@" +EOF +chmod +x "$STUB13/realpath" +OUT13=$(cd "$FIXTURE12" && PATH="$STUB13:$PATH" bash "$SCRIPT" --config "$VALE_CONFIG" \ + plugins/testplugin/skills/zzzskill/SKILL.md 2>&1 || true) +if echo "$OUT13" | grep -q "illegal option"; then + fail "invoked realpath -m — fails on macOS's BSD realpath, the bug this test guards against" +elif echo "$OUT13" | grep -q "VagueWording"; then + pass "flattens and flags with no GNU realpath available" +else + fail "produced no alert under a BSD-style realpath: $OUT13" +fi + +# --- 14. A directory argument is walked and its files flattened. The classifier +# used to accept only regular files, so a directory fell through to the vale +# flag list, left the file list empty, and exec'd bare vale — silently skipping +# the flattening. `lint`'s vale-run skill documents `vale ` as +# normal usage, so this is a reachable path. +echo "" +echo "--- flattens files reached through a directory argument ---" +FIXTURE14="$(make_fixture 2)" +trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12" "$STUB13" "$FIXTURE14"' EXIT +WRAPPED_DIR=$(run_wrap "$FIXTURE14" --config "$VALE_CONFIG" plugins) +BARE_DIR=$(cd "$FIXTURE14" && vale --config "$VALE_CONFIG" plugins 2>&1 || true) +if ! echo "$WRAPPED_DIR" | grep -q "VagueWording"; then + fail "a directory argument produced no alert — flattening was silently skipped, the bug this test guards against" +elif echo "$BARE_DIR" | grep -q "VagueWording"; then + fail "bare vale already flags this fixture, so the test can't detect a silently-skipped flattening" +else + pass "a directory argument is walked and its files flattened" +fi + +# --- 15. Directory walking must survive paths with spaces --- +echo "" +echo "--- walks a directory containing a path with spaces ---" +SPACED15="$FIXTURE14/plugins/testplugin/skills/zzz skill" +mkdir -p "$SPACED15" +cp "$FIXTURE14/plugins/testplugin/skills/zzzskill/SKILL.md" "$SPACED15/SKILL.md" +rm -rf "$FIXTURE14/plugins/testplugin/skills/zzzskill" +OUT15=$(run_wrap "$FIXTURE14" --config "$VALE_CONFIG" plugins/testplugin/skills) +if echo "$OUT15" | grep -q "zzz skill" && echo "$OUT15" | grep -q "VagueWording"; then + pass "a file under a directory whose name contains a space is walked and flattened" +else + fail "a path with a space was dropped from the directory walk" +fi + echo "" echo "Results: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]]