feat(gates): enforce metadata.version bumps on changed skills at pre-push
check-skill-version-bump fails a push when a skill directory changed against its merge-base with main (tests/ excluded) without a strictly higher metadata.version than main. New, renamed and deleted skills are exempt; every plugin is covered. Recorded as a dated section in ADR-0022 and documented in gates.md. Patch-bumps the 17 skills that changed on this branch without a bump, so the branch passes its own gate. Simplification audit finding 33. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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 <pushed ref>`** (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.
|
||||
- **`<skill>/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$`,
|
||||
|
||||
Reference in New Issue
Block a user