fix(lint): make the Vale gate actually gate, drop VagueQualifier
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
This commit is contained in:
@@ -83,31 +83,33 @@ make_line_fixture() {
|
||||
echo "$file"
|
||||
}
|
||||
|
||||
# The line ceiling is exclusive of the limit itself ("stay under $MAX_LINES
|
||||
# lines", per skill-authoring.md), enforced via `>=` — so $((MAX_LINES - 1))
|
||||
# must pass and $MAX_LINES itself must already fail.
|
||||
# The line ceiling is inclusive of the limit itself, enforced via `>` — so
|
||||
# exactly $MAX_LINES must pass and $((MAX_LINES + 1)) must fail. This matches
|
||||
# skill-audit/scripts/validate.sh's `line_count <= 500` pass condition; the two
|
||||
# previously disagreed at exactly $MAX_LINES lines, so a SKILL.md could pass its
|
||||
# own audit and still be blocked by the commit hook.
|
||||
echo ""
|
||||
echo "--- passes a file at $((MAX_LINES - 1)) lines, just under the $MAX_LINES-line boundary ---"
|
||||
AT_LINES="$(make_line_fixture at-line-limit "$((MAX_LINES - 1))")"
|
||||
echo "--- passes a file at exactly the $MAX_LINES-line boundary ---"
|
||||
AT_LINES="$(make_line_fixture at-line-limit "$MAX_LINES")"
|
||||
ACTUAL_LINES=$(awk 'END{print NR}' "$AT_LINES")
|
||||
if [[ "$ACTUAL_LINES" -ne "$((MAX_LINES - 1))" ]]; then
|
||||
fail "fixture has $ACTUAL_LINES lines, expected exactly $((MAX_LINES - 1))"
|
||||
if [[ "$ACTUAL_LINES" -ne "$MAX_LINES" ]]; then
|
||||
fail "fixture has $ACTUAL_LINES lines, expected exactly $MAX_LINES"
|
||||
elif "$SCRIPT" "$AT_LINES"; then
|
||||
pass "file at $((MAX_LINES - 1)) lines exits 0"
|
||||
pass "file at exactly $MAX_LINES lines exits 0"
|
||||
else
|
||||
fail "file at $((MAX_LINES - 1)) lines should have exited 0"
|
||||
fail "file at exactly $MAX_LINES lines should have exited 0 (the off-by-one this test guards against)"
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "--- fails a file at exactly the $MAX_LINES-line boundary ---"
|
||||
OVER_LINES="$(make_line_fixture over-line-limit "$MAX_LINES")"
|
||||
echo "--- fails a file one line over the $MAX_LINES-line boundary ---"
|
||||
OVER_LINES="$(make_line_fixture over-line-limit "$((MAX_LINES + 1))")"
|
||||
ACTUAL_OVER_LINES=$(awk 'END{print NR}' "$OVER_LINES")
|
||||
if [[ "$ACTUAL_OVER_LINES" -ne "$MAX_LINES" ]]; then
|
||||
fail "fixture has $ACTUAL_OVER_LINES lines, expected exactly $MAX_LINES"
|
||||
if [[ "$ACTUAL_OVER_LINES" -ne "$((MAX_LINES + 1))" ]]; then
|
||||
fail "fixture has $ACTUAL_OVER_LINES lines, expected exactly $((MAX_LINES + 1))"
|
||||
elif "$SCRIPT" "$OVER_LINES" 2>/dev/null; then
|
||||
fail "file at exactly $MAX_LINES lines should have exited non-zero (>= ceiling, the boundary bug this test guards against)"
|
||||
fail "file at $((MAX_LINES + 1)) lines should have exited non-zero"
|
||||
else
|
||||
pass "file at exactly $MAX_LINES lines exits non-zero"
|
||||
pass "file at $((MAX_LINES + 1)) lines exits non-zero"
|
||||
fi
|
||||
|
||||
# make_word_fixture builds a file with an exact total word count (frontmatter
|
||||
|
||||
@@ -39,13 +39,24 @@ make_fixture() {
|
||||
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 (cd "$FIXTURE1" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" \
|
||||
plugins/testplugin/skills/zzzskill/SKILL.md) | grep -q "VagueWording"; then
|
||||
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"
|
||||
@@ -56,8 +67,8 @@ echo ""
|
||||
echo "--- catches vague wording in a multi-line folded description ---"
|
||||
FIXTURE2="$(make_fixture 2)"
|
||||
trap 'rm -rf "$FIXTURE1" "$FIXTURE2"' EXIT
|
||||
if (cd "$FIXTURE2" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" \
|
||||
plugins/testplugin/skills/zzzskill/SKILL.md) | grep -q "VagueWording"; then
|
||||
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"
|
||||
@@ -69,8 +80,8 @@ 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=$(cd "$FIXTURE3" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" \
|
||||
plugins/testplugin/skills/zzzskill/SKILL.md 2>&1 || true)
|
||||
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"
|
||||
@@ -107,8 +118,8 @@ Body.
|
||||
EOF
|
||||
)"
|
||||
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4"' EXIT
|
||||
if (cd "$FIXTURE4" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" \
|
||||
plugins/testplugin/skills/zzzskill/SKILL.md) | grep -q "VagueWording"; then
|
||||
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"
|
||||
@@ -129,8 +140,8 @@ Body.
|
||||
EOF
|
||||
)"
|
||||
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5"' EXIT
|
||||
OUT5=$(cd "$FIXTURE5" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" \
|
||||
plugins/testplugin/skills/zzzskill/SKILL.md 2>&1)
|
||||
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
|
||||
@@ -157,8 +168,8 @@ Body.
|
||||
EOF
|
||||
)"
|
||||
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6"' EXIT
|
||||
if (cd "$FIXTURE6" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" \
|
||||
plugins/testplugin/skills/zzzskill/SKILL.md) | grep -q "VagueWording"; then
|
||||
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"
|
||||
@@ -180,8 +191,8 @@ Body.
|
||||
EOF
|
||||
)"
|
||||
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7"' EXIT
|
||||
OUT7=$(cd "$FIXTURE7" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" \
|
||||
plugins/testplugin/skills/zzzskill/SKILL.md 2>&1)
|
||||
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
|
||||
@@ -190,9 +201,16 @@ else
|
||||
fail "silently missed vague wording in the second paragraph after a blank line — the bug this test guards against"
|
||||
fi
|
||||
|
||||
# --- 8. --config=<path> (equals form) resolves the same as the two-argv form ---
|
||||
# --- 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 "--- --config=<path> equals form resolves from a subdirectory like the two-argv form ---"
|
||||
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"
|
||||
@@ -210,13 +228,53 @@ mkdir -p "$FIXTURE8/plugins/testplugin/skills/zzzskill"
|
||||
} > "$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"
|
||||
REL8="plugins/testplugin/skills/zzzskill/SKILL.md"
|
||||
OUT_EQ=$(cd "$SUBDIR8" && bash "$SCRIPT" --config=.vale.ini "$REL8" 2>&1)
|
||||
OUT_TWO=$(cd "$SUBDIR8" && bash "$SCRIPT" --config .vale.ini "$REL8" 2>&1)
|
||||
# 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 "--config=<path> from a subdirectory resolves and matches the two-argv form"
|
||||
pass "cwd-relative --config resolves from a subdirectory in both argv forms"
|
||||
else
|
||||
fail "--config=<path> equals form did not resolve the same as the two-argv form"
|
||||
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 ---
|
||||
@@ -252,10 +310,10 @@ 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 (cd "$FIXTURE10" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" "$ABS_FILE10") | grep -q "VagueWording"; then
|
||||
pass "an absolute path under repo_root is linted, not silently skipped"
|
||||
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 under repo_root was silently skipped — the bug this test guards against"
|
||||
fail "an absolute path was silently skipped — the bug this test guards against"
|
||||
fi
|
||||
|
||||
# --- 11. A literal (|) block scalar passes through unflattened (no regression) ---
|
||||
@@ -274,7 +332,7 @@ EOF
|
||||
)"
|
||||
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11"' EXIT
|
||||
REL11="plugins/testplugin/skills/zzzskill/SKILL.md"
|
||||
WRAPPED_OUT=$(cd "$FIXTURE11" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" "$REL11" 2>&1 || true)
|
||||
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"
|
||||
|
||||
Reference in New Issue
Block a user