From 68cfef334467d818e2fad29346c18027606bbf5b Mon Sep 17 00:00:00 2001 From: "@scott-wueschinski-GTMify" Date: Wed, 29 Jul 2026 08:40:59 -0400 Subject: [PATCH] fix: frontmatter gate crashed on a tracked symlink with an unreachable target Found by the gate's own first real CI run, against gtmify-config, where it died with a FileNotFoundError on skills/_gstack-command/SKILL.md. The cause is a genuine repo defect, not a CI quirk. That path is a committed symlink whose target is the ABSOLUTE /Users/scottwueschinski/claude/config/gtmify/skills/ gstack/SKILL.md, and it points into skills/gstack/, which that repo gitignores. So it resolves only on the machine that created it and is dangling everywhere else, including on the second machine and in CI. An audit of that repo found roughly 145 tracked symlinks, the large majority with absolute targets. Three behaviors were possible and only one is right. Crashing is obviously wrong. Failing would turn tier A permanently red over a pre-existing problem the gate was not built to solve, and a permanently red gate gets ignored within a week. So these are counted, named in the output with their unreachable target, and excluded from the verdict, and the summary states plainly how many were not assessed. A gate that quietly skips work reads as full coverage and is worse than one that admits a gap. A dangling symlink still cannot mask a real violation elsewhere in the same run; there is a test for exactly that. Adds tests/frontmatter_gate_test.sh, 8 cases, including the one that crashed. The self-test now iterates tests/*_test.sh rather than naming one file, so a new suite is picked up without editing the workflow. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/self-test.yml | 9 ++- scripts/frontmatter_gate.sh | 38 ++++++++++- tests/frontmatter_gate_test.sh | 110 ++++++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+), 4 deletions(-) create mode 100755 tests/frontmatter_gate_test.sh 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