From eb2f8c26ba591560a89cb7f6cb795805cdf42c1a Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:38:49 -0400 Subject: [PATCH 1/5] feat(skill-quality): add the shared listing-budget report, fix check 2's joiner (#1404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check-skill.sh check 2 only ever guarded the per-skill listing-entry cap (skillListingMaxDescChars, 1536 chars); the shared budget every loaded skill draws from together (skillListingBudgetFraction, default 1% of the model's context window) had no check at all. New check-listing-budget.sh pools one or more skills roots into a single aggregate estimate against a documented, overridable default (the harness's own SLASH_COMMAND_TOOL_CHAR_BUDGET fallback of 8000 chars), reports the biggest contributors on overflow, and never hardcodes a resolved live value (context window and skillListingBudgetFraction are both consumer settings this static check cannot observe) — always advisory, exit 0. Wired into the skill-quality-gate CI job as a report-only step pooling every plugin's skills/ root into one marketplace-wide aggregate. Also fixes check 2 itself: the harness assembles a listing entry as description + " - " + when_to_use (a literal 3-char joiner) that check 2's sum omitted, under-counting by 3 whenever when_to_use is populated. Settles the discrepancy the issue raised between two derivations of the budget formula: fetched current docs confirm SLASH_COMMAND_TOOL_CHAR_BUDGET's 8000-char fallback is exactly contextTokens(200000) x ~4 chars/token x skillListingBudgetFraction(0.01) — the contextTokens x 4 x fraction derivation, not a bare tokens x fraction reading. Item 3 (amending check 12 to accept a populated when_to_use as satisfying the trigger-spec requirement) is deferred per the issue's own sequencing note: PR #1096 is an open, active PR already editing check-skill.sh (claims check 21). #1404 stays open to track it. Refs #1404 Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 9 + .../skill-quality/.claude-plugin/plugin.json | 4 +- plugins/skill-quality/CHANGELOG.md | 27 +++ plugins/skill-quality/README.md | 32 ++- .../scripts/check-listing-budget.sh | 211 ++++++++++++++++++ .../scripts/check-listing-budget.test.sh | 139 ++++++++++++ plugins/skill-quality/scripts/check-skill.sh | 21 +- .../skill-quality/scripts/check-skill.test.sh | 27 +++ plugins/skill-quality/skills/check/SKILL.md | 42 +++- 9 files changed, 492 insertions(+), 20 deletions(-) create mode 100644 plugins/skill-quality/scripts/check-listing-budget.sh create mode 100644 plugins/skill-quality/scripts/check-listing-budget.test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 574fc7e22..0201e0f33 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -679,6 +679,15 @@ jobs: with: schemafile: plugins/skill-quality/reference/evals.schema.json files: plugins/*/skills/*/evals/evals.json + - name: Test the shared listing-budget reporter + run: bash plugins/skill-quality/scripts/check-listing-budget.test.sh + # Report-only (always exit 0 — see the script's own header): pools every + # plugin's skills root into ONE shared aggregate, the shape a consumer who + # installs the whole marketplace actually experiences. Runs on every + # event, unlike the PR-diff-gated step above, since the aggregate is a + # whole-repo property, not a per-change one. + - name: Report the shared skill-listing budget across every plugin + run: bash plugins/skill-quality/scripts/check-listing-budget.sh plugins/*/skills # Portability lint: skills declared ecosystem/forge/tracker-agnostic must not # ship bare hardcoded stack/forge/branch/tracker defaults — the coupling class diff --git a/plugins/skill-quality/.claude-plugin/plugin.json b/plugins/skill-quality/.claude-plugin/plugin.json index 1f541c354..f5d5604f4 100644 --- a/plugins/skill-quality/.claude-plugin/plugin.json +++ b/plugins/skill-quality/.claude-plugin/plugin.json @@ -1,8 +1,8 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "skill-quality", - "version": "0.10.2", - "description": "Skill-authoring QA tooling: a static contract checker that runs twenty deterministic checks over a Claude Code skill (frontmatter, listing-budget cap, trigger-keyword preservation, line caps, broken internal refs, markdownlint, gotchas surface, evals presence, precompute opportunity, injection shell-declaration) and a bundled evals.json schema for validation. Runs against any repo's skills directory via the convention-resolution ladder — no baked layout.", + "version": "0.11.0", + "description": "Skill-authoring QA tooling: a static contract checker that runs twenty deterministic checks over a Claude Code skill (frontmatter, per-skill listing-entry cap, trigger-keyword preservation, line caps, broken internal refs, markdownlint, gotchas surface, evals presence, precompute opportunity, injection shell-declaration), a shared skill-listing budget reporter across a set of skills, and a bundled evals.json schema for validation. Runs against any repo's skills directory via the convention-resolution ladder — no baked layout.", "author": { "name": "Melodic Software", "email": "info@melodicsoftware.com" diff --git a/plugins/skill-quality/CHANGELOG.md b/plugins/skill-quality/CHANGELOG.md index 1e5005552..bc929b6d6 100644 --- a/plugins/skill-quality/CHANGELOG.md +++ b/plugins/skill-quality/CHANGELOG.md @@ -3,6 +3,33 @@ All notable changes to the `skill-quality` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.11.0] + +### Added + +- **`listing-budget` action + `check-listing-budget.sh`: reports the SHARED skill-listing budget, + the aggregate limit nothing in the gate previously checked (#1404).** `check-skill.sh` check 2 only + ever guarded the per-skill entry cap (`skillListingMaxDescChars`, 1536 chars); the shared budget + every loaded skill draws from together (`skillListingBudgetFraction`, default 1% of the model's + context window) had no check at all — measured evidence found the aggregate overflowing by a large + multiple with no gate ever reporting it. The new script pools one or more skills roots into one + aggregate estimate against a documented, overridable default (8000 chars — the harness's own + `SLASH_COMMAND_TOOL_CHAR_BUDGET` fallback) and reports the biggest contributors on overflow. It is + always advisory (exit 0) since the live budget depends on a model's context window and a consumer's + own settings, neither of which a static check can observe — never hardcode + `skillListingBudgetFraction`'s documented default as a resolved live value; `/doctor` is the + authoritative source per machine. Wired into this repo's `skill-quality-gate` CI job as a report-only + step pooling every plugin's `skills/` root into one marketplace-wide aggregate. + +### Fixed + +- **Check 2 now counts the description/when_to_use joiner (#1404).** The harness assembles a skill's + listing entry as `description` + `" - "` + `when_to_use` — a literal 3-character joiner. Check 2 + summed only `len(description) + len(when_to_use)`, under-counting by 3 whenever `when_to_use` is + populated, so an entry sitting exactly at the boundary could pass a cap it had actually crossed. Not + currently binding at present description lengths in this repo, but wrong in exactly the direction + the listing-budget work is about. + ## [0.10.2] ### Fixed diff --git a/plugins/skill-quality/README.md b/plugins/skill-quality/README.md index 14192a0e2..d20fcc030 100644 --- a/plugins/skill-quality/README.md +++ b/plugins/skill-quality/README.md @@ -1,8 +1,9 @@ # skill-quality A Claude Code plugin for **skill-authoring QA**: it runs a static, deterministic contract gate over a -skill directory and validates a skill's `evals.json` against a bundled schema. No model invocation in -the gate — the same eighteen checks run identically in a session, a pre-commit hook, or CI. +skill directory, reports the shared listing-budget estimate across a set of skills, and validates a +skill's `evals.json` against a bundled schema. No model invocation in the gate — the same twenty checks +run identically in a session, a pre-commit hook, or CI. The one failure static analysis catches best is a rewrite silently dropping a `description` trigger phrase, which quietly degrades a skill's auto-invocation. Check 3 compares the trigger phrases against @@ -10,15 +11,16 @@ phrase, which quietly degrades a skill's auto-invocation. Check 3 compares the t | Skill | What it does | |---|---| -| `/skill-quality:check` | Runs the contract gate (`check`) or schema-validates evals (`validate-evals`), for one skill or every skill. | +| `/skill-quality:check` | Runs the contract gate (`check`), reports the shared listing budget (`listing-budget`), or schema-validates evals (`validate-evals`) — for one skill, a set of roots, or every skill. | | `/skill-quality:setup` | `check` (default) resolves and verifies the skills directory; `apply` routes a non-default `skills_root` change through Claude Code. | ## Checks -`check` runs `check-skill.sh` — eighteen checks, reported as `FAIL:` (blocking) or `WARN:` (advisory): +`check` runs `check-skill.sh` — twenty checks, reported as `FAIL:` (blocking) or `WARN:` (advisory): - Frontmatter parses; `name` + `description` present. -- `description` + `when_to_use` within the 1536-char listing budget (overflow truncates the listing). +- `description` + `when_to_use` within the 1536-char **per-skill** listing-entry cap (overflow + truncates that entry) — a different, narrower limit from the shared budget below. - Trigger-keyword preservation vs `HEAD` (skipped for a new, uncommitted skill). - `SKILL.md` under 500 lines (hard) / 200 lines (soft, advisory). - Backtick- and link-cited skill-internal supporting files resolve. @@ -30,11 +32,23 @@ phrase, which quietly degrades a skill's auto-invocation. Check 3 compares the t - Precompute opportunity (advisory) — a fenced shell block gathers read-only context the skill could inline at load time via [`!` injection](https://code.claude.com/docs/en/skills#inject-dynamic-context) instead of a per-invocation tool call. +- Dynamic-context injection portability (a bash-only `!` command with no `shell:` declared) and + defensive-fallback presence (`|| ` on every injected command). + +`listing-budget` runs `check-listing-budget.sh` — an always-advisory report on the **shared** budget +every loaded skill draws from together (`skillListingBudgetFraction`, default 1% of the model's context +window). This is the aggregate limit `check`'s per-skill cap above does not cover: nothing else in the +gate checks it, so a marketplace's skill count can silently overflow the live listing with no local +signal. It never asserts a live value it cannot observe (the model's context window and a consumer's +settings are both unknowable statically) — it reports against a documented, overridable default and +always exits 0. ```shell -/skill-quality:check my-skill # gate one skill -/skill-quality:check # gate every skill under the resolved root -/skill-quality:check validate-evals my-skill # schema-check evals.json +/skill-quality:check my-skill # gate one skill +/skill-quality:check # gate every skill under the resolved root +/skill-quality:check validate-evals my-skill # schema-check evals.json +/skill-quality:check listing-budget # report the shared budget over the resolved root +/skill-quality:check listing-budget plugins/*/skills # pool every plugin's root into one aggregate ``` ## Skills directory — never baked in @@ -65,4 +79,4 @@ Evals are warranted, not mandatory — a skill shipping none is not a failure. - A git repository — several checks read `git show HEAD:` / `git ls-files`; outside a repo the script exits 2. - `npx` (Node) is optional; without it the markdownlint check downgrades to a warning and the other - seventeen still gate. + nineteen still gate. diff --git a/plugins/skill-quality/scripts/check-listing-budget.sh b/plugins/skill-quality/scripts/check-listing-budget.sh new file mode 100644 index 000000000..704a87c9d --- /dev/null +++ b/plugins/skill-quality/scripts/check-listing-budget.sh @@ -0,0 +1,211 @@ +#!/usr/bin/env bash +# Static, deterministic report of the SHARED skill-listing character budget +# across one or more skills roots — the aggregate limit `check-skill.sh` never +# checks (see its header: check 2 is a PER-SKILL entry cap, not this). +# +# Claude Code loads a listing of every skill's name + description into context +# each turn. Two independent limits apply to it: +# - Per-entry cap (skillListingMaxDescChars, default 1536): check-skill.sh +# check 2 already guards this. +# - Shared/aggregate cap (skillListingBudgetFraction, default 0.01 = 1% of +# the model's context window): NOTHING checks this before now. When the +# aggregate overflows, Claude Code drops descriptions for the +# least-invoked skills first (name-only), degrading their auto-invocation +# with no local signal that it happened. +# https://code.claude.com/docs/en/skills#skill-descriptions-are-cut-short +# https://code.claude.com/docs/en/settings (skillListingBudgetFraction, +# skillListingMaxDescChars, SLASH_COMMAND_TOOL_CHAR_BUDGET) — fetched and +# verified current at authoring time. +# +# The aggregate budget is inherently a MACHINE-DEPENDENT estimate: it scales +# with the live model's context window and the resolved +# skillListingBudgetFraction, which a consumer's settings.json can override. +# This script therefore never asserts a live value it cannot observe — it +# reports against a documented, overridable default and always exits 0 +# (advisory only, never blocking CI or a pre-commit hook). `/doctor` is the +# live, authoritative source for the resolved value and biggest contributors +# on a given machine and model; this script is the static, reproducible +# proxy that runs without a live session. +# +# The default budget (8000 chars) is the harness's own documented fallback — +# SLASH_COMMAND_TOOL_CHAR_BUDGET's schema description: "The budget scales +# dynamically at 1% of the context window, with a fallback of 8000 +# characters." That fallback is exactly contextTokens(200000, the +# non-extended-context default) x ~4 chars/token x skillListingBudgetFraction +# (0.01) — the derivation is reconstructed only when an operator supplies a +# context-window override (below); the default asserts nothing beyond the one +# documented number. +# +# Usage: +# check-listing-budget.sh [ ...] +# check-listing-budget.sh --help +# +# No args: resolves ONE root via the same convention ladder as check-skill.sh +# (CHECK_SKILL_SKILLS_ROOT, then ${CLAUDE_PROJECT_DIR}/.claude/skills, then +# /.claude/skills) — the shape a single consumer project has. +# One or more args: each is scanned as an independent skills root and every +# skill under every root is pooled into ONE shared aggregate. This is how a +# consumer who installs multiple plugins actually experiences the listing +# (every loaded skill shares one budget in a live session) — the intended +# way to gate a marketplace repo, where each plugin owns its own +# plugins//skills/ root: e.g. `check-listing-budget.sh plugins/*/skills`. +# +# Overrides (never hardcode a resolved value as ground truth — a consumer's +# settings.json can diverge from every documented default below): +# CHECK_SKILL_LISTING_BUDGET_CHARS - fixed aggregate budget in characters +# (default 8000; skips the +# token/fraction reconstruction below) +# CHECK_SKILL_LISTING_CONTEXT_TOKENS - reconstructs the budget as +# TOKENS x CHARS_PER_TOKEN x FRACTION +# instead of the flat default; set +# this to match a machine's actual +# model context window (e.g. 1000000 +# for a 1M-context model) +# CHECK_SKILL_LISTING_BUDGET_FRACTION - default 0.01 (skillListingBudgetFraction's +# documented default) — set to match +# a machine's configured value +# CHECK_SKILL_LISTING_CHARS_PER_TOKEN - default 4 — set only if a more +# precise ratio is known +# CHECK_SKILL_LISTING_MAX_DESC_CHARS - per-entry truncation cap applied +# before summing (default 1536, +# matching skillListingMaxDescChars' +# documented default) — the harness +# truncates each entry to this cap +# before the aggregate ever sees it +# CHECK_SKILL_SKILLS_ROOT - single-root resolution override, +# only consulted in the no-args form +# +# Exit 0 always (report-only), except a usage/env error (exit 2) — this is an +# advisory rollup, not a pass/fail gate; see the "reported aggregate" framing +# in the issue this script closes. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +usage() { + sed -n '2,79p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' +} + +if [[ "${1:-}" == "--help" || "${1:-}" == "-h" ]]; then + usage + exit 0 +fi + +REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null | tr -d '\r')" +if [[ -z "$REPO_ROOT" || ! -d "$REPO_ROOT" ]]; then + printf 'Error: not in a git repo\n' >&2 + exit 2 +fi + +# shellcheck source=./skill-frontmatter.sh +source "$SCRIPT_DIR/skill-frontmatter.sh" + +MAX_DESC_CHARS="${CHECK_SKILL_LISTING_MAX_DESC_CHARS:-1536}" +JOINER_CHARS=3 # the literal " - " the harness inserts between description and when_to_use + +if [[ -n "${CHECK_SKILL_LISTING_CONTEXT_TOKENS:-}" ]]; then + CONTEXT_TOKENS="$CHECK_SKILL_LISTING_CONTEXT_TOKENS" + CHARS_PER_TOKEN="${CHECK_SKILL_LISTING_CHARS_PER_TOKEN:-4}" + FRACTION="${CHECK_SKILL_LISTING_BUDGET_FRACTION:-0.01}" + if ! BUDGET_CHARS="$(awk -v t="$CONTEXT_TOKENS" -v c="$CHARS_PER_TOKEN" -v f="$FRACTION" \ + 'BEGIN { printf "%d", t * c * f }' 2>/dev/null)" || [[ -z "$BUDGET_CHARS" ]]; then + printf 'Error: could not compute a budget from CHECK_SKILL_LISTING_CONTEXT_TOKENS=%s CHECK_SKILL_LISTING_CHARS_PER_TOKEN=%s CHECK_SKILL_LISTING_BUDGET_FRACTION=%s\n' \ + "$CONTEXT_TOKENS" "$CHARS_PER_TOKEN" "$FRACTION" >&2 + exit 2 + fi + BUDGET_SOURCE="reconstructed: $CONTEXT_TOKENS tokens x $CHARS_PER_TOKEN chars/token x $FRACTION" +else + BUDGET_CHARS="${CHECK_SKILL_LISTING_BUDGET_CHARS:-8000}" + BUDGET_SOURCE="documented default (SLASH_COMMAND_TOOL_CHAR_BUDGET fallback)" +fi + +# --- Resolve the roots to scan ---------------------------------------------- + +ROOTS=() +if (($# > 0)); then + ROOTS=("$@") +else + if [[ -n "${CHECK_SKILL_SKILLS_ROOT:-}" ]]; then + SINGLE_ROOT="$CHECK_SKILL_SKILLS_ROOT" + elif [[ -n "${CLAUDE_PROJECT_DIR:-}" ]]; then + SINGLE_ROOT="$CLAUDE_PROJECT_DIR/.claude/skills" + else + SINGLE_ROOT="$REPO_ROOT/.claude/skills" + fi + if [[ "$SINGLE_ROOT" != /* && ! "$SINGLE_ROOT" =~ ^[A-Za-z]:[\\/] ]]; then + SINGLE_ROOT="${CLAUDE_PROJECT_DIR:-$REPO_ROOT}/$SINGLE_ROOT" + fi + ROOTS=("$SINGLE_ROOT") +fi + +# --- Collect every skill under every root ----------------------------------- + +TOTAL=0 +ENTRY_COUNT=0 +CONTRIB_FILE="$(mktemp)" +trap 'rm -f "$CONTRIB_FILE"' EXIT + +found_any_root=0 +for root in "${ROOTS[@]}"; do + [[ -d "$root" ]] || continue + found_any_root=1 + for skill_md in "$root"/*/SKILL.md; do + [[ -f "$skill_md" ]] || continue + skill_name="${skill_md%/SKILL.md}" + skill_name="${skill_name##*/}" + fm="$(skill_frontmatter::extract <"$skill_md")" + [[ -n "$fm" ]] || continue + desc="$(skill_frontmatter::strip_quotes "$(skill_frontmatter::field description <<<"$fm")")" + wtu="$(skill_frontmatter::strip_quotes "$(skill_frontmatter::field when_to_use <<<"$fm")")" + desc_len=${#desc} + wtu_len=${#wtu} + joiner=0 + ((wtu_len > 0)) && joiner=$JOINER_CHARS + entry_len=$((desc_len + joiner + wtu_len)) + # The harness truncates each entry to the per-skill cap BEFORE the shared + # budget ever sees it — mirror that here so an already-oversized single + # entry (check 2's own FAIL) does not inflate this aggregate beyond what + # Claude Code would actually load. + ((entry_len > MAX_DESC_CHARS)) && entry_len=$MAX_DESC_CHARS + TOTAL=$((TOTAL + entry_len)) + ENTRY_COUNT=$((ENTRY_COUNT + 1)) + printf '%d\t%s\t%s\n' "$entry_len" "$skill_name" "$root" >>"$CONTRIB_FILE" + done +done + +if ((found_any_root == 0)); then + printf 'Error: no skills root found among: %s\n' "${ROOTS[*]}" >&2 + exit 2 +fi + +if ((ENTRY_COUNT == 0)); then + printf 'No skills found under: %s\n' "${ROOTS[*]}" + printf '\nCHECK-LISTING-BUDGET: 0 skills, nothing to report\n' + exit 0 +fi + +# --- Report ------------------------------------------------------------------ + +printf 'Shared listing-budget estimate over %d skill(s) across %d root(s):\n' "$ENTRY_COUNT" "${#ROOTS[@]}" +printf ' aggregate: %d chars\n' "$TOTAL" +printf ' budget: %d chars (%s)\n' "$BUDGET_CHARS" "$BUDGET_SOURCE" + +if ((TOTAL > BUDGET_CHARS)); then + OVERFLOW=$((TOTAL - BUDGET_CHARS)) + MULT="$(awk -v t="$TOTAL" -v b="$BUDGET_CHARS" 'BEGIN { printf "%.1f", t / b }' 2>/dev/null || echo '?')" + printf 'WARN: aggregate exceeds the budget by %d chars (~%sx over) — Claude Code drops the\n' "$OVERFLOW" "$MULT" + printf ' least-invoked skills'"'"' descriptions to name-only when this happens live.\n' + printf ' Biggest contributors (entry chars, skill, root):\n' + sort -t $'\t' -k1,1nr "$CONTRIB_FILE" | head -10 | while IFS=$'\t' read -r len name root; do + printf ' %6d %s (%s)\n' "$len" "$name" "$root" + done + printf '\nCHECK-LISTING-BUDGET: WARN — aggregate %d/%d chars over budget by %s at the configured budget.\n' \ + "$TOTAL" "$BUDGET_CHARS" "$OVERFLOW" + printf 'This is an estimate against a configurable default, not a live measurement — run\n' + # shellcheck disable=SC2016 # single quotes deliberate: the backticks are literal, not shell expansion + printf '`/doctor` in a live session for the authoritative resolved cost and contributors.\n' +else + printf 'CHECK-LISTING-BUDGET: OK — aggregate %d/%d chars within budget.\n' "$TOTAL" "$BUDGET_CHARS" +fi + +exit 0 diff --git a/plugins/skill-quality/scripts/check-listing-budget.test.sh b/plugins/skill-quality/scripts/check-listing-budget.test.sh new file mode 100644 index 000000000..6abd1dcae --- /dev/null +++ b/plugins/skill-quality/scripts/check-listing-budget.test.sh @@ -0,0 +1,139 @@ +#!/usr/bin/env bash +# Black-box contract test for check-listing-budget.sh. +# +# Self-contained and cwd-independent: builds a throwaway git repo with fixture +# skill roots, runs the reporter, and asserts on exit code + output. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SUT="$SCRIPT_DIR/check-listing-budget.sh" + +fails=0 +pass() { printf 'ok - %s\n' "$1"; } +fail() { + printf 'FAIL - %s\n' "$1" >&2 + fails=$((fails + 1)) +} + +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + +export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null +unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_COMMON_DIR GIT_PREFIX 2>/dev/null || true +git -C "$TMP" init -q +git -C "$TMP" config user.email test@example.com +git -C "$TMP" config user.name test + +ROOT_A="$TMP/plugin-a/skills" + +make_skill() { + local root="$1" name="$2" desc="$3" wtu="${4:-}" + mkdir -p "$root/$name" + { + printf -- '---\n' + printf 'name: %s\n' "$name" + printf 'description: "%s"\n' "$desc" + [[ -n "$wtu" ]] && printf 'when_to_use: "%s"\n' "$wtu" + printf -- '---\n\n## Purpose\n\nFixture.\n' + } >"$root/$name/SKILL.md" +} + +run() { (cd "$TMP" && bash "$SUT" "$@"); } + +# 1. --help exits 0. +if run --help >/dev/null 2>&1; then + pass "--help exits 0" +else + fail "--help should exit 0" +fi + +# 2. No matching root at all is a usage/env error (exit 2). +out="$(run "$TMP/does-not-exist" 2>&1)" +rc=$? +if [[ $rc -eq 2 ]] && grep -q 'no skills root found' <<<"$out"; then + pass "missing root(s) exits 2 with a clear message" +else + fail "missing root(s) should exit 2 (rc=$rc): $out" +fi + +# 3. A root that exists but has no skills reports zero and exits 0. +mkdir -p "$TMP/empty-root" +out="$(run "$TMP/empty-root" 2>&1)" +rc=$? +if [[ $rc -eq 0 ]] && grep -q '0 skills, nothing to report' <<<"$out"; then + pass "empty root reports zero skills and exits 0" +else + fail "empty root should report zero and exit 0 (rc=$rc): $out" +fi + +# 4. A small aggregate, well under the default 8000-char budget, is OK and +# exits 0 — advisory-only, never fails even implicitly. +make_skill "$ROOT_A" small-one "A small fixture skill." +make_skill "$ROOT_A" small-two "Another small fixture skill." +out="$(run "$ROOT_A" 2>&1)" +rc=$? +if [[ $rc -eq 0 ]] && grep -q 'CHECK-LISTING-BUDGET: OK' <<<"$out"; then + pass "small aggregate under the default budget reports OK" +else + fail "small aggregate should report OK (rc=$rc): $out" +fi + +# 5. Forcing a tiny budget via CHECK_SKILL_LISTING_BUDGET_CHARS trips the WARN +# path — proves the override plumbs through and the check never hardcodes +# 8000 as unconditional ground truth (item 4's caveat). +out="$(cd "$TMP" && CHECK_SKILL_LISTING_BUDGET_CHARS=10 bash "$SUT" "$ROOT_A" 2>&1)" +rc=$? +if [[ $rc -eq 0 ]] && grep -q 'WARN: aggregate exceeds the budget' <<<"$out" && grep -q 'CHECK-LISTING-BUDGET: WARN' <<<"$out"; then + pass "a forced tiny budget trips the WARN path but still exits 0 (advisory only)" +else + fail "forced tiny budget should WARN and still exit 0 (rc=$rc): $out" +fi + +# 6. The joiner is counted: a description + when_to_use entry is exactly +# DESC_LEN + 3 + WTU_LEN, not DESC_LEN + WTU_LEN (item 2's fix, mirrored +# here since the aggregate must match what check-skill.sh's check 2 now +# computes, or the two checks would disagree on the same entry's size). +mkdir -p "$TMP/joiner-root" +desc_40='1234567890123456789012345678901234567890' +wtu_10='1234567890' +make_skill "$TMP/joiner-root" joiner-skill "$desc_40" "$wtu_10" +# Budget set to exactly DESC(40) + WTU(10) = 50 so the un-joined sum would +# read as "OK" (50/50) but the joined sum (53) must WARN (53 > 50). +out="$(cd "$TMP" && CHECK_SKILL_LISTING_BUDGET_CHARS=50 bash "$SUT" "$TMP/joiner-root" 2>&1)" +if grep -q 'aggregate: 53 chars' <<<"$out" && grep -q 'WARN: aggregate exceeds the budget by 3 chars' <<<"$out"; then + pass "the 3-char description/when_to_use joiner is counted in the aggregate" +else + fail "aggregate should count the 3-char joiner (expected 53 chars, WARN by 3): $out" +fi + +# 7. An oversized single entry is capped at CHECK_SKILL_LISTING_MAX_DESC_CHARS +# before summing — mirrors the harness's own per-entry truncation, so one +# already-failing (check 2) entry cannot silently inflate the aggregate +# past what Claude Code would actually load. +mkdir -p "$TMP/cap-root" +long_desc="$(printf 'x%.0s' {1..200})" +make_skill "$TMP/cap-root" cap-skill "$long_desc" +out="$(cd "$TMP" && CHECK_SKILL_LISTING_MAX_DESC_CHARS=50 CHECK_SKILL_LISTING_BUDGET_CHARS=1000 bash "$SUT" "$TMP/cap-root" 2>&1)" +if grep -q 'aggregate: 50 chars' <<<"$out"; then + pass "an oversized entry is capped at CHECK_SKILL_LISTING_MAX_DESC_CHARS before summing" +else + fail "oversized entry should cap at 50 chars, not count all 200 (expected 'aggregate: 50 chars'): $out" +fi + +# 8. Multiple roots pool into ONE shared aggregate (the marketplace-repo use +# case: plugins/*/skills passed as separate positional roots). +mkdir -p "$TMP/root-x" "$TMP/root-y" +make_skill "$TMP/root-x" x-skill "12345" +make_skill "$TMP/root-y" y-skill "67890" +out="$(run "$TMP/root-x" "$TMP/root-y" 2>&1)" +if grep -q 'over 2 skill(s) across 2 root(s)' <<<"$out" && grep -q 'aggregate: 10 chars' <<<"$out"; then + pass "multiple roots pool into one shared aggregate" +else + fail "multiple roots should pool into one aggregate of 10 chars across 2 skills: $out" +fi + +if [[ $fails -ne 0 ]]; then + printf '%d assertion(s) failed\n' "$fails" >&2 + exit 1 +fi +printf 'all assertions passed\n' diff --git a/plugins/skill-quality/scripts/check-skill.sh b/plugins/skill-quality/scripts/check-skill.sh index df8f45f7d..56134bca5 100755 --- a/plugins/skill-quality/scripts/check-skill.sh +++ b/plugins/skill-quality/scripts/check-skill.sh @@ -24,9 +24,16 @@ # run on a clean tree with CHECK_SKILL_BASE_REF pointing before the change # (e.g. HEAD^ or a merge-base). # +# NOT covered here: the SHARED listing budget (skillListingBudgetFraction) that +# every loaded skill draws from together, a different cross-skill limit from +# check 2's per-skill entry cap below. See the companion +# check-listing-budget.sh for that aggregate report (always advisory). +# # Checks: # 1. Frontmatter parses; name matches dir; description present -# 2. description + when_to_use <= 1536 chars (listing-truncation guard) +# 2. description + when_to_use <= 1536 chars (per-skill listing-entry cap; +# counts the literal " - " joiner the harness inserts when when_to_use is +# populated) # 3. Trigger-keyword preservation vs the base ref (skipped for new skills; # a phrase moved verbatim to a sibling skill's listing text — one the # sibling did not carry at the base ref — WARNs, since the marketplace @@ -218,17 +225,21 @@ fi # --- Check 2: description + when_to_use <= DESC_CHAR_CAP chars -------------- # Cap is per-skill listing entry (description + when_to_use combined) — overflow -# truncates the listing and degrades auto-invocation. - +# truncates the listing and degrades auto-invocation. The harness assembles the +# entry as description + " - " + when_to_use — a literal 3-char joiner — so the +# combined length must include it whenever when_to_use is populated, or this +# check under-counts by 3 and can pass an entry that actually overflows. +JOINER_LEN=0 CUR_DESC="$(skill_frontmatter::strip_quotes "$(skill_frontmatter::field description <<<"$FRONTMATTER")")" CUR_WTU="$(skill_frontmatter::strip_quotes "$(skill_frontmatter::field when_to_use <<<"$FRONTMATTER")")" DESC_LEN=${#CUR_DESC} WTU_LEN=${#CUR_WTU} -COMBINED_LEN=$((DESC_LEN + WTU_LEN)) +((WTU_LEN > 0)) && JOINER_LEN=3 +COMBINED_LEN=$((DESC_LEN + JOINER_LEN + WTU_LEN)) if ((COMBINED_LEN > DESC_CHAR_CAP)); then err "description+when_to_use is $COMBINED_LEN chars (cap $DESC_CHAR_CAP — overflow truncates the listing)" elif ((WTU_LEN > 0)); then - note "description+when_to_use $COMBINED_LEN/$DESC_CHAR_CAP chars (desc $DESC_LEN + when_to_use $WTU_LEN)" + note "description+when_to_use $COMBINED_LEN/$DESC_CHAR_CAP chars (desc $DESC_LEN + joiner $JOINER_LEN + when_to_use $WTU_LEN)" else note "description length $DESC_LEN/$DESC_CHAR_CAP chars" fi diff --git a/plugins/skill-quality/scripts/check-skill.test.sh b/plugins/skill-quality/scripts/check-skill.test.sh index 942d326a9..59860431a 100755 --- a/plugins/skill-quality/scripts/check-skill.test.sh +++ b/plugins/skill-quality/scripts/check-skill.test.sh @@ -1267,6 +1267,33 @@ else fail "prose #! code span should not trip the injection checks (rc=$rc): $out" fi +# 29. Check 2 counts the 3-char " - " joiner: desc(1500) + wtu(34) = 1534, +# under the 1536 cap WITHOUT the joiner (the pre-fix bug would pass this), +# but + the 3-char joiner = 1537 — one char over. FAILing here proves +# item 2's fix (check-skill.sh:227) at the exact boundary, not merely an +# already-overflowing entry that would fail either way. +desc_1500="$(printf 'd%.0s' $(seq 1 1500))" +wtu_34="$(printf 'w%.0s' $(seq 1 34))" +make_skill joiner-boundary "--- +name: joiner-boundary +description: \"$desc_1500\" +when_to_use: \"$wtu_34\" +--- + +## Purpose + +Boundary fixture: desc(1500) + wtu(34) = 1534 (would pass check 2 if the +joiner were omitted, per the pre-fix bug); + the 3-char joiner = 1537, one +char over the 1536 cap. +" +out="$(run joiner-boundary 2>&1)" +rc=$? +if [[ $rc -eq 1 ]] && grep -q 'description+when_to_use is 1537 chars (cap 1536' <<<"$out"; then + pass "check 2 counts the joiner: a desc+wtu sum at the cap without it now fails at 1537" +else + fail "check 2 should count the 3-char joiner and fail at 1537/1536 (rc=$rc): $out" +fi + if [[ $fails -ne 0 ]]; then printf '%d assertion(s) failed\n' "$fails" >&2 exit 1 diff --git a/plugins/skill-quality/skills/check/SKILL.md b/plugins/skill-quality/skills/check/SKILL.md index 1ee0e0345..4c31176ee 100644 --- a/plugins/skill-quality/skills/check/SKILL.md +++ b/plugins/skill-quality/skills/check/SKILL.md @@ -1,7 +1,7 @@ --- name: check -description: "Skill-authoring QA for Claude Code skills. Use when: 'check this skill', 'skill quality', 'lint my skill', 'is this SKILL.md valid', 'validate skill frontmatter', 'check skill before publishing', 'validate evals.json', or before shipping a skill or plugin. Actions: `check []` runs a twenty-check static contract gate (frontmatter, listing-budget cap, trigger-keyword preservation vs HEAD, line caps, broken internal refs, markdownlint, gotchas surface, evals presence, precompute opportunity, injection shell-declaration) and reports PASS/FAIL with warnings; `validate-evals []` checks a skill's evals/evals.json against the bundled schema. Not for: writing new skills, or running model-graded evals." -argument-hint: "[check|validate-evals] [] — omit the action for check; omit the skill name to run over every skill" +description: "Skill-authoring QA for Claude Code skills. Use when: 'check this skill', 'skill quality', 'lint my skill', 'is this SKILL.md valid', 'validate skill frontmatter', 'check skill before publishing', 'validate evals.json', 'shared listing budget', 'is the skill listing overflowing', or before shipping a skill or plugin. Actions: `check []` runs a twenty-check static contract gate (frontmatter, per-skill listing-entry cap, trigger-keyword preservation vs HEAD, line caps, broken internal refs, markdownlint, gotchas surface, evals presence, precompute opportunity, injection shell-declaration) and reports PASS/FAIL with warnings; `validate-evals []` checks a skill's evals/evals.json against the bundled schema; `listing-budget [ ...]` reports the SHARED aggregate listing-budget estimate across every skill under the resolved root(s) — advisory only, never blocks. Not for: writing new skills, or running model-graded evals." +argument-hint: "[check|validate-evals|listing-budget] [ ...] — omit the action for check; omit the name/root to run over every skill under the resolved root" user-invocable: true disable-model-invocation: false shell: bash @@ -12,8 +12,10 @@ shell: bash Static, deterministic quality gate for skill authoring. The `check` action runs the bundled `check-skill.sh` — twenty checks with no model invocation, so results are reproducible in CI or a pre-commit hook. The `validate-evals` action checks a skill's `/evals/evals.json` against the bundled -JSON schema. Catches the failure that static analysis catches best: a rewrite silently dropping a -`description` trigger phrase, which degrades auto-invocation. +JSON schema. The `listing-budget` action runs `check-listing-budget.sh` — a separate, always-advisory +report on the SHARED listing budget every loaded skill draws from together (a different, cross-skill +limit from `check`'s per-skill entry cap). Catches the failure that static analysis catches best: a +rewrite silently dropping a `description` trigger phrase, which degrades auto-invocation. ## Skills-directory resolution @@ -59,6 +61,10 @@ Parse `$ARGUMENTS`: - **`check`** *(no name)* — run the gate over every skill under the resolved root. - **`validate-evals `** — validate one skill's `/evals/evals.json` against the schema. - **`validate-evals`** *(no name)* — validate every skill's `/evals/evals.json` that exists. +- **`listing-budget`** *(no root)* — report the shared listing-budget estimate over every skill under + the resolved root. +- **`listing-budget [ ...]`** — pool every skill under each given root into ONE shared + aggregate (e.g. every plugin's skills dir in a marketplace repo). ## Action: check @@ -97,6 +103,27 @@ that line before editing, since it may be an illustrative example path rather th may add `name` (kebab-case), `expected_output`, `files`, and one of `assertions` / `expectations`. 4. Report each violation with its JSON path, or confirm the file conforms. +## Action: listing-budget + +1. Resolve the root(s): explicit ` ...` arguments if given; otherwise the same + skills-root resolution as `check` (above). +2. Run: + + ```shell + bash "${CLAUDE_PLUGIN_ROOT}/scripts/check-listing-budget.sh" [ ...] + ``` + +3. Report the printed aggregate, the budget it was compared against (and whether that budget is the + documented default or a reconstructed override — see the script's own header), and the biggest + contributors when it overflows. + +This is a **different, cross-skill limit** from `check`'s per-skill entry cap (`description` + +`when_to_use` <= 1536 chars): the shared budget every loaded skill draws from together +(`skillListingBudgetFraction`, default 1% of the model's context window). It is always advisory — +exit 0 regardless of overflow — because the live budget depends on the model's context window and a +consumer's own settings, neither of which this static check can observe. Point `/doctor` at the live +session for the authoritative resolved cost. + ## Gotchas - The script needs a git repository — several checks (trigger-keyword preservation, vendor @@ -139,3 +166,10 @@ that line before editing, since it may be an illustrative example path rather th defect — like a check-5 ref, hand-verify the block before converting it. It reads only fenced shell blocks (not prose "run `git status` first") and stays silent whenever the skill already uses any `!` injection, so it under-reports by design; a clean run is not proof there is no precompute opportunity. +- `listing-budget` never asserts a resolved live value (context window and `skillListingBudgetFraction` + are both consumer settings this static check cannot observe) — it reports against a documented, + overridable default and always exits 0. A clean report is a signal to investigate against `/doctor` + in a live session, not a guarantee nothing is dropped there. In this marketplace's own repo, each + plugin owns its own `plugins//skills/` root, so gating the whole marketplace means pooling + every plugin's root into one call (`check-listing-budget.sh plugins/*/skills`) rather than running it + once per plugin in isolation — the repo's `check-changed-skills.sh` CI gate does this on every run. From 041e5f6f6e8bddcbd6821d49447be8089e88b970 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:44:32 -0400 Subject: [PATCH 2/5] =?UTF-8?q?fix(skill-quality):=20CI=20fixups=20for=20t?= =?UTF-8?q?he=20listing-budget=20PR=20=E2=80=94=20exec=20bit=20+=20catalog?= =?UTF-8?q?=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things the local test/lint pass didn't catch before push, both flagged by this repo's own CI gates on the first run: - The two new scripts (check-listing-budget.sh, check-listing-budget.test.sh) were committed without the executable bit — the hygiene lane's shebang-vs-mode check caught it. - The root README.md's generated catalog block was stale against the bumped skill-quality plugin.json description — regenerated via scripts/generate-catalog.mjs. Co-Authored-By: Claude Sonnet 5 --- README.md | 2 +- plugins/skill-quality/scripts/check-listing-budget.sh | 0 plugins/skill-quality/scripts/check-listing-budget.test.sh | 0 3 files changed, 1 insertion(+), 1 deletion(-) mode change 100644 => 100755 plugins/skill-quality/scripts/check-listing-budget.sh mode change 100644 => 100755 plugins/skill-quality/scripts/check-listing-budget.test.sh diff --git a/README.md b/README.md index 152160bec..137f6cd7d 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ user opts in with `/plugin enable`; an existing install is never flipped by cata - [`rate-limit-guard`](plugins/rate-limit-guard) — Shared rate-limit guard for loop lanes: a statusline wrapper tees the subscription rate-limit windows to a fixed machine-scope file, a StopFailure hook records rate-limit stops reactively, and a reader contract fixes how consuming sessions pause and resume. - [`context-guard`](plugins/context-guard) — Per-session context-window observability: a statusline wrapper tees each session's context_window fields to a per-session snapshot file, a zone resolver classifies usage into smart/acceptable/dumb bands (zones.json SSOT with shipped defaults), and a reader contract fixes how consuming sessions interpret the snapshots. - [`plugin-quality`](plugins/plugin-quality) — Post-use behavioral audit of Claude Code plugin components: a six-step audit workflow (evidence capture, grounded mapping in a fresh subagent, blindspot pass, interactive contract lock, presence-gated review seams, work-item emit with draft+confirm) over any skill, agent, hook, command, or config you have actually used — zone-informed by context-guard snapshots when present, conservative when not. -- [`skill-quality`](plugins/skill-quality) — Skill-authoring QA tooling: a static contract checker that runs twenty deterministic checks over a Claude Code skill (frontmatter, listing-budget cap, trigger-keyword preservation, line caps, broken internal refs, markdownlint, gotchas surface, evals presence, precompute opportunity, injection shell-declaration) and a bundled evals.json schema for validation. Runs against any repo's skills directory via the convention-resolution ladder — no baked layout. +- [`skill-quality`](plugins/skill-quality) — Skill-authoring QA tooling: a static contract checker that runs twenty deterministic checks over a Claude Code skill (frontmatter, per-skill listing-entry cap, trigger-keyword preservation, line caps, broken internal refs, markdownlint, gotchas surface, evals presence, precompute opportunity, injection shell-declaration), a shared skill-listing budget reporter across a set of skills, and a bundled evals.json schema for validation. Runs against any repo's skills directory via the convention-resolution ladder — no baked layout. ### Autonomy diff --git a/plugins/skill-quality/scripts/check-listing-budget.sh b/plugins/skill-quality/scripts/check-listing-budget.sh old mode 100644 new mode 100755 diff --git a/plugins/skill-quality/scripts/check-listing-budget.test.sh b/plugins/skill-quality/scripts/check-listing-budget.test.sh old mode 100644 new mode 100755 From b2d3f7f82b3ab4015c286be73514bcd66a819950 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:49:48 -0400 Subject: [PATCH 3/5] fix(skill-quality): include the last header line in --help output Automated review on #1450 caught it: the usage() sed range (2,79p) clipped the header's final line ("in the issue this script closes.") since the comment block actually runs through line 80. 2,80p now captures the whole header. Co-Authored-By: Claude Sonnet 5 --- plugins/skill-quality/scripts/check-listing-budget.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/skill-quality/scripts/check-listing-budget.sh b/plugins/skill-quality/scripts/check-listing-budget.sh index 704a87c9d..febc05880 100755 --- a/plugins/skill-quality/scripts/check-listing-budget.sh +++ b/plugins/skill-quality/scripts/check-listing-budget.sh @@ -83,7 +83,7 @@ set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" usage() { - sed -n '2,79p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' + sed -n '2,80p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' } if [[ "${1:-}" == "--help" || "${1:-}" == "-h" ]]; then From d218d15db78a3966a90f75a997161881ba72d981 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:11:30 -0400 Subject: [PATCH 4/5] fix(skill-quality): count only listing-eligible skills, harden budget inputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the five open Codex review findings on the new listing-budget reporter, all confirmed against branch content and current official docs. Exclude `disable-model-invocation: true` skills from the aggregate. The invocation-control table at https://code.claude.com/docs/en/skills records "Description not in context" for that frontmatter, and "Hide individual skills" states it "removes the skill from Claude's context entirely" — such a skill spends none of the shared description budget, so counting it overstated the report. On this marketplace: 183 skills / 109,205 chars -> 132 / 83,594, still ~10.4x over the 8000-char default, so the finding the check exists to surface is unchanged. Reject a missing explicit skills root as an environment error (exit 2) instead of silently skipping it, which omitted a whole plugin subtree from a falsely low "OK" aggregate. The no-args resolution path keeps its own "no skills root found" message. Report the count of roots actually scanned rather than the count of arguments given. Validate every numeric override as a positive number, integer or decimal, routing failures into the documented exit 2. A nonnumeric value previously either coerced to zero in awk — fabricating a zero-character budget and a bogus overflow WARN while still exiting 0 — or crashed with an undocumented exit 1. Give a fixed CHECK_SKILL_LISTING_BUDGET_CHARS precedence over the token/fraction reconstruction, as the script's own header always claimed, announcing the ignored reconstruction input rather than discarding it silently; and label it an override rather than the "documented default". The budget verdict stays advisory throughout: OK and WARN both exit 0. Only operator/environment errors use exit 2, which was already documented. Derive --help's range from the header block itself so editing that block can no longer clip or overrun the help text. Tests grow 8 -> 21 assertions, covering every path above; the three input-handling defects had shipped green precisely because none was asserted. Co-Authored-By: Claude Opus 5 (1M context) --- plugins/skill-quality/CHANGELOG.md | 22 +++ plugins/skill-quality/README.md | 5 + .../scripts/check-listing-budget.sh | 100 +++++++++++-- .../scripts/check-listing-budget.test.sh | 138 ++++++++++++++++-- plugins/skill-quality/skills/check/SKILL.md | 23 ++- 5 files changed, 261 insertions(+), 27 deletions(-) diff --git a/plugins/skill-quality/CHANGELOG.md b/plugins/skill-quality/CHANGELOG.md index bc929b6d6..79cc01612 100644 --- a/plugins/skill-quality/CHANGELOG.md +++ b/plugins/skill-quality/CHANGELOG.md @@ -21,6 +21,28 @@ All notable changes to the `skill-quality` plugin are documented here. Format fo authoritative source per machine. Wired into this repo's `skill-quality-gate` CI job as a report-only step pooling every plugin's `skills/` root into one marketplace-wide aggregate. + The report counts only **listing-eligible** skills. A skill with `disable-model-invocation: true` + is skipped: the invocation-control table at records + "Description not in context" for that frontmatter, and "Hide individual skills" states it + "removes the skill from Claude's context entirely" — such a skill spends none of the shared + description budget, so counting it overstates the aggregate. A consumer's `skillOverrides` can + free further descriptions via `"name-only"`, which repository content cannot reveal, so the + figure is an upper bound for anyone who sets it. On this marketplace the filter moves the reported + aggregate from 183 skills / 109,205 chars to 132 / 83,594 — still an order of magnitude over the + 8000-char default, so the finding the check exists to surface is unchanged. + + Input handling is fail-closed on operator error, while the budget verdict stays advisory: every + numeric override is validated as a positive number and every explicit root must exist, both + reported as the documented environment error (exit 2). Previously a nonnumeric override was + either coerced to zero by `awk` — fabricating a zero-character budget and a bogus overflow WARN + while still exiting 0 — or crashed with an undocumented exit 1, and a misspelled root among + several was silently skipped while its subtree vanished from an "OK" aggregate. A fixed + `CHECK_SKILL_LISTING_BUDGET_CHARS` now takes precedence over the token/fraction reconstruction as + its own documentation always claimed, announcing the ignored input rather than discarding it + silently, and is labelled an override instead of the "documented default". The report header + counts roots actually scanned rather than arguments given, and `--help` derives its range from the + header block so editing that block can no longer clip or overrun the help text. + ### Fixed - **Check 2 now counts the description/when_to_use joiner (#1404).** The harness assembles a skill's diff --git a/plugins/skill-quality/README.md b/plugins/skill-quality/README.md index d20fcc030..773aaf880 100644 --- a/plugins/skill-quality/README.md +++ b/plugins/skill-quality/README.md @@ -43,6 +43,11 @@ signal. It never asserts a live value it cannot observe (the model's context win settings are both unknowable statically) — it reports against a documented, overridable default and always exits 0. +Only **listing-eligible** skills are counted: `disable-model-invocation: true` keeps a skill's +description out of the model-visible listing entirely, so it spends none of the shared budget. A +consumer's `skillOverrides` can free further descriptions with `"name-only"`, which repository +content cannot reveal — so the reported figure is an upper bound for anyone who sets it. + ```shell /skill-quality:check my-skill # gate one skill /skill-quality:check # gate every skill under the resolved root diff --git a/plugins/skill-quality/scripts/check-listing-budget.sh b/plugins/skill-quality/scripts/check-listing-budget.sh index febc05880..0e5192380 100755 --- a/plugins/skill-quality/scripts/check-listing-budget.sh +++ b/plugins/skill-quality/scripts/check-listing-budget.sh @@ -17,6 +17,24 @@ # skillListingMaxDescChars, SLASH_COMMAND_TOOL_CHAR_BUDGET) — fetched and # verified current at authoring time. # +# What counts, and what deliberately does not: +# - This models DESCRIPTION characters only. The listing "always contains +# every skill name" (docs, above), so names are a floor this report does +# not estimate; the budget is spent on description + when_to_use text, +# which is what is summed here. +# - Skills with `disable-model-invocation: true` are SKIPPED. Per +# https://code.claude.com/docs/en/skills the invocation-control table +# records "Description not in context" for that frontmatter, and "Hide +# individual skills" states it "removes the skill from Claude's context +# entirely" — such a skill spends none of the shared description budget, +# so counting it overstates the aggregate. (Fetched and verified current +# when this filter was added.) +# - A consumer's `skillOverrides` can collapse further entries to +# `"name-only"`, which also frees their description characters. That is +# consumer settings.json state, not repository content, so a static check +# cannot observe it — this report is therefore an UPPER bound for any +# consumer who sets it. +# # The aggregate budget is inherently a MACHINE-DEPENDENT estimate: it scales # with the live model's context window and the resolved # skillListingBudgetFraction, which a consumer's settings.json can override. @@ -42,7 +60,11 @@ # # No args: resolves ONE root via the same convention ladder as check-skill.sh # (CHECK_SKILL_SKILLS_ROOT, then ${CLAUDE_PROJECT_DIR}/.claude/skills, then -# /.claude/skills) — the shape a single consumer project has. +# /.claude/skills) — the shape a single consumer project has. A +# resolved root that does not exist is reported as "no skills root found". +# One or more args: EVERY explicit root must exist — a missing one is an +# environment error (exit 2), never a silent skip, because skipping it +# would omit a whole plugin subtree and report a falsely low aggregate. # One or more args: each is scanned as an independent skills root and every # skill under every root is pooled into ONE shared aggregate. This is how a # consumer who installs multiple plugins actually experiences the listing @@ -75,6 +97,11 @@ # CHECK_SKILL_SKILLS_ROOT - single-root resolution override, # only consulted in the no-args form # +# Every numeric override above is validated as a positive number before use. +# A nonnumeric value is an environment error (exit 2) — never a silent +# coercion to zero, which would fabricate a zero-character budget and an +# overflow WARN out of a typo. +# # Exit 0 always (report-only), except a usage/env error (exit 2) — this is an # advisory rollup, not a pass/fail gate; see the "reported aggregate" framing # in the issue this script closes. @@ -82,8 +109,11 @@ set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# The header comment block above IS the --help text. Print from line 2 to the +# last consecutive `#` line rather than a hardcoded range, so editing the +# header can never again silently clip or overrun the help output. usage() { - sed -n '2,80p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' + awk 'NR == 1 { next } /^#/ { sub(/^# ?/, ""); print; next } { exit }' "${BASH_SOURCE[0]}" } if [[ "${1:-}" == "--help" || "${1:-}" == "-h" ]]; then @@ -100,30 +130,63 @@ fi # shellcheck source=./skill-frontmatter.sh source "$SCRIPT_DIR/skill-frontmatter.sh" +# Reject a nonnumeric override up front. Without this, `awk` coerces a typo to +# 0 and the report exits 0 announcing a zero-character budget and a bogus +# overflow, while the bash-arithmetic call sites die with an undocumented +# exit 1. Both failure modes are routed into the documented env error (exit 2). +# `kind` is `int` for counts and `num` for ratios/fractions, which are decimal. +require_positive_number() { + local name="$1" val="$2" kind="${3:-int}" + local pattern='^[0-9]+$' + [[ "$kind" == "num" ]] && pattern='^([0-9]+(\.[0-9]+)?|\.[0-9]+)$' + if [[ ! "$val" =~ $pattern ]] || ! awk -v v="$val" 'BEGIN { exit (v > 0) ? 0 : 1 }'; then + printf 'Error: %s must be a positive number, got: %s\n' "$name" "$val" >&2 + exit 2 + fi +} + MAX_DESC_CHARS="${CHECK_SKILL_LISTING_MAX_DESC_CHARS:-1536}" +require_positive_number CHECK_SKILL_LISTING_MAX_DESC_CHARS "$MAX_DESC_CHARS" int JOINER_CHARS=3 # the literal " - " the harness inserts between description and when_to_use -if [[ -n "${CHECK_SKILL_LISTING_CONTEXT_TOKENS:-}" ]]; then +# Precedence matches the documented contract above: a fixed aggregate budget +# SKIPS the token/fraction reconstruction. Checking it first is what makes that +# sentence true — the reconstruction branch used to win and silently discard a +# supplied fixed budget, which could flip the OK/WARN verdict. +if [[ -n "${CHECK_SKILL_LISTING_BUDGET_CHARS:-}" ]]; then + BUDGET_CHARS="$CHECK_SKILL_LISTING_BUDGET_CHARS" + require_positive_number CHECK_SKILL_LISTING_BUDGET_CHARS "$BUDGET_CHARS" int + BUDGET_SOURCE="override (CHECK_SKILL_LISTING_BUDGET_CHARS)" + if [[ -n "${CHECK_SKILL_LISTING_CONTEXT_TOKENS:-}" ]]; then + printf 'Note: CHECK_SKILL_LISTING_BUDGET_CHARS takes precedence; ignoring CHECK_SKILL_LISTING_CONTEXT_TOKENS=%s\n' \ + "$CHECK_SKILL_LISTING_CONTEXT_TOKENS" >&2 + fi +elif [[ -n "${CHECK_SKILL_LISTING_CONTEXT_TOKENS:-}" ]]; then CONTEXT_TOKENS="$CHECK_SKILL_LISTING_CONTEXT_TOKENS" CHARS_PER_TOKEN="${CHECK_SKILL_LISTING_CHARS_PER_TOKEN:-4}" FRACTION="${CHECK_SKILL_LISTING_BUDGET_FRACTION:-0.01}" + require_positive_number CHECK_SKILL_LISTING_CONTEXT_TOKENS "$CONTEXT_TOKENS" int + require_positive_number CHECK_SKILL_LISTING_CHARS_PER_TOKEN "$CHARS_PER_TOKEN" num + require_positive_number CHECK_SKILL_LISTING_BUDGET_FRACTION "$FRACTION" num if ! BUDGET_CHARS="$(awk -v t="$CONTEXT_TOKENS" -v c="$CHARS_PER_TOKEN" -v f="$FRACTION" \ - 'BEGIN { printf "%d", t * c * f }' 2>/dev/null)" || [[ -z "$BUDGET_CHARS" ]]; then + 'BEGIN { printf "%d", t * c * f }' 2>/dev/null)" || [[ -z "$BUDGET_CHARS" ]] || ((BUDGET_CHARS <= 0)); then printf 'Error: could not compute a budget from CHECK_SKILL_LISTING_CONTEXT_TOKENS=%s CHECK_SKILL_LISTING_CHARS_PER_TOKEN=%s CHECK_SKILL_LISTING_BUDGET_FRACTION=%s\n' \ "$CONTEXT_TOKENS" "$CHARS_PER_TOKEN" "$FRACTION" >&2 exit 2 fi BUDGET_SOURCE="reconstructed: $CONTEXT_TOKENS tokens x $CHARS_PER_TOKEN chars/token x $FRACTION" else - BUDGET_CHARS="${CHECK_SKILL_LISTING_BUDGET_CHARS:-8000}" + BUDGET_CHARS=8000 BUDGET_SOURCE="documented default (SLASH_COMMAND_TOOL_CHAR_BUDGET fallback)" fi # --- Resolve the roots to scan ---------------------------------------------- ROOTS=() +EXPLICIT_ROOTS=0 if (($# > 0)); then ROOTS=("$@") + EXPLICIT_ROOTS=1 else if [[ -n "${CHECK_SKILL_SKILLS_ROOT:-}" ]]; then SINGLE_ROOT="$CHECK_SKILL_SKILLS_ROOT" @@ -145,16 +208,31 @@ ENTRY_COUNT=0 CONTRIB_FILE="$(mktemp)" trap 'rm -f "$CONTRIB_FILE"' EXIT -found_any_root=0 +FOUND_ROOTS=0 for root in "${ROOTS[@]}"; do - [[ -d "$root" ]] || continue - found_any_root=1 + if [[ ! -d "$root" ]]; then + # An EXPLICIT root that does not exist is an environment error: silently + # skipping it omits a whole subtree and reports a falsely low aggregate + # under an "OK". The no-args resolved root falls through to the + # "no skills root found" check below instead. + if ((EXPLICIT_ROOTS)); then + printf 'Error: skills root does not exist: %s\n' "$root" >&2 + exit 2 + fi + continue + fi + FOUND_ROOTS=$((FOUND_ROOTS + 1)) for skill_md in "$root"/*/SKILL.md; do [[ -f "$skill_md" ]] || continue skill_name="${skill_md%/SKILL.md}" skill_name="${skill_name##*/}" fm="$(skill_frontmatter::extract <"$skill_md")" [[ -n "$fm" ]] || continue + # `disable-model-invocation: true` keeps this skill's description out of + # the model-visible listing entirely, so it spends none of the shared + # budget — counting it would overstate the aggregate. See the header. + dmi="$(skill_frontmatter::strip_quotes "$(skill_frontmatter::field disable-model-invocation <<<"$fm")")" + [[ "$dmi" == "true" ]] && continue desc="$(skill_frontmatter::strip_quotes "$(skill_frontmatter::field description <<<"$fm")")" wtu="$(skill_frontmatter::strip_quotes "$(skill_frontmatter::field when_to_use <<<"$fm")")" desc_len=${#desc} @@ -173,20 +251,20 @@ for root in "${ROOTS[@]}"; do done done -if ((found_any_root == 0)); then +if ((FOUND_ROOTS == 0)); then printf 'Error: no skills root found among: %s\n' "${ROOTS[*]}" >&2 exit 2 fi if ((ENTRY_COUNT == 0)); then - printf 'No skills found under: %s\n' "${ROOTS[*]}" + printf 'No listing-eligible skills found under: %s\n' "${ROOTS[*]}" printf '\nCHECK-LISTING-BUDGET: 0 skills, nothing to report\n' exit 0 fi # --- Report ------------------------------------------------------------------ -printf 'Shared listing-budget estimate over %d skill(s) across %d root(s):\n' "$ENTRY_COUNT" "${#ROOTS[@]}" +printf 'Shared listing-budget estimate over %d listing-eligible skill(s) across %d root(s):\n' "$ENTRY_COUNT" "$FOUND_ROOTS" printf ' aggregate: %d chars\n' "$TOTAL" printf ' budget: %d chars (%s)\n' "$BUDGET_CHARS" "$BUDGET_SOURCE" diff --git a/plugins/skill-quality/scripts/check-listing-budget.test.sh b/plugins/skill-quality/scripts/check-listing-budget.test.sh index 6abd1dcae..70952c2bb 100755 --- a/plugins/skill-quality/scripts/check-listing-budget.test.sh +++ b/plugins/skill-quality/scripts/check-listing-budget.test.sh @@ -27,33 +27,63 @@ git -C "$TMP" config user.name test ROOT_A="$TMP/plugin-a/skills" make_skill() { - local root="$1" name="$2" desc="$3" wtu="${4:-}" + local root="$1" name="$2" desc="$3" wtu="${4:-}" dmi="${5:-}" mkdir -p "$root/$name" { printf -- '---\n' printf 'name: %s\n' "$name" printf 'description: "%s"\n' "$desc" [[ -n "$wtu" ]] && printf 'when_to_use: "%s"\n' "$wtu" + [[ -n "$dmi" ]] && printf 'disable-model-invocation: %s\n' "$dmi" printf -- '---\n\n## Purpose\n\nFixture.\n' } >"$root/$name/SKILL.md" } run() { (cd "$TMP" && bash "$SUT" "$@"); } -# 1. --help exits 0. -if run --help >/dev/null 2>&1; then - pass "--help exits 0" +# 1. --help exits 0 and prints the WHOLE header block — no clipping, no +# overrun past the header into code. usage() derives its range from the +# comment block itself, so adding header lines can never desync it again. +out="$(run --help 2>&1)" +rc=$? +if [[ $rc -eq 0 ]] && + grep -q 'in the issue this script closes' <<<"$out" && + ! grep -q 'set -uo pipefail' <<<"$out"; then + pass "--help exits 0 and prints the full header without spilling into code" else - fail "--help should exit 0" + fail "--help should print the complete header and stop at the first code line (rc=$rc): $out" fi -# 2. No matching root at all is a usage/env error (exit 2). +# 2. An explicit root that does not exist is a usage/env error (exit 2). out="$(run "$TMP/does-not-exist" 2>&1)" rc=$? +if [[ $rc -eq 2 ]] && grep -q 'skills root does not exist' <<<"$out"; then + pass "a missing explicit root exits 2 with a clear message" +else + fail "a missing explicit root should exit 2 (rc=$rc): $out" +fi + +# 2b. A missing explicit root is rejected even when ANOTHER root is valid. +# Silently skipping it would omit a whole plugin subtree and report a +# falsely low aggregate under an "OK" — the failure mode this guards. +mkdir -p "$TMP/valid-root/only-skill" +make_skill "$TMP/valid-root" only-skill "A fixture." +out="$(run "$TMP/valid-root" "$TMP/does-not-exist" 2>&1)" +rc=$? +if [[ $rc -eq 2 ]] && grep -q 'skills root does not exist' <<<"$out"; then + pass "a missing explicit root is rejected even when another root is valid" +else + fail "a partially-missing explicit root list should exit 2, not silently skip (rc=$rc): $out" +fi + +# 2c. The no-args resolution path still reports "no skills root found" rather +# than the explicit-root error — the two branches stay distinct. +out="$(cd "$TMP" && CHECK_SKILL_SKILLS_ROOT="$TMP/nope-root" bash "$SUT" 2>&1)" +rc=$? if [[ $rc -eq 2 ]] && grep -q 'no skills root found' <<<"$out"; then - pass "missing root(s) exits 2 with a clear message" + pass "an unresolvable no-args root exits 2 via the resolution-path message" else - fail "missing root(s) should exit 2 (rc=$rc): $out" + fail "no-args resolution failure should exit 2 with 'no skills root found' (rc=$rc): $out" fi # 3. A root that exists but has no skills reports zero and exits 0. @@ -126,12 +156,102 @@ mkdir -p "$TMP/root-x" "$TMP/root-y" make_skill "$TMP/root-x" x-skill "12345" make_skill "$TMP/root-y" y-skill "67890" out="$(run "$TMP/root-x" "$TMP/root-y" 2>&1)" -if grep -q 'over 2 skill(s) across 2 root(s)' <<<"$out" && grep -q 'aggregate: 10 chars' <<<"$out"; then +if grep -q 'over 2 listing-eligible skill(s) across 2 root(s)' <<<"$out" && grep -q 'aggregate: 10 chars' <<<"$out"; then pass "multiple roots pool into one shared aggregate" else fail "multiple roots should pool into one aggregate of 10 chars across 2 skills: $out" fi +# 9. `disable-model-invocation: true` skills are excluded from the aggregate. +# Their descriptions are never loaded into the model-visible listing +# (https://code.claude.com/docs/en/skills — "Description not in context"), +# so counting them would overstate the shared budget. +mkdir -p "$TMP/dmi-root" +make_skill "$TMP/dmi-root" eligible-skill "12345" +make_skill "$TMP/dmi-root" manual-skill "9999999999999999999999999" "" "true" +make_skill "$TMP/dmi-root" explicit-false-skill "67890" "" "false" +out="$(run "$TMP/dmi-root" 2>&1)" +if grep -q 'over 2 listing-eligible skill(s)' <<<"$out" && grep -q 'aggregate: 10 chars' <<<"$out"; then + pass "disable-model-invocation: true skills are excluded; false is still counted" +else + fail "expected 2 eligible skills totalling 10 chars (the dmi:true skill excluded): $out" +fi + +# 10. A nonnumeric override is an environment error (exit 2), never a silent +# awk coercion to zero (which fabricated a 0-char budget and a bogus +# overflow WARN while still exiting 0) and never an undocumented exit 1. +# Each case names the variable it expects to be blamed. The two ratio vars +# are only consulted when CONTEXT_TOKENS selects the reconstruction branch, +# so those cases set a valid CONTEXT_TOKENS alongside. +assert_env_error() { + local var="$1" desc="$2" + shift 2 + local out rc + out="$(cd "$TMP" && env "$@" bash "$SUT" "$ROOT_A" 2>&1)" + rc=$? + if [[ $rc -eq 2 ]] && grep -q "$var must be a positive number" <<<"$out"; then + pass "$desc is rejected as an environment error (exit 2)" + else + fail "$desc should exit 2 blaming $var (rc=$rc): $out" + fi +} + +assert_env_error CHECK_SKILL_LISTING_CONTEXT_TOKENS "a nonnumeric context-token count" \ + CHECK_SKILL_LISTING_CONTEXT_TOKENS=nope +assert_env_error CHECK_SKILL_LISTING_BUDGET_FRACTION "a nonnumeric budget fraction" \ + CHECK_SKILL_LISTING_CONTEXT_TOKENS=200000 CHECK_SKILL_LISTING_BUDGET_FRACTION=nope +assert_env_error CHECK_SKILL_LISTING_CHARS_PER_TOKEN "a nonnumeric chars-per-token ratio" \ + CHECK_SKILL_LISTING_CONTEXT_TOKENS=200000 CHECK_SKILL_LISTING_CHARS_PER_TOKEN=nope +assert_env_error CHECK_SKILL_LISTING_MAX_DESC_CHARS "a nonnumeric per-entry cap" \ + CHECK_SKILL_LISTING_MAX_DESC_CHARS=nope +assert_env_error CHECK_SKILL_LISTING_BUDGET_CHARS "a nonnumeric fixed budget" \ + CHECK_SKILL_LISTING_BUDGET_CHARS=nope +assert_env_error CHECK_SKILL_LISTING_BUDGET_CHARS "a zero fixed budget" \ + CHECK_SKILL_LISTING_BUDGET_CHARS=0 + +# 11. A valid decimal ratio/fraction is NOT rejected by the numeric guard — +# the validation must accept the documented 0.01 default shape. +out="$(cd "$TMP" && CHECK_SKILL_LISTING_CONTEXT_TOKENS=1000000 CHECK_SKILL_LISTING_BUDGET_FRACTION=0.02 \ + CHECK_SKILL_LISTING_CHARS_PER_TOKEN=3.5 bash "$SUT" "$ROOT_A" 2>&1)" +rc=$? +if [[ $rc -eq 0 ]] && grep -q 'reconstructed: 1000000 tokens x 3.5 chars/token x 0.02' <<<"$out"; then + pass "decimal ratio and fraction overrides are accepted and reconstructed" +else + fail "decimal overrides should reconstruct the budget and exit 0 (rc=$rc): $out" +fi + +# 12. A supplied CHECK_SKILL_LISTING_BUDGET_CHARS is labelled an override, not +# the "documented default" — the provenance the report promises to state. +out="$(cd "$TMP" && CHECK_SKILL_LISTING_BUDGET_CHARS=4000 bash "$SUT" "$ROOT_A" 2>&1)" +if grep -q 'budget:.*4000 chars (override (CHECK_SKILL_LISTING_BUDGET_CHARS))' <<<"$out"; then + pass "a supplied fixed budget is labelled an override, not the documented default" +else + fail "an overridden budget should not be labelled the documented default: $out" +fi + +# 12b. With no override, the default IS labelled the documented default. +out="$(run "$ROOT_A" 2>&1)" +if grep -q 'budget:.*8000 chars (documented default' <<<"$out"; then + pass "the unoverridden budget is labelled the documented default" +else + fail "the default budget should carry the documented-default label: $out" +fi + +# 13. The fixed budget takes precedence over the token reconstruction, per the +# documented contract ("skips the token/fraction reconstruction"). The +# reconstruction branch used to win and silently discard the fixed value, +# which could flip the OK/WARN verdict. +out="$(cd "$TMP" && CHECK_SKILL_LISTING_BUDGET_CHARS=99999 CHECK_SKILL_LISTING_CONTEXT_TOKENS=200000 \ + bash "$SUT" "$ROOT_A" 2>&1)" +rc=$? +if [[ $rc -eq 0 ]] && + grep -q 'budget:.*99999 chars (override (CHECK_SKILL_LISTING_BUDGET_CHARS))' <<<"$out" && + grep -q 'takes precedence; ignoring CHECK_SKILL_LISTING_CONTEXT_TOKENS=200000' <<<"$out"; then + pass "a fixed budget wins over the token reconstruction and says so" +else + fail "the fixed budget should win and announce the ignored reconstruction input (rc=$rc): $out" +fi + if [[ $fails -ne 0 ]]; then printf '%d assertion(s) failed\n' "$fails" >&2 exit 1 diff --git a/plugins/skill-quality/skills/check/SKILL.md b/plugins/skill-quality/skills/check/SKILL.md index 4c31176ee..f3cc8791e 100644 --- a/plugins/skill-quality/skills/check/SKILL.md +++ b/plugins/skill-quality/skills/check/SKILL.md @@ -1,6 +1,6 @@ --- name: check -description: "Skill-authoring QA for Claude Code skills. Use when: 'check this skill', 'skill quality', 'lint my skill', 'is this SKILL.md valid', 'validate skill frontmatter', 'check skill before publishing', 'validate evals.json', 'shared listing budget', 'is the skill listing overflowing', or before shipping a skill or plugin. Actions: `check []` runs a twenty-check static contract gate (frontmatter, per-skill listing-entry cap, trigger-keyword preservation vs HEAD, line caps, broken internal refs, markdownlint, gotchas surface, evals presence, precompute opportunity, injection shell-declaration) and reports PASS/FAIL with warnings; `validate-evals []` checks a skill's evals/evals.json against the bundled schema; `listing-budget [ ...]` reports the SHARED aggregate listing-budget estimate across every skill under the resolved root(s) — advisory only, never blocks. Not for: writing new skills, or running model-graded evals." +description: "Skill-authoring QA for Claude Code skills. Use when: 'check this skill', 'skill quality', 'lint my skill', 'is this SKILL.md valid', 'validate skill frontmatter', 'check skill before publishing', 'validate evals.json', 'shared listing budget', 'is the skill listing overflowing', or before shipping a skill or plugin. Actions: `check []` runs a twenty-check static contract gate (frontmatter, per-skill listing-entry cap, trigger-keyword preservation vs HEAD, line caps, broken internal refs, markdownlint, gotchas surface, evals presence, precompute opportunity, injection shell-declaration) and reports PASS/FAIL with warnings; `validate-evals []` checks a skill's evals/evals.json against the bundled schema; `listing-budget [ ...]` reports the SHARED aggregate listing-budget estimate across every listing-eligible skill under the resolved root(s) — advisory only, never blocks. Not for: writing new skills, or running model-graded evals." argument-hint: "[check|validate-evals|listing-budget] [ ...] — omit the action for check; omit the name/root to run over every skill under the resolved root" user-invocable: true disable-model-invocation: false @@ -61,10 +61,11 @@ Parse `$ARGUMENTS`: - **`check`** *(no name)* — run the gate over every skill under the resolved root. - **`validate-evals `** — validate one skill's `/evals/evals.json` against the schema. - **`validate-evals`** *(no name)* — validate every skill's `/evals/evals.json` that exists. -- **`listing-budget`** *(no root)* — report the shared listing-budget estimate over every skill under - the resolved root. -- **`listing-budget [ ...]`** — pool every skill under each given root into ONE shared - aggregate (e.g. every plugin's skills dir in a marketplace repo). +- **`listing-budget`** *(no root)* — report the shared listing-budget estimate over every + listing-eligible skill under the resolved root. +- **`listing-budget [ ...]`** — pool every listing-eligible skill under each given root + into ONE shared aggregate (e.g. every plugin's skills dir in a marketplace repo). Every root given + must exist. ## Action: check @@ -114,8 +115,8 @@ that line before editing, since it may be an illustrative example path rather th ``` 3. Report the printed aggregate, the budget it was compared against (and whether that budget is the - documented default or a reconstructed override — see the script's own header), and the biggest - contributors when it overflows. + documented default, a fixed override, or a reconstructed one — the script labels which), and the + biggest contributors when it overflows. This is a **different, cross-skill limit** from `check`'s per-skill entry cap (`description` + `when_to_use` <= 1536 chars): the shared budget every loaded skill draws from together @@ -124,6 +125,14 @@ exit 0 regardless of overflow — because the live budget depends on the model's consumer's own settings, neither of which this static check can observe. Point `/doctor` at the live session for the authoritative resolved cost. +**Only listing-eligible skills count.** A skill with `disable-model-invocation: true` has its +description kept out of the model-visible listing entirely, so it spends none of the shared budget +and the report skips it — counting those would overstate the aggregate. A consumer's +`skillOverrides` can free further descriptions by collapsing entries to `"name-only"`, which +repository content cannot reveal, so the reported figure is an upper bound for anyone who sets it. +A missing explicit root and a nonnumeric override are both environment errors (exit 2), never a +silent skip or a coerced-to-zero budget. + ## Gotchas - The script needs a git repository — several checks (trigger-keyword preservation, vendor From f4121042ef1e90fc0e0fd5172cf3fe22ddd20739 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:34:48 -0400 Subject: [PATCH 5/5] fix(skill-quality): normalize the invocation-control boolean before filtering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `skill_frontmatter::field` returns the raw YAML scalar, so the exact-string `== "true"` filter missed spellings a YAML reader treats as the same boolean. Reproduced before fixing: a nine-skill fixture root covering the variants reported 7 listing-eligible skills instead of 2 — an inline `# comment`, a quoted value carrying a comment, `TRUE`, `True`, and a trailing-whitespace value all leaked through and were counted, re-inflating the aggregate with descriptions the harness keeps out of context. Normalize before comparing: strip a whitespace-preceded YAML comment, trim surrounding whitespace, strip one quote layer, and fold ASCII case. Scope deliberately bounded, and stated in the code: - Comment-stripping applies only to this boolean, never to `description` or `when_to_use`, where a whitespace-preceded `#` is content rather than a comment. Folding it there would silently truncate a description. - YAML 1.1's `yes` / `on` aliases are NOT folded. The documented spelling is `true`, and over-matching risks dropping a skill over a value the harness may read as a plain string. - This is a pragmatic normalizer for one known field, not a YAML parser. Audited the sibling call sites for the same class of gap: this was the only boolean compared against raw field output in the plugin. Every other `skill_frontmatter::field` consumer extracts free text, and `skill_frontmatter::metadata_field` already strips trailing comments internally, so the shared library needed no change — keeping the blast radius off `check-skill.sh`'s PASS/FAIL surface. Corrects the changelog's marketplace figure to 132 skills / 83,611 chars. The previously recorded 83,594 was measured before this PR's own SKILL.md description edit added 17 characters, and the figure now carries the commit it was measured at, per this repo's own measurement convention. Tests 21 -> 22; the new case fails against the pre-fix code. Co-Authored-By: Claude Opus 5 (1M context) --- plugins/skill-quality/CHANGELOG.md | 16 ++++++++--- .../scripts/check-listing-budget.sh | 27 ++++++++++++++++++- .../scripts/check-listing-budget.test.sh | 24 +++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/plugins/skill-quality/CHANGELOG.md b/plugins/skill-quality/CHANGELOG.md index 79cc01612..df97b738d 100644 --- a/plugins/skill-quality/CHANGELOG.md +++ b/plugins/skill-quality/CHANGELOG.md @@ -27,9 +27,19 @@ All notable changes to the `skill-quality` plugin are documented here. Format fo "removes the skill from Claude's context entirely" — such a skill spends none of the shared description budget, so counting it overstates the aggregate. A consumer's `skillOverrides` can free further descriptions via `"name-only"`, which repository content cannot reveal, so the - figure is an upper bound for anyone who sets it. On this marketplace the filter moves the reported - aggregate from 183 skills / 109,205 chars to 132 / 83,594 — still an order of magnitude over the - 8000-char default, so the finding the check exists to surface is unchanged. + figure is an upper bound for anyone who sets it. On this marketplace the filter excludes 51 of the + 183 `SKILL.md` files, leaving a reported **132 listing-eligible skills / 83,611 characters** as + measured at this commit — still an order of magnitude over the 8000-char default, so the finding + the check exists to surface is unchanged. (A figure without its commit goes stale: the population + itself moves, so re-measure rather than quoting this one forward.) + + The flag is read through a normalizing comparison rather than an exact string match, since valid + YAML can spell the same boolean as `true # manual-only`, `"true"`, `TRUE`, or with surrounding + whitespace, and a bare `== "true"` silently re-counted every one of those. Comment-stripping is + scoped to the boolean and deliberately never applied to `description` / `when_to_use`, where a + whitespace-preceded `#` is content rather than a comment. YAML 1.1's `yes` / `on` aliases are not + folded — the documented spelling is `true`, and over-matching would risk dropping a skill over a + value the harness may read as a plain string. Input handling is fail-closed on operator error, while the budget verdict stays advisory: every numeric override is validated as a positive number and every explicit root must exist, both diff --git a/plugins/skill-quality/scripts/check-listing-budget.sh b/plugins/skill-quality/scripts/check-listing-budget.sh index 0e5192380..ad30bbbc9 100755 --- a/plugins/skill-quality/scripts/check-listing-budget.sh +++ b/plugins/skill-quality/scripts/check-listing-budget.sh @@ -130,6 +130,31 @@ fi # shellcheck source=./skill-frontmatter.sh source "$SCRIPT_DIR/skill-frontmatter.sh" +# Fold a frontmatter boolean to a bare lowercase token before comparing it. +# `skill_frontmatter::field` returns the raw scalar, so an exact-string compare +# against "true" misses spellings a YAML reader treats as the same boolean: a +# trailing `# comment` (YAML requires whitespace before the `#`), surrounding +# whitespace, and quoted or differently-cased forms. Missing one silently +# re-counts a skill whose description the harness keeps out of context. +# Deliberately NOT folded: YAML 1.1's `yes` / `on` aliases — the docs only ever +# spell this field `true`, and treating a bare `yes` as the boolean risks +# dropping a skill over a value the harness may read as a plain string. +# Comment-stripping stays scoped to booleans and is never applied to +# `description` / `when_to_use`, where a ` #` is content, not a comment. +# A pragmatic normalizer for one known field, not a YAML parser. +trim_ws() { + local v="$1" + v="${v#"${v%%[![:space:]]*}"}" + printf '%s' "${v%"${v##*[![:space:]]}"}" +} + +normalize_bool() { + local v + v="$(trim_ws "$(sed -E 's/[[:space:]]+#.*$//' <<<"$1")")" + v="$(trim_ws "$(skill_frontmatter::strip_quotes "$v")")" + printf '%s' "$(tr '[:upper:]' '[:lower:]' <<<"$v")" +} + # Reject a nonnumeric override up front. Without this, `awk` coerces a typo to # 0 and the report exits 0 announcing a zero-character budget and a bogus # overflow, while the bash-arithmetic call sites die with an undocumented @@ -231,7 +256,7 @@ for root in "${ROOTS[@]}"; do # `disable-model-invocation: true` keeps this skill's description out of # the model-visible listing entirely, so it spends none of the shared # budget — counting it would overstate the aggregate. See the header. - dmi="$(skill_frontmatter::strip_quotes "$(skill_frontmatter::field disable-model-invocation <<<"$fm")")" + dmi="$(normalize_bool "$(skill_frontmatter::field disable-model-invocation <<<"$fm")")" [[ "$dmi" == "true" ]] && continue desc="$(skill_frontmatter::strip_quotes "$(skill_frontmatter::field description <<<"$fm")")" wtu="$(skill_frontmatter::strip_quotes "$(skill_frontmatter::field when_to_use <<<"$fm")")" diff --git a/plugins/skill-quality/scripts/check-listing-budget.test.sh b/plugins/skill-quality/scripts/check-listing-budget.test.sh index 70952c2bb..bf28bca4a 100755 --- a/plugins/skill-quality/scripts/check-listing-budget.test.sh +++ b/plugins/skill-quality/scripts/check-listing-budget.test.sh @@ -177,6 +177,30 @@ else fail "expected 2 eligible skills totalling 10 chars (the dmi:true skill excluded): $out" fi +# 9b. The invocation-control flag is NORMALIZED before comparison. A bare +# string compare against "true" silently re-counts a skill whose +# description the harness excludes, because valid YAML can spell the same +# boolean several ways. Covered: a trailing `# comment` (a YAML comment +# requires preceding whitespace), surrounding whitespace, quoted forms, +# and ASCII case variants. A `false` carrying a comment must still be +# counted — that is what stops the normalization from over-matching. +mkdir -p "$TMP/dmi-norm-root" +make_skill "$TMP/dmi-norm-root" norm-inline-comment "12345" "" "true # manual-only" +make_skill "$TMP/dmi-norm-root" norm-quoted "12345" "" '"true"' +make_skill "$TMP/dmi-norm-root" norm-single-quoted "12345" "" "'true'" +make_skill "$TMP/dmi-norm-root" norm-quoted-comment "12345" "" "'true' # manual" +make_skill "$TMP/dmi-norm-root" norm-upper "12345" "" "TRUE" +make_skill "$TMP/dmi-norm-root" norm-title "12345" "" "True" +make_skill "$TMP/dmi-norm-root" norm-trailing-space "12345" "" "true " +make_skill "$TMP/dmi-norm-root" norm-false-comment "12345" "" "false # still listed" +make_skill "$TMP/dmi-norm-root" norm-eligible "67890" +out="$(run "$TMP/dmi-norm-root" 2>&1)" +if grep -q 'over 2 listing-eligible skill(s)' <<<"$out" && grep -q 'aggregate: 10 chars' <<<"$out"; then + pass "the invocation-control boolean is normalized (comment, quotes, case, whitespace)" +else + fail "only norm-false-comment and norm-eligible should count (2 skills, 10 chars): $out" +fi + # 10. A nonnumeric override is an environment error (exit 2), never a silent # awk coercion to zero (which fabricated a 0-char budget and a bogus # overflow WARN while still exiting 0) and never an undocumented exit 1.