diff --git a/.github/workflows/self-test.yml b/.github/workflows/self-test.yml index 752b332..d8f4caa 100644 --- a/.github/workflows/self-test.yml +++ b/.github/workflows/self-test.yml @@ -39,8 +39,13 @@ jobs: # Preinstalled on the ubuntu runner images. run: shellcheck --severity=warning scripts/*.sh tests/*.sh - - name: Run the junk-gate test suite - run: bash tests/junk_file_gate_test.sh + - name: Run every test suite + run: | + set -euo pipefail + for t in tests/*_test.sh; do + echo "──── $t ────" + bash "$t" + done - name: Prove the suite can fail # A test suite that has only ever been seen passing is indistinguishable diff --git a/scripts/frontmatter_gate.sh b/scripts/frontmatter_gate.sh index 108d8c5..a65e974 100755 --- a/scripts/frontmatter_gate.sh +++ b/scripts/frontmatter_gate.sh @@ -56,14 +56,36 @@ fi echo ">> frontmatter gate: checking ${#targets[@]} file(s)" failures=0 +unreadable=0 for f in "${targets[@]}"; do + # Tracked but not readable in this checkout. Overwhelmingly this means a committed + # symlink with an absolute target that exists only on the authoring machine, and + # gtmify-config has roughly 145 tracked symlinks of which most are absolute. That is + # a real and separate problem, but failing a hundred files here would make this gate + # permanently red and therefore ignored, so these are counted and reported rather + # than treated as frontmatter violations. An earlier version simply crashed with a + # FileNotFoundError traceback. + if [ ! -r "$f" ]; then + if [ -L "$f" ]; then + printf ' skip %s\n tracked symlink with an unreachable target: %s\n' "$f" "$(readlink "$f")" + else + printf ' skip %s\n tracked but not readable in this checkout\n' "$f" + fi + unreadable=$((unreadable + 1)) + continue + fi + problem="$(F="$f" python3 <<'PY' import os, sys path = os.environ["F"] -with open(path, encoding="utf-8", errors="replace") as fh: - lines = fh.read().split("\n") +try: + with open(path, encoding="utf-8", errors="replace") as fh: + lines = fh.read().split("\n") +except OSError as e: + sys.stdout.write(f"could not be read: {e.strerror}") + sys.exit(0) if not lines or lines[0].strip() != "---": sys.stdout.write("no YAML frontmatter; file must open with ---") @@ -107,7 +129,19 @@ done if [ "$failures" -ne 0 ]; then echo echo "!! frontmatter gate FAILED: ${failures} file(s) would never trigger." + [ "$unreadable" -gt 0 ] && echo " (${unreadable} further file(s) were unreadable and not assessed; see above)" exit 1 fi +# Say plainly what was not checked. A gate that quietly skips work reads as full +# coverage and is worse than one that admits a gap. +if [ "$unreadable" -gt 0 ]; then + echo + echo ">> frontmatter gate passed on $(( ${#targets[@]} - unreadable )) file(s)." + echo " ${unreadable} were NOT assessed because they are tracked but unreadable here," + echo " almost certainly committed symlinks with absolute targets. Those are broken for" + echo " any machine other than the one that created them; worth fixing separately." + exit 0 +fi + echo ">> frontmatter gate passed." diff --git a/tests/frontmatter_gate_test.sh b/tests/frontmatter_gate_test.sh new file mode 100755 index 0000000..6e5aceb --- /dev/null +++ b/tests/frontmatter_gate_test.sh @@ -0,0 +1,110 @@ +#!/usr/bin/env bash +# +# Tests for scripts/frontmatter_gate.sh. +# +# The dangling-symlink case is the reason this file exists. gtmify-config tracks +# roughly 145 symlinks, most with absolute /Users/... targets that resolve only on the +# machine that created them. In CI they are tracked but unreadable, and the first +# version of the gate died on one with a FileNotFoundError traceback. Failing them +# instead would have turned tier A permanently red over a pre-existing problem, so the +# gate counts and reports them and does not fail. +# +# Usage: tests/frontmatter_gate_test.sh +set -uo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +GATE="$HERE/../scripts/frontmatter_gate.sh" +[ -f "$GATE" ] || { echo "cannot find $GATE" >&2; exit 2; } + +pass_count=0 +fail_count=0 + +new_repo() { + local d + d="$(mktemp -d)" + git -C "$d" init -q + git -C "$d" config user.email t@t.t + git -C "$d" config user.name t + printf '%s\n' "$d" +} + +# check [needle that must appear in output] +check() { + local name="$1" expect="$2" d="$3" needle="${4:-}" rc=0 out got + out="$(cd "$d" && bash "$GATE" 2>&1)" || rc=$? + got="pass"; [ "$rc" -ne 0 ] && got="fail" + + local ok=1 + [ "$got" = "$expect" ] || ok=0 + if [ -n "$needle" ] && ! printf '%s' "$out" | grep -q -- "$needle"; then ok=0; fi + + if [ "$ok" -eq 1 ]; then + printf ' ok %-40s expected %-4s got %s\n' "$name" "$expect" "$got" + pass_count=$((pass_count + 1)) + else + printf ' FAIL %-40s expected %-4s got %s' "$name" "$expect" "$got" + [ -n "$needle" ] && printf ' (needle: %s)' "$needle" + printf '\n' + printf '%s\n' "$out" | sed 's/^/ /' + fail_count=$((fail_count + 1)) + fi + rm -rf "$d" +} + +valid_skill() { + local d="$1" n="$2" + mkdir -p "$d/skills/$n" + printf -- '---\nname: %s\ndescription: does a real thing\n---\nbody\n' "$n" > "$d/skills/$n/SKILL.md" +} + +echo "== frontmatter gate ==" + +d="$(new_repo)"; valid_skill "$d" good +git -C "$d" add -A >/dev/null 2>&1; git -C "$d" commit -qm i >/dev/null 2>&1 +check "valid_skill_passes" pass "$d" + +d="$(new_repo)"; mkdir -p "$d/skills/nofm" +printf '# just a heading\n' > "$d/skills/nofm/SKILL.md" +git -C "$d" add -A >/dev/null 2>&1; git -C "$d" commit -qm i >/dev/null 2>&1 +check "missing_frontmatter_fails" fail "$d" "no YAML frontmatter" + +d="$(new_repo)"; mkdir -p "$d/skills/nodesc" +printf -- '---\nname: nodesc\n---\nbody\n' > "$d/skills/nodesc/SKILL.md" +git -C "$d" add -A >/dev/null 2>&1; git -C "$d" commit -qm i >/dev/null 2>&1 +check "missing_description_fails" fail "$d" "missing or empty: description" + +d="$(new_repo)"; mkdir -p "$d/skills/unclosed" +printf -- '---\nname: x\ndescription: y\nbody with no closing marker\n' > "$d/skills/unclosed/SKILL.md" +git -C "$d" add -A >/dev/null 2>&1; git -C "$d" commit -qm i >/dev/null 2>&1 +check "unclosed_frontmatter_fails" fail "$d" "never closed" + +# The case that crashed the first implementation. +d="$(new_repo)"; valid_skill "$d" good +mkdir -p "$d/skills/dangling" +ln -s /nonexistent/absolute/target/SKILL.md "$d/skills/dangling/SKILL.md" +git -C "$d" add -A >/dev/null 2>&1; git -C "$d" commit -qm i >/dev/null 2>&1 +check "dangling_symlink_is_skipped_not_fatal" pass "$d" "unreachable target" + +# A dangling symlink must not mask a genuine violation elsewhere. +d="$(new_repo)"; mkdir -p "$d/skills/nofm" "$d/skills/dangling" +printf '# heading only\n' > "$d/skills/nofm/SKILL.md" +ln -s /nonexistent/x/SKILL.md "$d/skills/dangling/SKILL.md" +git -C "$d" add -A >/dev/null 2>&1; git -C "$d" commit -qm i >/dev/null 2>&1 +check "dangling_does_not_mask_real_failure" fail "$d" "no YAML frontmatter" + +# Nested paths must not be swept in: git pathspec wildcards cross slashes by default, +# which is what produced 55 false positives against gtmify-config. +d="$(new_repo)"; mkdir -p "$d/agents/instructions" +printf '# prose, no frontmatter by design\n' > "$d/agents/instructions/some_agent.md" +git -C "$d" add -A >/dev/null 2>&1; git -C "$d" commit -qm i >/dev/null 2>&1 +check "nested_agent_docs_are_out_of_scope" pass "$d" "nothing to check" + +d="$(new_repo)" +git -C "$d" commit -q --allow-empty -m i >/dev/null 2>&1 +check "repo_with_no_skills_noops" pass "$d" "nothing to check" + +echo +echo "════════════════════════════════════════" +echo " ${pass_count} passed, ${fail_count} failed" +echo "════════════════════════════════════════" +[ "$fail_count" -eq 0 ] || exit 1