diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2d58a43..a985ec6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -192,6 +192,19 @@ repos: pass_filenames: false always_run: true + - id: check-skill-version-bump + name: Check changed skills bump metadata.version + description: On every push, fail if a skill directory changed (tests/ excluded) since the merge-base with main without its SKILL.md metadata.version rising (ADR-0022) + entry: bash scripts/check-skill-version-bump.sh + language: system + stages: [pre-push] + pass_filenames: false + always_run: true + # Baseline is the merge-base with origin/main (falling back to main), + # not the remote branch tip: readers install from main. Fails closed + # when no main ref resolves. Merges through Gitea's merge button run no + # local hook, so they bypass this, just as they bypass check-release-needed. + - id: validate-marketplace name: Validate marketplace manifest description: Run claude plugin validate --strict on the root marketplace manifest diff --git a/docs/adr/0022-skill-metadata-version-is-mandatory.md b/docs/adr/0022-skill-metadata-version-is-mandatory.md index 7885a9a..9ba050a 100644 --- a/docs/adr/0022-skill-metadata-version-is-mandatory.md +++ b/docs/adr/0022-skill-metadata-version-is-mandatory.md @@ -76,3 +76,39 @@ own SKILL.md drops the "with `metadata.version` present" conditional in its bump presence is no longer in question. `.pre-commit-config.yaml`'s `skill-frontmatter` hook is extended to require the field, closing the gap #113 and #118 both named in the same audit pass: a stated rule with nothing enforcing it drifts the same way an unstated one does. + +## Amendment (2026-09-16) — the bump is enforced at push, not only required to exist + +**Context.** Making the field mandatory did not make it move. The only thing that bumped it was +`skill-author` Step 4, so every hand edit and every trim pass skipped the bump: on +`docs/simplification-audit`, 17 of the 40 skill directories that changed against `main` carried +the same `metadata.version` as `main`, and `gitea` alone sat at six different values. Both +validators checked presence and semver shape, never movement, so the field could not answer the +question this ADR gives it — "did this change since I last read it". (Simplification audit +finding 33.) + +**Decision.** `scripts/check-skill-version-bump.sh` runs as a pre-push hook on every push. For each +skill directory under `plugins/*/.apm/skills/` that differs between the pushed ref and its +merge-base with `main`, ignoring `tests/`, the pushed `metadata.version` must be strictly greater +than `main`'s. + +- **Baseline is the merge-base with `main`, not the previous commit.** Readers only ever see + `main` — installs resolve against the default branch (ADR-0018) — so one bump per branch is what + the field owes them. A per-commit check would bump a skill once per commit and inflate the + number past meaning. +- **"Greater", not "exactly one patch higher".** A second `skill-author` pass on the same branch + that bumps again still passes. +- **`tests/` is excluded.** No agent loads it; a fixture-only change does not change the skill. +- **Skills absent from either side are exempt.** New, renamed and merged skills start fresh under + the rules above; deleted skills have nothing to check. +- **Every plugin is covered, `bin` included,** and the gate is repo-local — it is not exported + through `.pre-commit-hooks.yaml`. + +**Considered options.** *Declare the field advisory* — cheapest, but concedes the field cannot do +its job. *Drop the field* — rejected by this ADR already, and costlier now. *Check at commit +time against `HEAD`* — rejected for the inflation above. + +**Consequences.** The 17 unbumped skills took a patch bump in the commit that added the gate. +Like `check-release-needed`, the gate only fires on a local `git push` through pre-commit: a merge +made with Gitea's merge button runs no local hooks and is not checked. A typo fix in a skill now +costs a version bump; that is the rule working, not noise. diff --git a/docs/spec/gates.md b/docs/spec/gates.md index 6f6c492..3fbc1a0 100644 --- a/docs/spec/gates.md +++ b/docs/spec/gates.md @@ -21,24 +21,24 @@ Install hooks via `pc-run`, wiring **all three stages**. This repo's `.pre-commi `default_install_hook_types`, so a plain install silently skips `commit-msg` (Conventional Commits) and `pre-push` (everything below). -The pre-push command reports **10** hooks, not 8. The extra two are pre-commit's own `meta` hooks, +The pre-push command reports **11** hooks, not 9. The extra two are pre-commit's own `meta` hooks, `check-hooks-apply` and `check-useless-excludes`: they declare no `stages:`, so they run at every stage including this one. Both are declared in this repo's `.pre-commit-config.yaml` like everything -else — what separates them is `repo: meta` (pre-commit's own built-ins) from `repo: local`. Eight +else — what separates them is `repo: meta` (pre-commit's own built-ins) from `repo: local`. Nine is the count of hooks this repo authors itself. -**The caveat: one of those 8 is a silent no-op under that invocation.** +**The caveat: one of those 9 is a silent no-op under that invocation.** `check-release-needed` exits 0 immediately unless `PRE_COMMIT_REMOTE_BRANCH` equals `refs/heads/main`, and pre-commit exports that variable only from the real pre-push git hook during an actual `git push`. Running the stage by hand — or from a CI runner — therefore reports it `Passed` having checked nothing. That is by design for feature branches — pushing WIP must not be blocked on cutting a premature tag — but it means `--hook-stage pre-push --all-files` is a full -rehearsal of 7 hooks and a skip of the eighth. The script's own header records the same gap for +rehearsal of 8 hooks and a skip of the ninth. The script's own header records the same gap for a PR merged through Gitea's merge button, where no local push happens at all. ## The pre-push gate -Eight hooks, grouped below by what they guard rather than by the order `.pre-commit-config.yaml` declares them in. +Nine hooks, grouped below by what they guard rather than by the order `.pre-commit-config.yaml` declares them in. **Core checks** @@ -80,12 +80,30 @@ drift in generated text. | Hook | Guards | |---|---| | `check-release-needed` | on a real `git push` to `main` only — fails if files exposed via `.pre-commit-hooks.yaml` changed since the last tag. A no-op everywhere else, including under `pre-commit run --hook-stage pre-push` (see [the caveat above](#running-the-gates)) | +| `check-skill-version-bump` | on every push — fails if a skill directory changed since the merge-base with `main` without its `metadata.version` rising (see [below](#check-skill-version-bump)) | Two of these shell out to `apm`: `apm-audit-ci` and `apm-pack-check-clean`. The second is a bare `apm …` entry and the first is a `bash -c` loop calling `apm` once per package, so without the CLI the push dies with an unhelpful "command not found". Install with `apm-install`, or `curl -sSL https://aka.ms/apm-unix | sh`; verify with `apm --version`. +### `check-skill-version-bump` + +ADR-0022 makes `metadata.version` mandatory and a skill change carries a bump; `skill-size-check` +only checks the field's presence and shape, so this hook holds the bump itself. + +- **Baseline is `git merge-base origin/main `** (local `main` if `origin/main` does not + resolve). Readers install from `main`, so "changed" means changed against what `main` ships. The + remote branch tip is not the baseline: a second push would excuse an unbumped change the first + push already carried. With no `main` ref or no merge-base, the hook fails closed. +- **A changed skill must end strictly above its baseline version**, compared numerically + (`1.0.10` > `1.0.9`). Any bump size passes. A missing or non-`MAJOR.MINOR.PATCH` version at the + pushed ref fails. Skills absent at the baseline (new, renamed, merged) or at the pushed ref + (deleted) are exempt. +- **`/tests/` is excluded**: no agent loads it, so a test-only change ships nothing. +- **Known gap:** a PR merged through Gitea's merge button runs no local hook, the same gap + `check-release-needed` has. + ## Skill and agent context gates (ADR-0020) The `skill-size-check` pre-commit hook, scoped to `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$`, diff --git a/plugins/bin/.apm/skills/caveman/SKILL.md b/plugins/bin/.apm/skills/caveman/SKILL.md index 542810f..b10085c 100644 --- a/plugins/bin/.apm/skills/caveman/SKILL.md +++ b/plugins/bin/.apm/skills/caveman/SKILL.md @@ -5,7 +5,7 @@ description: > Ultra-compressed output mode that drops articles, filler and pleasantries while keeping technical substance exact, cutting token usage by roughly 75%. metadata: - version: "1.0.0" + version: "1.0.1" --- Respond terse like smart caveman. All technical substance stay. Only fluff die. diff --git a/plugins/bin/.apm/skills/grill-me/SKILL.md b/plugins/bin/.apm/skills/grill-me/SKILL.md index 70bb682..7af0774 100644 --- a/plugins/bin/.apm/skills/grill-me/SKILL.md +++ b/plugins/bin/.apm/skills/grill-me/SKILL.md @@ -6,7 +6,7 @@ description: > decision tree. Not a plan to challenge against `CONTEXT.md` and ADRs -> `grill-with-docs`. metadata: - version: "1.0.0" + version: "1.0.1" --- Interview me relentlessly about every aspect of this plan until we reach a shared understanding. Walk down each branch of the design tree, resolving dependencies between decisions one-by-one. For each question, provide your recommended answer. diff --git a/plugins/bin/.apm/skills/grill-with-docs/SKILL.md b/plugins/bin/.apm/skills/grill-with-docs/SKILL.md index 30df96e..ee5cdc5 100644 --- a/plugins/bin/.apm/skills/grill-with-docs/SKILL.md +++ b/plugins/bin/.apm/skills/grill-with-docs/SKILL.md @@ -5,7 +5,7 @@ description: > the interview challenges terms against `CONTEXT.md` and writes decisions into it and into ADRs as they land. Not a plain interview -> `grill-me`. metadata: - version: "1.0.0" + version: "1.0.1" --- diff --git a/plugins/bin/.apm/skills/prototype/SKILL.md b/plugins/bin/.apm/skills/prototype/SKILL.md index b544772..457d9e6 100644 --- a/plugins/bin/.apm/skills/prototype/SKILL.md +++ b/plugins/bin/.apm/skills/prototype/SKILL.md @@ -6,7 +6,7 @@ description: > variations. Not production code -> `tdd`. Not talking a design through -> `grill-me`. metadata: - version: "1.0.0" + version: "1.0.1" --- # Prototype diff --git a/plugins/bin/.apm/skills/research/SKILL.md b/plugins/bin/.apm/skills/research/SKILL.md index 1d114ab..b25bb47 100644 --- a/plugins/bin/.apm/skills/research/SKILL.md +++ b/plugins/bin/.apm/skills/research/SKILL.md @@ -6,7 +6,7 @@ description: >- documentation written from existing code or specs -> `write-docs`. Not a bug or incident -> `diagnose`. metadata: - version: "1.0.0" + version: "1.0.1" category: research allowed-tools: - Grep diff --git a/plugins/bin/.apm/skills/write-docs/SKILL.md b/plugins/bin/.apm/skills/write-docs/SKILL.md index 7ca0b97..95c176d 100644 --- a/plugins/bin/.apm/skills/write-docs/SKILL.md +++ b/plugins/bin/.apm/skills/write-docs/SKILL.md @@ -9,7 +9,7 @@ description: > updated: 2026-05-17 when: invoked by explicit trigger ("write docs for X", "document this module", "create docs for this feature") or implicit request to produce technical documentation from code or spec metadata: - version: "1.0.0" + version: "1.0.1" category: implement source: - repo: anthropics/skills diff --git a/plugins/core/.apm/skills/agentsmd-audit/SKILL.md b/plugins/core/.apm/skills/agentsmd-audit/SKILL.md index e852ebd..efb5814 100644 --- a/plugins/core/.apm/skills/agentsmd-audit/SKILL.md +++ b/plugins/core/.apm/skills/agentsmd-audit/SKILL.md @@ -14,7 +14,7 @@ metadata: - context7-websites-agents-md - context7-agentsmd-agents-md - governance-secrets-hard-prohibition - version: "0.1.2" + version: "0.1.3" --- ## Gotchas diff --git a/plugins/core/.apm/skills/agentsmd-author/SKILL.md b/plugins/core/.apm/skills/agentsmd-author/SKILL.md index a9b35f3..ccac923 100644 --- a/plugins/core/.apm/skills/agentsmd-author/SKILL.md +++ b/plugins/core/.apm/skills/agentsmd-author/SKILL.md @@ -12,7 +12,7 @@ metadata: - agents-md-official - context7-websites-agents-md - context7-agentsmd-agents-md - version: "0.1.2" + version: "0.1.3" --- ## Gotchas diff --git a/plugins/core/.apm/skills/provider-adapter-author/SKILL.md b/plugins/core/.apm/skills/provider-adapter-author/SKILL.md index 254196e..9f8b34d 100644 --- a/plugins/core/.apm/skills/provider-adapter-author/SKILL.md +++ b/plugins/core/.apm/skills/provider-adapter-author/SKILL.md @@ -11,7 +11,7 @@ metadata: category: docs source_keys: - adr-0002-0003-two-tier-claude-md - version: "0.1.1" + version: "0.1.2" --- ## Gotchas diff --git a/plugins/git/.apm/skills/git-submodules/SKILL.md b/plugins/git/.apm/skills/git-submodules/SKILL.md index f3969f7..560345b 100644 --- a/plugins/git/.apm/skills/git-submodules/SKILL.md +++ b/plugins/git/.apm/skills/git-submodules/SKILL.md @@ -9,7 +9,7 @@ description: > Not the superproject's own remotes -> `git-remotes`. metadata: - version: "1.0.0" + version: "1.0.1" category: git source_keys: - git-scm-submodule-docs diff --git a/plugins/git/.apm/skills/pc-author/SKILL.md b/plugins/git/.apm/skills/pc-author/SKILL.md index 8b7c411..9bddb84 100644 --- a/plugins/git/.apm/skills/pc-author/SKILL.md +++ b/plugins/git/.apm/skills/pc-author/SKILL.md @@ -6,7 +6,7 @@ description: > shellcheck"). Not running, installing, or updating hooks -> `pc-run`. allowed-tools: Bash Read Write Edit metadata: - version: "1.0.0" + version: "1.0.1" category: devtools source_keys: - context7-pre-commit-com diff --git a/plugins/git/.apm/skills/pc-run/SKILL.md b/plugins/git/.apm/skills/pc-run/SKILL.md index 2e58fa5..90164c4 100644 --- a/plugins/git/.apm/skills/pc-run/SKILL.md +++ b/plugins/git/.apm/skills/pc-run/SKILL.md @@ -8,7 +8,7 @@ description: > compatibility: Requires pre-commit installed and available on PATH. metadata: - version: "1.0.1" + version: "1.0.2" category: devtools source_keys: - context7-pre-commit-com diff --git a/plugins/gitea/.apm/skills/gitea-branches/SKILL.md b/plugins/gitea/.apm/skills/gitea-branches/SKILL.md index 820a55e..d1eee2b 100644 --- a/plugins/gitea/.apm/skills/gitea-branches/SKILL.md +++ b/plugins/gitea/.apm/skills/gitea-branches/SKILL.md @@ -12,7 +12,7 @@ compatibility: Requires Gitea MCP server configured with a token with write:repo metadata: category: integration - version: "0.1.3" + version: "0.1.4" source_keys: - gitea-mcp-repo - gitea-mcp-slim-go diff --git a/plugins/kyberforge/.apm/skills/apm-install/SKILL.md b/plugins/kyberforge/.apm/skills/apm-install/SKILL.md index d576504..6c94bfc 100644 --- a/plugins/kyberforge/.apm/skills/apm-install/SKILL.md +++ b/plugins/kyberforge/.apm/skills/apm-install/SKILL.md @@ -6,7 +6,7 @@ description: > authoring, publishing, auditing, or dependency installation for an apm package -> `apm-workflow`. metadata: - version: "1.0.0" + version: "1.0.1" category: apm source_keys: - context7-microsoft-apm diff --git a/plugins/kyberforge/.apm/skills/apm-workflow/SKILL.md b/plugins/kyberforge/.apm/skills/apm-workflow/SKILL.md index 85e2f30..3032c32 100644 --- a/plugins/kyberforge/.apm/skills/apm-workflow/SKILL.md +++ b/plugins/kyberforge/.apm/skills/apm-workflow/SKILL.md @@ -5,7 +5,7 @@ description: > the dependencies it declares, or an apm marketplace — even when the user does not say "apm". Not the apm binary or an agent runtime -> `apm-install`. metadata: - version: "1.0.0" + version: "1.0.1" category: apm source_keys: - context7-microsoft-apm diff --git a/plugins/lint/.apm/skills/vale-config/SKILL.md b/plugins/lint/.apm/skills/vale-config/SKILL.md index 7511181..7ae5a67 100644 --- a/plugins/lint/.apm/skills/vale-config/SKILL.md +++ b/plugins/lint/.apm/skills/vale-config/SKILL.md @@ -8,7 +8,7 @@ description: > metadata: category: lint - version: "0.1.2" + version: "0.1.3" source_keys: - context7-websites-vale-sh - house-vale-3-15-2-repro diff --git a/plugins/lint/.apm/skills/vale-run/SKILL.md b/plugins/lint/.apm/skills/vale-run/SKILL.md index 5baae10..5d90c43 100644 --- a/plugins/lint/.apm/skills/vale-run/SKILL.md +++ b/plugins/lint/.apm/skills/vale-run/SKILL.md @@ -6,7 +6,7 @@ description: > as in "lint the docs", "check prose style", or "why is CI failing on the docs check". Not setting up Vale config or styles -> `vale-config`. metadata: - version: "0.1.3" + version: "0.1.4" category: lint source_keys: - context7-websites-vale-sh diff --git a/scripts/check-skill-version-bump.sh b/scripts/check-skill-version-bump.sh new file mode 100755 index 0000000..ff9c0c7 --- /dev/null +++ b/scripts/check-skill-version-bump.sh @@ -0,0 +1,156 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Fails a push when a skill changed without its SKILL.md `metadata.version` +# being bumped. ADR-0022 makes the field mandatory and the bump the rule; this +# is the gate that holds the rule, since skill-size-check only checks presence +# and shape. +# +# Baseline: `git merge-base
`. Readers install skills from +# main, so "changed" means changed relative to what main ships, not relative to +# the remote branch's current tip. Diffing from PRE_COMMIT_FROM_REF would let +# the second push of a feature branch excuse a change the first push already +# carried unbumped. Unlike check-release-needed this runs on EVERY push, not +# only pushes to main: a missing bump is cheapest to fix on the branch, before +# review, and a push to main is then already covered. +# +# Scope: every skill directory plugins//.apm/skills//, bin +# included. Anything under /tests/ is ignored — no agent ever loads it, +# so a test-only change ships nothing to a reader. A skill counts as changed +# when any other file under its directory differs between baseline and pushed +# ref. Renames are diffed as delete + add (--no-renames), so: +# - a skill absent at the baseline (new, renamed-to, merged-into) is exempt; +# it has no prior version to exceed. +# - a skill absent at the pushed ref (deleted, renamed-from) is exempt; +# there is nothing left to version. +# A changed skill present at both refs must carry a three-part semver +# `metadata.version` at the pushed ref (same SEMVER_RE as skill-size-check.sh, +# so pre-release suffixes are rejected here as they are there) that is +# numerically greater than the baseline's. A baseline with no parseable +# version (a skill predating ADR-0022) accepts any valid version. Versions are +# read from git objects, never the working tree. +# +# Missing baseline: if neither origin/main nor main resolves, or no merge-base +# exists (shallow clone, unrelated history), the gate FAILS closed — the same +# choice check-release-needed makes for an unreadable tag. Passing would make +# a fresh clone without main the one place the rule is silently off. +# +# Known gap: this only fires on a local `git push` through pre-commit's +# pre-push hook (or a manual `pre-commit run --hook-stage pre-push`). A PR +# merged via Gitea's merge button runs no local hook at all — closing that +# requires a server-side CI job, which this repo does not have yet. + +# See check-release-needed.sh: PRE_COMMIT_TO_REF is the local sha actually +# being pushed, which is only HEAD for the common case. +PUSHED_REF="${PRE_COMMIT_TO_REF:-HEAD}" + +# All-zeros sha: the push deletes a branch. Nothing ships; bail out. +if [[ "$PUSHED_REF" =~ ^0+$ ]]; then + exit 0 +fi + +REPO_ROOT="$(git rev-parse --show-toplevel)" +cd "$REPO_ROOT" + +MAIN_REF="" +for candidate in origin/main main; do + if git rev-parse --verify -q "$candidate^{commit}" > /dev/null; then + MAIN_REF="$candidate" + break + fi +done + +if [[ -z "$MAIN_REF" ]]; then + echo "FAIL: neither origin/main nor main resolves, so there is no baseline to compare skill versions against." >&2 + echo " Fix: git fetch origin main (or create a local main) and retry." >&2 + exit 1 +fi + +if ! BASELINE="$(git merge-base "$MAIN_REF" "$PUSHED_REF" 2>/dev/null)"; then + echo "FAIL: no merge-base between $MAIN_REF and $PUSHED_REF, so there is no baseline to compare skill versions against." >&2 + echo " Fix: ensure full history is available (e.g. git fetch --unshallow) and retry." >&2 + exit 1 +fi + +if ! CHANGED="$(git diff --no-renames --name-only "$BASELINE" "$PUSHED_REF" -- plugins)"; then + echo "FAIL: could not diff $BASELINE..$PUSHED_REF (see git error above)." >&2 + exit 1 +fi + +SKILL_PATH_RE='^(plugins/[^/]+/\.apm/skills/[^/]+)/(.+)$' +SKILL_DIRS=() +while IFS= read -r path; do + [[ "$path" =~ $SKILL_PATH_RE ]] || continue + [[ "${BASH_REMATCH[2]}" == tests/* ]] && continue + dir="${BASH_REMATCH[1]}" + seen=false + for existing in ${SKILL_DIRS[@]+"${SKILL_DIRS[@]}"}; do + [[ "$existing" == "$dir" ]] && { seen=true; break; } + done + $seen || SKILL_DIRS+=("$dir") +done <<< "$CHANGED" + +[[ ${#SKILL_DIRS[@]} -eq 0 ]] && exit 0 + +# Prints the three-part semver metadata.version from SKILL.md on stdin, or +# nothing when it is missing or malformed. Same acceptance rule as +# skill-size-check.sh: str()-coerce, strip whitespace and quotes, then +# ^\d+\.\d+\.\d+$ — so `1.0` (a YAML float) and `1.0.0-rc1` both print nothing. +read_version() { + python3 -c ' +import re, sys, yaml +text = sys.stdin.read() +m = re.match(r"^---\s*\n(.*?)\n---\s*(\n|$)", text, re.S) +if not m: + sys.exit(0) +try: + data = yaml.safe_load(m.group(1)) +except Exception: + sys.exit(0) +meta = data.get("metadata") if isinstance(data, dict) else None +ver = meta.get("version") if isinstance(meta, dict) else None +if ver is None: + sys.exit(0) +ver = str(ver).strip().strip("\x27\"") +if re.match(r"^\d+\.\d+\.\d+$", ver): + print(ver) +' +} + +# Exit 0 when $1 > $2, both MAJOR.MINOR.PATCH, compared numerically so +# 1.0.10 > 1.0.9. 10# forces base 10 on a leading zero. +semver_gt() { + local -a a b + local i + IFS=. read -ra a <<< "$1" + IFS=. read -ra b <<< "$2" + for i in 0 1 2; do + if (( 10#${a[i]} > 10#${b[i]} )); then return 0; fi + if (( 10#${a[i]} < 10#${b[i]} )); then return 1; fi + done + return 1 +} + +OFFENDERS=() +for dir in ${SKILL_DIRS[@]+"${SKILL_DIRS[@]}"}; do + # Absent at baseline: new, renamed-to, or merged-into. Exempt. + git cat-file -e "$BASELINE:$dir/SKILL.md" 2>/dev/null || continue + # Absent at pushed ref: deleted or renamed-from. Exempt. + [[ "$(git cat-file -t "$PUSHED_REF:$dir" 2>/dev/null)" == "tree" ]] || continue + + base_ver="$(git show "$BASELINE:$dir/SKILL.md" | read_version)" + cur_ver="$(git show "$PUSHED_REF:$dir/SKILL.md" 2>/dev/null | read_version || true)" + + if [[ -z "$cur_ver" ]]; then + OFFENDERS+=("$dir: metadata.version missing or not MAJOR.MINOR.PATCH at $PUSHED_REF (baseline: ${base_ver:-none})") + elif [[ -n "$base_ver" ]] && ! semver_gt "$cur_ver" "$base_ver"; then + OFFENDERS+=("$dir: $base_ver -> $cur_ver") + fi +done + +if [[ ${#OFFENDERS[@]} -gt 0 ]]; then + echo "FAIL: skills changed since merge-base with $MAIN_REF without a metadata.version bump (ADR-0022):" >&2 + printf ' %s\n' ${OFFENDERS[@]+"${OFFENDERS[@]}"} >&2 + echo " Fix: raise metadata.version in each SKILL.md above the baseline — bump PATCH at minimum." >&2 + exit 1 +fi diff --git a/tests/test-skill-version-bump.sh b/tests/test-skill-version-bump.sh new file mode 100755 index 0000000..cc15b8b --- /dev/null +++ b/tests/test-skill-version-bump.sh @@ -0,0 +1,252 @@ +#!/usr/bin/env bash +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SCRIPT="$REPO_ROOT/scripts/check-skill-version-bump.sh" +PASS=0 +FAIL=0 + +pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } +fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } + +# The gate reads versions with python3 + PyYAML, the same hard requirement +# skill-size-check carries. Without them this suite cannot run: exit 77 so +# run-tests reports SKIPPED (and --strict fails the push). +if ! python3 -c 'import yaml' 2>/dev/null; then + echo "SKIP: python3 with PyYAML is required" >&2 + exit 77 +fi + +# Output assertions use here-strings, never `echo | grep -q` (pipefail race; +# see tests/test-check-release-needed.sh). + +CLEANUP_DIRS=() +trap 'rm -rf ${CLEANUP_DIRS[@]+"${CLEANUP_DIRS[@]}"}' EXIT + +# write_skill [body] +# An empty version line writes a SKILL.md with no metadata block at all. +write_skill() { + local repo="$1" plugin="$2" skill="$3" vline="$4" body="${5:-body}" + local d="$repo/plugins/$plugin/.apm/skills/$skill" + mkdir -p "$d" + { + echo "---" + echo "name: $skill" + echo "description: Use when testing." + if [[ -n "$vline" ]]; then + echo "metadata:" + echo " $vline" + fi + echo "---" + echo "$body" + } > "$d/SKILL.md" +} + +commit() { (cd "$1" && git add -A && git commit -q -m "${2:-change}"); } + +# A fixture with skills alpha (1.0.0) and beta (1.0.9) on main, then checked out +# onto a feature branch. +make_fixture() { + local dir + dir="$(mktemp -d)" + CLEANUP_DIRS+=("$dir") + (cd "$dir" && git init -q -b main && git config user.email t@t.t && git config user.name t) + write_skill "$dir" demo alpha 'version: "1.0.0"' + write_skill "$dir" demo beta "version: 1.0.9" + commit "$dir" initial + (cd "$dir" && git checkout -q -b feature) + echo "$dir" +} + +# Every PRE_COMMIT_* input comes from here: a value inherited from the pre-push +# hook running this suite names a sha of the real repo, not the fixture. +run_check() { + local dir="$1" + if [[ $# -ge 2 ]]; then + (cd "$dir" && unset PRE_COMMIT_FROM_REF PRE_COMMIT_REMOTE_BRANCH \ + && PRE_COMMIT_TO_REF="$2" bash "$SCRIPT" 2>&1) + else + (cd "$dir" && unset PRE_COMMIT_FROM_REF PRE_COMMIT_TO_REF PRE_COMMIT_REMOTE_BRANCH \ + && bash "$SCRIPT" 2>&1) + fi +} + +# expect_pass [to-ref] +expect_pass() { + local desc="$1"; shift + local out + if out="$(run_check "$@")" && [[ -z "$out" ]]; then + pass "$desc" + else + fail "$desc — expected silent exit 0, got: $out" + fi +} + +# expect_fail [to-ref] +expect_fail() { + local desc="$1" pattern="$2"; shift 2 + local out + if out="$(run_check "$@")"; then + fail "$desc — expected non-zero exit, got 0" + elif grep -qE "$pattern" <<< "$out"; then + pass "$desc" + else + fail "$desc — output did not match /$pattern/: $out" + fi +} + +echo "" +echo "--- 1. unchanged skill passes ---" +F="$(make_fixture)" +echo "unrelated" > "$F/README.md"; commit "$F" +expect_pass "no skill change passes silently" "$F" + +echo "" +echo "--- 2. changed without bump fails ---" +F="$(make_fixture)" +write_skill "$F" demo alpha 'version: "1.0.0"' "new body"; commit "$F" +expect_fail "unbumped change fails and names baseline and current" "alpha: 1\.0\.0 -> 1\.0\.0" "$F" + +echo "" +echo "--- 3. changed with patch / minor / major bump passes ---" +for v in 1.0.1 1.1.0 2.0.0; do + F="$(make_fixture)" + write_skill "$F" demo alpha "version: \"$v\"" "new body"; commit "$F" + expect_pass "bump to $v passes" "$F" +done + +echo "" +echo "--- 4. version decreased fails ---" +F="$(make_fixture)" +write_skill "$F" demo beta "version: 1.0.8" "new body"; commit "$F" +expect_fail "decrease fails" "beta: 1\.0\.9 -> 1\.0\.8" "$F" + +echo "" +echo "--- 5. change to a non-SKILL.md file still counts ---" +F="$(make_fixture)" +mkdir -p "$F/plugins/demo/.apm/skills/alpha/references" +echo "ref" > "$F/plugins/demo/.apm/skills/alpha/references/x.md"; commit "$F" +expect_fail "references/ change without bump fails" "alpha: 1\.0\.0 -> 1\.0\.0" "$F" + +echo "" +echo "--- 6. tests/-only change passes without bump ---" +F="$(make_fixture)" +mkdir -p "$F/plugins/demo/.apm/skills/alpha/tests" +echo "t" > "$F/plugins/demo/.apm/skills/alpha/tests/test-x.sh"; commit "$F" +expect_pass "tests/-only change is exempt" "$F" + +echo "" +echo "--- 7. new skill exempt ---" +F="$(make_fixture)" +write_skill "$F" demo gamma 'version: "0.1.0"'; commit "$F" +expect_pass "new skill passes" "$F" + +echo "" +echo "--- 8. deleted skill exempt ---" +F="$(make_fixture)" +rm -rf "$F/plugins/demo/.apm/skills/alpha"; commit "$F" +expect_pass "deleted skill passes" "$F" + +echo "" +echo "--- 9. renamed skill exempt ---" +F="$(make_fixture)" +(cd "$F" && git mv plugins/demo/.apm/skills/alpha plugins/demo/.apm/skills/alpha2) +commit "$F" +expect_pass "renamed skill passes (old absent at pushed, new absent at baseline)" "$F" + +echo "" +echo "--- 10. missing version on changed skill fails ---" +F="$(make_fixture)" +write_skill "$F" demo alpha "" "new body"; commit "$F" +expect_fail "missing version fails" "alpha: metadata\.version missing" "$F" + +echo "" +echo "--- 11. malformed / prerelease version fails ---" +for v in 'version: 1.1' 'version: "1.0.1-rc1"'; do + F="$(make_fixture)" + write_skill "$F" demo alpha "$v" "new body"; commit "$F" + expect_fail "'$v' is rejected like skill-size-check rejects it" "alpha: metadata\.version missing or not" "$F" +done + +echo "" +echo "--- 12. multiple offenders all reported ---" +F="$(make_fixture)" +write_skill "$F" demo alpha 'version: "1.0.0"' "x" +write_skill "$F" demo beta "version: 1.0.9" "x" +commit "$F" +OUT="$(run_check "$F" || true)" +if grep -q "alpha: 1.0.0 -> 1.0.0" <<< "$OUT" && grep -q "beta: 1.0.9 -> 1.0.9" <<< "$OUT" \ + && grep -q "bump PATCH at minimum" <<< "$OUT"; then + pass "both offenders and the fix are reported" +else + fail "not every offender reported: $OUT" +fi + +echo "" +echo "--- 13. PRE_COMMIT_TO_REF respected over HEAD ---" +F="$(make_fixture)" +write_skill "$F" demo alpha 'version: "1.0.0"' "unbumped"; commit "$F" +BAD="$(cd "$F" && git rev-parse HEAD)" +(cd "$F" && git checkout -q -b clean main) +expect_fail "unbumped TO_REF fails while HEAD is clean" "alpha: 1\.0\.0 -> 1\.0\.0" "$F" "$BAD" +(cd "$F" && git checkout -q "$BAD") +expect_pass "clean TO_REF passes while HEAD is unbumped" "$F" "$(cd "$F" && git rev-parse main)" + +echo "" +echo "--- 14. all-zeros delete sha no-ops ---" +F="$(make_fixture)" +write_skill "$F" demo alpha 'version: "1.0.0"' "unbumped"; commit "$F" +expect_pass "40-zero sha exits 0" "$F" "0000000000000000000000000000000000000000" +expect_pass "64-zero sha exits 0" "$F" "$(printf '0%.0s' {1..64})" + +echo "" +echo "--- 15. bin plugin covered ---" +F="$(make_fixture)" +write_skill "$F" bin tool 'version: "1.0.0"' +(cd "$F" && git checkout -q main); commit "$F" "add bin skill" +(cd "$F" && git checkout -q feature && git merge -q main) +write_skill "$F" bin tool 'version: "1.0.0"' "changed"; commit "$F" +expect_fail "bin skill change without bump fails" "plugins/bin/\.apm/skills/tool: 1\.0\.0 -> 1\.0\.0" "$F" + +echo "" +echo "--- 16. multi-digit semver compare ---" +F="$(make_fixture)" +write_skill "$F" demo beta "version: 1.0.10" "x"; commit "$F" +expect_pass "1.0.10 > 1.0.9 passes" "$F" + +echo "" +echo "--- 17. baseline is the merge-base, not main's tip ---" +F="$(make_fixture)" +write_skill "$F" demo alpha 'version: "1.0.1"' "branch change"; commit "$F" +(cd "$F" && git checkout -q main) +write_skill "$F" demo alpha 'version: "1.0.5"' "main moved on"; commit "$F" +(cd "$F" && git checkout -q feature) +expect_pass "bump over the merge-base passes even though main is ahead" "$F" + +echo "" +echo "--- 18. origin/main preferred over local main ---" +F="$(make_fixture)" +write_skill "$F" demo alpha 'version: "1.0.1"' "x"; commit "$F" +# A stale local main pointing at the feature tip would make the diff empty; +# origin/main at the original commit must win and still see the change. +(cd "$F" && git update-ref refs/remotes/origin/main main && git branch -f main feature) +write_skill "$F" demo alpha 'version: "1.0.1"' "y"; commit "$F" +if OUT="$(run_check "$F")" && [[ -z "$OUT" ]]; then + pass "origin/main used as baseline (1.0.0 -> 1.0.1 counted as a bump)" +else + fail "unexpected output: $OUT" +fi +(cd "$F" && git update-ref refs/remotes/origin/main feature~1) +expect_fail "origin/main at the bumped commit flags the further unbumped change" \ + "merge-base with origin/main" "$F" + +echo "" +echo "--- 19. no main ref fails closed ---" +F="$(mktemp -d)"; CLEANUP_DIRS+=("$F") +(cd "$F" && git init -q -b trunk && git config user.email t@t.t && git config user.name t) +write_skill "$F" demo alpha 'version: "1.0.0"'; commit "$F" +expect_fail "missing main fails with a clear message" "neither origin/main nor main resolves" "$F" + +echo "" +echo "Results: $PASS passed, $FAIL failed" +[[ $FAIL -eq 0 ]]