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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58
This commit is contained in:
@@ -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
|
||||
|
||||
127
tests/test-vale-hooks-consumer.sh
Executable file
127
tests/test-vale-hooks-consumer.sh
Executable file
@@ -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" <<EOF
|
||||
repos:
|
||||
- repo: file://$HOOK_REPO
|
||||
rev: $HOOK_REV
|
||||
hooks:
|
||||
- id: kyberforge-vale-audit-skill
|
||||
- id: kyberforge-vale-audit-agent
|
||||
EOF
|
||||
|
||||
write_fixtures() {
|
||||
local body="$1"
|
||||
cat > "$CONSUMER/skills/demo/SKILL.md" <<EOF
|
||||
---
|
||||
name: demo
|
||||
description: >
|
||||
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" <<EOF
|
||||
---
|
||||
name: demo
|
||||
description: >
|
||||
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 ]]
|
||||
@@ -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" <<EOF
|
||||
#!/usr/bin/env bash
|
||||
for a in "\$@"; do
|
||||
case "\$a" in
|
||||
-m|--canonicalize-missing)
|
||||
echo "realpath: illegal option -- m" >&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 <path-or-glob>` 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 ]]
|
||||
|
||||
Reference in New Issue
Block a user