From a6eedacfd8bc3c25cf8e9e06c806a12145cfd870 Mon Sep 17 00:00:00 2001
From: Defame1297 <gitea@rkdr.net>
Date: Wed, 9 Sep 2026 05:13:52 +0000
Subject: [PATCH] fix(skill-frontmatter): check every file, scope checks to
 frontmatter
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The hook is `entry: bash` with `args: ['-c', <script>]`. pre-commit appends filenames after the
script string, so the first becomes `$0` and never enters `"$@"` — on a single-file commit, the
common case, the loop body never ran and the hook reported Passed having measured nothing.
ADR-0022 leans on this hook as the enforcement for a mandatory `metadata.version`, so the vacuous
green was the whole gate.

Implementation notes:
- An arg0 placeholder absorbs `$0` so every filename lands in `"$@"`.
- Checks now run against the YAML frontmatter block only, extracted with awk. The old
  `grep -A10 "^metadata:"` matched a `metadata:` inside a body code fence, spanned past the block
  into a following `source:` entry's `version:`, accepted any indentation, and missed a `version:`
  more than ten lines in. An unreadable frontmatter block is now an error, never a pass.
- The value is asserted against three-part semver. `write-docs` carried "1.0" through the entire
  ADR-0022 retrofit undetected, which a presence-only check cannot catch.

Impact: `tests/test-skill-frontmatter.sh` is the first test this hook has ever had. It drives the
real `entry`/`args` composition read out of the config rather than a copy of the script, which is
the only shape that catches the arg0 bug; against the pre-fix hook it scores 7/20.

gates.md described the hook wrongly in both directions and is rewritten, with a carve-out
explaining why this one stays a shell parser next to the "python3 and PyYAML are hard
requirements" reasoning that argues otherwise.

Refs: #127
ADR: 0022
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EeH8SCbcrCAQrtymkNuhKP
---
 .pre-commit-config.yaml         |  82 +++++--
 docs/spec/gates.md              |  78 ++++++-
 tests/test-skill-frontmatter.sh | 366 ++++++++++++++++++++++++++++++++
 3 files changed, 504 insertions(+), 22 deletions(-)
 create mode 100644 tests/test-skill-frontmatter.sh

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 79c9540..ef3d3cc 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -254,27 +254,79 @@ repos:
         entry: bash
         language: system
         files: '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$'
+        # Pinned by tests/test-skill-frontmatter.sh, which drives this exact
+        # `bash -c <script> <arg0> <files...>` call shape rather than a copy of
+        # the script -- the bug below was invisible to any test that did not.
         args:
           - -c
           - |
+            # Every check reads the FRONTMATTER only, never the whole file. A
+            # `metadata:` / `name:` / `description:` line inside a body code
+            # fence is documentation (skill-author quotes exactly such a block)
+            # and used to satisfy these greps.
             for f in "$@"; do
