#!/usr/bin/env bash
# check-skill-frontmatter.sh — fail when an image-baked skill's frontmatter
# violates the Agent Skills spec that pi's own loader measures against.
#
# THE DEFECT THIS EXISTS TO CATCH, measured 2026-10-23 while adding the
# pi-devbox-firstboot skill. Its `description` was drafted at 1056 characters.
# pi's shipped loader (dist/core/skills.js) defines MAX_DESCRIPTION_LENGTH =
# 1024 and pushes a diagnostic when a description is longer — and then says, in
# a comment directly above the return:
#
#     // Still load the skill even with warnings, unless description is
#     // missing or empty.
#
# So an over-long description does NOT break the skill. That is exactly why it
# needs a gate rather than trust: the skill works, the violation is real, and
# the only thing that would ever tell you is a diagnostic emitted inside a pi
# session. Nothing in CI starts a pi session and reads skill diagnostics, so the
# warning is invisible here by construction. A spec violation that still works
# is the most durable kind of rot — nothing ever forces the fix.
#
# One failure in this family is NOT a warning but a silent drop: a missing or
# empty description returns `{ skill: null }`, so the skill is never offered to
# the model at all. The skill file ships, `test -f` passes in smoke-test.sh, the
# symlink resolves, `pi-devbox-version` lists it — and the skill does not exist
# as far as any agent is concerned. This gate separates the two cases in its
# output, because they need different urgency even though both are red.
#
# WHY THE BAKED TREE SPECIFICALLY. These skills are the FALLBACK layer (see
# skills/VENDORED.md): they are what a container resolves with no skillset repo
# mounted, which is also the case of a brand-new device that has nothing else.
# A skillset-owned skill has its own upstream review; a baked one has only this.
#
# MEASURED HEADROOM when this gate was added, which is why it is worth having:
#   credential-incident-response  1022  (2 chars spare)
#   pi-extensions                 1003  (21)
#   pi-devbox-firstboot            952  (72)
#   pi-devbox-environment          923  (101)
#   mempalace                      254  (770)
# Two of five are within one sentence of the limit, so "nobody would write a
# description that long" is already false here.
#
# A KNOWN COUPLING, stated rather than discovered later: `pi-extensions` and
# `mempalace` are VENDORED copies, so an upstream edit that pushes one over the
# limit turns this gate red on a file this repo does not author. That is
# deliberate and precedented — check-skill-floor.sh already couples to the
# pi-extensions package — because the image SHIPS those bytes, so the violation
# is ours to carry regardless of who typed it. The fix in that case is to
# refresh the snapshot after upstream trims, not to exempt the file.
#
# WHY NAME IS CHECKED TOO, including against the directory. The loader falls
# back to the PARENT DIRECTORY NAME when frontmatter has no usable `name`
# (`frontmatterName || parentDirName`), so a mismatch does not fail — it
# silently renames the skill, and every pointer written as
# `~/.agents/skills/<dir>/SKILL.md` then refers to a skill the model knows under
# a different name. The managed AGENTS.md block points at skills by path, so
# that divergence is load-bearing here.
#
# EXIT CODES (same contract as lint-shell.sh / check-skill-floor.sh /
# check-doc-drift.sh — a gate that cannot run must not pass):
#   0  every baked skill's frontmatter is valid
#   1  VIOLATION — at least one skill breaks the spec
#   2  cannot run — no skills directory, or no python3 with a YAML module
#
# WHY PYTHON AND NOT grep/awk. `description: >-` is a YAML folded block scalar:
# its value spans lines and is joined with spaces, so a line-oriented length
# check measures the wrong string. python3-yaml is already installed in the
# actionlint job and already used by check-doc-drift.sh, so this adds no new
# dependency. NB the heredoc form below is `python3 - "$arg"` (script on stdin,
# argument on the command line); `echo x | python3 <<'EOF'` cannot work, which
# is the SC2259 defect the shellcheck gate was added for.
set -euo pipefail

REPO_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)
SKILLS_DIR="${REPO_ROOT}/rootfs/usr/local/share/pi-devbox/skills"

usage() {
  cat <<'USAGE'
Usage: scripts/check-skill-frontmatter.sh [options]

Options:
  --skills-dir DIR   Validate DIR instead of the repo's baked skills tree
                     (used by the truth-table self-test).
  -h, --help         This text.

Exit: 0 valid | 1 violation | 2 cannot run
USAGE
}

