ci: gate baked skill frontmatter against the loader's own limits
Lint / skill-floor (push) Successful in 9s
Lint / hadolint (push) Successful in 10s
Lint / doc-drift (push) Successful in 18s
Lint / skill-frontmatter (push) Successful in 19s
Lint / actionlint (push) Successful in 32s

Found while adding pi-devbox-firstboot, whose description was drafted at 1056
characters against pi's MAX_DESCRIPTION_LENGTH of 1024. pi does not reject it:
dist/core/skills.js pushes a diagnostic and then loads the skill anyway --
"Still load the skill even with warnings, unless description is missing or
empty". So the skill works, the violation is real, and the only thing that
would ever report it is a diagnostic inside a live pi session. Nothing in CI
starts one, so the warning is invisible here by construction. A spec violation
that still works is the most durable kind of rot: nothing forces the fix.

Unlike check-skill-floor.sh, this was NOT free to add. Measured headroom on the
five baked skills: credential-incident-response 1022 (2 chars spare),
pi-extensions 1003 (21), pi-devbox-firstboot 952, pi-devbox-environment 923,
mempalace 254. Two of five are within one sentence of the limit, so "nobody
would write a description that long" is already false here.

It also catches the sibling failure that is a silent DROP rather than a
warning: a missing or empty description returns skill: null, so the file ships,
the symlink resolves, smoke-test's `test -f` passes, pi-devbox-version lists it
-- and no agent can ever see it. Name rules are checked too, including against
the directory, because the loader falls back to parentDirName and a mismatch
silently renames the skill while every path-based pointer (the managed AGENTS
block included) keeps naming the directory.

Truth table, each mutation verified to differ materially from the shipped tree:
shipped 0; over-long description 1; missing description 1 (reported as fatal,
not a mere violation); name != directory 1; no frontmatter 1; directory with no
SKILL.md 1; absent skills tree 2. Exit codes match lint-shell.sh,
check-skill-floor.sh and check-doc-drift.sh -- a gate that cannot run must not
pass.

python3-yaml rather than grep/awk because `description: >-` is a folded block
scalar whose value spans lines and joins with spaces, so a line-oriented length
check measures the wrong string. Same package the actionlint job already
installs, same parser check-doc-drift.sh already uses.

The vendored copies (pi-extensions, mempalace) are deliberately in scope: an
upstream edit over the limit turns this red on a file this repo does not author,
which is precedented by check-skill-floor.sh's coupling and correct because the
image ships those bytes. The fix in that case is to refresh the snapshot, not to
exempt the file.
This commit is contained in:
pi@mbp-m1-2020
2026-10-24 00:32:13 +02:00
parent 5d6ff537c4
commit 206fd1ec35
3 changed files with 257 additions and 0 deletions
+46
View File
@@ -249,3 +249,49 @@ jobs:
- name: Doc claims match the build files - name: Doc claims match the build files
run: bash scripts/check-doc-drift.sh run: bash scripts/check-doc-drift.sh
skill-frontmatter:
# Gate the image-baked skills' frontmatter against the spec pi's own loader
# measures. Its own job, for the same reason as skill-floor and doc-drift:
# "a shipped skill violates the spec" should be a distinct red name, not a
# line buried in a job about workflow syntax.
#
# The gap it closes, measured 2026-10-23 while adding pi-devbox-firstboot:
# that skill's description was drafted at 1056 chars against the loader's
# MAX_DESCRIPTION_LENGTH of 1024. pi does NOT reject it — dist/core/skills.js
# pushes a diagnostic and then loads the skill anyway ("Still load the skill
# even with warnings, unless description is missing or empty"). So the only
# thing that would ever report the violation is a diagnostic inside a live pi
# session, and nothing in CI starts one. A spec violation that still works is
# the most durable kind of rot: nothing forces the fix.
#
# Not free in the way skill-floor was free: two of the five baked skills are
# already within ~20 chars of the limit (credential-incident-response 1022,
# pi-extensions 1003), so this fires on a plausible next edit rather than on
# a hypothetical one.
#
# The sibling failure it also catches is a silent DROP rather than a warning:
# a missing/empty description returns skill: null, so the file ships, the
# symlink resolves, smoke-test's `test -f` passes, pi-devbox-version lists
# it — and no agent can ever see it.
#
# Exit codes 0 valid / 1 violation / 2 cannot-run, matching lint-shell.sh,
# check-skill-floor.sh and check-doc-drift.sh. A missing skills tree or a
# python3 without the yaml module is a red 2, not a green tick.
runs-on: ubuntu-latest
container:
image: catthehacker/ubuntu:act-latest
steps:
- uses: actions/checkout@v4
# python3-yaml because `description: >-` is a folded block scalar: the
# value spans lines and joins with spaces, so a line-oriented length
# check measures the wrong string. Same package the actionlint job
# installs, same parser check-doc-drift.sh already relies on.
- name: Install python3-yaml
run: |
apt-get update
apt-get install -y --no-install-recommends python3-yaml
- name: Baked skill frontmatter is valid
run: bash scripts/check-skill-frontmatter.sh
+22
View File
@@ -51,6 +51,28 @@ Pre-v1.0.0 tags followed the pi npm version (`v{pi_version}[letter]`).
the round only through that line, so rewording the paragraph must not be able the round only through that line, so rewording the paragraph must not be able
to break the link silently. to break the link silently.
- **`scripts/check-skill-frontmatter.sh` + a `skill-frontmatter` lint job:
baked skill frontmatter is now gated against the limits pi's own loader
measures.** Found while adding the skill above, whose `description` was
drafted at 1056 characters against the loader's `MAX_DESCRIPTION_LENGTH` of
1024. pi does **not** reject that — `dist/core/skills.js` pushes a diagnostic
and then loads the skill anyway ("Still load the skill even with warnings,
unless description is missing or empty"), so the only thing that would ever
report the violation is a diagnostic inside a live pi session — and nothing in
CI starts one. A spec violation that still works is the most durable kind of
rot, because nothing forces the fix.
Unlike `check-skill-floor.sh`, this one was **not** free to add: two of the
five baked skills are already within ~20 characters of the limit
(`credential-incident-response` 1022, `pi-extensions` 1003), so it fires on a
plausible next edit rather than a hypothetical one. It also catches the
sibling failure that is a silent **drop** rather than a warning — missing or
empty `description` returns `skill: null`, so the file ships, the symlink
resolves, smoke-test's `test -f` passes, `pi-devbox-version` lists it, and no
agent can ever see it. Exit codes 0 valid / 1 violation / 2 cannot-run, and
the vendored copies are deliberately in scope: the image ships those bytes,
so the violation is ours to carry whoever typed it.
### Fixed ### Fixed
- **CI: a smoke job's image could be deleted mid-test by another job's disk - **CI: a smoke job's image could be deleted mid-test by another job's disk
+189
View File
@@ -0,0 +1,189 @@
#!/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