holocron/tests/test-adr0020-body-checks.sh

#!/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 "---"
    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 ]]