while [ $# -gt 0 ]; do
  case "$1" in
    --skills-dir) shift; [ $# -gt 0 ] || { echo "::error::--skills-dir needs a value" >&2; exit 2; }; SKILLS_DIR="$1" ;;
    -h|--help) usage; exit 0 ;;
    *) echo "::error::unknown option: $1" >&2; usage >&2; exit 2 ;;
  esac
  shift
done

if [ ! -d "$SKILLS_DIR" ]; then
  echo "::error::skills directory not found: ${SKILLS_DIR}"
  echo "A gate that cannot run must not pass — this is a red 2, not a green tick."
  exit 2
fi

if ! command -v python3 >/dev/null 2>&1; then
  echo "::error::python3 not found; cannot parse YAML frontmatter"
  exit 2
fi

if ! python3 -c 'import yaml' >/dev/null 2>&1; then
  echo "::error::python3 has no yaml module (apt: python3-yaml); cannot parse frontmatter"
  exit 2
fi

python3 - "$SKILLS_DIR" <<'PY'
import os, re, sys
import yaml

# Mirrors pi's dist/core/skills.js. If pi changes these, change them here.
MAX_NAME_LENGTH = 64
MAX_DESCRIPTION_LENGTH = 1024

skills_dir = sys.argv[1]
fatal, violations, checked = [], [], 0

for name in sorted(os.listdir(skills_dir)):
    d = os.path.join(skills_dir, name)
    if not os.path.isdir(d):
        continue
    path = os.path.join(d, "SKILL.md")
    rel = os.path.relpath(path, skills_dir)

    if not os.path.isfile(path):
        fatal.append(f"{rel}: directory has no SKILL.md — the loader cannot see it")
        continue
    checked += 1

    raw = open(path, encoding="utf-8").read()
    m = re.match(r"^---\n(.*?)\n---\n", raw, re.S)
    if not m:
        fatal.append(f"{rel}: no YAML frontmatter block (must open on line 1 with ---)")
        continue
    try:
        fm = yaml.safe_load(m.group(1)) or {}
    except Exception as e:
        fatal.append(f"{rel}: frontmatter is not valid YAML: {e}")
        continue
    if not isinstance(fm, dict):
        fatal.append(f"{rel}: frontmatter is not a mapping")
        continue

    desc = fm.get("description")
    if not isinstance(desc, str) or desc.strip() == "":
        # The one hard drop: the loader returns skill: null for this.
        fatal.append(f"{rel}: description missing or empty — THE SKILL IS NEVER LOADED")
    elif len(desc) > MAX_DESCRIPTION_LENGTH:
        violations.append(
            f"{rel}: description is {len(desc)} chars, exceeds {MAX_DESCRIPTION_LENGTH} "
            f"(loader warns and still loads, so nothing else will tell you)"
        )

    fm_name = fm.get("name") if isinstance(fm.get("name"), str) else None
    eff = fm_name or name
    if len(eff) > MAX_NAME_LENGTH:
        violations.append(f"{rel}: name is {len(eff)} chars, exceeds {MAX_NAME_LENGTH}")
    if not re.fullmatch(r"[a-z0-9-]+", eff):
        violations.append(f"{rel}: name {eff!r} must be lowercase a-z, 0-9 and hyphens only")
    if eff.startswith("-") or eff.endswith("-"):
        violations.append(f"{rel}: name {eff!r} must not start or end with a hyphen")
    if "--" in eff:
        violations.append(f"{rel}: name {eff!r} must not contain a double hyphen")
    if fm_name is not None and fm_name != name:
        violations.append(
            f"{rel}: frontmatter name {fm_name!r} != directory {name!r} — the loader "
            f"would use {fm_name!r} while every path-based pointer says {name!r}"
        )

print(f"checked {checked} baked skill(s) in {skills_dir}")

if not fatal and not violations:
    print("OK: every baked skill's frontmatter is valid.")
    sys.exit(0)

for f in fatal:
    print(f"::error::{f}")
for v in violations:
    print(f"::error::{v}")
print(f"FAIL: {len(fatal)} fatal, {len(violations)} spec violation(s).")
sys.exit(1)
PY