-              if [[ -f "$f" ]]; then
-                missing=""
-                if ! grep -q "^name:" "$f"; then
-                  missing="${missing}name: "
-                fi
-                if ! grep -q "^description:" "$f"; then
-                  missing="${missing}description: "
-                fi
-                if ! grep -A10 "^metadata:" "$f" | grep -q "  version:"; then
-                  missing="${missing}metadata.version "
-                fi
-                if [[ -n "$missing" ]]; then
-                  echo "ERROR: $f is missing required frontmatter fields (${missing})"
-                  exit 1
-                fi
+              [[ -f "$f" ]] || continue
+
+              fm="$(awk '
+                { sub(/\r$/, "") }
+                NR == 1 { sub(/^\357\273\277/, "") }
+                !opened && /^[[:blank:]]*$/ { next }
+                !opened {
+                  if ($0 ~ /^---[[:blank:]]*$/) { opened = 1; next }
+                  exit
+                }
+                /^---[[:blank:]]*$/ { closed = 1; exit }
+                { print }
+                END { if (!opened || !closed) exit 3 }
+              ' "$f")" || {
+                echo "ERROR: $f has no closing YAML frontmatter block (expected --- ... --- at the top of the file)"
+                exit 1
+              }
+
+              missing=""
+              printf '%s\n' "$fm" | grep -q "^name:" || missing="${missing}name: "
+              printf '%s\n' "$fm" | grep -q "^description:" || missing="${missing}description: "
+
+              # Scoped to the `metadata:` block and stopped at the next
+              # top-level key, so a `version:` under a following `source:` list
+              # cannot stand in for it; the `^  version:` anchor is exact, so a
+              # deeper-nested `    version:` cannot either. No line budget, so a
+              # long `metadata:` block does not hide the key.
+              ver="$(printf '%s\n' "$fm" | awk '
+                /^metadata:/ { inm = 1; next }
+                inm && /^[A-Za-z]/ { exit }
+                inm && /^  version:/ {
+                  v = $0
+                  sub(/^  version:[[:blank:]]*/, "", v)
+                  sub(/[[:blank:]]+#.*$/, "", v)
+                  sub(/[[:blank:]]+$/, "", v)
+                  print "found:" v
+                  exit
+                }
+              ')"
+              [[ -n "$ver" ]] || missing="${missing}metadata.version "
+
+              if [[ -n "$missing" ]]; then
+                echo "ERROR: $f is missing required frontmatter fields (${missing})"
+                exit 1
+              fi
+
+              raw="${ver#found:}"
+              v="$raw"
+              case "$v" in
+                \"*\") v="${v#\"}"; v="${v%\"}" ;;
+                \'*\') v="${v#\'}"; v="${v%\'}" ;;
+              esac
+              if [[ ! "$v" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
+                echo "ERROR: $f has a malformed frontmatter metadata.version (${raw:-<empty>}) -- expected a three-part semver, e.g. \"1.0.0\""
+                exit 1
               fi
             done
+          # arg0 for `bash -c`. WITHOUT it pre-commit's first filename lands in
+          # $0 and is dropped from "$@" -- so a single-file commit, the normal
+          # case, ran the loop zero times and reported Passed having checked
+          # nothing. Do not remove; tests/test-skill-frontmatter.sh pins it.
+          - skill-frontmatter
 
       - id: skill-size-check
         stages: ['pre-commit']
diff --git a/docs/spec/gates.md b/docs/spec/gates.md
index cdd1e3c..7280c1b 100644
--- a/docs/spec/gates.md
+++ b/docs/spec/gates.md
@@ -128,12 +128,43 @@ not an authoring change.
 ### `skill-frontmatter`, the other hook on that scope
 
 A second `repo: local` pre-commit hook, `skill-frontmatter`, runs on the **same** `files:` pattern at
-the same stage. It is a short shell loop: for each file, `grep -q "^name:"` and
-`grep -q "^description:"`, failing with "missing required frontmatter fields" if either is absent.
+the same stage. It is a shell loop that, **for the YAML frontmatter block only** — everything between
+the opening `---` and the next `---` — asserts four things per file:
 
-**It overlaps ADR-0020's "description present and non-empty" FAIL, and the overlap is not clean.**
-The ADR (`:95-101`) requires that question be decided on the **YAML-folded value** and nowhere else,
-precisely because a line regex gets it wrong in both directions. Measured on fixtures:
+| Check | Rejects with |
+|---|---|
+| a `^name:` line is present | "missing required frontmatter fields (name: …)" |
+| a `^description:` line is present | "missing required frontmatter fields (description: …)" |
+| `metadata:` contains a `^  version:` key, anchored, scanning to the next top-level key | "missing required frontmatter fields (metadata.version)" |
+| that version's value is three-part semver (`1.0.0`, quoted or not) | "has a malformed frontmatter metadata.version (…)" |
+
+Every one of those qualifiers is load-bearing, and each replaced a defect that let the hook report
+Passed having measured nothing. `tests/test-skill-frontmatter.sh` pins all of them:
+
+- **Frontmatter-scoped, not whole-file.** The checks used to `grep` the entire file, so a `metadata:`
+  or `name:` block quoted in a **body code fence** satisfied them — `skill-author`'s own docs quote
+  exactly such a block.
+- **Bounded by the next top-level key, not by `-A10`.** The version check was
+  `grep -A10 "^metadata:" | grep -q "  version:"`, which ran ten lines past the end of the block: a
+  `version:` belonging to a following `source:` list entry counted (`write-docs` and `research` both
+  have a `source:` list immediately after `metadata:`), while a `metadata:` block with more than ten
+  lines before its `version:` was reported missing.
+- **`^  version:` anchored.** `"  version:"` was an unanchored substring, so a deeper-nested
+  `    version:` matched too.
+- **The value is asserted, not just the key.** `plugins/bin/.apm/skills/write-docs/SKILL.md` carried
+  `version: "1.0"` — present, correctly nested, and not a version — through an entire PR under a
+  presence-only check. Two-part `1.0` is a YAML float, not a version string.
+- **The call shape is pinned.** `entry: bash` with `args: ['-c', <script>, …]` needs an explicit
+  arg0 placeholder after the script: without it `bash -c` puts pre-commit's **first** filename in
+  `$0`, where `for f in "$@"` never sees it. A single-file commit — the normal case — therefore ran
+  the loop body zero times and exited 0. The third `args` entry (`skill-frontmatter`) exists solely
+  to absorb `$0`; do not remove it.
+- **An unreadable file is an error, not a pass.** A file with no closing `---` fails with "no closing
+  YAML frontmatter block" rather than falling through to a green.
+
+**It still overlaps ADR-0020's "description present and non-empty" FAIL, and the overlap is not
+clean.** The ADR (`:95-101`) requires that question be decided on the **YAML-folded value** and
+nowhere else, precisely because a line regex gets it wrong in both directions. Measured on fixtures:
 
 | Frontmatter | `skill-frontmatter` | `skill-size-check` |
 |---|---|---|
@@ -145,10 +176,34 @@ against, and it is the only one of the two that objects to a quoted key. Neither
 currently live in the corpus, and the honest reading is that presence is `skill-size-check`'s
 question — the grep's contribution to it is noise on one shape and silence on the other.
 
-What the grep does add is the `name:` key, which **no** ADR-0020 check reads: a `SKILL.md` with no
-`name:` passes `skill-size-check` at exit 0. That is its real and only unique coverage, and the
+What the hook adds that **no** ADR-0020 check reads is two keys: `name:` and `metadata.version`. A
+`SKILL.md` missing either passes `skill-size-check` at exit 0. That is its unique coverage, and the
 reason not to fold it into the size gate on the grounds of redundancy.
 
+#### Why this one stays a shell parser
+
+[`python3` and PyYAML are hard requirements](#python3-and-pyyaml-are-hard-requirements) below records
+that a hand-rolled frontmatter reader on this exact `files:` scope was **deliberately deleted**,
+because "a reader that mis-parses an unfamiliar scalar shape reports a clean pass on a file it never
+measured." That reasoning is about `skill-size-check` and does **not** transfer here. Do not delete
+this hook citing it. Three differences:
+
+1. **It answers a strictly narrower question.** `skill-size-check` must know the *folded value* of a
+   `>`-block scalar to count its characters, which is where a line reader diverges from a parser —
+   one corpus description measured 270 characters parsed and 412 unparsed. This hook asks only
+   whether a key is on a line and whether one short **plain scalar** matches `N.N.N`. There is no
+   folding, no multi-line value, and no measurement to get subtly wrong.
+2. **It is frontmatter-scoped.** The failure mode that killed the old fallback was silently reading
+   past or short of the block. This one extracts the block explicitly and errors out when it cannot
+   find a closing marker, so "could not parse" is a red, never a green.
+3. **It is pinned by tests.** `tests/test-skill-frontmatter.sh` drives the hook through pre-commit's
+   real `bash -c <script> <arg0> <files…>` invocation and asserts each defect class above. The
+   deleted fallback had no such suite; that is how its disagreement with a real parser survived.
+
+The trade it buys is that the hook stays repo-local. Moving it to a script would change the
+externally exposed `.pre-commit-hooks.yaml` contract for consumers, for a check that has no need of a
+YAML parser.
+
 ### Two independent gate families, neither replaced the other
 
 **Family 1 — agentskills.io spec backstop** (unchanged, conformance not quality):
@@ -413,6 +468,15 @@ reader that mis-parses an unfamiliar scalar shape reports a clean pass on a file
 which is the exact vacuous-green failure the `python3` check exists to avoid. `pip install pyyaml`
 (or `python3 -m pip install PyYAML`, or the distro's `python3-yaml`) if the hook reports it missing.
 
+**Neither requirement generalises to every hook on this scope, and one deliberate exception sits
+right next to it.** [`skill-frontmatter`](#skill-frontmatter-the-other-hook-on-that-scope) runs on the
+same `files:` pattern as a **shell** parser, on purpose — it asks only whether a key is on a line and
+whether one short plain scalar matches `N.N.N`, with no folding to get wrong, and moving it to a
+script would change the externally exposed `.pre-commit-hooks.yaml` contract for consumers. That
+section carries the full argument. A reader arriving here first should not read this one as
+condemning it. `check-rtk-prefix` needs `python3` but **not** PyYAML: it reads the markdown body and
+never touches frontmatter, so it has no scalar to fold.
+
 ## Agent files take the description gates, not the body gate
 
 `check-apm-agents-valid` runs agent-audit's `validate.sh` over every real
diff --git a/tests/test-skill-frontmatter.sh b/tests/test-skill-frontmatter.sh
new file mode 100644
index 0000000..f9de65c
--- /dev/null
+++ b/tests/test-skill-frontmatter.sh
@@ -0,0 +1,366 @@
+#!/usr/bin/env bash
+# Regression test for the `skill-frontmatter` pre-commit hook.
+#
+# The hook is `entry: bash` with `args: ['-c', <script>, <arg0>]`, and this file
+# drives that exact call shape -- read out of .pre-commit-config.yaml, never
+# re-implemented here. That is the whole point. `grep -rn skill-frontmatter
+# tests/` returned nothing before this file existed, and the two defect classes
+# below are both invisible to a test that copies the script body and calls it
+# some other way:
+#
+#   1. THE POSITIONAL DROP. `bash -c <script> fileA fileB` puts fileA in $0, not
+#      in "$@". The hook had no arg0 placeholder, so pre-commit's FIRST filename
+#      was swallowed -- and a single-file commit, the normal case, ran the loop
+#      body zero times and reported Passed having checked nothing. A test that
+#      sources or inlines the script never sees this; only the real invocation
+#      shape does. Cases 2 and 3 below are that regression.
+#
+#   2. THE UNSCOPED GREP. The checks used to run over the WHOLE file, and the
+#      version check was `grep -A10 "^metadata:" | grep -q "  version:"`. Four
+#      confirmed ways to pass while measuring nothing, all pinned below:
+#        * `metadata:` in a BODY code fence satisfies it (skill-author's own
+#          docs quote exactly such a block);
+#        * `-A10` runs past the end of the metadata block, so a `version:`
+#          belonging to a following `source:` list entry satisfies it
+#          (write-docs and research both have `source:` right after `metadata:`);
+#        * `"  version:"` is an unanchored substring, so `    version:` at a
+#          deeper nesting satisfies it;
+#        * and the mirror image, a false NEGATIVE: a metadata block with more
+#          than 10 lines before `version:` was reported missing.
+#
+# Plus the semver assertion, which is not redundant with presence: write-docs
+# carried `version: "1.0"` -- present, well-nested, and not a version -- through
+# an entire PR under a check that only ever asked whether the key was there.
+set -euo pipefail
+
+REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+CONFIG="$REPO_ROOT/.pre-commit-config.yaml"
+PASS=0
+FAIL=0
+
+pass() { echo "  PASS: $1"; PASS=$((PASS + 1)); }
+fail() { echo "  FAIL: $1"; FAIL=$((FAIL + 1)); }
+
+# Same guard shape as tests/test-check-executables-allow-sync.sh: the hook
+# definition is YAML and reading it any other way is guessing. README.md lists
+# python3/PyYAML as a pre-push prerequisite, and `run-tests.sh --strict` turns
+# this skip into a failure, which is the correct reading when it runs as a gate.
+command -v python3 > /dev/null 2>&1 || { echo "SKIP: python3 is required to read the hook definition out of .pre-commit-config.yaml"; exit 77; }
+python3 -c 'import yaml' > /dev/null 2>&1 || { echo "SKIP: PyYAML is required to read the hook definition out of .pre-commit-config.yaml"; exit 77; }
+
+TMPDIR_T="$(mktemp -d)"
+trap 'rm -rf "$TMPDIR_T"' EXIT
+
+# ---------------------------------------------------------------------------
+# The hook, as pre-commit will run it
+# ---------------------------------------------------------------------------
+# ENTRY and ARGS come straight out of the config. run_hook() then reproduces
+# pre-commit's own composition -- entry, then args, then the filenames appended
+# LAST -- so the positional handling under test is the real one.
+HOOK_JSON="$TMPDIR_T/hook.json"
+python3 - "$CONFIG" "$HOOK_JSON" <<'PY'
+import json
+import sys
+
+import yaml
+
+config_path, out_path = sys.argv[1:3]
+with open(config_path, encoding='utf-8') as fh:
+    cfg = yaml.safe_load(fh) or {}
+
+found = None
+for repo in cfg.get('repos') or []:
+    for hook in repo.get('hooks') or []:
+        if hook.get('id') == 'skill-frontmatter':
+            found = hook
+
+if found is None:
+    sys.exit("skill-frontmatter hook not found in " + config_path)
+
+with open(out_path, 'w', encoding='utf-8') as fh:
+    json.dump(
+        {
+            'entry': found.get('entry'),
+            'args': found.get('args') or [],
+            'files': found.get('files'),
+            'pass_filenames': found.get('pass_filenames', True),
+            'always_run': found.get('always_run', False),
+        },
+        fh,
+    )
+PY
+
+# Reads NUL-delimited fields on stdin into the named array. bash 3.2 (macOS) has
+# no `mapfil[e]`/`readarra[y]` builtin, and tests/test-vale-wrap.sh's static scan
+# rejects both — the bracket classes above match what a bare spelling would while
+# keeping one out of this file, the same trick that file uses for `npro[c]`.
+read_nul_array() {
+  local __var="$1"
+  shift
+  local __item
+  eval "$__var=()"
+  while IFS= read -r -d '' __item; do
+    eval "$__var+=(\"\$__item\")"
+  done
+}
+
+ENTRY="$(python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["entry"])' "$HOOK_JSON")"
+ARGS=()
+read_nul_array ARGS < <(python3 -c '
+import json
+import sys
+
+data = json.load(open(sys.argv[1]))
+for arg in data["args"]:
+    sys.stdout.write(arg + "\0")
+' "$HOOK_JSON")
+
+RUN_OUT=""
+RUN_RC=0
+# run_hook <file>... -- exactly `entry args... files...`, pre-commit's ordering.
+run_hook() {
+  RUN_RC=0
+  RUN_OUT="$("$ENTRY" ${ARGS[@]+"${ARGS[@]}"} "$@" 2>&1)" || RUN_RC=$?
+}
+
+# write_skill <path> <frontmatter-body> [markdown-body]
+write_skill() {
+  local path="$1" frontmatter="$2" body="${3:-# Heading
+
+Body text.}"
+  mkdir -p "$(dirname "$path")"
+  {
+    printf -- '---\n'
+    printf '%s\n' "$frontmatter"
+    printf -- '---\n\n'
+    printf '%s\n' "$body"
+  } > "$path"
+}
+
+VALID_FM='name: valid-skill
+description: A skill whose frontmatter is complete and well formed.
+metadata:
+  version: "1.0.0"'
+
+assert_passes() { # <label> <file>...
+  local label="$1"
+  shift
+  run_hook "$@"
+  if [[ $RUN_RC -eq 0 ]]; then
+    pass "$label"
+  else
+    fail "$label -- expected exit 0, got $RUN_RC. Output: $RUN_OUT"
+  fi
+}
+
+assert_fails_with() { # <label> <needle> <file>...
+  local label="$1" needle="$2"
+  shift 2
+  run_hook "$@"
+  if [[ $RUN_RC -eq 0 ]]; then
+    fail "$label -- exited 0, so the hook reported Passed having checked nothing. Output: $RUN_OUT"
+  elif [[ "$RUN_OUT" != *"$needle"* ]]; then
+    fail "$label -- exited $RUN_RC but the message lacked '$needle'. Output: $RUN_OUT"
+  else
+    pass "$label"
+  fi
+}
+
+# ---------------------------------------------------------------------------
+# 1. The call shape itself
+# ---------------------------------------------------------------------------
+# Asserted as a contract as well as behaviourally, because the behavioural
+# symptom of losing arg0 is a GREEN run -- the least likely thing to be noticed.
+echo "--- the hook passes an arg0 placeholder so pre-commit's filenames land in \"\$@\" ---"
+if [[ "$ENTRY" != "bash" ]]; then
+  fail "entry is '$ENTRY', not bash -- the arg0 reasoning below assumes bash -c"
+elif [[ ${#ARGS[@]} -lt 3 ]]; then
+  fail "args has ${#ARGS[@]} entries; a 'bash -c <script>' hook needs a third, the arg0 placeholder, or the first filename is dropped from \"\$@\""
+elif [[ "${ARGS[0]}" != "-c" ]]; then
+  fail "args[0] is '${ARGS[0]}', not -c"
+elif [[ "${ARGS[2]}" == -* ]]; then
+  fail "args[2] is '${ARGS[2]}', which bash will read as a flag rather than as \$0"
+else
+  pass "args is [-c, <script>, '${ARGS[2]}'] -- the third entry becomes \$0 and every filename reaches \"\$@\""
+fi
+
+FILES_PATTERN="$(python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["files"] or "")' "$HOOK_JSON")"
+PASS_FILENAMES="$(python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["pass_filenames"])' "$HOOK_JSON")"
+echo ""
+echo "--- the hook is filename-driven, so the loop is the only thing that ever runs ---"
+if [[ "$PASS_FILENAMES" != "True" ]]; then
+  fail "pass_filenames is $PASS_FILENAMES; with no filenames the loop body never executes and the hook is a permanent no-op"
+elif [[ "$FILES_PATTERN" != '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$' ]]; then
+  fail "files: is '$FILES_PATTERN', not the .apm/ skill scope -- ADR-0020's gates and this one must agree on scope"
+else
+  pass "pass_filenames is on and files: is the .apm/ skill scope"
+fi
+
+# ---------------------------------------------------------------------------
+# 2. A single bad file, on its own -- THE regression
+# ---------------------------------------------------------------------------
+# This is the shipped bug in one line: one file, which is what a commit
+# touching one skill hands the hook. Before the arg0 fix the file landed in $0,
+# "$@" was empty, and this exited 0.
+echo ""
+echo "--- a SINGLE bad file fails (before the arg0 fix this exited 0 having read nothing) ---"
+write_skill "$TMPDIR_T/single/SKILL.md" 'name: single
+description: Missing its metadata block entirely.'
+assert_fails_with "one file with no metadata block is rejected" "missing required frontmatter fields" "$TMPDIR_T/single/SKILL.md"
+
+echo ""
+echo "--- and a single GOOD file still passes, so the case above is not failing for some other reason ---"
+write_skill "$TMPDIR_T/single-ok/SKILL.md" "$VALID_FM"
+assert_passes "one valid file passes" "$TMPDIR_T/single-ok/SKILL.md"
+
+# ---------------------------------------------------------------------------
+# 3. Position within the argument list must not matter
+# ---------------------------------------------------------------------------
+echo ""
+echo "--- the bad file is caught wherever it sits in the argument list ---"
+write_skill "$TMPDIR_T/ok-a/SKILL.md" "$VALID_FM"
+write_skill "$TMPDIR_T/ok-b/SKILL.md" "$VALID_FM"
+write_skill "$TMPDIR_T/bad/SKILL.md" 'name: bad
+description: No metadata block.'
+
+assert_fails_with "bad file FIRST is caught" "$TMPDIR_T/bad/SKILL.md" \
+  "$TMPDIR_T/bad/SKILL.md" "$TMPDIR_T/ok-a/SKILL.md" "$TMPDIR_T/ok-b/SKILL.md"
+assert_fails_with "bad file LAST is caught (the loop reaches the end of \"\$@\")" "$TMPDIR_T/bad/SKILL.md" \
+  "$TMPDIR_T/ok-a/SKILL.md" "$TMPDIR_T/ok-b/SKILL.md" "$TMPDIR_T/bad/SKILL.md"
+assert_passes "three valid files pass" "$TMPDIR_T/ok-a/SKILL.md" "$TMPDIR_T/ok-b/SKILL.md" "$TMPDIR_T/single-ok/SKILL.md"
+
+# ---------------------------------------------------------------------------
+# 4. The four grep defects
+# ---------------------------------------------------------------------------
+echo ""
+echo "--- defect 1: a \`metadata:\` block in a BODY code fence is documentation, not frontmatter ---"
+# skill-author's docs quote a metadata block verbatim. Under the old whole-file
+# grep that quotation satisfied the check for the file quoting it.
+write_skill "$TMPDIR_T/fence/SKILL.md" 'name: fence
+description: Frontmatter has no metadata block; the body quotes one.' '# Fence
+
+Skills declare their version like this:
+
+```yaml
+metadata:
+  version: "1.0.0"
+```'
+assert_fails_with "a quoted metadata block in the body does not satisfy metadata.version" "missing required frontmatter fields" "$TMPDIR_T/fence/SKILL.md"
+
+echo ""
+echo "--- ... and the same for name: and description: quoted in the body ---"
+write_skill "$TMPDIR_T/fence-keys/SKILL.md" 'metadata:
+  version: "1.0.0"' '# Fence keys
+
+```yaml
+name: not-the-real-name
+description: not the real description
+```'
+assert_fails_with "body-fenced name:/description: do not satisfy the presence checks" "name: description:" "$TMPDIR_T/fence-keys/SKILL.md"
+
+echo ""
+echo "--- defect 2: a \`version:\` under a following \`source:\` list is not metadata.version ---"
+# `-A10` ran ten lines past `metadata:` regardless of where the block ended.
+# write-docs and research both carry a `source:` list immediately after it.
+write_skill "$TMPDIR_T/source-list/SKILL.md" 'name: source-list
+description: metadata has no version; the next top-level key does.
+metadata:
+  author: someone
+source:
+  - name: upstream
+    version: "2.3.4"'
+assert_fails_with "a version: belonging to source[] does not satisfy metadata.version" "missing required frontmatter fields" "$TMPDIR_T/source-list/SKILL.md"
+
+echo ""
+echo "--- defect 3: a deeper-nested \`    version:\` is not metadata.version ---"
+# `grep -q "  version:"` was an unanchored substring match, so any indentation
+# of two spaces or more matched.
+write_skill "$TMPDIR_T/deep-indent/SKILL.md" 'name: deep-indent
+description: The only version: key sits one level too deep.
+metadata:
+  provenance:
+    version: "1.0.0"'
+assert_fails_with "a four-space-indented version: does not satisfy metadata.version" "missing required frontmatter fields" "$TMPDIR_T/deep-indent/SKILL.md"
+
+echo ""
+echo "--- defect 4: a version: more than ten lines into the metadata block is FOUND ---"
+# The mirror image: the old check reported this one missing.
+write_skill "$TMPDIR_T/long-metadata/SKILL.md" 'name: long-metadata
+description: A long metadata block whose version sits well past line ten.
+metadata:
+  a: 1
+  b: 2
+  c: 3
+  d: 4
+  e: 5
+  f: 6
+  g: 7
+  h: 8
+  i: 9
+  j: 10
+  k: 11
+  version: "1.0.0"'
+assert_passes "a version: 13 lines into the metadata block is found" "$TMPDIR_T/long-metadata/SKILL.md"
+
+# ---------------------------------------------------------------------------
+# 5. Present is not the same as well formed
+# ---------------------------------------------------------------------------
+echo ""
+echo "--- a present-but-non-semver version is rejected, with its own message ---"
+# plugins/bin/.apm/skills/write-docs/SKILL.md carried exactly this through a
+# whole PR: the key was present, so a presence-only check had nothing to say.
+write_skill "$TMPDIR_T/two-part/SKILL.md" 'name: two-part
+description: Its version is two-part, which is a float in YAML, not a version.
+metadata:
+  version: "1.0"'
+assert_fails_with "\"1.0\" is rejected as malformed, not reported as missing" "malformed frontmatter metadata.version" "$TMPDIR_T/two-part/SKILL.md"
+
+echo ""
+echo "--- ... and the malformed message quotes the offending value ---"
+run_hook "$TMPDIR_T/two-part/SKILL.md"
+if [[ "$RUN_OUT" == *'"1.0"'* ]]; then
+  pass "the message names the value it rejected"
+else
+  fail "the message did not quote the rejected value. Output: $RUN_OUT"
+fi
+
+echo ""
+echo "--- other non-semver shapes ---"
+write_skill "$TMPDIR_T/empty-version/SKILL.md" 'name: empty-version
+description: The key is present with no value at all.
+metadata:
+  version:'
+assert_fails_with "a valueless version: is rejected" "metadata.version" "$TMPDIR_T/empty-version/SKILL.md"
+
+write_skill "$TMPDIR_T/word-version/SKILL.md" 'name: word-version
+description: A non-numeric version.
+metadata:
+  version: latest'
+assert_fails_with "version: latest is rejected" "malformed frontmatter metadata.version" "$TMPDIR_T/word-version/SKILL.md"
+
+echo ""
+echo "--- an unquoted three-part version is accepted (both YAML spellings are legal) ---"
+write_skill "$TMPDIR_T/unquoted/SKILL.md" 'name: unquoted
+description: An unquoted semver, which YAML reads as a string.
+metadata:
+  version: 0.1.4'
+assert_passes "version: 0.1.4 unquoted passes" "$TMPDIR_T/unquoted/SKILL.md"
+
+# ---------------------------------------------------------------------------
+# 6. A file that cannot be read as frontmatter must not report green
+# ---------------------------------------------------------------------------
+echo ""
+echo "--- a file with no frontmatter block fails loudly rather than passing vacuously ---"
+printf '# Just a document\n\nname: not-frontmatter\ndescription: nor this\n' > "$TMPDIR_T/no-fm.md"
+assert_fails_with "a file with no --- block is an error" "no closing YAML frontmatter block" "$TMPDIR_T/no-fm.md"
+
+printf -- '---\nname: unterminated\ndescription: the block is never closed\nmetadata:\n  version: "1.0.0"\n' > "$TMPDIR_T/unterminated.md"
+assert_fails_with "an unterminated frontmatter block is an error" "no closing YAML frontmatter block" "$TMPDIR_T/unterminated.md"
+
+echo ""
+echo "--- a path that does not exist is skipped, not crashed on ---"
+assert_passes "a nonexistent path is ignored" "$TMPDIR_T/nope/SKILL.md"
+
+echo ""
+echo "Results: $PASS passed, $FAIL failed"
+[[ $FAIL -eq 0 ]]