diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index fbb2161..89d3cd5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -50,6 +50,12 @@ jobs: # skill needs a row in the catalogue's skill-license table. run: bash scripts/check-skill-licenses.sh + - name: ✍️ Check first-party skill authoring rules + # A first-party skill's references sit one level deep and are linked from its + # entry file, and its description stays inside the recorded limit. Exceptions + # carry a reason in scripts/skill-authoring-exceptions.json and can only shrink. + run: bash scripts/check-skill-authoring.sh + lint-scripts: name: Lint scripts runs-on: ubuntu-latest @@ -310,6 +316,9 @@ jobs: - name: 🧪 Self-test the skill license check run: bash scripts/check-skill-licenses.test.sh + - name: 🧪 Self-test the skill authoring check + run: bash scripts/check-skill-authoring.test.sh + - name: 🧪 Self-test the desired-state digest generator # Proves the generator writes exactly what validate-manifests.sh demands — a # generator that disagreed with the gate would leave a sync branch just as diff --git a/docs/scripts.md b/docs/scripts.md index 2cb5b6b..00fb909 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -14,7 +14,7 @@ complete inventory; read a script's header for its arguments and safeguards. | Propose or publish a marketplace release | [Release guide](marketplace-releases.md), which explains the separate opt-in steps and recovery | | Recheck open pull requests after changing CI | [recheck-open-prs.sh](../scripts/recheck-open-prs.sh) | -The directory also contains checks for package discovery, skill provenance and licensing, +The directory also contains checks for package discovery, skill provenance, licensing and authoring rules, version changes, changelogs, generated writes, release observations and instruction size. The [validation guide](validation.md) explains which checks to run and what they establish. diff --git a/docs/skill-lifecycle.md b/docs/skill-lifecycle.md index bafaf2b..b5da61a 100644 --- a/docs/skill-lifecycle.md +++ b/docs/skill-lifecycle.md @@ -96,9 +96,27 @@ catalogued, external skills keep their provenance and are not edited, every bund licence record, each `SKILL.md` validates against the open skill format, and a changed plugin has a changed version. [`validation.md`](validation.md) has the commands. -Not checked by CI: whether a skill's examples are correct, and whether it loads for the tasks -it is meant for. The first is the correctness gate in the scorecard. The second is what the -activation suite in #651 will measure; until then it is the trial in each pull request. +Checked for first-party skills only, because external skills are not edited here: + +- **Entry file size.** `SKILL.md` is at most 500 lines and 20,000 bytes. +- **References.** Every file under `references/` sits directly in that folder. Each Markdown + reference is linked from `SKILL.md`. A worked example or data file is linked from `SKILL.md` + or from the Markdown reference that explains it. +- **Description length.** At most 600 characters. The limit was set on 2026-11-10 from the 18 + first-party skills: 16 were at or under it, and the format itself allows 1,024. CI prints + every first-party description's length on each run. + +A skill that cannot meet a reference or description rule yet is recorded in +[`skill-authoring-exceptions.json`](../scripts/skill-authoring-exceptions.json) with its +reason. The record can only shrink: CI fails when an exception is no longer needed, when a +recorded description grows, and when its recorded ceiling is higher than the measured length. +The five first exceptions are tracked in #680. + +Not checked by CI: whether a skill's examples are correct, whether it loads for the tasks it +is meant for, and whether its acceptance scenarios exist. The first is the correctness gate in +the scorecard. The other two belong to the activation suite in #651, which decides where +scenarios live; until then they are the trial in each pull request and the scenarios recorded +on the skill's issue. ## Keeping sources current diff --git a/docs/validation.md b/docs/validation.md index d408000..1a68f28 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -55,6 +55,11 @@ bash scripts/plugin-scorecard.test.sh # plugins//resources/THIRD-PARTY-NOTICES.md; every skill needs a row in docs/plugins.md. bash scripts/check-skill-licenses.sh bash scripts/check-skill-licenses.test.sh +# A first-party skill's references sit one level deep and are linked from its entry file, and its +# description stays inside the limit (CI's "Check first-party skill authoring rules" step). Needs +# jq and the Go yq. Exceptions, each with its reason: scripts/skill-authoring-exceptions.json. +bash scripts/check-skill-authoring.sh +bash scripts/check-skill-authoring.test.sh # Remote MCP URL syntax uses the repository-local Go helper (Go ≥1.22, standard library only). GOENV=off GOWORK=off GO111MODULE=off GOTOOLCHAIN=local GOFLAGS='' CGO_ENABLED=0 \ go test scripts/mcp-url-go/main.go scripts/mcp-url-go/main_test.go diff --git a/scripts/check-skill-authoring.sh b/scripts/check-skill-authoring.sh new file mode 100755 index 0000000..051ec55 --- /dev/null +++ b/scripts/check-skill-authoring.sh @@ -0,0 +1,237 @@ +#!/usr/bin/env bash +# Check the authoring rules for first-party skills that no other check enforces (#678). +# +# A first-party skill is one whose SKILL.md records no install provenance: the same test +# validate-manifests.sh applies. An external skill is owned upstream, is never edited here +# and is not judged by this check. +# +# RULES (the skill-lifecycle skill's references/authoring.md states them) +# 1. Everything under a skill's references/ folder is a regular file directly inside it. +# 2. Every Markdown file in references/ is linked from SKILL.md. Any other file there (a +# worked example, a data file) is linked from SKILL.md or from a Markdown file in +# references/. +# 3. The description is no longer than the recorded limit, in characters. +# The entry file's own 500-line / 20000-byte budget is checked by validate-manifests.sh. +# +# EXCEPTIONS are recorded, with a reason each, in scripts/skill-authoring-exceptions.json +# (read from the working directory). They can only shrink: an exception whose rule now +# holds, whose file is gone, or whose recorded description ceiling no longer equals the +# measured length fails the check until the record is corrected. +# +# LINKS are read from Markdown outside fenced code blocks: inline `[text](target)` links +# and `[label]: target` definitions. A `#fragment` is ignored. A target in angle brackets +# or with spaces is not recognised and is reported as a missing link. +# +# Operates on the current working directory (run from the repository root, as CI does). +# +# USAGE check-skill-authoring.sh +# EXIT 0 every first-party skill follows the rules +# 1 a rule is broken, an exception is stale, or a skill cannot be classified +# 2 usage error, a required tool is missing, or the exception record is unusable +set -euo pipefail + +here=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=scripts/frontmatter.lib.sh +. "$here/frontmatter.lib.sh" + +unusable() { + printf 'check-skill-authoring: %s\n' "$*" >&2 + exit 2 +} + +[ "$#" -eq 0 ] || unusable 'usage: check-skill-authoring.sh (no arguments; run from the repository root)' +for tool in jq yq awk find sort; do + command -v "$tool" > /dev/null 2>&1 || unusable "$tool is required" +done +# The description is decoded by a YAML parser, never by pattern matching. Only the Go yq +# (mikefarah) reads Markdown front matter; another yq must not be trusted to mean the same. +case $(yq --version 2>&1) in + *mikefarah*) ;; + *) unusable 'the Go yq (github.com/mikefarah/yq) is required' ;; +esac +[ -d plugins ] || unusable 'no plugins directory here: run from the repository root' + +exceptions=scripts/skill-authoring-exceptions.json +[ -f "$exceptions" ] || unusable "$exceptions is missing" +# One exact shape, so a typo cannot silently drop a rule or an exception. +jq -e ' + def text: type == "string" and (gsub("\\s"; "") | length) > 0; + type == "object" and (keys == ["description", "references"]) and + (.description | type == "object" and (keys == ["exceptions", "limit"]) and + (.limit | type == "number" and . == floor and . >= 1 and . <= 1024)) and + (.description.limit as $limit | .description.exceptions | type == "array" and all(.[]; + type == "object" and (keys == ["ceiling", "reason", "skill"]) and + (.skill | text) and (.reason | text) and + (.ceiling | type == "number" and . == floor and . > $limit and . <= 1024))) and + ((.description.exceptions | map(.skill)) as $skills | ($skills | unique | length) == ($skills | length)) and + (.references | type == "object" and (keys == ["exceptions"]) and + (.exceptions | type == "array" and all(.[]; + type == "object" and (keys == ["file", "reason"]) and (.file | text) and (.reason | text)))) and + ((.references.exceptions | map(.file)) as $files | ($files | unique | length) == ($files | length)) +' "$exceptions" > /dev/null 2>&1 || unusable "$exceptions does not have the expected shape" + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT +limit=$(jq -r '.description.limit' "$exceptions") +jq -r '.description.exceptions[] | [.skill, (.ceiling | tostring)] | @tsv' "$exceptions" > "$work/description-exceptions" +jq -r '.references.exceptions[].file' "$exceptions" > "$work/reference-exceptions" +: > "$work/description-exceptions-used" +: > "$work/reference-exceptions-used" + +failed=0 +fail() { # file message + echo "::error file=$1::$2" + failed=1 +} + +# Print every link target in a Markdown file, one per line, without its #fragment. +link_targets() { + awk ' + /^[ \t]*(```|~~~)/ { fence = !fence; next } + fence { next } + { + line = $0 + if (match(line, /^ ? ? ?\[[^]]+\]:[ \t]*[^ \t]+/)) { + target = substr(line, RSTART, RLENGTH) + sub(/^ ? ? ?\[[^]]+\]:[ \t]*/, "", target) + sub(/#.*$/, "", target) + if (target != "") print target + } + while (match(line, /\]\([^) \t]+/)) { + target = substr(line, RSTART + 2, RLENGTH - 2) + line = substr(line, RSTART + RLENGTH) + sub(/#.*$/, "", target) + if (target != "") print target + } + } + ' "$1" +} + +# Count characters, not bytes: every UTF-8 character has exactly one non-continuation byte. +character_count() { + printf '%s' "$1" | LC_ALL=C tr -d '\200-\277' | wc -c | tr -d '[:space:]' +} + +check_description() { # skill-dir + local dir=$1 skill=$1/SKILL.md tag description length ceiling + tag=$(yq --front-matter=extract '.description | tag' "$skill" 2> /dev/null) || tag='' + if [ "$tag" != '!!str' ]; then + fail "$skill" "the description could not be read as text, so its length is unknown" + return 0 + fi + description=$(yq --front-matter=extract '.description' "$skill") + length=$(character_count "$description") + printf ' %4s characters %s\n' "$length" "$dir" + ceiling=$(awk -F '\t' -v skill="$dir" '$1 == skill { print $2 }' "$work/description-exceptions") + if [ -n "$ceiling" ]; then + printf '%s\n' "$dir" >> "$work/description-exceptions-used" + if [ "$length" -le "$limit" ]; then + fail "$exceptions" "$dir: the description is now $length characters, within the limit of $limit; remove its exception" + elif [ "$length" -gt "$ceiling" ]; then + fail "$skill" "the description is $length characters; its recorded exception allows $ceiling. Shorten it" + elif [ "$length" -lt "$ceiling" ]; then + fail "$exceptions" "$dir: the description is now $length characters; lower its recorded ceiling from $ceiling to $length" + fi + elif [ "$length" -gt "$limit" ]; then + fail "$skill" "the description is $length characters; the limit for a first-party skill is $limit. Shorten it, or record an exception with its reason in $exceptions" + fi +} + +check_references() { # skill-dir + local dir=$1 refs=$1/references entry rel name document + if [ ! -e "$refs" ] && [ ! -L "$refs" ]; then return 0; fi + if [ -L "$refs" ] || [ ! -d "$refs" ]; then + fail "$refs" 'references must be a real directory' + return 0 + fi + link_targets "$dir/SKILL.md" | sed 's|^\./||' > "$work/entry-links" + : > "$work/reference-links" + if ! find "$refs" -mindepth 1 -maxdepth 1 -type f -name '*.md' -print0 > "$work/documents" || + ! find "$refs" -mindepth 1 -print0 > "$work/entries-unsorted" || + ! LC_ALL=C sort -z "$work/entries-unsorted" > "$work/entries"; then + fail "$refs" 'the references folder could not be read, so nothing in it was checked' + return 0 + fi + while IFS= read -r -d '' document; do + link_targets "$document" | sed 's|^\./||' >> "$work/reference-links" + done < "$work/documents" + while IFS= read -r -d '' entry; do + rel=${entry#"$dir"/} + if [ -L "$entry" ]; then + fail "$entry" 'a reference must be a regular file, not a link' + continue + fi + [ ! -d "$entry" ] || continue + if [ ! -f "$entry" ]; then + fail "$entry" 'a reference must be a regular file' + continue + fi + name=${rel#references/} + case $name in + */*) + fail "$entry" "references sit one level deep: move this file directly into $refs" + continue + ;; + esac + if grep -Fxq -- "$rel" "$work/entry-links"; then + if grep -Fxq -- "$entry" "$work/reference-exceptions"; then + printf '%s\n' "$entry" >> "$work/reference-exceptions-used" + fail "$exceptions" "$entry is now linked from SKILL.md; remove its exception" + fi + continue + fi + case $name in + *.md) ;; + *) + # A worked example belongs to the document that explains it. + if grep -Fxq -- "$name" "$work/reference-links"; then continue; fi + fail "$entry" "this file is not linked from SKILL.md or from a Markdown file in $refs" + continue + ;; + esac + if grep -Fxq -- "$entry" "$work/reference-exceptions"; then + printf '%s\n' "$entry" >> "$work/reference-exceptions-used" + continue + fi + fail "$entry" "this reference is not linked from SKILL.md. Link it there, so a reader of the entry file can find it" + done < "$work/entries" +} + +if ! find plugins -type f -path '*/skills/*/SKILL.md' -print0 > "$work/skills-unsorted" || + ! LC_ALL=C sort -z "$work/skills-unsorted" > "$work/skills"; then + unusable 'the skills under plugins could not be listed' +fi +[ -s "$work/skills" ] || unusable 'no skills found under plugins: run from the repository root' + +echo "First-party skill descriptions (limit: $limit characters):" +first_party=0 +while IFS= read -r -d '' skill; do + if owner=$(frontmatter_repository < "$skill") && ! frontmatter_has_value "$skill" github-repo; then + [ -z "$owner" ] || continue + else + fail "$skill" 'the skill cannot be classified as first-party or external (validate-manifests.sh reports why)' + continue + fi + first_party=$((first_party + 1)) + dir=${skill%/SKILL.md} + check_description "$dir" + check_references "$dir" +done < "$work/skills" + +# An exception that matched nothing protects nothing: it names a skill or file that is gone. +while IFS=$'\t' read -r skill _; do + [ -n "$skill" ] || continue + grep -Fxq -- "$skill" "$work/description-exceptions-used" || + fail "$exceptions" "$skill: the description exception names no first-party skill; remove it" +done < "$work/description-exceptions" +while IFS= read -r file; do + [ -n "$file" ] || continue + grep -Fxq -- "$file" "$work/reference-exceptions-used" || + fail "$exceptions" "$file: the reference exception names no reference of a first-party skill; remove it" +done < "$work/reference-exceptions" + +if [ "$failed" -ne 0 ]; then + echo "✗ first-party skill authoring rules: see the errors above" >&2 + exit 1 +fi +echo "✓ $first_party first-party skills follow the authoring rules" diff --git a/scripts/check-skill-authoring.test.sh b/scripts/check-skill-authoring.test.sh new file mode 100755 index 0000000..350ae3c --- /dev/null +++ b/scripts/check-skill-authoring.test.sh @@ -0,0 +1,292 @@ +#!/usr/bin/env bash +# Self-test for check-skill-authoring.sh: run the real check against fixture repositories and +# assert its exit status and the message that names the broken rule. +# Markdown and YAML fragments below are matched and written literally. +# shellcheck disable=SC2016 +set -euo pipefail +here=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT +failures=0 +ours=plugins/alpha/skills/ours +record=scripts/skill-authoring-exceptions.json + +# Write the exception record of a fixture: limit, description exceptions, reference exceptions. +exceptions() { # root limit description-exceptions-json reference-exceptions-json + printf '{"description":{"limit":%s,"exceptions":%s},"references":{"exceptions":%s}}\n' \ + "$2" "$3" "$4" > "$1/$record" +} + +# Write the first-party skill's entry file with a given description line and body. +entry() { # root description-yaml body + printf -- '---\nname: ours\n%s\n---\n# ours\n\n%s\n' "$2" "$3" > "$1/$ours/SKILL.md" +} + +# A consistent repository. The first-party skill links its one reference, which links its +# worked example. The external skill breaks every rule, and must not be judged. +fixture() { + local root=$work/$1 + mkdir -p "$root/scripts" "$root/$ours/references" "$root/plugins/alpha/skills/theirs/references/deep" + exceptions "$root" 60 '[]' '[]' + entry "$root" 'description: Fixture skill for the authoring check.' 'Read [the guide](references/guide.md).' + printf '# Guide\n\nSee [the example](example.json).\n' > "$root/$ours/references/guide.md" + printf '{}\n' > "$root/$ours/references/example.json" + { + printf -- '---\nname: theirs\ndescription: %s\n' "$(printf 'x%.0s' $(seq 1 200))" + printf 'metadata:\n github-repo: https://github.com/example/skills\n---\n# theirs\n' + } > "$root/plugins/alpha/skills/theirs/SKILL.md" + printf 'Unlinked and nested.\n' > "$root/plugins/alpha/skills/theirs/references/deep/note.md" + printf '%s\n' "$root" +} + +# Run the real check in a fixture and compare its exit status and messages. +expect() { # label expected-exit root [message...] + local label=$1 want=$2 root=$3 rc=0 message + shift 3 + (cd "$root" && bash "$here/check-skill-authoring.sh") > "$work/out" 2>&1 || rc=$? + if [ "$rc" -ne "$want" ]; then + echo "FAIL $label: expected exit $want, got $rc" + sed 's/^/ /' "$work/out" + failures=$((failures + 1)) + return 0 + fi + for message in "$@"; do + if ! grep -Fq -- "$message" "$work/out"; then + echo "FAIL $label: the output lacks: $message" + sed 's/^/ /' "$work/out" + failures=$((failures + 1)) + return 0 + fi + done + echo "PASS $label" +} + +repeat() { # text count + local out='' i + for ((i = 0; i < $2; i++)); do out+=$1; done + printf '%s' "$out" +} + +# --- the consistent fixture, and what is out of scope --- +root=$(fixture consistent) +expect 'a consistent repository passes and reports the measured length' 0 "$root" \ + " 38 characters $ours" '1 first-party skills follow the authoring rules' +if grep -Fq 'theirs' "$work/out"; then + echo 'FAIL an external skill was judged' + failures=$((failures + 1)) +else + echo 'PASS an external skill is not judged' +fi + +# --- references sit one level deep --- +root=$(fixture nested) +mkdir -p "$root/$ours/references/deep" +printf 'Nested.\n' > "$root/$ours/references/deep/note.md" +entry "$root" 'description: Fixture skill.' 'Read [the guide](references/guide.md) and [a note](references/deep/note.md).' +expect 'a nested reference fails even when linked' 1 "$root" \ + "file=$ours/references/deep/note.md::references sit one level deep" + +# --- every Markdown reference is linked from the entry file --- +root=$(fixture orphan) +printf 'Nobody links this.\n' > "$root/$ours/references/orphan.md" +expect 'an unlinked Markdown reference fails' 1 "$root" \ + "file=$ours/references/orphan.md::this reference is not linked from SKILL.md" + +root=$(fixture chained) +printf '# Guide\n\nSee [the example](example.json) and [more](more.md).\n' > "$root/$ours/references/guide.md" +printf 'Reached only through the guide.\n' > "$root/$ours/references/more.md" +expect 'a Markdown reference reached only through another reference fails' 1 "$root" \ + "file=$ours/references/more.md::this reference is not linked from SKILL.md" +exceptions "$root" 60 '[]' "[{\"file\":\"$ours/references/more.md\",\"reason\":\"Fixture reason.\"}]" +expect 'a recorded exception admits a chained Markdown reference' 0 "$root" + +root=$(fixture fenced) +entry "$root" 'description: Fixture skill.' '``` +Read [the guide](references/guide.md). +```' +expect 'a link inside a fenced code block is not a link' 1 "$root" \ + "file=$ours/references/guide.md::this reference is not linked from SKILL.md" + +root=$(fixture fragment) +entry "$root" 'description: Fixture skill.' 'Read [the guide](./references/guide.md#part "Title").' +expect 'a ./ prefix, a fragment and a title still link the reference' 0 "$root" + +root=$(fixture definition) +entry "$root" 'description: Fixture skill.' 'Read [the guide][guide]. + +[guide]: references/guide.md' +expect 'a link definition links the reference' 0 "$root" + +root=$(fixture mention) +entry "$root" 'description: Fixture skill.' 'Read `references/guide.md`.' +expect 'a mention without a link fails' 1 "$root" \ + "file=$ours/references/guide.md::this reference is not linked from SKILL.md" + +# --- a worked example belongs to the entry file or to a Markdown reference --- +root=$(fixture stray) +printf '{}\n' > "$root/$ours/references/stray.json" +expect 'a data file linked from nowhere fails' 1 "$root" \ + "file=$ours/references/stray.json::this file is not linked from SKILL.md or from a Markdown file" +exceptions "$root" 60 '[]' "[{\"file\":\"$ours/references/stray.json\",\"reason\":\"Fixture reason.\"}]" +expect 'an exception cannot admit a data file linked from nowhere' 1 "$root" \ + "file=$ours/references/stray.json::this file is not linked" + +root=$(fixture entry-example) +printf '# Guide\n\nNo example here.\n' > "$root/$ours/references/guide.md" +entry "$root" 'description: Fixture skill.' 'Read [the guide](references/guide.md) and [the example](references/example.json).' +expect 'a data file linked from the entry file passes' 0 "$root" + +# --- only regular files --- +root=$(fixture link) +ln -s guide.md "$root/$ours/references/alias.md" +expect 'a symbolic link among the references fails' 1 "$root" \ + "file=$ours/references/alias.md::a reference must be a regular file, not a link" + +root=$(fixture linked-folder) +mv "$root/$ours/references" "$root/$ours/elsewhere" +ln -s elsewhere "$root/$ours/references" +expect 'a references folder that is a link fails' 1 "$root" \ + "file=$ours/references::references must be a real directory" + +# --- stale reference exceptions --- +root=$(fixture stale-linked) +exceptions "$root" 60 '[]' "[{\"file\":\"$ours/references/guide.md\",\"reason\":\"Fixture reason.\"}]" +expect 'an exception for a reference that is linked fails as stale' 1 "$root" \ + "$ours/references/guide.md is now linked from SKILL.md; remove its exception" + +root=$(fixture stale-missing) +exceptions "$root" 60 '[]' "[{\"file\":\"$ours/references/gone.md\",\"reason\":\"Fixture reason.\"}]" +expect 'an exception for a missing reference fails as stale' 1 "$root" \ + "$ours/references/gone.md: the reference exception names no reference of a first-party skill" + +# --- description length, in characters --- +root=$(fixture at-limit) +entry "$root" "description: $(repeat a 60)" 'Read [the guide](references/guide.md).' +expect 'a description at the limit passes' 0 "$root" " 60 characters $ours" + +root=$(fixture over-limit) +entry "$root" "description: $(repeat a 61)" 'Read [the guide](references/guide.md).' +expect 'a description over the limit fails' 1 "$root" \ + "file=$ours/SKILL.md::the description is 61 characters; the limit for a first-party skill is 60" + +root=$(fixture multibyte) +entry "$root" "description: $(repeat é 60)" 'Read [the guide](references/guide.md).' +expect 'characters are counted, not bytes' 0 "$root" " 60 characters $ours" +entry "$root" "description: $(repeat é 61)" 'Read [the guide](references/guide.md).' +expect 'a multi-byte description over the limit fails' 1 "$root" 'the description is 61 characters' + +root=$(fixture folded) +entry "$root" 'description: >- + line one + line two' 'Read [the guide](references/guide.md).' +expect 'a folded description is measured as the text a host reads' 0 "$root" " 17 characters $ours" + +root=$(fixture no-description) +printf -- '---\nname: ours\n---\n# ours\n\nRead [the guide](references/guide.md).\n' > "$root/$ours/SKILL.md" +expect 'a description that cannot be read fails' 1 "$root" \ + "file=$ours/SKILL.md::the description could not be read as text" + +# --- description exceptions are a ratchet --- +root=$(fixture ceiling) +entry "$root" "description: $(repeat a 70)" 'Read [the guide](references/guide.md).' +exceptions "$root" 60 "[{\"skill\":\"$ours\",\"ceiling\":70,\"reason\":\"Fixture reason.\"}]" '[]' +expect 'a recorded ceiling equal to the measured length passes' 0 "$root" +exceptions "$root" 60 "[{\"skill\":\"$ours\",\"ceiling\":80,\"reason\":\"Fixture reason.\"}]" '[]' +expect 'a ceiling above the measured length fails as stale' 1 "$root" \ + "$ours: the description is now 70 characters; lower its recorded ceiling from 80 to 70" +exceptions "$root" 60 "[{\"skill\":\"$ours\",\"ceiling\":65,\"reason\":\"Fixture reason.\"}]" '[]' +expect 'a description above its ceiling fails' 1 "$root" \ + "file=$ours/SKILL.md::the description is 70 characters; its recorded exception allows 65" +entry "$root" "description: $(repeat a 50)" 'Read [the guide](references/guide.md).' +expect 'an exception for a description inside the limit fails as stale' 1 "$root" \ + "$ours: the description is now 50 characters, within the limit of 60; remove its exception" + +root=$(fixture ceiling-missing) +exceptions "$root" 60 '[{"skill":"plugins/alpha/skills/gone","ceiling":70,"reason":"Fixture reason."}]' '[]' +expect 'an exception for a missing skill fails as stale' 1 "$root" \ + 'plugins/alpha/skills/gone: the description exception names no first-party skill' + +root=$(fixture ceiling-external) +exceptions "$root" 60 '[{"skill":"plugins/alpha/skills/theirs","ceiling":200,"reason":"Fixture reason."}]' '[]' +expect 'an exception cannot name an external skill' 1 "$root" \ + 'plugins/alpha/skills/theirs: the description exception names no first-party skill' + +# --- a skill that cannot be classified is never skipped --- +root=$(fixture unclassified) +printf -- '---\nname: ours\ndescription: Partial install.\nmetadata:\n github-path: skills/ours\n---\nBody.\n' \ + > "$root/$ours/SKILL.md" +expect 'a skill with partial install provenance fails' 1 "$root" \ + "file=$ours/SKILL.md::the skill cannot be classified as first-party or external" + +# --- an unusable setup is never a pass --- +root=$(fixture bad-record) +printf '{}\n' > "$root/$record" +expect 'an empty exception record is refused' 2 "$root" 'does not have the expected shape' +printf '{"description":{"limit":0,"exceptions":[]},"references":{"exceptions":[]}}\n' > "$root/$record" +expect 'a limit of zero is refused' 2 "$root" 'does not have the expected shape' +exceptions "$root" 60 "[{\"skill\":\"$ours\",\"ceiling\":60,\"reason\":\"Fixture reason.\"}]" '[]' +expect 'a ceiling that is not above the limit is refused' 2 "$root" 'does not have the expected shape' +exceptions "$root" 60 "[{\"skill\":\"$ours\",\"ceiling\":70,\"reason\":\" \"}]" '[]' +expect 'an exception without a reason is refused' 2 "$root" 'does not have the expected shape' +exceptions "$root" 60 "[{\"skill\":\"$ours\",\"ceiling\":70,\"reason\":\"One.\"},{\"skill\":\"$ours\",\"ceiling\":80,\"reason\":\"Two.\"}]" '[]' +expect 'a skill recorded twice is refused' 2 "$root" 'does not have the expected shape' +printf '{"description":{"limit":60,"exceptions":[]},"references":{"exceptions":[]},"extra":true}\n' > "$root/$record" +expect 'an unknown key is refused' 2 "$root" 'does not have the expected shape' +rm "$root/$record" +expect 'a missing exception record is refused' 2 "$root" "$record is missing" + +root=$(fixture no-plugins) +rm -rf "$root/plugins" +expect 'a directory without plugins is refused' 2 "$root" 'no plugins directory here' + +root=$(fixture other-yq) +mkdir "$work/stub" +printf '#!/bin/sh\necho "yq 3.4.3"\n' > "$work/stub/yq" +chmod +x "$work/stub/yq" +rc=0 +(cd "$root" && PATH="$work/stub:$PATH" bash "$here/check-skill-authoring.sh") > "$work/out" 2>&1 || rc=$? +if [ "$rc" -eq 2 ] && grep -Fq 'the Go yq' "$work/out"; then + echo 'PASS another yq is refused' +else + echo "FAIL another yq was not refused (exit $rc)" + failures=$((failures + 1)) +fi + +rc=0 +(cd "$root" && bash "$here/check-skill-authoring.sh" --anything) > "$work/out" 2>&1 || rc=$? +if [ "$rc" -eq 2 ]; then + echo 'PASS an argument is refused' +else + echo "FAIL an argument was not refused (exit $rc)" + failures=$((failures + 1)) +fi + +# --- this repository --- +expect 'this repository passes with its recorded exceptions' 0 "$here/.." 'first-party skills follow the authoring rules' + +# Without its exceptions the same tree must fail on exactly the recorded places, which proves +# the check reads the real skills and that no exception is recorded without need. +copy=$work/repository +mkdir -p "$copy/scripts" +cp -R "$here/../plugins" "$copy/plugins" +limit=$(jq -r '.description.limit' "$here/../$record") +exceptions "$copy" "$limit" '[]' '[]' +rc=0 +(cd "$copy" && bash "$here/check-skill-authoring.sh") > "$work/out" 2>&1 || rc=$? +recorded=$(jq -r '[.description.exceptions[], .references.exceptions[]] | length' "$here/../$record") +reported=$(grep -c '^::error' "$work/out" || true) +if [ "$rc" -eq 1 ] && [ "$reported" -eq "$recorded" ]; then + echo "PASS without its exceptions this repository fails in exactly the $recorded recorded places" +elif [ "$recorded" -eq 0 ] && [ "$rc" -eq 0 ]; then + echo 'PASS this repository needs no exceptions' +else + echo "FAIL without its exceptions this repository reported $reported errors for $recorded recorded exceptions (exit $rc)" + sed 's/^/ /' "$work/out" + failures=$((failures + 1)) +fi + +if [ "$failures" -ne 0 ]; then + echo "FAIL: $failures case(s) failed" + exit 1 +fi +echo 'PASS: the first-party skill authoring check holds every rule and refuses every unusable setup' diff --git a/scripts/ci-script-test-sharding-workflow.test.sh b/scripts/ci-script-test-sharding-workflow.test.sh index 7d9f4b2..5b7713c 100755 --- a/scripts/ci-script-test-sharding-workflow.test.sh +++ b/scripts/ci-script-test-sharding-workflow.test.sh @@ -104,6 +104,7 @@ check_workflow() { 'bash scripts/package-discovery.test.sh' 'bash scripts/desired-state-boundaries.test.sh' 'bash scripts/check-skill-licenses.test.sh' + 'bash scripts/check-skill-authoring.test.sh' './scripts/refresh-desired-state-digests.test.sh || result=1' 'bash scripts/digest-declarations.test.sh || result=1' 'bash scripts/generated-write-safety.test.sh' diff --git a/scripts/skill-authoring-exceptions.json b/scripts/skill-authoring-exceptions.json new file mode 100644 index 0000000..8897f06 --- /dev/null +++ b/scripts/skill-authoring-exceptions.json @@ -0,0 +1,33 @@ +{ + "description": { + "limit": 600, + "exceptions": [ + { + "skill": "plugins/agentic-engineering/skills/agent-improvement", + "ceiling": 718, + "reason": "Older than the limit. A description change alters when the skill loads, so it is shortened together with a re-run of the skill's scenarios (#680)." + }, + { + "skill": "plugins/agentic-engineering/skills/product-engineering", + "ceiling": 601, + "reason": "Older than the limit. A description change alters when the skill loads, so it is shortened together with a re-run of the skill's scenarios (#680)." + } + ] + }, + "references": { + "exceptions": [ + { + "file": "plugins/agentic-engineering/skills/agent-improvement/references/prioritization-flow.md", + "reason": "Reached through references/scoring.md. The entry file is within 50 bytes of its 20,000-byte budget, so the direct link waits until the entry file is trimmed (#680)." + }, + { + "file": "plugins/agentic-engineering/skills/product-engineering/references/shared-code.md", + "reason": "Reached through references/code-quality.md. The entry file is within 50 bytes of its 20,000-byte budget, so the direct link waits until the entry file is trimmed (#680)." + }, + { + "file": "plugins/agentic-engineering/skills/product-engineering/references/tool-maturation.md", + "reason": "Reached through references/code-quality.md. The entry file is within 50 bytes of its 20,000-byte budget, so the direct link waits until the entry file is trimmed (#680)." + } + ] + } +} diff --git a/scripts/validate-manifests.test.sh b/scripts/validate-manifests.test.sh index 0642bba..ce32c6d 100755 --- a/scripts/validate-manifests.test.sh +++ b/scripts/validate-manifests.test.sh @@ -473,6 +473,24 @@ Body. EOF check_pass "locally authored skill without install metadata passes" "$d" +# An authored entry file stays inside its 500-line / 20000-byte budget (#618, #678). +authored_skill() { # dir body-lines line-text + { + printf '%s\n' '---' 'name: example-skill' 'description: Hand-authored skill.' '---' + local i + for ((i = 0; i < $2; i++)); do printf '%s\n' "$3"; done + } > "$1/plugins/alpha/skills/example-skill/SKILL.md" +} +d=$(fresh) +authored_skill "$d" 496 'Body.' +check_pass "authored skill of exactly 500 lines passes" "$d" +d=$(fresh) +authored_skill "$d" 497 'Body.' +check_fail "authored skill over 500 lines fails" "authored skill exceeds its 500-line / 20000-byte budget" "$d" +d=$(fresh) +authored_skill "$d" 100 "$(printf 'x%.0s' $(seq 1 200))" +check_fail "authored skill over 20000 bytes fails" "authored skill exceeds its 500-line / 20000-byte budget" "$d" + # Orphaned installation fields must never become local ownership. for install_field in github-path github-ref github-tree-sha github-pinned; do d=$(fresh)