Files
holocron/tests/test-adr0020-body-checks.sh
Defame1297 c8a7c9ea87 chore: fold skill-frontmatter into skill-size-check
skill-frontmatter was a 62-line bash script inlined in
.pre-commit-config.yaml, re-parsing SKILL.md frontmatter with grep and
awk to check for name/description/metadata.version fields.
skill-size-check.sh already parses the same frontmatter block with
PyYAML for its ADR-0020 checks, so the two checks belonged in one
script.

Adds a ~20-line required-frontmatter check (name, description,
metadata.version as three-part semver) to scripts/skill-size-check.sh.
Removes the inline skill-frontmatter hook from .pre-commit-config.yaml
and deletes tests/test-skill-frontmatter.sh (366 lines). Removes the
79-line "the other hook on that scope" discussion from
docs/spec/gates.md and its now-dangling cross-reference, replacing
both with a one-line note of the fold, and updates the pre-push hook
counts there.

Updates fixture builders in test-skill-size-check.sh,
test-adr0020-body-checks.sh, test-adr0020-targets.sh,
test-adr0020-differential.sh, and test-vale-hooks-consumer.sh to carry
valid metadata.version so the new check doesn't spuriously fail
existing fixtures that predate it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
2026-09-12 20:00:43 +00:00

479 lines
15 KiB
Bash
Executable File

