Round-3 review of PR #85 found the "enforcing" pre-commit hook enforced nothing. Vale's exit code keys on error-level alerts alone: five of the six rules were level: warning, so they exited 0, and pre-commit hides output from a passing hook — the alerts were invisible and blocked nothing. ADR-0013 rejected a report-only trial tier and then shipped one by accident. Flatten every rule to level: error. Vale's own exit code is then correct, so the hook entry drops to a bare vale-wrap.sh call and the graded error->FAIL / warning->SUGGESTION mapping disappears from both audit skills: every alert is a FAIL, in the gate and the audit alike. No ignorable tier, matching shellcheck, the test suite and conventional-pre-commit. Delete Kyberforge.VagueQualifier. Measured against the 41 skill/agent files as they stood before the rule ever ran: 2 hits. One marginal ("very different" -> "fundamentally different"), one an unfixable false positive — caveman/SKILL.md quotes "of course" as an example of filler, a mention not a use — which forced the only Vale suppression comments in the repo. Those four lines go with it; two of them were dead anyway, suppressing a frontmatter-scoped rule on a body line. Held-out prose (273 files) fired 15 times, 9 inside out-of-scope research examples and the rest one word in two idioms in a single doc. SentenceOpenerThereIs survives: 22 held-out hits, both in-corpus hits clean rewrites, zero suppressions. Widen .vale.ini's globs to [**/SKILL.md], [**/agents/*.md] and [**/*.agent.md]. The plugins/*/-prefixed globs scoped nothing — Vale's * crosses /, so they already matched docs/research/examples/**/agents/*.md and assets/templates/SKILL.md, the two paths CONTEXT.md claimed they excluded. Scoping is and was the hook's files: regex. The old globs also hid a silent false negative: a skill outside plugins/ matched no section, so Vale reported 0 files and exited 0, which both audits read as clean. They now treat a 0-file run as NOT RUN and fall back to full judgment. Also: - vale-wrap.sh resolves relative --config values and file arguments against the caller's cwd, as vale does, instead of the repo root, which hard-errored from a subdirectory and silently skipped flattening for file args that did not resolve from the root. Absolute paths inside the cwd are relativized so reports cite resolvable paths, not scratch ones. - vale-run's exit-code model was documented backwards ("exits non-zero whenever it finds an alert at or above MinAlertLevel") and would have led anyone following it to build a gate that passes everything. Its Markdown suppression syntax was MDX-only and does not suppress in .md; corrected in the skill and its troubleshooting reference, with backtick/fence exemption documented as the first resort. - skill-size-check.sh fails only above 500 lines, agreeing with skill-audit's validate.sh <= 500 pass. - ADR-0013 and CONTEXT.md amended to match, recording why graded severities cannot gate. Verified: 9 test scripts / 15 vale-wrap cases pass; vale-audit-prefilter, skill-size-check and shellcheck pass --all-files; check-manifests and claude plugin validate --strict clean. New tests fail against the old script (3 of them) and pass against the new one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58
346 lines
14 KiB
Bash
Executable File
346 lines
14 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# Regression test for scripts/vale-wrap.sh: Vale's `text.frontmatter.description`
|
|
# NLP scope silently stops matching when the description value is a YAML block
|
|
# scalar spanning 2+ physical lines. vale-wrap.sh flattens it to one line before
|
|
# handing off to the real vale binary — this asserts that actually happens.
|
|
set -euo pipefail
|
|
|
|
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
|
SCRIPT="$REPO_ROOT/scripts/vale-wrap.sh"
|
|
PASS=0
|
|
FAIL=0
|
|
|
|
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
|
|
fi
|
|
|
|
make_fixture() {
|
|
local dir desc_lines file
|
|
dir="$(mktemp -d)"
|
|
(cd "$dir" && git init -q)
|
|
mkdir -p "$dir/plugins/testplugin/skills/zzzskill"
|
|
desc_lines="$1"
|
|
file="$dir/plugins/testplugin/skills/zzzskill/SKILL.md"
|
|
{
|
|
echo "---"
|
|
echo "name: zzzskill"
|
|
echo "description: >"
|
|
for ((i = 1; i <= desc_lines; i++)); do
|
|
echo " Line $i mentions helps with and utilize, plus a colon: like this."
|
|
done
|
|
echo "---"
|
|
echo ""
|
|
echo "Body."
|
|
} > "$file"
|
|
echo "$dir"
|
|
}
|
|
|
|
# Every Kyberforge rule is `level: error`, so vale exits non-zero whenever a
|
|
# fixture trips one — which is the expected outcome for nearly every case here.
|
|
# run_wrap therefore captures output and swallows the exit status; assertions
|
|
# are made on the report text. Cases that genuinely care about the exit code
|
|
# capture it explicitly instead.
|
|
run_wrap() {
|
|
local dir="$1"
|
|
shift
|
|
(cd "$dir" && bash "$SCRIPT" "$@" 2>&1) || true
|
|
}
|
|
|
|
# --- 1. A known-bad single-line description is caught (sanity check on Vale itself) ---
|
|
echo ""
|
|
echo "--- catches vague wording in a single-line description ---"
|
|
FIXTURE1="$(make_fixture 1)"
|
|
trap 'rm -rf "$FIXTURE1"' EXIT
|
|
if run_wrap "$FIXTURE1" --config "$REPO_ROOT/.vale.ini" \
|
|
plugins/testplugin/skills/zzzskill/SKILL.md | grep -q "VagueWording"; then
|
|
pass "flags vague wording when description is a single physical line"
|
|
else
|
|
fail "did not flag known-bad single-line description"
|
|
fi
|
|
|
|
# --- 2. The same known-bad wording across 2+ physical lines is still caught ---
|
|
echo ""
|
|
echo "--- catches vague wording in a multi-line folded description ---"
|
|
FIXTURE2="$(make_fixture 2)"
|
|
trap 'rm -rf "$FIXTURE1" "$FIXTURE2"' EXIT
|
|
if run_wrap "$FIXTURE2" --config "$REPO_ROOT/.vale.ini" \
|
|
plugins/testplugin/skills/zzzskill/SKILL.md | grep -q "VagueWording"; then
|
|
pass "flags vague wording when description spans 2+ physical lines"
|
|
else
|
|
fail "silently missed known-bad wording in a multi-line description — the bug this test guards against"
|
|
fi
|
|
|
|
# --- 3. Line count is preserved so unrelated report line numbers don't shift ---
|
|
echo ""
|
|
echo "--- preserves total line count when flattening ---"
|
|
FIXTURE3="$(make_fixture 3)"
|
|
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3"' EXIT
|
|
ORIG_LINES=$(wc -l < "$FIXTURE3/plugins/testplugin/skills/zzzskill/SKILL.md")
|
|
OUT=$(run_wrap "$FIXTURE3" --config "$REPO_ROOT/.vale.ini" \
|
|
plugins/testplugin/skills/zzzskill/SKILL.md)
|
|
MAX_LINE=$(echo "$OUT" | grep -oE '^[[:space:]]*[0-9]+:[0-9]+' | tr -d '[:space:]' | cut -d: -f1 | sort -n | tail -1)
|
|
if [[ -n "$MAX_LINE" ]] && (( MAX_LINE <= ORIG_LINES )); then
|
|
pass "reported line numbers stay within the original file's line count"
|
|
else
|
|
fail "reported line number ($MAX_LINE) exceeds original file line count ($ORIG_LINES)"
|
|
fi
|
|
|
|
# make_raw_fixture writes stdin verbatim to a fresh fixture's SKILL.md, for
|
|
# cases where the exact description body needs to be hand-crafted rather than
|
|
# generated from the desc_lines loop above.
|
|
make_raw_fixture() {
|
|
local dir
|
|
dir="$(mktemp -d)"
|
|
(cd "$dir" && git init -q)
|
|
mkdir -p "$dir/plugins/testplugin/skills/zzzskill"
|
|
cat > "$dir/plugins/testplugin/skills/zzzskill/SKILL.md"
|
|
echo "$dir"
|
|
}
|
|
|
|
# --- 4. A folded description containing a double quote is still caught ---
|
|
# This is the exact case that silently passed (zero alerts) before switching
|
|
# from json.dumps (double-quoted, backslash-escaped) to a single-quoted scalar.
|
|
echo ""
|
|
echo "--- catches vague wording when the folded description contains a double quote ---"
|
|
FIXTURE4="$(make_raw_fixture <<'EOF'
|
|
---
|
|
name: zzzskill
|
|
description: >
|
|
Use when the user says "audit this skill" and helps with and utilize things.
|
|
Second line continues the same folded scalar for flattening.
|
|
---
|
|
|
|
Body.
|
|
EOF
|
|
)"
|
|
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4"' EXIT
|
|
if run_wrap "$FIXTURE4" --config "$REPO_ROOT/.vale.ini" \
|
|
plugins/testplugin/skills/zzzskill/SKILL.md | grep -q "VagueWording"; then
|
|
pass "flags vague wording when the description contains a double quote"
|
|
else
|
|
fail "silently missed vague wording in a description containing a double quote — the bug this test guards against"
|
|
fi
|
|
|
|
# --- 5. A folded description containing an apostrophe fires and stays valid YAML ---
|
|
echo ""
|
|
echo "--- catches vague wording when the folded description contains an apostrophe ---"
|
|
FIXTURE5="$(make_raw_fixture <<'EOF'
|
|
---
|
|
name: zzzskill
|
|
description: >
|
|
Use when the user's task helps with and utilize things across two lines.
|
|
Second continuation line for the fold.
|
|
---
|
|
|
|
Body.
|
|
EOF
|
|
)"
|
|
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5"' EXIT
|
|
OUT5=$(run_wrap "$FIXTURE5" --config "$REPO_ROOT/.vale.ini" \
|
|
plugins/testplugin/skills/zzzskill/SKILL.md)
|
|
if echo "$OUT5" | grep -q "VagueWording"; then
|
|
pass "flags vague wording when the description contains an apostrophe"
|
|
else
|
|
fail "silently missed vague wording in a description containing an apostrophe"
|
|
fi
|
|
if echo "$OUT5" | grep -qi "yaml:"; then
|
|
fail "flattened copy with an apostrophe produced a YAML parse error"
|
|
else
|
|
pass "flattened copy with an apostrophe is valid YAML (no parse error)"
|
|
fi
|
|
|
|
# --- 6. A folded description with a backslash and a non-ASCII character ---
|
|
echo ""
|
|
echo "--- catches vague wording when the folded description has a backslash and non-ASCII text ---"
|
|
FIXTURE6="$(make_raw_fixture <<'EOF'
|
|
---
|
|
name: zzzskill
|
|
description: >
|
|
Use when the café résumé naïve thing helps with and utilize things here.
|
|
Path is C:\Users\test and this is the second continuation line.
|
|
---
|
|
|
|
Body.
|
|
EOF
|
|
)"
|
|
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6"' EXIT
|
|
if run_wrap "$FIXTURE6" --config "$REPO_ROOT/.vale.ini" \
|
|
plugins/testplugin/skills/zzzskill/SKILL.md | grep -q "VagueWording"; then
|
|
pass "flags vague wording when the description has a backslash and non-ASCII text"
|
|
else
|
|
fail "silently missed vague wording in a description with a backslash and non-ASCII text"
|
|
fi
|
|
|
|
# --- 7. A folded description with a blank line between two paragraphs ---
|
|
echo ""
|
|
echo "--- handles a blank line inside a folded description without crashing ---"
|
|
FIXTURE7="$(make_raw_fixture <<'EOF'
|
|
---
|
|
name: zzzskill
|
|
description: >
|
|
Use when the user needs a general helper.
|
|
|
|
Do not use when this helps with and utilize things instead.
|
|
---
|
|
|
|
Body.
|
|
EOF
|
|
)"
|
|
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7"' EXIT
|
|
OUT7=$(run_wrap "$FIXTURE7" --config "$REPO_ROOT/.vale.ini" \
|
|
plugins/testplugin/skills/zzzskill/SKILL.md)
|
|
if echo "$OUT7" | grep -q "Traceback"; then
|
|
fail "crashed while flattening a description with a blank line between paragraphs"
|
|
elif echo "$OUT7" | grep -q "VagueWording"; then
|
|
pass "still flags vague wording in the second paragraph after a blank line"
|
|
else
|
|
fail "silently missed vague wording in the second paragraph after a blank line — the bug this test guards against"
|
|
fi
|
|
|
|
# --- 8. Relative paths resolve against the caller's cwd, exactly as bare vale
|
|
# resolves them. Every path below is deliberately relative to $SUBDIR8, not to
|
|
# the fixture's repo root: an earlier version of the wrapper resolved relative
|
|
# paths against the git toplevel instead, which (a) hard-errored on a
|
|
# `--config ../../..` that bare vale accepts and (b) silently dropped file
|
|
# arguments that didn't resolve from the repo root, skipping the flattening the
|
|
# wrapper exists to perform. The old tests only ever passed repo-root-relative
|
|
# paths from a subdirectory, so neither failure mode was caught.
|
|
echo ""
|
|
echo "--- resolves a cwd-relative --config from a subdirectory (equals and two-argv forms) ---"
|
|
FIXTURE8="$(mktemp -d)"
|
|
(cd "$FIXTURE8" && git init -q)
|
|
cp "$REPO_ROOT/.vale.ini" "$FIXTURE8/.vale.ini"
|
|
cp -r "$REPO_ROOT/styles" "$FIXTURE8/styles"
|
|
mkdir -p "$FIXTURE8/plugins/testplugin/skills/zzzskill"
|
|
{
|
|
echo "---"
|
|
echo "name: zzzskill"
|
|
echo "description: >"
|
|
echo " Line one mentions helps with and utilize, plus a colon: like this."
|
|
echo " Line two continues the same folded scalar for flattening."
|
|
echo "---"
|
|
echo ""
|
|
echo "Body."
|
|
} > "$FIXTURE8/plugins/testplugin/skills/zzzskill/SKILL.md"
|
|
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8"' EXIT
|
|
SUBDIR8="$FIXTURE8/plugins/testplugin/skills/zzzskill"
|
|
# Both paths are relative to $SUBDIR8 (four levels below the fixture root).
|
|
REL_CFG8="../../../../.vale.ini"
|
|
REL_FILE8="SKILL.md"
|
|
OUT_EQ=$(run_wrap "$SUBDIR8" "--config=$REL_CFG8" "$REL_FILE8")
|
|
OUT_TWO=$(run_wrap "$SUBDIR8" --config "$REL_CFG8" "$REL_FILE8")
|
|
if echo "$OUT_EQ" | grep -q "VagueWording" && [[ "$OUT_EQ" == "$OUT_TWO" ]]; then
|
|
pass "cwd-relative --config resolves from a subdirectory in both argv forms"
|
|
else
|
|
fail "cwd-relative --config did not resolve from a subdirectory (equals form vs two-argv form)"
|
|
fi
|
|
|
|
# --- 8b. A cwd-relative --config matches the equivalent absolute invocation ---
|
|
# Regression for failure mode (a): resolving --config against the repo root made
|
|
# `--config ../../../../.vale.ini` expand to a path above the toplevel, and vale
|
|
# hard-errored with "does not exist" (exit 2) on args bare vale handles fine.
|
|
echo ""
|
|
echo "--- a cwd-relative --config produces the same result as the absolute-path form ---"
|
|
set +e
|
|
OUT_REL_CFG=$(cd "$SUBDIR8" && bash "$SCRIPT" --config "$REL_CFG8" "$REL_FILE8" 2>&1)
|
|
RC_REL_CFG=$?
|
|
OUT_ABS_CFG=$(cd "$SUBDIR8" && bash "$SCRIPT" --config "$FIXTURE8/.vale.ini" "$REL_FILE8" 2>&1)
|
|
RC_ABS_CFG=$?
|
|
set -e
|
|
if echo "$OUT_REL_CFG" | grep -qi "does not exist"; then
|
|
fail "cwd-relative --config hard-errored ('does not exist') — the bug this test guards against"
|
|
elif [[ "$OUT_REL_CFG" == "$OUT_ABS_CFG" && "$RC_REL_CFG" -eq "$RC_ABS_CFG" ]]; then
|
|
pass "cwd-relative --config matches the absolute-path invocation (output and exit code)"
|
|
else
|
|
fail "cwd-relative --config (rc=$RC_REL_CFG) diverged from the absolute-path form (rc=$RC_ABS_CFG)"
|
|
fi
|
|
|
|
# --- 8c. A cwd-relative FILE argument is still flattened, not silently skipped ---
|
|
# Regression for failure mode (b): a relative file path that didn't resolve from
|
|
# the repo root failed the wrapper's file test, fell through to the vale flag
|
|
# list, and left the file list empty — so the wrapper exec'd bare vale and
|
|
# silently skipped the flattening. Bare vale reports nothing here, so asserting
|
|
# on the alert (not just the exit code) is what makes the silence detectable.
|
|
echo ""
|
|
echo "--- flattens a cwd-relative file argument passed from a subdirectory ---"
|
|
WRAPPED_REL=$(run_wrap "$SUBDIR8" --config "$FIXTURE8/.vale.ini" "$REL_FILE8")
|
|
BARE_REL=$(cd "$SUBDIR8" && vale --config "$FIXTURE8/.vale.ini" "$REL_FILE8" 2>&1 || true)
|
|
if ! echo "$WRAPPED_REL" | grep -q "VagueWording"; then
|
|
fail "cwd-relative file argument produced no alert — flattening was silently skipped, the bug this test guards against"
|
|
elif echo "$BARE_REL" | grep -q "VagueWording"; then
|
|
fail "bare vale already flags this fixture, so the test can't detect a silently-skipped flattening"
|
|
else
|
|
pass "cwd-relative file argument is flattened and flagged where bare vale reports nothing"
|
|
fi
|
|
|
|
# --- 9. Zero file args (or a file list that filters to nothing) exits promptly ---
|
|
echo ""
|
|
echo "--- exits promptly instead of hanging on stdin when no files are passed ---"
|
|
if timeout 5 bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" < <(sleep 100) >/dev/null 2>&1; then
|
|
pass "exits promptly with zero file args"
|
|
else
|
|
RC=$?
|
|
if [[ $RC -eq 124 ]]; then
|
|
fail "hung waiting on stdin with zero file args — the bug this test guards against"
|
|
else
|
|
pass "exits promptly (nonzero exit) with zero file args"
|
|
fi
|
|
fi
|
|
|
|
echo ""
|
|
echo "--- exits promptly when a file list filters down to nothing ---"
|
|
if timeout 5 bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" --no-such-flag < <(sleep 100) >/dev/null 2>&1; then
|
|
pass "exits promptly when no file-shaped args remain"
|
|
else
|
|
RC=$?
|
|
if [[ $RC -eq 124 ]]; then
|
|
fail "hung waiting on stdin when the file list filtered to nothing — the bug this test guards against"
|
|
else
|
|
pass "exits promptly (nonzero exit) when the file list filters to nothing"
|
|
fi
|
|
fi
|
|
|
|
# --- 10. An absolute path to the fixture SKILL.md is still linted, not skipped ---
|
|
echo ""
|
|
echo "--- lints an absolute path to a skill file instead of silently skipping it ---"
|
|
FIXTURE10="$(make_fixture 2)"
|
|
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10"' EXIT
|
|
ABS_FILE10="$FIXTURE10/plugins/testplugin/skills/zzzskill/SKILL.md"
|
|
if run_wrap "$FIXTURE10" --config "$REPO_ROOT/.vale.ini" "$ABS_FILE10" | grep -q "VagueWording"; then
|
|
pass "an absolute path is linted, not silently skipped"
|
|
else
|
|
fail "an absolute path was silently skipped — the bug this test guards against"
|
|
fi
|
|
|
|
# --- 11. A literal (|) block scalar passes through unflattened (no regression) ---
|
|
echo ""
|
|
echo "--- leaves a literal (|) block scalar untouched (narrowed >-only scope) ---"
|
|
FIXTURE11="$(make_raw_fixture <<'EOF'
|
|
---
|
|
name: zzzskill
|
|
description: |
|
|
Line one mentions helps with and utilize things here.
|
|
Line two continues the literal block scalar for this test.
|
|
---
|
|
|
|
Body.
|
|
EOF
|
|
)"
|
|
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11"' EXIT
|
|
REL11="plugins/testplugin/skills/zzzskill/SKILL.md"
|
|
WRAPPED_OUT=$(run_wrap "$FIXTURE11" --config "$REPO_ROOT/.vale.ini" "$REL11")
|
|
BARE_OUT=$(cd "$FIXTURE11" && vale --config "$REPO_ROOT/.vale.ini" "$REL11" 2>&1 || true)
|
|
if [[ "$WRAPPED_OUT" == "$BARE_OUT" ]]; then
|
|
pass "literal (|) block scalar output matches bare vale exactly — untouched by flattening"
|
|
else
|
|
fail "wrapper altered output for a literal (|) block scalar description — should be left untouched"
|
|
fi
|
|
|
|
echo ""
|
|
echo "Results: $PASS passed, $FAIL failed"
|
|
[[ $FAIL -eq 0 ]]
|