diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 574fc7e22c..0201e0f335 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/README.md b/README.md index 152160bec2..137f6cd7d6 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/.claude-plugin/plugin.json b/plugins/skill-quality/.claude-plugin/plugin.json index 1f541c3547..f5d5604f4a 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 1e50055525..df97b738d3 100644 --- a/plugins/skill-quality/CHANGELOG.md +++ b/plugins/skill-quality/CHANGELOG.md @@ -3,6 +3,65 @@ 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. + + 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 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 + 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 + 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 14192a0e24..773aaf880f 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,28 @@ 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. + +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 -/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 +84,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 100755 index 0000000000..ad30bbbc98 --- /dev/null +++ b/plugins/skill-quality/scripts/check-listing-budget.sh @@ -0,0 +1,314 @@ +#!/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. +# +# 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. +# 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. 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 +# (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 +# +# 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. +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() { + awk 'NR == 1 { next } /^#/ { sub(/^# ?/, ""); print; next } { exit }' "${BASH_SOURCE[0]}" +} + +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" + +# 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 +# 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 + +# 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" ]] || ((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=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" + 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_ROOTS=0 +for root in "${ROOTS[@]}"; do + 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="$(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")")" + 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_ROOTS == 0)); then + printf 'Error: no skills root found among: %s\n' "${ROOTS[*]}" >&2 + exit 2 +fi + +if ((ENTRY_COUNT == 0)); then + 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 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" + +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 100755 index 0000000000..bf28bca4a3 --- /dev/null +++ b/plugins/skill-quality/scripts/check-listing-budget.test.sh @@ -0,0 +1,283 @@ +#!/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:-}" 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 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 print the complete header and stop at the first code line (rc=$rc): $out" +fi + +# 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 "an unresolvable no-args root exits 2 via the resolution-path message" +else + 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. +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 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 + +# 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. +# 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 +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 df8f45f7d6..56134bca52 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 942d326a92..59860431a7 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 1ee0e03459..f3cc8791e5 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 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 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,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 + 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 @@ -97,6 +104,35 @@ 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, 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 +(`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. + +**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 @@ -139,3 +175,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.