#!/usr/bin/env bash
# Regression test for the ADR-0020 body-shape checks and, just as importantly,
# for the false-positive fixes each of them needed. Every check here was
# completely untested.
#
# * Gotchas section over 5 entries — SUGGESTION
# * Gotchas section over 25% of the body — SUGGESTION
# * a references/<file>.md named but absent — ERROR (a broken pointer is not a
# style opinion)
# * description with no boundary clause — SUGGESTION
#
# The false-positive half is not optional extra coverage. Each of these checks
# scans prose, and the first naive version of each one fired on ordinary writing:
# a ```-fenced EXAMPLE of a Gotchas section became the section itself, indented
# child bullets were counted as top-level entries, `## Gotcha handling` was read
# as the Gotchas section, and a documented-then-removed references/ file became a
# hard ERROR. The skills most likely to carry such an example are skill-author and
# skill-audit — the two that DOCUMENT these conventions — so a gate that fires on
# them is a gate nobody can turn on.
#
# Every case is a matched pair: the check fires just over its boundary, and stays
# silent just under it (or on the shape it must not match). A test asserting only
# that a bad file fails proves nothing about a check that fires on everything.
set -euo pipefail
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
HOOK="$REPO_ROOT/scripts/skill-size-check.sh"
PASS=0
FAIL=0
pass() { echo " PASS: $1"; PASS=$((PASS + 1)); }
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
TMPDIR_T="$(mktemp -d)"
trap 'rm -rf "$TMPDIR_T"' EXIT
# A description with a boundary clause and no routing target — the neutral
# default, so a fixture about Gotchas or references does not also trip the
# missing-boundary-clause SUGGESTION and stop isolating what it names.
CLEAN_DESC="Use when doing the thing. Do not use for anything else."
# make_skill <name> <desc> — SKILL.md with the body read from stdin. Echoes the
# path. Each skill gets its own directory so references/ fixtures are isolated.
make_skill() {
local name="$1" desc="$2" dir
dir="$TMPDIR_T/$name"
mkdir -p "$dir"
{
echo "---"
echo "name: $name"
echo "description: $desc"
echo "metadata:"
echo " version: \"1.0.0\""
echo "---"
cat
} > "$dir/SKILL.md"
echo "$dir/SKILL.md"
}
# expect <label> <file> <expect: silent|suggests|errors> [needle] [absent-needle]
#
# `silent` is the strict one: exit 0 AND completely empty output. It is what
# makes every "must not fire" case below real — a check that fired with some
# other wording would still be caught.
expect() {
local label="$1" file="$2" mode="$3" needle="${4:-}" absent="${5:-}" out status=0
set +e
out="$(bash "$HOOK" "$file" 2>&1)"
status=$?
set -e
case "$mode" in
silent)
if [[ $status -eq 0 && -z "$out" ]]; then
pass "$label"
else
fail "$label (exit $status, output: ${out:-<empty>})"
fi
;;
suggests)
if [[ $status -ne 0 ]]; then
fail "$label — a SUGGESTION must never change the exit code (exit $status, output: $out)"
elif [[ "$out" != *"SUGGESTION"* || "$out" != *"$needle"* ]]; then
fail "$label (exit $status, output: ${out:-<empty>})"
elif [[ -n "$absent" && "$out" == *"$absent"* ]]; then
fail "$label — output also contained '$absent', which must not fire here: $out"
else
pass "$label"
fi
;;
errors)
if [[ $status -eq 0 ]]; then
fail "$label — expected a hard ERROR, got exit 0 (output: ${out:-<empty>})"
elif [[ "$out" != *"$needle"* ]]; then
fail "$label (exit $status, output: ${out:-<empty>})"
else
pass "$label"
fi
;;
quiet-about)
# Exit 0 and the named text absent, but other output permitted. Used where
# a second, unrelated finding legitimately fires.
if [[ $status -ne 0 ]]; then
fail "$label (exit $status, output: $out)"
elif [[ "$out" == *"$needle"* ]]; then
fail "$label — '$needle' fired when it must not: $out"
else
pass "$label"
fi
;;
esac
}
filler() { python3 -c "print(' '.join(['word'] * $1))"; }
# ---------------------------------------------------------------------------
# Gotchas: entry count (guideline 5)
# ---------------------------------------------------------------------------
# The filler after the section keeps the 25% fraction check well clear, so these
# two cases isolate the ENTRY count. Without it a six-entry section in a short
# body would fire both and the pair would not distinguish them.
echo ""
echo "--- Gotchas entry count: 5 is fine, 6 is a SUGGESTION ---"
F_FIVE="$(make_skill gotchas-five "$CLEAN_DESC" <<EOF
## Gotchas
- first trap here
- second trap here
- third trap here
- fourth trap here
- fifth trap here
## Notes
$(filler 200)
EOF
)"
expect "a Gotchas section with exactly 5 entries is silent" "$F_FIVE" silent
F_SIX="$(make_skill gotchas-six "$CLEAN_DESC" <<EOF
## Gotchas
- first trap here
- second trap here
- third trap here
- fourth trap here
- fifth trap here
- sixth trap here
## Notes
$(filler 200)
EOF
)"
expect "a Gotchas section with 6 entries raises a SUGGESTION and still exits 0" \
"$F_SIX" suggests "Gotchas section has 6 entries" "over the 25% guideline"
# ---------------------------------------------------------------------------
# Gotchas: share of the body (guideline 25%)
# ---------------------------------------------------------------------------
# Exact boundary arithmetic, not an approximation. The section is prose (no list
# items) so the entry check cannot fire and confuse the result; body words are
# then section + filler + the two two-word headings. At a body of 100 words a
# 25-word section is exactly the guideline (inclusive — `>` is the comparison, so
# it passes) and a 26-word section is one word past it.
echo ""
echo "--- Gotchas share of body: exactly 25% is fine, 26% is a SUGGESTION ---"
F_AT="$(make_skill gotchas-at-fraction "$CLEAN_DESC" <<EOF
## Gotchas
$(filler 25)
## Notes
$(filler 71)
EOF
)"
expect "a Gotchas section at exactly 25% of the body is silent" "$F_AT" silent
F_OVER="$(make_skill gotchas-over-fraction "$CLEAN_DESC" <<EOF
## Gotchas
$(filler 26)
## Notes
$(filler 70)
EOF
)"
expect "a Gotchas section at 26% of the body raises a SUGGESTION" \
"$F_OVER" suggests "Gotchas section is 26 of 100 body words (26%)" "entries"
# ---------------------------------------------------------------------------
# Gotchas: false positives
# ---------------------------------------------------------------------------
echo ""
echo "--- a ## Gotchas heading inside a fenced block is not the Gotchas section ---"
# skill-author and skill-audit both document this convention by showing it. If a
# fenced example counted, the two skills that define the rule would be the two
# most likely to fail it.
F_FENCED_HEADING="$(make_skill gotchas-fenced-heading "$CLEAN_DESC" <<EOF
## How to write one
\`\`\`markdown
## Gotchas
- example one
- example two
- example three
- example four
- example five
- example six
- example seven
\`\`\`
$(filler 200)
EOF
)"
expect "a fenced ## Gotchas heading is not read as the section" \
"$F_FENCED_HEADING" quiet-about "Gotchas section"
echo ""
echo "--- list items inside a fenced block do not count as Gotchas entries ---"
F_FENCED_ENTRIES="$(make_skill gotchas-fenced-entries "$CLEAN_DESC" <<EOF
## Gotchas
- a real trap
- another real trap
Shown as an example of what NOT to write:
\`\`\`markdown
- fake one
- fake two
- fake three
- fake four
- fake five
- fake six
- fake seven
- fake eight
\`\`\`
## Notes
$(filler 300)
EOF
)"
expect "eight fenced bullets plus two real ones counts as two entries, not ten" \
"$F_FENCED_ENTRIES" quiet-about "Gotchas section has"
echo ""
echo "--- the heading must END in gotcha(s): '## Gotcha handling' is not the section ---"
# `## Gotcha handling` and `## Why gotchas matter` are prose sections. Treating
# one as the Gotchas section measures a span that was never a gotcha list.
F_HANDLING="$(make_skill gotcha-handling "$CLEAN_DESC" <<EOF
## Gotcha handling
- item one
- item two
- item three
- item four
- item five
- item six
- item seven
## Notes
$(filler 200)
EOF
)"
expect "'## Gotcha handling' is not matched as the Gotchas section" \
"$F_HANDLING" quiet-about "Gotchas section"
# The control for the three cases above. Without it, "no Gotchas finding" could
# equally mean the whole check is dead, and all three would still be green.
F_CONTROL="$(make_skill gotchas-control "$CLEAN_DESC" <<EOF
## Common gotchas
- item one
- item two
- item three
- item four
- item five
- item six
- item seven
## Notes
$(filler 200)
EOF
)"
expect "control: a real '## Common gotchas' heading with 7 entries IS matched" \
"$F_CONTROL" suggests "Gotchas section has 7 entries"
# ---------------------------------------------------------------------------
# references/<file>.md pointers
# ---------------------------------------------------------------------------
echo ""
echo "--- a references/ pointer that is not on disk is a hard ERROR ---"
F_REF_MISSING="$(make_skill ref-missing "$CLEAN_DESC" <<EOF
If the caller needs the long form, read references/nowhere.md first.
EOF
)"
expect "a body pointing at an absent references/nowhere.md ERRORs" \
"$F_REF_MISSING" errors "points at references/nowhere.md"
F_REF_PRESENT="$(make_skill ref-present "$CLEAN_DESC" <<EOF
If the caller needs the long form, read references/here.md first.
EOF
)"
mkdir -p "$TMPDIR_T/ref-present/references"
echo "content" > "$TMPDIR_T/ref-present/references/here.md"
expect "the same pointer is silent once the file exists" "$F_REF_PRESENT" silent
echo ""
echo "--- a references/ pointer inside a fenced block is not a dispatch entry ---"
F_REF_FENCED="$(make_skill ref-fenced "$CLEAN_DESC" <<EOF
Dispatch tables look like this:
\`\`\`markdown
If X, read references/example-file.md.
\`\`\`
EOF
)"
expect "a fenced references/example-file.md does not ERROR" "$F_REF_FENCED" silent
echo ""
echo "--- an UNTERMINATED fence does not blank the rest of the body ---"
# The fenced-block exemptions above all rest on mask_fenced(), and an unclosed
# fence used to run to EOF: everything after it was blanked, so the ERROR-tier
# references/ check and both Gotchas counts silently stopped seeing any of it.
# That is the worst shape a masking bug can take — a stray ``` line, which is a
# typo an author makes while writing the very examples the masking exists for,
# turned the rest of the file invisible and the gate green. Masking may narrow
# what a check reads; it may never delete content from every check at once.
#
# Both suppressed checks are asserted, because they are separate call sites and
# a fix that restored only one would leave the other silent.
F_FENCE_REF="$(make_skill fence-unclosed-ref "$CLEAN_DESC" <<EOF
Here is how it is invoked:
\`\`\`bash
some-command --all
If the caller needs the long form, read references/behind-the-fence.md first.
EOF
)"
expect "an absent references/ pointer after an unclosed fence still ERRORs" \
"$F_FENCE_REF" errors "points at references/behind-the-fence.md"
F_FENCE_GOTCHAS="$(make_skill fence-unclosed-gotchas "$CLEAN_DESC" <<EOF
Here is how it is invoked:
\`\`\`bash
some-command --all
## Common gotchas
- first trap here
- second trap here
- third trap here
- fourth trap here
- fifth trap here
- sixth trap here
- seventh trap here
## Notes
$(filler 200)
EOF
)"
expect "a Gotchas section after an unclosed fence is still counted" \
"$F_FENCE_GOTCHAS" suggests "Gotchas section has 7 entries"
# The control. Closing the fence must still mask, or the fix above would have
# been "stop masking", which re-breaks every false-positive case in this file.
F_FENCE_CLOSED="$(make_skill fence-closed-ref "$CLEAN_DESC" <<EOF
Here is how it is invoked:
\`\`\`bash
some-command --all
\`\`\`
Dispatch tables look like this:
\`\`\`markdown
If X, read references/behind-the-fence.md.
\`\`\`
EOF
)"
expect "control: the same pointer inside a CLOSED fence is still masked" \
"$F_FENCE_CLOSED" silent
echo ""
echo "--- a references/ pointer in a same-line removal context is history, not dispatch ---"
# Narrow on purpose: a live dispatch table never describes its own target as
# removed, so the exemption costs no recall. Each phrasing is checked separately
# because they are separate alternatives in one regex, and a typo in any one of
# them turns ordinary prose back into a hard ERROR.
#
# The file names are deliberately NEUTRAL (detail-a.md, not gone-a.md). An
# earlier draft of this block named them gone-*.md and every case passed for the
# wrong reason: "gone" is itself one of the removal words, so the exemption fired
# off the FILENAME and the phrase under test was never exercised. The control
# below is what surfaced that — it is the assertion that keeps these five honest.
i=0
for phrase in \
"The old references/detail-a.md was removed in v2." \
"references/detail-b.md is no longer part of this skill." \
"references/detail-c.md is deprecated and should not be read." \
"references/detail-d.md was renamed, so nothing points at it now." \
"references/detail-e.md has been superseded by the body itself."; do
i=$((i + 1))
F_REF_PAST="$(make_skill "ref-past-$i" "$CLEAN_DESC" <<EOF
$phrase
EOF
)"
expect "removal-context pointer is not an ERROR: \"$phrase\"" "$F_REF_PAST" silent
done
# The control: the SAME sentence shape without a removal word must still ERROR,
# or the exemption above has swallowed the check rather than narrowed it.
F_REF_LIVE="$(make_skill ref-live "$CLEAN_DESC" <<EOF
The details live in references/detail-a.md, which the agent should read first.
EOF
)"
expect "control: the same pointer with no removal word still ERRORs" \
"$F_REF_LIVE" errors "points at references/detail-a.md"
# ---------------------------------------------------------------------------
# Missing boundary clause
# ---------------------------------------------------------------------------
echo ""
echo "--- a description with no boundary clause raises a SUGGESTION ---"
F_NO_BOUNDARY="$(make_skill no-boundary "Use when the user wants the thing done." <<EOF
Do the thing.
EOF
)"
expect "a description with no boundary clause raises a SUGGESTION and still exits 0" \
"$F_NO_BOUNDARY" suggests "description has no boundary clause"
# Both accepted shapes, asserted separately: the prose markers and ADR-0020's
# compressed arrow form. Dropping either from the detector would leave the other
# green.
F_PROSE_BOUNDARY="$(make_skill prose-boundary "Use when the user wants the thing done. Do not use for anything else." <<EOF
Do the thing.
EOF
)"
expect "the prose boundary form satisfies the check" "$F_PROSE_BOUNDARY" silent
F_ARROW_BOUNDARY="$(make_skill arrow-boundary "Use when the user wants the thing done. Not the other thing -> sibling-skill." <<EOF
Do the thing.
EOF
)"
expect "ADR-0020's compressed 'Not X -> y' form satisfies the check" \
"$F_ARROW_BOUNDARY" quiet-about "no boundary clause"
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]]