16 Commits

Author SHA1 Message Date
14248e04b9 docs: fix the review findings on the hook-contract retirement
Why: a review of 4de5b6b and 3c5f6a6 found seven places that still
described the retired external hook contract as current, or that could
pass without checking anything.

Implementation Notes:
- gates.md: drop the "one caveat below" pointer; the caveat is gone.
- SIMPLIFICATION-AUDIT.md: the §7 note now says all but one entry is
  closed and lists 36 (4de5b6b) as struck. Finding 2's count chain gets
  a closing note: pre-push is 8 repo-authored hooks (10 reported).
- .pre-commit-config.yaml: the check-vale-style-sync comment points at
  case 32 (one-plugin narrowing guard), not the deleted case 33.
- ADR-0014: the retirement pointer now covers the ADR-0025 amendment
  above it too, naming case 33 and the exported hook IDs. ADR-0022 gets
  a reciprocal amended-by note on its check-release-needed comparison.
  Historical body text is unchanged.
- test-vale-wrap.sh case 32: property 3 fails when a class's corpus
  regex matches no tracked file, instead of passing vacuously after a
  layout move. The corpus regexes are now globals so a new Part D can
  point them at a missing layout and require that failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 12:30:42 +00:00
3c5f6a6e37 docs: mark finding 36 done in the simplification audit
Why: 4de5b6b retired the external pre-commit hook contract, so the audit
still listed 36 as open, with a deferred decision.

Implementation Notes: finding 36 is struck through and carries a dated
Done note (measured line counts, the case 33 cost and how its guard was
kept). The §7 status notes, the §8 consumer question and the §10
dispositions now leave 15 as the only open finding, with 22 deferred
with bin.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 11:53:25 +00:00
4de5b6b355 chore(gates): retire the external pre-commit hook contract
Why: .pre-commit-hooks.yaml and its release-tag gate served external
consumers that do not exist. No repo on the Gitea instance pins these
hooks, and the README names apm as the only supported install path. The
mechanism was also already failing: skill-size-check.sh changed after
v2.0.1 with no tag cut, and the gate cannot fire through Gitea's merge
button. (Simplification audit finding 36.)

Implementation Notes:
- Delete .pre-commit-hooks.yaml, scripts/check-release-needed.sh,
  tests/test-check-release-needed.sh and tests/test-vale-hooks-consumer.sh,
  and remove the check-release-needed pre-push hook. The repo: local
  skill-size-check and vale-audit-prefilter-* hooks are unchanged.
- ADR-0014 is amended, not retired: its runtime decision to bundle Vale
  inside factory-audit stands. The amendment keeps the entry[0]-only
  constraint (LESSONS.md:101,105) in case the export returns. ADR-0025
  gets a pointer.
- test-vale-wrap.sh: drop case 33 (the cross-manifest drift check) and
  case 28's hook-scope half, which read the published manifest. Case 32
  now also requires each hook to select every tracked file of its class,
  which keeps case 33's one-plugin-narrowing guard, with a mutation test.
- test-skill-size-check.sh and test-adr0020-contract.sh now assert the
  hook contract and verbose: true on .pre-commit-config.yaml only.
- gates.md: pre-push count goes from 9 to 8 authored hooks (11 to 10
  reported), and the Release table, the External consumers section and
  the two-manifest scope table are removed. README and script/test
  comments no longer describe the export as live. The resolver comment
  is edited identically in both copies.
- The v1.0.0/v2.0.0/v2.0.1 tags are left in place; they are inert.

ADR: 0014
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 11:52:14 +00:00
2119da9700 docs: record the second review round in the simplification audit
Why: the audit carried stale figures and working-tree citations after the
grill commits landed, and this review round's decisions needed recording.

Implementation Notes: line totals pinned to c07ca07, working-tree
citations replaced with commits, version-location count corrected, and a
new section lists this round's dispositions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 11:24:33 +00:00
807caf22ee fix(kyberforge): give branch-aware advice for the refreshed apm lock
Why: the docs said to discard a refreshed apm.lock.yaml on a feature
branch because the refresh records main's commit, but the branch's own
lock records a (older) main commit too, and the SessionStart notice gave
the same "commit or discard" advice on every branch.

Implementation Notes:
- check-apm-current.sh picks fixed advice by branch: commit or discard
  deliberately on the default branch (origin/HEAD, else main), discard and
  reinstall on a feature branch; the branch name is never interpolated.
- README, AGENTS.md and ADR-0019 give the real reasons (no lock churn in
  the branch diff, deployed tree matches the committed lock), the cost
  (the session runs the older main) and that the next session start
  refreshes again.
- ADR-0019's check-clean and stale-server claims restated to match apm's
  source.

ADR: 0019
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 11:24:32 +00:00
398515bcad chore: drop the duplicated package descriptions from root apm.yml
Why: for a local-path marketplace entry, apm treats a root description: as
an override of the package's own apm.yml, reported only at verbose level,
so the "two copies stay identical" rule had no enforcement. Same fix as
2def060 made for version:.

Implementation Notes:
- All six root copies matched their package apm.yml before removal; the
  compiled marketplace.json descriptions are unchanged.
- apm-workflow references now scope the "omit it" advice to local-path
  entries: on a remote entry, version: is the semver range that selects
  the tag, and version: or ref: is required.

Impact: ADR-0021 amended; the package apm.yml is the single source.

ADR: 0021
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 11:24:22 +00:00
c5d43b4d63 chore(release): bump bin, git, gitea, core and lint patch versions
Why: each of these plugins changed shipped .apm/ content on this branch
without a package version bump, which configure.md requires and no gate
catches. No skill, agent or hook was removed, so a patch bump fits.

Implementation Notes: marketplace.json regenerated with apm pack; the diff
is the five version strings only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 11:24:21 +00:00
1d40544075 fix(gates): hold skill versions above main's tip as well as the merge-base
Why: two branches that both bump a skill 1.0.0 -> 1.0.1 with different
content merge without a conflict, and each passed the gate against its own
merge-base, so main could ship two changes under one version.

Implementation Notes:
- check-skill-version-bump requires the pushed version to exceed both the
  merge-base and the main tip; failures name the baseline they missed.
- Presence is read from the tree, so a blob missing from a partial clone is
  a read failure instead of a silently exempt "new" skill.
- A leading UTF-8 BOM no longer reads as a missing version.
- Version parts reject leading zeros in all three validators
  (check-skill-version-bump, skill-size-check, factory-audit).
- New tests cover equal bumps, moved files, major/minor ordering, bad refs,
  unreadable blobs, mode-only changes, symlinks and tag peeling.

Impact: ADR-0022 amended (reverses "not main's current tip"); gates.md
updated to match, including pre-commit 4.6.1's exact ref selection.

ADR: 0022
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 11:24:08 +00:00
b426460f75 docs: reconcile the audit with the review of the grill commits
Re-measures the figures the new hook and ADR edits moved, re-points
shifted gates.md and config citations, ticks the decided §8 questions,
and aligns §7, §10 and the finding 8/18/28/33 notes with the decisions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 10:33:36 +00:00
c07ca0767e docs(kyberforge): stop apm-workflow assuming root package versions
The root apm.yml packages[] entries no longer carry version:, and a
version there is a silent override that --check-versions does not
catch. configure.md and marketplace.md now name the package's own
apm.yml as the single source and drop version: from the examples.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 10:33:35 +00:00
dd0b9233e6 docs: correct the version-bump gate and branch-refresh documentation
ADR-0022 and gates.md now state the merge-base baseline, the fail-closed
cases, the PyYAML requirement and the multi-ref push gap (shared with
check-release-needed); the new hook gets its own gates.md group. Both
ADR additions follow each file's amendment format. README and AGENTS.md
now say to discard a refreshed apm.lock.yaml on a feature branch, and
that an .apm/ edit is live only once it is on the remote's main.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 10:33:34 +00:00
1ce596cdbe fix(gates): close the review findings in check-skill-version-bump
- Read changed paths NUL-delimited so non-ASCII paths are no longer
  silently skipped.
- Fail closed when only local main resolves and the pushed commit is
  the merge-base, instead of passing on an empty diff.
- Accept ASCII-only versions with at most nine digits per part.
- Check for python3/PyYAML up front, and report read failures as such
  rather than as a missing version; name a missing SKILL.md.
- Document that pre-commit gates only the first ref of a multi-ref push.

Tests grow to 29 cases covering each fix plus annotated tags, CRLF
frontmatter, unrelated histories and pushing main.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 10:33:33 +00:00
89b1c6fc85 docs: record the grill and declined-finding decisions in the audit
Findings 11, 17, 23, 25 and 27 are declined. From the grill: 33 and 34
done, 8, 18, 20 and 28 closed, 22 deferred with bin. Open: 15 and 36.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 10:10:00 +00:00
afcf477ede docs(adr): record the feature-branch refresh hazard in ADR-0019
The session-start refresh resolves against main, so on a branch it
redeploys content the branch removed and rewrites the lock to main's
commit. Documented as a consequence rather than skipped in code, since
a skip would only freeze the session on an older main. Also records the
re-measured refresh time (~24-26 s). Simplification audit finding 34.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 10:09:59 +00:00
2def06054a chore: drop the duplicated package versions from root apm.yml
The six packages[].version lines restated each plugin's own apm.yml
version and were unpoliced: on drift apm silently shipped the curator
value. apm reads the plugin's apm.yml when the entry is absent, and
apm pack --check-versions --check-clean still passes with the committed
marketplace.json unchanged. Simplification audit finding 33.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 10:09:58 +00:00
8451169d2b 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>
2026-09-16 10:09:56 +00:00
54 changed files with 1665 additions and 1511 deletions

View File

@@ -18,35 +18,35 @@
{
"name": "bin",
"description": "Skills for everyday AI-assisted development work that is not tied to a single tool, forge or language, and has not yet been split into a focused plugin.",
"version": "1.1.7",
"version": "1.1.8",
"category": "Utilities",
"source": "./plugins/bin"
},
{
"name": "git",
"description": "Skills and agents for working with a local Git clone over the git wire protocol, and for authoring and running the pre-commit hooks that guard it.",
"version": "1.3.7",
"version": "1.3.8",
"category": "Version Control",
"source": "./plugins/git"
},
{
"name": "gitea",
"description": "Skills and agents for working with a Gitea forge through its HTTP API — the forge's own objects, as distinct from the local git clone.",
"version": "1.3.8",
"version": "1.3.9",
"category": "Version Control",
"source": "./plugins/gitea"
},
{
"name": "core",
"description": "Skills for authoring and auditing a repo's AGENTS.md and the provider adapter files that defer to it.",
"version": "1.1.2",
"version": "1.1.3",
"category": "Productivity",
"source": "./plugins/core"
},
{
"name": "lint",
"description": "Skills and agents for configuring and running linters.",
"version": "1.1.7",
"version": "1.1.8",
"category": "Developer Tools",
"source": "./plugins/lint"
}

View File

@@ -167,7 +167,7 @@ repos:
# assertions diffed skill-audit's Vale copy against agent-audit's; the
# merge into factory-audit leaves one copy, so those are moot. The other
# 11 moved into tests/test-vale-wrap.sh (case 0, cases 28-31, its
# Vale-absent skip, and case 33 for cross-manifest files: agreement),
# Vale-absent skip, and case 32 for the one-plugin narrowing guard),
# which run-tests runs here at
# pre-push, so do not re-add the hook to restore coverage. Do not
# confuse its removal with check-scope-walkup-sync below, which survives:
@@ -183,14 +183,18 @@ repos:
pass_filenames: false
always_run: true
- id: check-release-needed
name: Check a release tag covers .pre-commit-hooks.yaml's paths
description: On push to main only, fail if files exposed via .pre-commit-hooks.yaml changed since the last tag
entry: bash scripts/check-release-needed.sh
- 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.
- id: validate-marketplace
name: Validate marketplace manifest

View File

@@ -1,31 +0,0 @@
# PUBLISHED CONTRACT. External repos consume these IDs with `rev: <tag>`, so an
# ID or a `files:` regex here may not change without breaking them on upgrade.
# ADR-0025 merged skill-audit and agent-audit into factory-audit and re-pointed
# both `entry:` paths at its single vale-wrap.sh; both IDs and both regexes are
# unchanged, deliberately. Collapsing them into one was considered and rejected:
# it breaks every consumer pinning kyberforge-vale-audit-agent, and it re-creates
# ADR-0014's measured failure where one hook against one config silently scanned
# 0 files of the other type. Two IDs are what keep both file scopes addressable.
- id: kyberforge-vale-audit-skill
name: Kyberforge Vale prose audit (SKILL.md)
description: Deterministic prose-pattern prefilter for kyberforge's factory-audit skill flow, via its own bundled Vale config/styles
entry: plugins/kyberforge/.apm/skills/factory-audit/scripts/vale-wrap.sh
language: script
files: '(^|/)SKILL\.md$'
- id: kyberforge-vale-audit-agent
name: Kyberforge Vale prose audit (agent files)
description: Deterministic prose-pattern prefilter for kyberforge's factory-audit agent flow, via its own bundled Vale config/styles
entry: plugins/kyberforge/.apm/skills/factory-audit/scripts/vale-wrap.sh
language: script
files: '(^|/)agents/[^/]+\.md$|\.agent\.md$'
- id: kyberforge-skill-size-check
name: SKILL.md size and context-budget ceilings
description: Enforce agentskills.io's 500-line/2,770-whole-file-word spec ceilings plus ADR-0020's context budget (description 250 chars SUGGESTION / 400 FAIL, body-only 600 words SUGGESTION / 900 FAIL, resolvable boundary-clause routing targets)
entry: scripts/skill-size-check.sh
language: script
files: '(^|/)SKILL\.md$'
# verbose so the SUGGESTION tier reaches a human -- pre-commit prints
# nothing for a passing hook, and a SUGGESTION deliberately does not fail.
verbose: true

View File

@@ -28,8 +28,8 @@ Fall back to raw shell only when no skill covers it.
## Session rules
- **Do not add repo-owned keys to `.claude/settings.json`.** apm treats it as its own deployed artifact and `apm audit --ci` replays the install and diffs, so anything apm would not have written is permanent drift that fails the `apm-audit-ci` pre-push hook. A hook you want here is authored in `plugins/<name>/.apm/hooks/` and deployed by apm, never hand-written into that file. The `SessionStart` entry already in it is exactly that: kyberforge authors it in `plugins/kyberforge/.apm/hooks/hooks.json` and apm merges it in, so it is apm's own output, it is what the replay expects, and it belongs in the commit — do not strip it (ADR-0019). Machine-specific settings go in the gitignored `.claude/settings.local.json`; shared enforcement goes in `.pre-commit-config.yaml`.
- **`apm.lock.yaml` turning up modified is expected, not a bug.** kyberforge's `SessionStart` hook keeps the install current on launch and rewrites the lock in the process (ADR-0019). Commit or discard it deliberately.
- **A `.apm/` edit is not live in this session until it is pushed.** The six dependencies resolve from the holocron remote, unpinned against the default branch. `apm install` deploys from the lock; `apm update` is what re-resolves refs.
- **`apm.lock.yaml` turning up modified is expected, not a bug.** kyberforge's `SessionStart` hook keeps the install current on launch and rewrites the lock in the process (ADR-0019). On `main`, commit or discard it deliberately. On a feature branch, discard it (`git checkout -- apm.lock.yaml`, then `apm install`). This keeps unrelated lock churn out of the branch diff and keeps `apm pack --check-clean` consistent with the committed lock. The session then runs the older `main` that the lock records, which is accepted on a branch, and the next session start refreshes again.
- **A `.apm/` edit is not live until it is on the remote's `main`.** The six dependencies resolve from the holocron remote, unpinned against the default branch, so pushing a feature branch does not deploy it (ADR-0019). `apm install` deploys from the lock; `apm update` is what re-resolves refs.
- **No pre-push hook needs the network.** Root `apm.yml`'s marketplace has no remote package entries, so every hook resolves locally.
- **This repo and Gitea are the only source of truth.** All project state, decisions, and working conventions live here. Do not use an external memory system for this project — cached state diverges from the repo and you get a split brain. Before answering any design or architecture question, check `docs/adr/` for an existing decision.

View File

@@ -31,8 +31,8 @@ Install all of these before setting up. Each one is a hard dependency of a git h
| Tool | Why | Install |
| --- | --- | --- |
| `apm` CLI | Two pre-push hooks shell out to it (`apm-audit-ci` and `apm-pack-check-clean`) | The `apm-install` skill, or `curl -sSL https://aka.ms/apm-unix \| sh`. Verify with `apm --version` |
| `python3` + PyYAML | Required by `scripts/skill-size-check.sh` (the `skill-size-check` pre-commit hook), which reads folded YAML frontmatter | `python3` is usually present — pre-commit is itself a Python application. `pip install pyyaml` if the hook reports PyYAML missing |
| `vale` | Required by the `vale-audit-prefilter-skill` / `-agent` pre-commit hooks, and by the `test-vale-wrap.sh` / `test-vale-hooks-consumer.sh` suites that `run-tests --strict` runs at pre-push | `brew install vale` (macOS), `snap install vale` (Linux), `choco install vale` (Windows), or https://vale.sh/docs/vale-cli/installation/ |
| `python3` + PyYAML | Required by `scripts/skill-size-check.sh` (the `skill-size-check` pre-commit hook) and `scripts/check-skill-version-bump.sh` (the `check-skill-version-bump` pre-push hook), which both parse YAML frontmatter | `python3` is usually present — pre-commit is itself a Python application. `pip install pyyaml` if the hook reports PyYAML missing |
| `vale` | Required by the `vale-audit-prefilter-skill` / `-agent` pre-commit hooks, and by the `test-vale-wrap.sh` suite that `run-tests --strict` runs at pre-push | `brew install vale` (macOS), `snap install vale` (Linux), `choco install vale` (Windows), or https://vale.sh/docs/vale-cli/installation/ |
| `claude` CLI | Required by the `validate-marketplace` pre-push hook | Claude Code |
Two notes worth reading before you skip one:
@@ -58,7 +58,7 @@ pre-commit install -t pre-commit -t commit-msg -t pre-push
## Keeping the install current
The six dependencies in root `apm.yml` are unpinned against the default branch, so deployed skills go stale whenever anyone merges. kyberforge's `SessionStart` hook keeps the install current automatically on launch, rewriting `apm.lock.yaml` in the process — an unexplained modification to it after opening a session is expected, not a bug; commit or discard it deliberately. Mechanism and rationale: `docs/adr/0019-session-start-hook-keeps-the-apm-install-current.md`.
The six dependencies in root `apm.yml` are unpinned against the default branch, so deployed skills go stale whenever anyone merges. kyberforge's `SessionStart` hook keeps the install current automatically on launch, rewriting `apm.lock.yaml` in the process — an unexplained modification to it after opening a session is expected, not a bug. On `main`, commit or discard it deliberately. On a feature branch, discard it (`git checkout -- apm.lock.yaml`, then `apm install`). The committed lock records a `main` commit too, just an older one. Discarding keeps lock churn unrelated to the branch out of its diff, and keeps the deployed tree consistent with the committed lock that `apm pack --check-clean` reads. The trade-off: the session then runs the older `main` the lock records, which is accepted on a feature branch. The discard also lasts only until the next session start, when the hook finds the lock behind `main` and refreshes again. Mechanism and rationale: `docs/adr/0019-session-start-hook-keeps-the-apm-install-current.md`.
Note the difference between the two commands:
@@ -84,11 +84,6 @@ Run the pre-push gate locally in one command:
pre-commit run --hook-stage pre-push --all-files
```
One caveat: `check-release-needed` is a silent no-op under this invocation. It exits 0 unless
`PRE_COMMIT_REMOTE_BRANCH` is `refs/heads/main`, and pre-commit exports that only from the real
pre-push git hook during an actual `git push` — so the hook reports `Passed` having checked nothing.
Every other pre-push hook does run.
See [`docs/spec/gates.md`](docs/spec/gates.md) for what each hook enforces and why.
**Offline?** No pre-push hook needs the network: root `apm.yml`'s marketplace has no remote package entries (the last one, `mattpocock-skills`, was removed), so `apm-pack-check-clean` resolves everything from local sources. All pre-push hooks pass offline.

View File

@@ -21,27 +21,33 @@ Counting convention: line counts are hand-edited `.apm/` source unless marked "i
>
> > **Re-measured (2026-09-14, at `a6434e0`):** the right-hand column originally read 31,473 / 6,050 / 3,471 / 2,360 / 923 / 2,083 = 46,360 and was labelled "Today" against "the current working tree". It did not reconcile to its own commit's tree — at `061bb3d`, where it was written, the six plugins measured 31,435 / 6,048 / 3,474 / 2,358 / 926 / 2,087 = 46,328 — and "the current working tree" is a basis that goes stale silently. Re-counted at `a6434e0` and the column now names its SHA. The baseline column is confirmed exact against `9eb8bc7`. Commits after `061bb3d` (`c96ca9c`, which deleted the six plugin-root `.mcp.json` files) account for most of the remaining drift.
> **Re-derived (2026-09-16, at HEAD on `docs/simplification-audit`):** the 2026-09-15 notes recording finding 14's merge (`467bbd7`, ADR-0025) and the pipefail fix (`4059cb4`) were written without correcting the headlines they annotate, so this pass re-counted every figure those two commits could have moved and corrected each in place above and below. Everything re-measured here came from a command run at HEAD — `git ls-files`, `wc -l`, `grep -c`, and `bash tests/run-tests.sh --strict` — never from an earlier note. What moved: finding 2 (two surviving sync gates → one), the `.pre-commit-config.yaml` hook counts (27/9 → 26/8), the skill census (39 → 38 and everything derived from it), finding 11's validator and `sources.md` figures, finding 16's whole numeric basis, and the stale `skill-audit/`, `agent-audit/` and `formatting-and-scripts.md` paths in findings 18, 19 and 33. §1's three rows re-measured at HEAD: **469** tracked files (465 regular plus the 4 submodule gitlinks) / **74,594** lines; `plugins/` **46,106** (62%); the 38 `SKILL.md` bodies **2,409** (5.2% of plugin lines); enforcement **20 `tests/test-*.sh` totalling 10,189 lines**, the two runners **502** (`run-tests.sh` 283 + `run-bats.sh` 219), and `scripts/` **2,901**; kyberforge's validator scripts and their bats tests **5,861 + 6,015** (the merge deduplicated scripts and left the test corpus larger, not smaller — `git ls-files 'plugins/kyberforge/.apm/skills/*/scripts/*.sh'` and `.../tests/*.bats`). `run-tests.sh --strict` reports **20 passed, 0 skipped, 0 failed**. Re-checked and unchanged, so left alone: `docs/research/` inside plugins (19,030) and repo-level `docs/research/` + `docs/notes/` (4,488). Not re-measured, and still carrying their last stated basis: the preload-tax and commit-share rows, §2's timings, and the per-plugin table in the note above.
> **Re-derived (2026-09-16, at HEAD on `docs/simplification-audit`):** the 2026-09-15 notes recording finding 14's merge (`467bbd7`, ADR-0025) and the pipefail fix (`4059cb4`) were written without correcting the headlines they annotate, so this pass re-counted every figure those two commits could have moved and corrected each in place above and below. Everything re-measured here came from a command run at HEAD — `git ls-files`, `wc -l`, `grep -c`, and `bash tests/run-tests.sh --strict` — never from an earlier note. What moved: finding 2 (two surviving sync gates → one), the `.pre-commit-config.yaml` hook counts (27/9 → 26/8, then back to 27/9 — see the correction at the end of this note), the skill census (39 → 38 and everything derived from it), finding 11's validator and `sources.md` figures, finding 16's whole numeric basis, and the stale `skill-audit/`, `agent-audit/` and `formatting-and-scripts.md` paths in findings 18, 19 and 33. §1's three rows re-measured: ~~**469**~~ → **471** tracked files (~~465~~ → 467 regular plus the 4 submodule gitlinks) / ~~**74,594**~~ → **75,441** lines (pinned to `c07ca07`; see the note below); `plugins/` ~~**46,106** (62%)~~ → **46,127** (61%); the 38 `SKILL.md` bodies **2,409** (5.2% of plugin lines); enforcement ~~**20 `tests/test-*.sh` totalling 10,189 lines**~~ → **21 `tests/test-*.sh` totalling 10,608 lines**, the two runners **502** (`run-tests.sh` 283 + `run-bats.sh` 219), and `scripts/` ~~**2,901**~~ → **3,139**; kyberforge's validator scripts and their bats tests ~~**5,861**~~ → **5,876** + **6,015** (the merge deduplicated scripts and left the test corpus larger, not smaller — `git ls-files 'plugins/kyberforge/.apm/skills/*/scripts/*.sh'` and `.../tests/*.bats`). `run-tests.sh --strict` reports ~~**20 passed, 0 skipped, 0 failed**~~ → **21 passed, 0 skipped, 0 failed**.
>
> > **Re-measured (2026-09-16, at `c07ca07`):** commit `8451169` added `check-skill-version-bump` — a pre-push hook, `scripts/check-skill-version-bump.sh` (238 lines) and `tests/test-skill-version-bump.sh` (410) — after the figures above were taken, so each was one short. `.pre-commit-config.yaml` now has **27** `- id:` entries and **9** `stages: [pre-push]` (`grep -c -- "- id:"`; `grep -c "stages: \[pre-push\]"`), all nine repo-authored. The struck figures are replaced from these commands. They were run against the working tree, and every figure reproduces exactly from the committed tree at `c07ca07`: `git ls-files | wc -l`; `cat` over every non-gitlink tracked path `| wc -l`; `git ls-files plugins | xargs cat | wc -l`; `git ls-files scripts | xargs wc -l` (no untracked files under `scripts/`); `ls tests/test-*.sh | wc -l` and `cat tests/test-*.sh | wc -l`; `bash tests/run-tests.sh --strict`. The earlier 469 / 74,594 / 46,106 did not reproduce exactly at `8451169^` either (469 / 74,638 / 46,121), so they were taken at an earlier commit than this note's "at HEAD" says. Re-checked and unchanged, so left alone: `docs/research/` inside plugins (19,030) and repo-level `docs/research/` + `docs/notes/` (4,488). Not re-measured, and still carrying their last stated basis: the preload-tax and commit-share rows, §2's timings, and the per-plugin table in the note above.
>
> > **Pinned (2026-09-16, review round):** the tracked-lines total is a moving figure, and this file moves it: `b426460` changed only this audit and took the committed total from 75,441 to 75,461. So the figure stays pinned to a commit instead of being chased. At `c07ca07` the committed tree has **471** paths (`git ls-tree -r`), **75,441** lines (every blob, `git cat-file -p … | wc -l`), and **46,127** lines under `plugins/` (61%). The 38 `SKILL.md` bodies there total **2,409** lines. At `c07ca07` the working-tree method above gives the same numbers, because nothing else was uncommitted. The `scripts/`, `tests/` and runner figures in the note above also reproduce at `c07ca07`, and at `b426460` too, since that commit changed no other file.
> **Reviewed (2026-09-14):** commits `718c79a` and `d2480b8` were put through a five-agent review. Result: **zero skill, agent or hook regressions** — 39 skills before and after, all gates passing, and both hook removals (`validate-plugins`, `check-plugin-content-sync`) genuinely moot rather than merely unenforced. One real functional regression was found — MCP propagation to consumers, broken by the same commit's manifest deletion; see finding 37 — along with the numeric and bookkeeping drift in this document's own 2026-09-14 notes, corrected in place above and below.
> **Reviewed again (2026-09-16):** the grill commits (`8451169` through `b426460`) went through a second review round. Its dispositions are in §11.
## 1. The shape of the problem
| Measure | Value |
| ---------------------------------------------------------------------------| -----------------------------------------------------------------------------------------|
| Tracked files / lines | ~~820 / 102,000~~ → ~~475 / 73,073~~ → 469 / 74,594 |
| Lines in `plugins/` | ~~70,600 (69% of repo)~~ → ~~46,301 (63% of repo)~~ → 46,106 (62% of repo) |
| Tracked files / lines | ~~820 / 102,000~~ → ~~475 / 73,073~~ → ~~469 / 74,594~~ → 471 / 75,441 (at `c07ca07`) |
| Lines in `plugins/` | ~~70,600 (69% of repo)~~ → ~~46,301 (63% of repo)~~ → ~~46,106 (62% of repo)~~ → 46,127 (61% of repo) |
| Of which the ~~39~~ → 38 `SKILL.md` files a model actually loads | ~~about 2,600 lines (under 4% of plugin lines)~~ → ~~2,509 lines (5.4% of plugin lines)~~ → 2,409 lines (5.2% of plugin lines) |
| Generated flat mirror files (byte copies of `.apm/`) | ~~263 files, ~22,000 lines~~ → 0 (deleted 2026-09-14, see below) |
| `docs/research/` vendored inside plugins | ~19,000 lines, nothing executable reads it |
| Repo-level `docs/research/` + `docs/notes/` | 4,500 lines, 47% of all prose words, 6 of 11 research files linked only from each other |
| Enforcement: hook entries in `.pre-commit-config.yaml` / pre-push hooks | 33 / 14 |
| Enforcement: `tests/*.sh` + runners + `scripts/` | ~~12,400 + 475 + 4,500 lines~~ → ~~9,123 + 490 + 3,308 lines~~ → 10,189 + 502 + 2,901 |
| Validator scripts inside kyberforge (+ their bats tests) | ~~6,800 + 5,300 lines~~ → 5,861 + 6,015 |
| Enforcement: `tests/*.sh` + runners + `scripts/` | ~~12,400 + 475 + 4,500 lines~~ → ~~9,123 + 490 + 3,308 lines~~ → ~~10,189 + 502 + 2,901~~ → 10,608 + 502 + 3,139 |
| Validator scripts inside kyberforge (+ their bats tests) | ~~6,800 + 5,300 lines~~ → ~~5,861 + 6,015~~ → 5,876 + 6,015 |
| Preload tax (39 skill names + descriptions) | 10,987 chars, ~2,750 tokens per session |
| Commits since 2026-05-10 / share touching hook, test, gate, vale, or sync | 447 / ~25% |
> **Corrected then done (2026-09-14):** the mirror row's figure was wrong. The true mirror was **213 files / 20,061 lines**, not 263 / ~22,000 — the original count swept in files that were never mirror output. All 213 were deleted in commit `718c79a` on `docs/simplification-audit` (245 files changed, 298 insertions, 22,602 deletions across the whole change), so the row is now zero. The enforcement row is stale on **both** halves — it was correct at the 2026-09-10 baseline (`9eb8bc7`: 33 `- id:` entries, 14 repo-authored pre-push hooks), but `.pre-commit-config.yaml` today has ~~**27 entries and 9 `stages: [pre-push]`**~~ → **26 entries and 8 `stages: [pre-push]`** (re-measured 2026-09-16 at HEAD; `467bbd7` removed `check-vale-style-sync` with finding 14's merge). Like for like that is 14 → ~~9~~ → 8 repo-authored pre-push hooks. The stage *reports* ~~11~~ → 10, because the 2 pre-commit `meta` hooks also run there — a different counting basis; see the corrected §3 target, which states it the same way.
> **Corrected then done (2026-09-14):** the mirror row's figure was wrong. The true mirror was **213 files / 20,061 lines**, not 263 / ~22,000 — the original count swept in files that were never mirror output. All 213 were deleted in commit `718c79a` on `docs/simplification-audit` (245 files changed, 298 insertions, 22,602 deletions across the whole change), so the row is now zero. The enforcement row is stale on **both** halves — it was correct at the 2026-09-10 baseline (`9eb8bc7`: 33 `- id:` entries, 14 repo-authored pre-push hooks), but `.pre-commit-config.yaml` today has ~~**27 entries and 9 `stages: [pre-push]`**~~ → ~~**26 entries and 8 `stages: [pre-push]`**~~ → **27 entries and 9 `stages: [pre-push]`** (`467bbd7` removed `check-vale-style-sync` with finding 14's merge; `8451169` then added `check-skill-version-bump`; re-measured 2026-09-16 at HEAD (`b426460`) with `grep -c -- "- id:"` and `grep -c "stages: \[pre-push\]"` on `.pre-commit-config.yaml`). Like for like that is 14 → ~~9~~ → ~~8~~ → 9 repo-authored pre-push hooks. The stage *reports* ~~11~~ → ~~10~~ → 11, because the 2 pre-commit `meta` hooks also run there — a different counting basis; see the corrected §3 target, which states it the same way.
> **Re-measured (2026-09-14, at `a6434e0`):** this table is a **dated snapshot corrected in place**, not a live figure — every arrow above reads "baseline (2026-09-10, `9eb8bc7`) → value at the stated commit". Three further rows were still carrying baseline values after `d2480b8`/`061bb3d` corrected their neighbours, and are now corrected at `a6434e0`:
>
@@ -98,8 +104,8 @@ This is the area you named as hardest to understand and slowest. Root cause: mos
> **Grilled and closed (2026-09-14):** `apm-audit-ci` — already resolved before this audit was written: `.pre-commit-config.yaml`'s own comment block (added in commit `a155af6`, months before this audit) already rebuts the "overclaimed description" complaint and gives a dated, verified justification for what the hook still checks. Keep, no action. `apm-marketplace-check` — its stated purpose ("the only hook that checks remote package references rather than local-source paths") is void: finding 35 (commit `568ca74`) already removed the only remote package entry, so every `marketplace.packages[]` entry is now a local `./plugins/<name>` path and the hook is pure overlap with `apm-pack-check-clean`. Removed the hook entry, and corrected the now-stale "does NOT join apm-marketplace-check ... on the offline SKIP= list" comment on `apm-audit-ci` (there is no offline skip list any more — every pre-push hook already passes offline per `README.md`). Updated `README.md` (tool table, "Offline?" section) and `docs/spec/gates.md` (hook table, hook counts 13→11 self-authored / 15→13 total, the "Three of these shell out to apm" paragraph, and the "Pushing without a network" section) accordingly. Verified: `apm audit --ci` still passes per-plugin, and the pre-push hook count now matches `.pre-commit-config.yaml`.
> **Corrected and closed (2026-09-14, at `a6434e0`):** two things above went stale within hours of being written, and the finding was never given a marker.
>
> - **"Keep the two `claude plugin validate` hooks"** is void. `718c79a` (ADR-0024) deleted `validate-plugins` — the ADR's own reasoning is that `claude plugin validate` reads manifests only and could never detect the empty-content defect it was credited with guarding, and with the per-plugin manifests gone it has nothing left to read. Only **`validate-marketplace`** survives, over the one manifest this repo still ships (`.claude-plugin/marketplace.json`). Of the six hooks this finding named, three now exist: `validate-marketplace`, `apm-pack-check-clean`, `apm-audit-ci`. Verified against `.pre-commit-config.yaml`: ~~27 `- id:` entries, 9 with `stages: [pre-push]`~~ → **26 `- id:` entries, 8 with `stages: [pre-push]`** (re-measured 2026-09-16 at HEAD), no `validate-plugins` entry.
> - **The gates.md figures above ("13→11 self-authored / 15→13 total") were correct for `0dffff3` and are no longer current.** `718c79a` removed two more pre-push hooks after that commit, and `docs/spec/gates.md:24` read **11 reported / 9 self-authored** when this note was written; finding 14's merge has since removed `check-vale-style-sync`, and it now reads **10 reported / 8 self-authored**. Read the count from that file, not from this note.
> - **"Keep the two `claude plugin validate` hooks"** is void. `718c79a` (ADR-0024) deleted `validate-plugins` — the ADR's own reasoning is that `claude plugin validate` reads manifests only and could never detect the empty-content defect it was credited with guarding, and with the per-plugin manifests gone it has nothing left to read. Only **`validate-marketplace`** survives, over the one manifest this repo still ships (`.claude-plugin/marketplace.json`). Of the six hooks this finding named, three now exist: `validate-marketplace`, `apm-pack-check-clean`, `apm-audit-ci`. Verified against `.pre-commit-config.yaml`: ~~27 `- id:` entries, 9 with `stages: [pre-push]`~~ → ~~**26 `- id:` entries, 8 with `stages: [pre-push]`**~~ → **27 `- id:` entries, 9 with `stages: [pre-push]`** (re-measured 2026-09-16 at HEAD, `b426460`; `8451169` added `check-skill-version-bump`), no `validate-plugins` entry.
> - **The gates.md figures above ("13→11 self-authored / 15→13 total") were correct for `0dffff3` and are no longer current.** `718c79a` removed two more pre-push hooks after that commit, and `docs/spec/gates.md:24` read **11 reported / 9 self-authored** when this note was written; finding 14's merge has since removed `check-vale-style-sync`, and it ~~now reads **10 reported / 8 self-authored**~~ → read **10 reported / 8 self-authored** until `8451169` added `check-skill-version-bump`; at HEAD (`b426460`, 2026-09-16) `gates.md:24-28` reads **11 reported / 9 self-authored** again. Read the count from that file, not from this note.
>
> Marked `[x]`: all three of this finding's decisions are resolved — `check-manifests` deleted (`e647f14`), `apm-audit-ci` kept on the grill above, `apm-marketplace-check` removed (`0dffff3`).
@@ -136,14 +142,20 @@ This is the area you named as hardest to understand and slowest. Root cause: mos
8. **`docs/spec/gates.md` (1,048 lines) is roughly 15% "what is enforced" and 85% post-mortems** of defects already fixed and pinned by tests. The 60-line hook table is the useful part. Target 200 lines. The same applies to the 106 comment lines in `.pre-commit-config.yaml` and to `scripts/`, where 8 of 15 files are 40 to 60% comments. Effort M.
> **Partially done (2026-09-13):** see commit `a35f5e8` on `docs/simplification-audit`. The 85%-post-mortem characterization was stale — the file had already shrunk to 966 lines by other findings, and most of what remained is load-bearing "why this design" rationale cited by ADRs and tests, not dead incident narration. Cut only the two genuinely stale passages: a reproduction paragraph carrying explicitly outdated numbers, and a retrofit-process narrative superseded by current state — a 36-line cut, 966 → 930 as measured at commit `a35f5e8`. Those two figures describe that commit only, not the file: `718c79a` and later findings have edited `gates.md` again, so read its current length from the file rather than quoting a number here. `.pre-commit-config.yaml`'s comments were left untouched; on inspection they're compact constraint notes, not filler. Target of 200 lines not reached and not recommended — would require deleting content the file itself flags as load-bearing.
>
> **Closed (2026-09-16, grill): done to the extent recommended.** The `gates.md` cut in `a35f5e8` stands; the 200-line target stays rejected (the file is ~~1,113 lines at HEAD~~ → **1,164** lines at HEAD (`b426460`), `wc -l docs/spec/gates.md`, grown by later findings' sections, and read on demand only). The tests target below is struck: ~~findings 3, 5 and 16 each found dense suites to be named-incident regression coverage~~ → findings 3 and 5 each found dense test suites to be named-incident regression coverage, and finding 16 found the same of dense validator code, whose comments are an incident log. Any future cut to a test suite is its own finding and starts by reading that suite's header.
**Proposed target.** ~~Pre-push 14 hooks to 6: `run-tests`, `validate-plugins`, `validate-marketplace`, `apm-pack-check-clean`, `check-plugin-content-sync`, `check-release-needed`.~~
> **Corrected (2026-09-14):** two of the six named targets no longer exist — `validate-plugins` and `check-plugin-content-sync` were deleted in commit `718c79a` (finding 7, ADR-0024). Actual state today: **9 repo-authored pre-push hooks** — `run-tests`, `check-executables-allow-sync`, `apm-audit-ci`, `check-apm-agents-valid`, `apm-pack-check-clean`, `check-vale-style-sync`, `check-scope-walkup-sync`, `check-release-needed`, `validate-marketplace` — plus the 2 pre-commit `meta` hooks that also run at this stage, so 11 are reported at pre-push. `validate-marketplace` was kept: the root `marketplace:` block in `apm.yml` and the root `.claude-plugin/marketplace.json` stay, because apm's own marketplace consumers read that same catalogue and `<name>@holocron` short names depend on it. (That manifest is the only tracked file under `.claude-plugin/` — `git ls-files .claude-plugin` returns it alone. The sibling `.claude-plugin/plugin.json` is a local `apm pack` byproduct, has never been tracked on any branch, and is ignored at `.gitignore:59`; it was not "kept", because it was never there.)
> **Superseded count (2026-09-15):** finding 14 deleted `check-vale-style-sync` with the merge into `factory-audit` (ADR-0025), so pre-push is now **8 repo-authored hooks** (10 reported). The dated note above is the state on 2026-09-14; see finding 14's note for the correction.
> **Superseded count (2026-09-15):** finding 14 deleted `check-vale-style-sync` with the merge into `factory-audit` (ADR-0025), so pre-push ~~is now~~ → was **8 repo-authored hooks** (10 reported). The dated note above is the state on 2026-09-14; see finding 14's note for the correction.
>
> > **Superseded count (2026-09-16, at HEAD `b426460`):** `8451169` added `check-skill-version-bump` (finding 33), so pre-push is **9 repo-authored hooks** (11 reported) — the eight above plus that one. Measured with `grep -c "stages: \[pre-push\]" .pre-commit-config.yaml`.
>
> > **Superseded count (2026-09-16, at `4de5b6b`):** `4de5b6b` removed `check-release-needed` (finding 36), so pre-push is **8 repo-authored hooks** (10 reported) — the nine above minus that one. Measured the same way.
Pre-commit stays roughly as is minus `skill-frontmatter`, and minus `check-ast` once finding 9 removes the only `.py` files. ~~Tests 26 files to about 10 (12,400 to about 5,000 lines).~~ Keep bats and its three submodules; the 351 bats tests ship inside plugins and are the right tool there. Do not port the bash suites to bats; delete them instead.
Pre-commit stays roughly as is minus `skill-frontmatter`, and minus `check-ast` once finding 9 removes the only `.py` files. ~~Tests 26 files to about 10 (12,400 to about 5,000 lines).~~ Keep bats and its three submodules; the 351 bats tests ship inside plugins and are the right tool there. ~~Do not port the bash suites to bats; delete them instead.~~ **Struck (2026-09-16, grill):** see finding 8's closing note — the suites are regression coverage (findings 3 and 5; finding 16 found the same of the validators they test).
> **Re-measured (2026-09-14, at `a6434e0`):** the tests target was stated against the 2026-09-10 baseline and both its numbers are stale. `tests/` now holds **20 `test-*.sh` suites totalling 9,123 lines** (plus the two runners, 490). Six suites have gone since the baseline: `test-check-manifests.sh` (`e647f14`), `test-skill-frontmatter.sh` (`c8a7c9e`), `test-governance-layer.sh` and `test-instructions-and-docs.sh` (`5f9f2b3`), `test-sync-marketplace-mirror.sh` (`0dffff3`), `test-sync-plugin-content.sh` (`718c79a`). Restated on the same basis the target is **20 files to about 10, 9,123 to about 5,000 lines** — the file half of the target is now the closer half, and finding 9's `check-ast` clause is moot anyway, since finding 9 is not proceeding.
@@ -172,6 +184,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
> Also breaks: `check-scope-walkup-sync` loses one of four walk-up ports (the hook exists because three scripts drifted); `tests/test-adr0020-contract.sh` loses its parser byte-identity assertion; `tests/test-check-scope-walkup-sync.sh` must re-base its fixture; ADR-0010 is superseded outright and ADR-0009/0016 need amending (`field-inventory.md`'s allowlist data line carries `source_keys`). `LESSONS.md:73` records this validator as the **only** thing that catches a skill authored outside `skill-author` — a failure that "recurred twice in one session" — so "git blame + a README URL do the same job" is false for the one thing the chain demonstrably catches. Side effect: 55 reference files have frontmatter containing *only* `source_keys:`, leaving empty `---\n---` blocks to delete.
>
> **Effort L, not M** (about ~~6,393~~ → **7,136** lines deleted across 242 files: the ~~4,641~~ → **5,380** validator and bats lines plus the ~~1,752~~ → **1,756** of `sources.md` measured above, across 196 `source_keys` carriers and ~~46~~ → **45** `sources.md` files. An earlier revision of this note said ~4,600 lines across ~230 files, which was internally inconsistent — 4,600 is validator-plus-bats only and silently drops the `sources.md` this same note measures, and ~230 inherited a carrier count of 172 that missed every `metadata:`-nested file.) Smaller alternative worth considering: scope the drop to the skill half only (~~1,217 lines, 1,198-line validator, 82 tests~~ → **1,207 lines of skill `sources.md`, the 1,145-line `lib-provenance-skill.sh`, 87 tests**, re-measured 2026-09-16 at HEAD) and leave the ADR-0010 plugin-root half alone — no ADR supersession needed.
>
> **Decision (2026-09-16):** Not proceeding — the human declined this finding. The provenance chain (`sources.md`, `source_keys:`, `validate-provenance.sh`) stays, and `research` keeps producing it. This also answers §8's provenance question.
12. [x] ~~**Strip ADR and changelog narration from model-facing files.** `ADR-0020` is cited in 3 of 7 kyberforge SKILL.md files and 16 references; ADR-0023 is cited inline 21 times in the git plugin. Examples: "was the old house rule and ADR-0020 deleted it", "were removed per ADR-0015 once issue #90 landed", "this file previously recorded `list_issues` as having neither a `type` nor a `milestones` parameter". `skill-author/references/retrofit.md` (197 lines) is a one-time migration guide; it is loaded from `improve.md` and listed in `sources.md`, so remove those in the same change. These belong in git history or the ADR, not in context. Effort S.~~
> **Done (2026-09-12):** see commit `edcc57c` on `docs/simplification-audit`. Historical narration stripped from kyberforge (ADR-0020) and git (ADR-0023) skill content; `retrofit.md` deleted along with its load-step and `sources.md` entries. Caught in review: some `ADR-0023` tags were not narration but the `check-rtk-prefix` hook's required opt-out marker for intentionally-bare git commands — those 12 were restored, not left stripped.
@@ -189,6 +203,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
> - **The §8 blocker was a non-issue.** The design question held open there — whether one `description` could carry both skills' trigger sets without breaching the ADR-0020 ceiling — was answered against the **400-character FAIL**, which the merged description clears. Read the number from the shipped file, not from a draft. *(Corrected later on 2026-09-15.)* The shipped description is **241 characters**, inside the 250-character SUGGESTION target, and `bash scripts/skill-size-check.sh plugins/kyberforge/.apm/skills/factory-audit/SKILL.md` prints nothing for it. A first cut shipped at **319** and accepted the SUGGESTION as the cost of carrying both artifact types' trigger phrases. That reasoning was wrong. The quoted phrases (`audit this skill`, `review my SKILL.md`, `audit this agent`, `review my agent file`) restated the "skill directory or agent definition audited" trigger in a second register, which ADR-0020 makes a FAIL. Removing them, and keeping both boundary arrows, gives 241. An earlier "241" in this document and ADR-0025's "240" came from a hypothetical single-arrow draft that was never reproduced. That today's figure is also 241 is a coincidence, not a confirmation of it. The real ceiling was the other one: a single body covering both artifact types ran past the **900-word body FAIL**. Solved the way ADR-0020 prescribes — a dispatch body that routes to per-type references, with the 16 per-type reference files namespaced `skill-*` and `agent-*` (plus the shared `sources.md`).
>
> **Finding 18 was deliberately kept out of scope.** Its re-scoped remainder is prose trimming inside these same files and would have made the merge diff unreviewable; it stays open against `factory-audit`'s files.
>
> > **Since closed (2026-09-16, grill):** finding 18 is no longer open — it closed as not proceeding; see its own closing note.
15. **Merge `skill-author` + `agent-author` likewise.** `contract.md` shares most of its Description section; `new-skill.sh` and `new-agent.sh` implement the same package-root walk-up with different mode names; step 1 dispatch tables and step 3 gates are near-identical. Keep the agent scope logic (plugin vs project/user) as its own reference. Effort M.
@@ -210,6 +226,7 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
> Where the savings actually are: **merge skill-audit + agent-audit (finding 14) → ~~−1,587 lines~~ → landed 2026-09-15 at −2,934 lines, zero coverage loss.** A second option — sourcing the resolver into `scripts/skill-size-check.sh` rather than embedding it (−1,061) — is technically possible but couples the root hook to plugin layout and dismantles the byte-identity contract test's design; needs a decision, not an assumption.
17. **Fold `forge` and `apm-install`.** `forge` is a four-row routing table plus 207 lines of references explaining fork vs inline; it should be 25 lines with no references. `apm-install` (53 lines + 17-line sources) becomes a sixth dispatch row in `apm-workflow`. Effort S.
> **Decision (2026-09-16):** Not proceeding — the human declined this finding. `forge` and `apm-install` stay as separate skills.
18. **Delete prose the model already knows.** "Valid characters: lowercase letters, numbers, hyphens"; what pipx does and PEP 668; "code blocks carry a language tag"; "data to stdout, diagnostics to stderr". Ironically `body-discipline.md` instructs auditors not to include "concepts the agent already knows". Effort S.
> **Re-scoped and folded into finding 22 (2026-09-14).** All four named examples were located, and they are four different classes of thing — only one is what the finding describes:
@@ -224,11 +241,13 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
> Three exemptions agreed, which is what re-scopes the finding:
>
> - **Audit criteria are exempt.** ~~`body-discipline.md:14`~~ → `factory-audit/references/skill-body-discipline.md:14` frames the rule as "Would the agent get this wrong without this instruction?" — an auditor *would*, because the criterion is what it reports against. Cutting criteria is a redesign of what ~~`skill-audit`~~ → `factory-audit` checks, which belongs with finding 14.
> > **Repointed (2026-09-16, at HEAD):** ADR-0025's merge both moved the directory and renamed the file (`references/formatting-and-scripts.md` → `references/skill-formatting-and-scripts.md`), so the two citations above were doubly stale. Line 19 and line 39 still land on the two criteria named, and `skill-body-discipline.md:14` still carries the core test — verified with `sed -n`. Per finding 14's note, finding 18 **stays open against `factory-audit`'s files**, so these are repointed, not struck.
> > **Repointed (2026-09-16, at HEAD):** ADR-0025's merge both moved the directory and renamed the file (`references/formatting-and-scripts.md` → `references/skill-formatting-and-scripts.md`), so the two citations above were doubly stale. Line 19 and line 39 still land on the two criteria named, and `skill-body-discipline.md:14` still carries the core test — verified with `sed -n`. Per finding 14's note, finding 18 **stays open against `factory-audit`'s files**, so these are repointed, not struck. *(Since closed, 2026-09-16 grill — see the closing note at the end of this finding. The repointed citations stay as the record.)*
> - **`assets/templates/` is exempt.** Scaffold output, not context.
> - **Sourced restatement of a spec this repo's own artifacts are built to is exempt.** `skill-author/references/scripts.md` carries `source_keys: agentskills-using-scripts` and deliberately restates the agentskills.io spec — the contract every skill here is written against, so the restatement governs this repo's artifacts and has to be in front of the author. **`source_keys:` alone is not the test**, and cannot be: `conventional-commits-spec.md` and `bisect.md` both carry it too, and finding 20 recommends reducing both to a pointer plus the house delta. The decidable line is what the content governs — a spec this repo's artifacts must satisfy (agentskills.io) is exempt; documentation of an external tool the model already has (Conventional Commits, `git bisect`) is not. Grounding, stated honestly: findings 9 and 26 closed as "Keep — vendored upstream content is intentional", but both closed over the `docs/research/` and `docs/notes/` *directories*, not over skill `references/*.md`; extending them to `scripts.md` is this note's inference, not a recorded decision. (An earlier revision added "finding 11 re-decides this content's status anyway" — withdrawn: finding 11 proposes dropping the provenance *metadata and validators*, not the sourced prose.)
>
> What remains is unsourced explanatory prose in skill bodies and non-criteria references — roughly **30–60 lines across kyberforge**, where `apm-install/SKILL.md` yields about one clause. Too small to stand alone, and the same class of writing as finding 22 with a larger surface and no sourced-content conflict. **Merged into finding 22 under these exemptions; not a separate work item.** Safety note established while scoping: `validate-provenance.sh` is not a pre-push gate (the only `.pre-commit-config.yaml` reference is `check-scope-walkup-sync`, over the walk-up port) and validates `sources.md` structure, never line-level traceability — so trimming sourced prose trips no gate provided frontmatter and `sources.md` are left intact. **Loose end in the fold, stated so it is not lost:** finding 22's total is computed over five `bin` skills (1,018 lines) and its implementation sizing names two agents, neither touching kyberforge — so these 30–60 lines sit outside the scope finding 22 states. Track them there as a separate line item with its own estimate; they are not covered by "bin: strip generic process theatre" as written.
>
> **Closed (2026-09-16, grill): not proceeding.** Finding 22 is deferred with `bin`, and the folded kyberforge remainder (30–60 thin lines) is too small to stand alone, as the note above already says. It would also bring no skill under budget — a judgment made at the grill, not a figure the note above states.
### 4.3 git and gitea (153 + 93 files, 9,889 + 6,047 lines incl. mirror; source 3,288 + 2,286)
@@ -251,6 +270,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
> Blast radius if ever revisited: **68 backticked references to git skill names, 99 to gitea names** under `plugins/`. Only the ones in a `SKILL.md` are boundary targets `skill-size-check` resolves and FAILs on if dangling — its `files:` regex is `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$`, so it opens nothing else — and that is **35 of the 68 git mentions and 26 of the 99 gitea ones**. The remaining 33 and 73 live in `references/*.md`, the two orchestrate agents (10 and 23 on their own), the plugin READMEs, kyberforge's audit and author references, and two `validate.sh` copies — none of which this gate opens: they still have to be rewritten by hand, but they fail no hook. Plus `AGENTS.md:16,18`, `README.md:21-22`, `CONTEXT.md:168` (uses `gitea-prs` as the naming exemplar), `architecture.md:34` (uses `git-branches` vs `gitea-branches` as the canonical boundary example), and ADRs 0011, 0020, 0021, 0022, 0023. Two false alarms not worth chasing: `tests/test-check-rtk-prefix.sh:97` reads from a pinned historical SHA, and `scripts/check-rtk-prefix.sh`'s mention is in a comment.
>
> **Salvageable independently, ~230 lines:** `conventional-commits-spec.md` (~98% generic) and `bisect.md` (~97%) are the only two files where the generic-restatement thesis fully holds — reduce each to a pointer plus the house delta. Also worth a finding-13-style trim-in-place: the issue-vs-PR disambiguation duplicated across 4 gitea files. Neither needs a merge.
>
> **Closed (2026-09-16, grill): not proceeding, salvage included.** Both files are loaded on demand only (`git-commits/SKILL.md:46`, `git-history/SKILL.md:33`), and exactly when the agent needs the precise rules; replacing them with a URL pointer trades a local, deterministic answer for a network fetch mid-commit. `bisect.md` is not ~97% generic — every command carries the `rtk` prefix (ADR-0023). `conventional-commits-spec.md` carries the commitlint 11-type set the repo's `conventional-pre-commit` hook enforces. The gitea issue-vs-PR duplication is forced by the no-cross-skill-sharing rule (§9). The saving would be repo lines, not context tokens, at the price of a version bump per file.
21. [x] ~~**Delete `config.example.json` / `.claude/plugins/git/config.json`.** Read by two steps, written by nothing. Default to GitHub Flow with the existing `develop` / `release/*` inference. Effort S.~~
> **Done (2026-09-12):** see commit `f5e4d0d`. Deleted `plugins/git/config.example.json` (the runtime `.claude/plugins/git/config.json` was never a tracked file). Removed the config-read step from `git-orchestrate`'s Process and from `git-branches`' Step 1, leaving the existing default-inference logic (GitHub Flow, with Gitflow inferred from a `develop`/`release/*` branch) as the sole path; updated `git-workflow`'s description of the orchestrator's behaviour to match. Dropped the now-dangling `applied_config` field from `git-orchestrate`'s output shape and the `config.example.json` example from `docs/spec/architecture.md`.
@@ -271,8 +292,11 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
> **Coupling — the important caution.** This corpus has already been through two trim passes, and the last one broke two of these five targets the same way. PR #129 (`598a7c3`) records: *"`prototype` and `vale-config` deleted rules outright that survived nowhere."* This finding proposes redoing that operation on `prototype` and `diagnose`. **Recommend dropping `prototype` and `grill-with-docs` from scope entirely** — both pass every gate and both have prior-regression history. Also: deleting a `references/*.md` named in a body is a hard ERROR (`skill-size-check.sh:1265`), so `tdd`'s reference deletions and its SKILL.md relinks must land in one commit; `improve-codebase-architecture/SKILL.md:77,79` hard-name `grill-with-docs`'s `context-format.md` and `adr-format.md` by path, so neither can be renamed; and shrinking `write-docs` falsifies live comments at `skill-size-check.sh:64,596`, `tests/test-adr0020-targets.sh:592` and `architecture.md:82`. Unlike finding 23's `caveman`/`zoom-out`, **none of these five is cited as a convention exemplar** anywhere.
>
> Sizing if implemented: two parallelizable agents over disjoint files — A on `write-docs` (self-contained, no references), B on `tdd` (reference deletion + same-commit relink, ERROR-gated, cannot be split). `diagnose` is ~15 lines, too small for its own agent.
>
> **Deferred (2026-09-16, grill).** The human is excluding `bin` from this audit: its skills are "binned for a reason" and will be fixed or relocated as a separate piece of work. Nothing in this finding is executed here; the itemised ~165-line estimate above is the starting point for that work. `bin` is still covered by the version-bump gate (finding 33) until then.
23. **bin: merge `grill-me` into `grill-with-docs`.** `grill-me` is 16 lines and a subset of the docs flow; `grill-with-docs` creates `CONTEXT.md` when missing, so the merged skill needs a no-write opt-out. `caveman` (50 lines) and `zoom-out` (9) are hand-invoked prompts rather than workflow skills; they are also the repo's `disable-model-invocation` exemplars in `CONTEXT.md`, `contract.md`, ADR-0020, ADR-0021, and `gates.md`, and `install.sh` has no path for `~/.claude/commands/`, so moving them means picking a new exemplar. `improve-codebase-architecture` defines its glossary twice (inline and in `language.md`; the README documents the split as intentional). Effort S.
> **Decision (2026-09-16):** Not proceeding — the human declined this finding. `grill-me` and `grill-with-docs` stay separate.
24. **core: `provider-adapter-author` is a 1,200-line wrapper around one instruction** ("replace duplicated lines with `@AGENTS.md`, keep provider-specific lines"): a 496-line validator with a 519-line bats suite for a check that is a grep. `agentsmd-author` already calls `agentsmd-audit` as mandatory closeout, and both route to `provider-adapter-author` in boundary clauses that must change with it. Target: one `agentsmd` skill with an audit mode, adapter conversion as a step, validator about 40 lines. Needs an ADR-0012 revisit. Effort L.
> **Refuted (2026-09-14, at HEAD `062ca47`). Not deferred — the target fails the repo's own gate before any judgment call is reached, so the ADR-0012 §8 question is moot for this finding.**
@@ -288,6 +312,7 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
> Two further blockers if it were ever revisited: the merge dissolves `agentsmd-author`'s standing prohibition *"Never write to a provider file yourself, in any circumstance"* (SKILL.md:21), a hazard `c59e4bf` closed after the validator's own size-FAIL remediation text "actively invited the prohibited edit"; and `skill-size-check.sh:121` + `tests/test-skill-size-check.sh:729` both cite `a8cd5e8`'s exit-2 split as precedent for their own, so deleting it orphans two live cross-references.
25. **lint: delete the `lint-runner` agent.** Its body is "call `vale-run`, reformat output", which `--output=JSON` already gives; it exists for backends that do not exist. It is the example boundary clause in three `agent-author` templates and ADR-0016, so those need a new example. About 40% of `vale-config` is install tables and settings lists the model can fetch from vale.sh. Keep the house-verified matrices (`E100`/`E201`, `Packages` below glob, frontmatter, ignore paths). `lint/docs/research/docs/vale/` overlaps the skill's own references by about two thirds. Effort S.
> **Decision (2026-09-16):** Not proceeding — the human declined this finding. The `lint-runner` agent stays.
## 5. Prose and docs (9,600 lines, 109,000 words outside plugins)
@@ -310,6 +335,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
> **Honest ceiling: 168 words / 17.7%** of the file's 949 — the cross-reference scaffolding only: preamble **43** (lines 3–5), non-governance block **77** (71–76), footer **48** (79–82), landing at ~67 lines with no rule loss. (An earlier revision said 47 for the preamble, which is only reachable by counting lines 1–7 — that sweeps in the `#` glyph and a `---` rule as words.) Plus 133 words from the constitution. Not 50 lines, not 20%.
>
> **Two defects the finding missed, both worth fixing independently of it.** (1) **A live bug: `docs/HUMANS.md` does not exist** — the file is `docs/wiki/HUMANS.md`. The wrong path appears **five times across three files**, including the **deployed** `core/instructions/governance.md:82`, which is self-inconsistent (line 73 correct, line 82 broken); the other four are `CONTROLS.md:5,101,106` and `ai-constitution.md:238`. (An earlier revision said "four times" while enumerating all five.) **Fixed (2026-09-15):** all five corrected to `docs/wiki/HUMANS.md`; the deployed copy under `~/.claude/` is now stale until `scripts/install.sh` re-runs. (2) The deployed always-on file carries **repo-relative pointers that dangle in every project but this one** — an agent told to "read it when making decisions not covered here" cannot. That is the substantive question this finding should have asked. The footer is additionally self-referential: `governance.md:80` lists the file as compatible with itself.
>
> **Decision (2026-09-16):** Not proceeding — the human declined this finding, including the 168-word cross-reference trim. `core/instructions/governance.md` and the other three governance documents stay as they are. The `docs/HUMANS.md` path defect was fixed separately (see §10).
28. **ADRs: 2,740 lines, 72% in eight ADRs over 150 lines.** ADR-0020 is 513 lines with a 71-line measurement log as Context; ADR-0017 has 173 lines of amendments against 45 of decision. ADR-0001 is superseded and ADR-0006 moot, both keeping full text below the banner. ADR-0002 is three lines. Truncate superseded ones to the banner, fold amendments into the decision, cap Context at 20 lines, add a 25-line `docs/adr/README.md` index with status. The rules already live in `gates.md`; the ADRs need only decision and consequences. Effort M.
> **Moved backwards (measured 2026-09-14 over `afa7187^`..`a6434e0`):** today's ADR-0024 work did the opposite of this finding on every axis, and that is recorded here so it is a known trade rather than a surprise. `docs/adr/` went from **23 files / 2,748 lines** to **24 / 3,084** — one new ADR (0024, 259 lines) plus amendment and banner text across **eleven existing ADRs** (0001, 0006, 0011, 0013, 0014, 0015, 0017, 0018, 0019, 0020, 0021 — 87 lines added, 10 removed, net **+77**), for a total of net **+336 lines (+12%)**. The two ADRs this finding names for truncation both grew *below* their banners instead: **ADR-0001 26 → 27** lines and **ADR-0006 22 → 27**, each gaining a fresh "as of ADR-0024" paragraph rather than losing the historical body beneath it. ADR-0017 gained a supersession banner while keeping its four amendments in full — the exact shape this finding proposes to fold.
@@ -322,7 +349,9 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
>
> Today those same figures read: **ten** ADRs exceed 150 lines, not eight; top-eight share is 68.3%, the over-150 cohort 79.2%. ADR-0020 is **514** lines. Its Context is **72** lines counting the `## Context` heading and **71** without — a counting convention, not drift: the section is byte-identical at `a3e721e` and at HEAD (`## Context` at :11 through `## Decision` at :83), so the finding's 71 and this note's 72 are the same span counted two ways. ADR-0017's "173 amendment lines against 45 of decision" and ADR-0002's three lines are exact.
>
> **"The rules already live in `gates.md`" is backwards.** `docs/spec/gates.md:349-352` explicitly *declines* to restate ADR-0020's numbers: *"they live in ADR-0020's Consequences section… Quoting them here would just create a second copy to go stale."* gates.md is a consumer of the ADR, not its replacement. **The index proposal also contradicts a recorded decision** — `docs/spec/architecture.md:90`: *"There is no index file — the directory holds numbered ADRs whose filenames state their decision, so `ls docs/adr/` is the index."*
> **"The rules already live in `gates.md`" is backwards.** ~~`docs/spec/gates.md:349-352`~~ → `docs/spec/gates.md:397-400` explicitly *declines* to restate ADR-0020's numbers: *"they live in ADR-0020's Consequences section… Quoting them here would just create a second copy to go stale."* gates.md is a consumer of the ADR, not its replacement. **The index proposal also contradicts a recorded decision** — `docs/spec/architecture.md:90`: *"There is no index file — the directory holds numbered ADRs whose filenames state their decision, so `ls docs/adr/` is the index."*
>
> > **Repointed (2026-09-16, at HEAD `b426460`):** the quoted `gates.md` passage moved from `:349-352` to `:397-400` as later sections were added above it; verified with `grep -n "Quoting them here" docs/spec/gates.md` and `sed -n 397,400p`. `architecture.md:90` still resolves.
>
> **No superseded body can be truncated — every one is quoted by content, not merely cited by number.** ADR-0001's body text is quoted verbatim at `docs/adr/0015:5,36`, and `factory-integration-decisions.md:133` lists "Pull-based distribution (ADR-0001)" as settled, a concept living only in its consequences bullets. ADR-0006's version-parity invariant is stated only at `0006:23` and is relied on by `0014:116` and `0024:183-185` — and its banner (17 lines) is already longer than its body (7). ADR-0017's own banner says its diagnosis "is still accurate about how Claude Code's installer works", and ADR-0024 cites its body in eight places. ADR-0002 is only partially superseded and is cited as a design source by a shipped skill.
>
@@ -333,6 +362,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
> **Honest ceiling ~235 lines (7.5%)**, and the one real win is not in the finding: **delete ADR-0017's four amendments (−173) now that ADR-0024 consequence 7 has restated them in full**, re-pointing eight citations. Plus ADR-0001/0006 compressed to banner-plus-one-line (−20) and ADR-0020's anecdotes (−42). No README index. Restate the headline as **79% in ten ADRs**.
>
> **The framing question this finding never notices:** it proposes reversing a convention the repo *just* re-affirmed — every banner added by the ADR-0024 work ends with some form of "kept below as the historical record". Is a superseded ADR's body a record or dead weight? Nothing here is mechanical; every proposed cut touches text another file quotes.
>
> **Closed (2026-09-16, grill): not proceeding.** Decision: a superseded or accepted ADR's text is the historical record (the Nygard convention, and what every ADR-0024 banner already says), so no body is truncated, no amendment is deleted, and ADR-0020's Context is left intact. That removes every remaining cut — ADR-0017's amendments are part of its record, and ADR-0024 consequence 7 (`0024:213-252`, ~40 lines) summarises them rather than restating them in full as the note above says. Corrected headline for anyone quoting it: ~~**79% of `docs/adr/` lines sit in ten ADRs over 150 lines** (measured 2026-09-14)~~ → **80% of `docs/adr/` lines (2,983 of 3,709, across 25 files) sit in eleven ADRs over 150 lines** (at HEAD `b426460`, 2026-09-16, from `wc -l docs/adr/*.md`; ADR-0025 joined the cohort and the ADR-0019 and ADR-0022 amendments grew the directory). The 79%-in-ten figure was the 2026-09-14 state; the 2,740 figure is the `a3e721e` state only.
29. [x] ~~**The same facts are stated in full three or four times.** "Edit `.apm/`, never the mirror": README (2 paragraphs), AGENTS.md (2 paragraphs), architecture.md (2 paragraphs plus the lost-README anecdote), ADR-0017. The apm.lock / SessionStart story: README (11 lines), AGENTS.md, ADR-0018, ADR-0019, gates.md. The offline `SKIP=` command and the three-stage install each appear three times. Rule: README has the how-to, AGENTS.md has one-line rules with links, architecture.md has mechanics. Effort S.~~
> **Corrected then partially done (2026-09-14):** independent re-verification found the "edit `.apm/`, never the mirror" and apm.lock/SessionStart clusters confirmed but the third overstated — no file documents an offline `SKIP=` command (the one `SKIP=`-adjacent mention in `gates.md` explicitly says a *different* opt-out "is not `SKIP=`"), and "three-stage install" appears twice, not three times, with no restatement worth trimming. Trimmed the two confirmed clusters: README's "Editing plugin content" and AGENTS.md's "Edit `.apm/`, never the flat mirror" sections cut to the how-to/one-line-plus-link split the finding itself proposed, full mechanics (the `rm -rf` behavior and the `plugins/kyberforge/hooks/README.md` anecdote) staying solely in `docs/spec/architecture.md`. README's "Keeping the install current" and AGENTS.md's apm.lock bullet trimmed to drop the restated `apm outdated`/`apm update --yes` timing narrative, pointing to ADR-0019 as the canonical mechanism instead. No test greps the trimmed wording (checked).
@@ -357,23 +388,25 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
Not covered by the area audits above; found on a final sweep of the root config and install pipeline. The install pipeline itself (`scripts/install.sh` 55 lines, `deploy-manifest.sh` 24, statusline 109) is fine and needs nothing.
33. **Every plugin version lives in four places (five for kyberforge), plus one per skill.** `plugins/<name>/apm.yml`, two generated `plugin.json` files, the root `apm.yml` packages list, the `executables.allow` key (`kyberforge#1.6.2`), and a `metadata.version` in all ~~39~~ → **38** SKILL.md files (ADR-0022) that nothing consumes and that drifts freely (gitea skills sit at five different values). Repo tags (`v2.0.1`) follow a third scheme that the declared `tagPattern: v{version}` can never match under `per_package` versioning. ADR-0006, ADR-0022, `check-executables-allow-sync`, `skill-frontmatter`, and `apm pack --check-versions` all exist to police this. Proposal: one version per plugin in its `apm.yml`; drop `metadata.version` and ADR-0022; let `apm pack` derive the rest. Effort M.
> **Partially advanced (2026-09-14):** see commit `718c79a` on `docs/simplification-audit`. Two of the four locations per plugin are gone: the twelve generated `plugin.json` manifests (`plugins/*/.claude-plugin/` and `plugins/*/.github/plugin/`) were deleted with the mirror. ADR-0006 needed no action — it was already moot and governed only those two now-deleted manifests, so no version bumps were required by the change. **Not closed.** Still outstanding: `plugins/<name>/apm.yml`, the root `apm.yml` packages list, the `executables.allow` pin, and `metadata.version` in all ~~39~~ → **38** SKILL.md files (still unconsumed, still drifting), plus ADR-0022 and the `v{version}` `tagPattern` mismatch.
> **Partially advanced (2026-09-14):** see commit `718c79a` on `docs/simplification-audit`. Two of the four locations per plugin are gone: the twelve generated `plugin.json` manifests (`plugins/*/.claude-plugin/` and `plugins/*/.github/plugin/`) were deleted with the mirror. ADR-0006 needed no action — it was already moot and governed only those two now-deleted manifests, so no version bumps were required by the change. **Not closed.** Still outstanding: `plugins/<name>/apm.yml`, the root `apm.yml` packages list, the `executables.allow` pin, and `metadata.version` in all ~~39~~ → **38** SKILL.md files (still unconsumed, still drifting), plus ADR-0022 and the `v{version}` `tagPattern` mismatch. *(Since closed, 2026-09-16 grill — see the decision note at the end of this finding.)*
> **Verified (2026-09-14, at HEAD `062ca47`): headline wrong, central claim inverted — and it contains the one zero-risk, empirically-verified win in this audit.**
>
> **Do this regardless of anything else: delete the six root `apm.yml` `packages[].version` lines.** Tested in an isolated scratch copy (repo untouched): setting `plugins/lint/apm.yml` to `9.9.9` while root says `1.1.7` **passes `apm pack --check-versions --check-clean` with exit 0**, reports `[matches]`, and emits `1.1.7` — the curator entry wins (`output_mappers.py:163-171`). Deleting the root `version:` line entirely leaves `marketplace.json` **byte-unchanged** (`builder._fetch_local_metadata` reads the plugin's own `apm.yml`). All six are removable with zero output diff. This is unpoliced duplication that silently ships the wrong number on drift. Effort S, no decision needed.
>
> Corrected headline: **two** hand-maintained per-plugin locations (**three** for kyberforge), not four — the audit's own "already done" note records the `plugin.json` deletion but never fixed the headline. Gitea skills drift across **six** values (`0.1.2, 0.1.3, 0.1.4, 0.1.5, 0.1.6, 1.0.1`), not five — still six at HEAD on 2026-09-16, re-derived by parsing `metadata.version` out of each `plugins/gitea/.apm/skills/*/SKILL.md`. ~~39 `SKILL.md` files ✓~~ → **38** carry it, and all 38 do (re-measured 2026-09-16; ADR-0025's merge took one). The `0.4.6` duplication between root `version:` and `marketplace.version:` is **forced by apm, not a repo choice** — deleting `marketplace.version` makes `--check-clean` go dirty.
> Corrected headline: ~~**two** hand-maintained per-plugin locations (**three** for kyberforge)~~ → **one** hand-maintained per-plugin version location, `plugins/<name>/apm.yml` (**two** for kyberforge, adding the `executables.allow` key), not four. `2def060` deleted the root `packages[].version` lines (corrected 2026-09-16, review round). The root `packages[].description:` duplicates dropped in the same round are a separate duplication, not a version location, so they do not change this count — the audit's own "already done" note records the `plugin.json` deletion but never fixed the headline. Gitea skills drift across **six** values (`0.1.2, 0.1.3, 0.1.4, 0.1.5, 0.1.6, 1.0.1`), not five — ~~still six at HEAD on 2026-09-16~~ → **five** again at HEAD (`b426460`) on 2026-09-16 (`0.1.2, 0.1.4, 0.1.5, 0.1.6, 1.0.1`), because `8451169` bumped `gitea-branches` 0.1.3 → 0.1.4 under the new version-bump gate and it was the only skill at 0.1.3; re-derived by parsing `metadata.version` out of each `plugins/gitea/.apm/skills/*/SKILL.md` with PyYAML. ~~39 `SKILL.md` files ✓~~ → **38** carry it, and all 38 do (re-measured 2026-09-16; ADR-0025's merge took one). The `0.4.6` duplication between root `version:` and `marketplace.version:` is **forced by apm, not a repo choice** — deleting `marketplace.version` makes `--check-clean` go dirty.
>
> **"Nothing consumes `metadata.version`" is false twice over.** Machine enforcers: ~~`scripts/skill-size-check.sh:1365-1374`~~ → `scripts/skill-size-check.sh:1370-1379` and ~~`skill-audit/scripts/validate.sh:1292-1332`~~ → `plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-checks-skill.sh:235-277`, both FAIL tier, the latter citing ADR-0022 by name, with four dedicated bats cases and ~10 fixture generators baking the field in.
> Instruction-level consumers: `skill-author/SKILL.md:60` (bump minor on create, patch on improve), `create.md:89,101`, `improve.md:82`, and `forge/SKILL.md:54` + `references/version-bump.md`. apm parses it for Chatmode/Instruction/Context primitives but not for Skills, and never emits it. Precise statement: the value is written, shape-validated, and never read *downstream* — it is an agent-visible revision counter, and the drift table shows the counter is not being maintained.
>
> > **Repointed (2026-09-16, at HEAD):** `skill-audit/scripts/validate.sh` no longer exists — ADR-0025's merge moved the ADR-0022 check into `factory-audit`'s skill-side check library, where it is the `SEMVER_RE` block (comment header at `:235`, `fail()` calls at `:261` and `:275`). `skill-size-check.sh` grew by 5 lines above the block since `062ca47`, hence the shifted range there. All five instruction-level citations still resolve at HEAD, verified with `sed -n`.
>
> **ADR-0022 already considered and rejected dropping the field**, on the grounds that `skill-author` depends on it to decide whether a pass owes a bump — a rationale still live today. Superseding costs: rewrite skill-author's bump rule, delete `forge`'s version-bump route premise, strip two scripts, delete four bats cases, fix ~10 fixture generators, edit the scaffold template, update `gates.md:97` — and re-open the "is this field present here?" question issue #127 closed, just from the other side. **Recommendation: keep it and fix the actual defect, which is that nobody bumps it.** Either enforce the bump in the skill-author workflow or declare the values advisory in the ADR.
> **ADR-0022 already considered and rejected dropping the field**, on the grounds that `skill-author` depends on it to decide whether a pass owes a bump — a rationale still live today. Superseding costs: rewrite skill-author's bump rule, delete `forge`'s version-bump route premise, strip two scripts, delete four bats cases, fix ~10 fixture generators, edit the scaffold template, update ~~`gates.md:97`~~ → `gates.md:145` — and re-open the "is this field present here?" question issue #127 closed, just from the other side. *(Repointed 2026-09-16, at HEAD `b426460`: the `metadata.version` frontmatter sentence formerly at `gates.md:97` is now at `:143-146`, the field itself on `:145`; verified with `grep -n "metadata.version" docs/spec/gates.md`.)* **Recommendation: keep it and fix the actual defect, which is that nobody bumps it.** Either enforce the bump in the skill-author workflow or declare the values advisory in the ADR.
>
> **The `tagPattern` claim is refuted — inert, not broken.** Under `versioning.strategy: per_package`, apm never reads it: `version_check.py:262` gates on `strategy == "tag_pattern"`, and `builder.py:641,781` are reachable only for *remote* source entries, while all six packages here are local paths. The `v1.0.0`/`v2.0.0`/`v2.0.1` tags are not "a third scheme" — they are the `.pre-commit-hooks.yaml` external-consumer contract tags from finding 36, a different axis entirely. Latent risk only: if `dependencies.apm` ever gains `ref:` pins, tagPattern goes live against per-package tags that do not exist.
>
> Also: **`executables.allow` should be kept** — it is version-keyed by apm's design and `check-executables-allow-sync` guards a real silent failure (ADR-0019). And ADR-0006's ADR-0024 amendment asserting *"`apm.yml`'s `version:` is the only version field a plugin has"* is inaccurate while root `packages[].version` exists — fixed by the deletion above.
>
> **Decided and done (2026-09-16, grill): enforce the bump.** Advisory status and dropping the field were both rejected. The six root `apm.yml` `packages[].version` lines are deleted (`apm pack --check-versions --check-clean` still passes, output unchanged), which also makes ADR-0006's "the only version field a plugin has" true. `scripts/check-skill-version-bump.sh` now runs at pre-push: any skill directory that changed against its merge-base with `main`, `tests/` excluded, must carry a strictly higher `metadata.version` than ~~`main`~~ → the same skill had at that merge-base (not `main`'s current tip; see `gates.md:90` and `:111-112`, and the hook comment at `.pre-commit-config.yaml:203-206`. *Amended in the 2026-09-16 review round: the gate now also compares against the `origin/main` tip; see §11.*); new, renamed and deleted skills are exempt; every plugin is covered, `bin` included. Recorded as a dated section in ADR-0022, not a new ADR. The 17 skills changed on this branch without a bump took a patch bump in the same commit. The `executables.allow` pin and the inert `tagPattern` are left as the note above recommends.
34. **The SessionStart hook auto-updates the install on every startup.** `check-apm-current.sh` runs `apm outdated` (network, 60 s timeout) and then `apm update --yes` (300 s timeout) at every session start, rewriting `apm.lock.yaml`. That is why the lock file is dirty at the start of this session and why `AGENTS.md` has to explain "commit or discard it deliberately". It is a 60-line script with a 368-line test, an ADR (0019), the `executables.allow` pin, and a sync hook behind it. For a repo that is its own source, the update belongs in `install.sh` or a manual `apm update`, not in session startup. Effort S to remove; the design question is whether auto-update at startup is wanted at all.
> **Refuted (2026-09-14, at HEAD `062ca47`). The evidence is inverted: the finding cites as proof of over-eagerness a session in which the mechanism did not fire, and the observed state is the exact silent failure ADR-0019 exists to prevent.**
@@ -389,6 +422,8 @@ Not covered by the area audits above; found on a final sweep of the root config
> **Recommendation: keep the hook.** Cost is 0.8 s on a current install; offline it fails fast (0.81 s, status `unknown`, grep misses, exit 0 — the 60 s timeout is a bound, not a latency). The benefit guards a failure that is silent by construction and that the repo is exhibiting right now.
>
> **Two things worth fixing, neither of which is removal.** (1) ADR-0019's ~10.4 s refresh figure is now **~18 s** measured warm on a LAN remote — it is quoted in the `timeout: 380` invariant reasoning and understates by 75%. (2) **An undocumented branch hazard, and the strongest argument the finding could have made:** the hook resolves against the remote *default* branch, so on a feature branch that changes `plugins/`, an auto-refresh reinstalls `main`'s version over it. Reproduced — running `apm update` today re-installs `main`'s `plugins/bin/.mcp.json` and writes back the obsidian MCP server that commit `c96ca9c` removed on this branch. That deserves a line in ADR-0019's Consequences; the proportionate fix if it bites is ~3 lines skipping the refresh when `HEAD` is not the default branch.
>
> **Decided and done (2026-09-16, grill): document, do not skip.** Both follow-ups landed in ADR-0019. The refresh figure is re-measured at ~24–26 s (two runs, six packages behind), not the ~18 s above — still inside the 360 s the `timeout: 380` invariant covers. The branch hazard is a new Consequences paragraph, written after it fired in this very session: the startup refresh redeployed `main`'s `skill-audit`/`agent-audit` and the `obsidian` server over this branch. Skipping the refresh off the default branch was rejected — it would freeze the session on an older `main` without making the branch live.
35. [x] ~~**Outputs and packages for consumers that do not exist.** The `codex` output profile generates `.agents/plugins/marketplace.json` (95 lines) although Codex is not a supported consumer. The `mattpocock-skills` remote package entry is the only reason `apm-marketplace-check` needs the network, and its pin is advanced by hand (ADR-0015). The `.github/plugin/marketplace.json` mirror is a legacy path (finding 2). Removing all three leaves one generated marketplace manifest (the per-plugin `plugin.json` pairs remain) and no network-dependent hook. Effort S.~~
> **Done (2026-09-13):** see commit `568ca74` on `docs/simplification-audit`. Removed the `codex` output profile from root `apm.yml` and its compiled `.agents/plugins/marketplace.json` (95 lines), and the `mattpocock-skills` remote package entry — the only remote marketplace entry, so `apm-marketplace-check` and `apm-pack-check-clean` no longer need network access at all. Updated `README.md`, `AGENTS.md`, `docs/spec/gates.md`, and `docs/spec/architecture.md` accordingly; added one-line superseded/updated notes to ADR-0015 and ADR-0021. Left `.github/plugin/marketplace.json` untouched — that's the Copilot legacy-path question in finding 2/§8, out of scope here; only re-ran the sync script to keep it consistent. `apm.lock.yaml` unaffected (`marketplace.packages[]` isn't part of the lockfile). Verified via `apm install`, `apm pack --marketplace=claude --check-versions`, and all four affected pre-push hooks.
@@ -397,7 +432,7 @@ Not covered by the area audits above; found on a final sweep of the root config
> - **"The per-plugin `plugin.json` pairs remain"** is void. All twelve were deleted in `718c79a` (ADR-0024); `git ls-files '*plugin.json'` returns nothing. The only tracked manifest left anywhere is the root `.claude-plugin/marketplace.json`. (The root `.claude-plugin/plugin.json` beside it is untracked local `apm pack` output, ignored at `.gitignore:59`.)
> - **"Left `.github/plugin/marketplace.json` untouched … out of scope here"** is void the same day: `0dffff3` deleted it under finding 2c, along with `scripts/sync-marketplace-mirror.sh` and its test. The "only re-ran the sync script to keep it consistent" step above refers to `sync-plugin-content.sh`, itself deleted in `718c79a`.
36. **The release-tag mechanism guards an external contract with no known consumer.** `.pre-commit-hooks.yaml` exports three hooks for other repos to pin by `rev: <tag>`. `check-release-needed` (242 lines + 442 test), `test-vale-hooks-consumer` (270 lines), ADR-0014, and three tags exist to serve that. If no other repo pins these hooks today, the whole mechanism can be deferred until one does. Effort S.
36. [x] ~~**The release-tag mechanism guards an external contract with no known consumer.** `.pre-commit-hooks.yaml` exports three hooks for other repos to pin by `rev: <tag>`. `check-release-needed` (242 lines + 442 test), `test-vale-hooks-consumer` (270 lines), ADR-0014, and three tags exist to serve that. If no other repo pins these hooks today, the whole mechanism can be deferred until one does. Effort S.~~
> **Verified (2026-09-14, at HEAD `062ca47`): premise holds — the only premise in this audit to survive verification, though not the finding whole: `test-vale-hooks-consumer.sh` is 272 lines, not 270. Not yet decided; deferred by the human on 2026-09-14.**
>
> Exact: three exported hooks (`kyberforge-vale-audit-skill`, `kyberforge-vale-audit-agent`, `kyberforge-skill-size-check`), `check-release-needed.sh` 242, its test 442, three tags (`v1.0.0`, `v2.0.0`, `v2.0.1`). `test-vale-hooks-consumer.sh` is **272** lines, not 270.
@@ -408,11 +443,15 @@ Not covered by the area audits above; found on a final sweep of the root config
>
> **The mechanism is already failing at its one job.** `scripts/skill-size-check.sh` changed on `origin/main` in `598a7c3` after `v2.0.1`, with no tag cut since — a consumer pinning `rev: v2.0.1` gets a stale hook today. The gate cannot fire: it is wholly gated on `PRE_COMMIT_REMOTE_BRANCH == refs/heads/main`, and PRs merge through Gitea's server-side button, which sets nothing. The script's own header documents this as needing "a server-side CI job, which this repo does not have yet".
>
> **The premise that it serves only the external contract holds** — all three exported hooks are *separately* wired internally via `repo: local` (`.pre-commit-config.yaml:216,249,258`), so deleting the export costs **zero** internal lint coverage.
> **The premise that it serves only the external contract holds** — all three exported hooks are *separately* wired internally via `repo: local` (~~`.pre-commit-config.yaml:216,249,258`~~ → `.pre-commit-config.yaml:221,254,269`, the three `entry:` lines), so deleting the export costs **zero** internal lint coverage.
>
> **Correction to the finding: ADR-0014 gets amended, not retired.** Its primary decision — moving Vale config/styles/wrapper into `skill-audit/assets/vale/` and `agent-audit/assets/vale/`, self-locating from `${BASH_SOURCE[0]}` so the prefilter works at *runtime* in any repo installing kyberforge — is independent of the release-tag mechanism and stands on its own. Only the `.pre-commit-hooks.yaml` half and the tag consequence retire.
>
> Removal is ~1,000 lines and mechanical: `.pre-commit-hooks.yaml`, `check-release-needed.sh`, both tests, the hook block at `.pre-commit-config.yaml:194-201`, the `gates.md:83` row and its "External consumers" section. Tags are inert and can stay. **The one real loss:** `test-vale-hooks-consumer.sh` is the sole test exercising the entry-resolution path that once shipped broken — it goes only *with* the manifest, never while it stays. Reversal cost is bounded provided ADR-0014 and `LESSONS.md:101,105` are kept: they preserve the `entry[0]`-only constraint that took three review rounds to find.
> Removal is ~1,000 lines and mechanical: `.pre-commit-hooks.yaml`, `check-release-needed.sh`, both tests, the hook block at ~~`.pre-commit-config.yaml:194-201`~~ → `.pre-commit-config.yaml:186-193`, the ~~`gates.md:83`~~ → `gates.md:96` row and its "External consumers" section (`gates.md:789`). Tags are inert and can stay. **The one real loss:** `test-vale-hooks-consumer.sh` is the sole test exercising the entry-resolution path that once shipped broken — it goes only *with* the manifest, never while it stays. Reversal cost is bounded provided ADR-0014 and `LESSONS.md:101,105` are kept: they preserve the `entry[0]`-only constraint that took three review rounds to find.
>
> > **Repointed (2026-09-16, at HEAD `b426460`):** the line citations in this note were taken at `062ca47` and have shifted. The `check-release-needed` block is now `.pre-commit-config.yaml:186-193` (`grep -n "id: check-release-needed"`); the three internal `repo: local` wirings' `entry:` lines are `:221` (`skill-size-check`), `:254` and `:269` (the two `vale-audit-prefilter-*` hooks, both now on `factory-audit`'s one `vale-wrap.sh`); the `check-release-needed` table row is `gates.md:96` and the "External consumers" section heading is `gates.md:789`. Verified with `grep -n` and `sed -n`. Line counts in this note were not re-measured.
>
> **Decided and done (2026-09-16):** see commit `4de5b6b` on `docs/simplification-audit`. The human took the deferred decision: remove the mechanism. Deleted `.pre-commit-hooks.yaml` (31 lines), `scripts/check-release-needed.sh` (242), `tests/test-check-release-needed.sh` (449 at HEAD, not the 442 above) and `tests/test-vale-hooks-consumer.sh` (276 at HEAD, not 272), plus the `check-release-needed` hook block, for **1,443 lines removed and 234 added** across 20 files. ADR-0014 is **amended, not retired**, as the note above says: its runtime bundling decision stands, and the amendment records why the export went and keeps the `entry[0]`-only constraint (`LESSONS.md:101,105`) in case it returns. ADR-0025 gets a pointer to that amendment. Tags are left in place. **One cost the finding did not count:** `tests/test-vale-wrap.sh` case 33, the cross-manifest `files:` drift check, and case 28's hook-scope half both read the published manifest and went with it. Case 33's one guard that did not need a second manifest, a local regex narrowed to one plugin, is now a third property of case 32, with its own mutation test, so that coverage is kept. `gates.md` now counts 8 authored pre-push hooks (10 reported), no longer 9 (11).
37. [x] ~~**Two `.mcp.json` files declare an Obsidian vault server over `docs/`** (root and `plugins/bin/`; the other five plugin `.mcp.json` files are empty stubs), while `AGENTS.md` forbids using an external memory system for this repo. If the Obsidian tools are unused, drop both and the `reinject_mcp_servers` explanation in the bin README; the bin `plugin.json` pair regenerates. Effort S.~~
> **Not proceeding (2026-09-13):** premise doesn't hold. The server exposes the repo's own git-tracked `docs/` folder — not an external/off-repo store — so it isn't the "external memory system" AGENTS.md's rule targets. It was deliberately added and versioned (3 commits), is documented as current intended behavior in both READMEs, and ADR-0018 uses it as its only concrete worked example of apm's MCP-dependency propagation mechanism actually working. No skill invokes the Obsidian tools as a workflow step, but that alone doesn't make the config dead. No changes made; recommend a human confirm whether the vault tooling is still wanted before removing it.
@@ -426,10 +465,14 @@ Not covered by the area audits above; found on a final sweep of the root config
## 7. Suggested order
1. Quick wins, all S, no design decisions needed: findings 9, 10, 26, 30, 31, 29, 12, 13, 1, 6, 4, 35, 37, 38, and the mirror-sync and executables-allow halves of 2. Removes roughly 25,000 to 30,000 lines and 6 hooks.
2. Structural changes that need a short discussion: ~~14~~, 15, ~~19~~, 20, 23, 25, 17, ~~3~~, 5, ~~7~~, 33, ~~34~~, 36.
3. The real complexity: ~~16 (validators)~~, 11 (provenance), ~~24 (core)~~, 8 and 28 (gates.md and ADRs).
2. Structural changes that need a short discussion: ~~14~~, 15, ~~19~~, ~~20~~, ~~23~~, ~~25~~, ~~17~~, ~~3~~, ~~5~~, ~~7~~, ~~33~~, ~~34~~, ~~36~~.
3. The real complexity: ~~16 (validators)~~, ~~11 (provenance)~~, ~~24 (core)~~, ~~8 and 28 (gates.md and ADRs)~~.
> **Re-derived (2026-09-16, at HEAD):** this ordering was written before the findings were worked, and seven of its entries are now closed. Struck above: **14** landed (`467bbd7`, ADR-0025); **7** was superseded then done (`718c79a`); **3** and **19** are not proceeding on refuted premises; **34**, **16** and **24** are refuted outright. **5** is left standing but is downstream of 16 by its own note, so it cannot be taken in this bucket's order. Read each finding's own marker, not this list — it is a plan of record, not a status board. Bucket 1 is left as written: every entry in it is marked `[x]` or carries a decision note at its own finding.
> **Status (2026-09-16, after the grill on 33, 28, 22/18, 20, 8, 34):** open findings were **15** (merge `skill-author` + `agent-author`) and **36** (release-tag mechanism, decision deferred by the human). **22** is deferred with the rest of `bin`. Every other finding is done, closed, or refuted at its own note.
>
> **Updated (2026-09-16, later):** **36** is decided and done (`4de5b6b`), so **15** is the only open finding. **22** stays deferred with `bin`.
> **Re-derived (2026-09-16, at HEAD):** this ordering was written before the findings were worked, and ~~seven of its entries are now closed~~ → ~~all but two~~ → all but one of its bucket-2 and bucket-3 entries are now closed (corrected later on 2026-09-16, after the grill, and again once **36** closed). Struck above: **14** landed (`467bbd7`, ADR-0025); **7** was superseded then done (`718c79a`); **3**, **5** and **19** are not proceeding on refuted premises; ~~**34**,~~ **16** and **24** are refuted outright; **34** was refuted as a removal and then decided and done as documentation of the branch hazard in ADR-0019 (`afcf477`), with the hook kept; **33** was decided and done (enforce the bump, `8451169`); **8**, **20** and **28** closed at the grill; **17**, **23** and **25** were declined by the human; **11** was declined by the human; **36** was done (`4de5b6b`). ~~**5** is left standing but is downstream of 16 by its own note, so it cannot be taken in this bucket's order.~~ **5** is closed with 16: its own note says it is downstream of 16, and 16 is refuted. Still open, per the Status note above: **15** alone, now that **36** is done (`4de5b6b`); **22** is deferred with `bin`. Read each finding's own marker, not this list — it is a plan of record, not a status board. Bucket 1 is left as written: every entry in it is marked `[x]` or carries a decision note at its own finding.
Findings 9, 10, 11, and 12 are coupled through the provenance validator and the audit criteria; land them together or the audit gates start reporting the removals.
@@ -445,17 +488,26 @@ Findings 9, 10, 11, and 12 are coupled through the provenance validator and the
> **The mechanism is host-independent, which is why this bullet had to change.** ADR-0024 consequence 1 and §9's first residual both state it for Claude Code: a native registration succeeds and installs six plugins containing zero skills, silently. Nothing in that chain is Claude-specific. The catalogue is a list of plugin *roots*; discovery of content inside a root is a convention-scan of flat `skills/`/`agents/`/`hooks/` directories, and that is the layout `718c79a` deleted. Whichever of the four paths a host resolves the catalogue through, it lands on the same empty roots. Copilot was in fact always the weaker case — ADR-0017's own `hooks` amendment records that the mirror only ever partially served it.
>
> **The delete still holds**, for a stronger reason than the one given: the file was a preferred discovery path to content that no longer exists. What changed is the accepted cost — this is no longer "preference lost", it is the same accepted silent-empty-install residual §9 records, now known to apply to Copilot as well.
- **Provenance chain.** Is "which upstream informed this file" a requirement you still want, or was it a governance experiment? Finding 11 hinges on this.
- [x] ~~**Provenance chain.** Is "which upstream informed this file" a requirement you still want, or was it a governance experiment? Finding 11 hinges on this.~~
> **Sharpened (2026-09-14):** still open, but ask it of the **producer** first. `plugins/bin/.apm/skills/research/` specifies the `sources.md` + `source_keys:` format and three evals in `plugins/bin/evals/research/research/eval.yaml` assert it. If `research` keeps emitting the chain, finding 11 collapses to "delete the validators" and the metadata stays. See finding 11's verification note.
- **ADR-0012 (three core skills) and the one-script-per-skill install constraint.** ~~The merges in 14, 15, and 24 need the first revisited and are the only way around the second.~~ **Corrected (2026-09-14):** this grouping was wrong, and finding 2b's note has said so since `0dffff3` while this bullet said the opposite. ADR-0012 governs only the `core` plugin's three `agentsmd-*` skills (`agentsmd-author`, `agentsmd-audit`, `provider-adapter-author`) — read it: it names those three and nothing else. **Only finding 24 touches them, so only finding 24 needs ADR-0012 revisited.** Findings 14 and 15 merge kyberforge's `skill-audit`/`agent-audit` and `skill-author`/`agent-author`, which ADR-0012 does not govern; what constrains them is the self-containment rule, and merging is the way *around* it rather than a reason to reverse anything. That rule survives ADR-0024 — see §9's negative result and ADR-0024 consequence 6, which also correct its source: it is the agentskills.io spec for APM package mode, not a property of Claude Code's plugin cache-install as finding 2b's note assumed. The open question for 14/15 is a design one — one `description` carrying both skills' trigger phrases — not an ADR supersession. Are you open to superseding ADR-0012, for finding 24?
>
> **Answered (2026-09-16):** keep it. The human declined finding 11; the chain and its validators stay, and `research` keeps producing it.
- [x] ~~**ADR-0012 (three core skills) and the one-script-per-skill install constraint.**~~ ~~The merges in 14, 15, and 24 need the first revisited and are the only way around the second.~~ **Corrected (2026-09-14):** this grouping was wrong, and finding 2b's note has said so since `0dffff3` while this bullet said the opposite. ADR-0012 governs only the `core` plugin's three `agentsmd-*` skills (`agentsmd-author`, `agentsmd-audit`, `provider-adapter-author`) — read it: it names those three and nothing else. **Only finding 24 touches them, so only finding 24 needs ADR-0012 revisited.** Findings 14 and 15 merge kyberforge's `skill-audit`/`agent-audit` and `skill-author`/`agent-author`, which ADR-0012 does not govern; what constrains them is the self-containment rule, and merging is the way *around* it rather than a reason to reverse anything. That rule survives ADR-0024 — see §9's negative result and ADR-0024 consequence 6, which also correct its source: it is the agentskills.io spec for APM package mode, not a property of Claude Code's plugin cache-install as finding 2b's note assumed. The open question for 14/15 is a design one — one `description` carrying both skills' trigger phrases — not an ADR supersession. ~~Are you open to superseding ADR-0012, for finding 24?~~
> **Closed on the 14/15 half (2026-09-16, at HEAD):** finding 14 landed as `factory-audit` on 2026-09-15 (`467bbd7`, ADR-0025), and the design question this bullet holds open was answered by doing it — the merged description ships at 241 characters, inside the 250 SUGGESTION target, and the binding ceiling turned out to be the 900-word **body**, solved with a dispatch body over `skill-*`/`agent-*` reference files. See finding 14's own note. What remains open here is finding 15 (`skill-author` + `agent-author`) and the ADR-0012 question below, which the next note already answers.
> **Moot (2026-09-14):** finding 24 is refuted on arithmetic before this question is reached — the three `core` bodies total 1,360 words against `BODY_MAX_WORDS=900`, and their descriptions 806 chars against a 400 cap. Nothing needs superseding because the merge it would unblock cannot be committed. Question closed unless finding 24 is rewritten.
- **Granularity of git/gitea skills.** One `git` skill vs seven trades routing precision for size. Is one broad description acceptable?
>
> **Closed (2026-09-16):** both halves are settled — finding 14 landed and finding 24 is refuted. The only finding left under this bullet is 15, which needs no ADR-0012 revisit (see above); its remaining question is the design one this bullet already names.
- [x] ~~**Granularity of git/gitea skills.** One `git` skill vs seven trades routing precision for size. Is one broad description acceptable?~~
> **Answered by measurement (2026-09-14): no, and it is not a preference question.** A merged git description measures **1,950 chars against a 400-char FAIL ceiling (4.9×)** and a 3,381-word body against 900 (3.8×). Both proposed gitea halves also FAIL at 2.5×, and the gitea split additionally puts a hard boundary through the edit-a-file-then-open-a-PR workflow. (An earlier revision also called the gitea split "blocked by ADR-0011, which already rejected a *smaller* bundling" — withdrawn; ADR-0011's objection is to a boundary being crossed, not to bundle size. See finding 20's verification note.)
- **Auto-update at session start.** Do you want the install refreshed from the remote every time a session opens (finding 34), or is a manual `apm update` acceptable?
>
> **Answered (2026-09-16):** keep the seven-and-seven granularity. Finding 20 closed at the grill as not proceeding, salvage included; see its closing note.
- [x] ~~**Auto-update at session start.** Do you want the install refreshed from the remote every time a session opens (finding 34), or is a manual `apm update` acceptable?~~
> **Recommendation on evidence (2026-09-14): keep it; finding 34 refuted.** The premise that it runs on every startup is false (the update is conditional on a real SHA check), the lock was not dirty, the hook did not fire this session, and the install is currently **9 commits behind `main` with nothing reporting it** — the failure the hook exists to prevent. `install.sh`, the proposed alternative host, has no apm step. Still formally the human's call, but the factual basis for removing it does not survive. See finding 34.
- **External hook consumers.** Does any other repo pin this repo's `.pre-commit-hooks.yaml` by tag today? If not, finding 36 defers the release mechanism entirely.
>
> **Decided (2026-09-16, grill):** keep the hook and document the feature-branch hazard; skipping the refresh off the default branch was rejected. Landed in ADR-0019 (`afcf477`). See finding 34's closing note.
- ~~**External hook consumers.** Does any other repo pin this repo's `.pre-commit-hooks.yaml` by tag today? If not, finding 36 defers the release mechanism entirely.~~
> **Evidence gathered, decision deferred (2026-09-14).** No consumer found: the Gitea instance holds two repos, and the other pins seven hook repos, none of them this one. No consumer-driven commit in the 13 touching the mechanism. Off-instance clones undeterminable — but ADR-0024 accepted exactly this standard when it deleted the mirror. The mechanism is additionally **already broken** (a consumer pinning `rev: v2.0.1` gets a stale `skill-size-check.sh`, and the guard cannot fire through Gitea's merge button). The human deferred the decision on 2026-09-14; the finding is ready to execute when it is taken. See finding 36.
> **Answered (2026-09-16):** no consumer, and the human took the decision: the mechanism is removed (`4de5b6b`), and ADR-0014 is amended to record why. See finding 36.
- [x] ~~**Obsidian MCP.** Are the Obsidian tools over `docs/` used by anyone? If not, finding 37 is a pure delete.~~
> **Answered (2026-09-14):** not used — remove entirely. All seven `.mcp.json` files are deleted and the repo-root path is gitignored; see finding 37, which also records the functional regression this uncovered (since `718c79a` deleted the per-plugin manifests, apm no longer propagated the server to consumers at all).
@@ -501,6 +553,8 @@ That base rate made "effort S, no decisions needed" an unreliable signal, and §
| 24 | Refuted — arithmetically impossible (1,360w vs a 900 cap) | 0 |
| 34 | Refuted — evidence inverted | 0 |
> **Dispositions since the wave (2026-09-16):** this table records the verdicts at `062ca47` and is left as written. Of the ten, none is still open: **36** was decided and done later the same day (`4de5b6b`), and before that it was the only open one. **33** and **34** are decided and done — the bump is enforced (`8451169`) and the feature-branch hazard is documented in ADR-0019 (`afcf477`), with the hook kept. **20** and **28** closed at the grill as not proceeding; **11** and **27** were declined by the human; **22** is deferred with `bin`. See each finding's own closing note.
**One premise of ten survived — finding 36's — but not the finding whole: its supporting figure was wrong (`test-vale-hooks-consumer.sh` is 272 lines, not 270). The other nine premises failed.**
**The headline figure was wrong in at most eight of the ten, not all ten.** Two exceptions, stated so the claim is not overstated:
@@ -526,3 +580,17 @@ The recurring failure mode is worth naming, because it has now produced six wron
- **~~A live bug in always-on context.~~ Fixed (2026-09-15).** The deployed `core/instructions/governance.md` cited `docs/HUMANS.md`, which does not exist — the file is `docs/wiki/HUMANS.md`. Five occurrences across three files (`governance.md:82`, which was self-inconsistent against its own correct line 73; `CONTROLS.md:5,101,106`; `ai-constitution.md:238`), in a file `@`-imported into every session in every project. All five now point at `docs/wiki/HUMANS.md`. Note the deployed copy under `~/.claude/` no longer matches the repo until `scripts/install.sh` re-runs.
- **This checkout's install is stale and there is a branch hazard.** At the time of the wave `apm outdated` reported 6 outdated dependencies, 9 commits behind `main`, with a clean tree and nothing reporting it. **Do not run `apm update` on this branch** — it resolves against `main` and restores the obsidian MCP server that commit `c96ca9c` removed here. Reproduced. The mechanism is worse than "reinstalls `plugins/bin/.mcp.json`": apm never writes into `plugins/`, it re-materialises the file under `apm_modules/` and regenerates the repo-root `/.mcp.json` — which `c96ca9c` gitignored, so the restoration would not appear in `git status` at all. This belongs in ADR-0019's Consequences; see finding 34.
> **Landed (2026-09-16):** commit `afcf477` amended ADR-0019's Consequences with the feature-branch hazard, and the discard guidance for a feature branch is now also in `AGENTS.md` and `README.md` (`dd0b923`). The reason those two files and ADR-0019 gave for discarding the lock was wrong, and the review round below corrected it. See finding 34's closing note.
## 11. Review round on the grill commits (2026-09-16)
A review of this branch's grill commits (`8451169` through `b426460`) raised the findings below. Each was decided in this round and fixed on this branch. Several agents made the fixes in parallel. This note records the decisions. It does not re-verify the details of fixes it did not make itself: for those, read the named file or ADR.
- **Version-bump gate baseline.** The gate now also compares against the `origin/main` tip, as well as the merge-base. ADR-0022 is amended to match. This supersedes the merge-base-only wording in finding 33's closing note.
- **Version-bump gate parsing.** Frontmatter that starts with a BOM, and version components with leading zeros, are now parsed correctly.
- **Version-bump gate tests.** The gaps the review found in `tests/test-skill-version-bump.sh` are closed.
- **Plugin patch bumps.** Five plugins took a patch bump: `bin`, `git`, `gitea`, `core` and `lint`. kyberforge was already at `2.0.0` on this branch and needed no further bump.
- **Root `apm.yml` descriptions.** The `packages[].description:` duplicates are dropped, following the `version:` lines `2def060` already removed. ADR-0021 is amended. This is not a version location, so finding 33's corrected count (one per plugin, two for kyberforge) is unaffected.
- **Remote-entry `version:` guidance.** The `apm-workflow` references now give the right guidance on a remote marketplace entry's `version:`.
- **Lock-file discard reasoning.** `README.md`, `AGENTS.md` and ADR-0019's 2026-09-16 amendment used to say to discard the refreshed lock on a feature branch "because it records `main`'s commit, not the branch's". That was wrong: the branch's committed lock records a `main` commit too, just an older one. In this checkout it is `b7bec71`, which `git branch -r --contains` finds on `origin/main`. All three now give the real reasons. Discarding keeps unrelated lock churn out of the branch diff, and it keeps the deployed tree consistent with the lock that `apm pack --check-clean` reads. They also state the cost: the session runs the older `main` until the next session start refreshes again. The SessionStart notice in `check-apm-current.sh` now gives branch-specific advice, and `tests/test-apm-current-hook.sh` pins it. ADR-0019 had two claims that were checked against apm's source. `apm pack` "refuses to run": precisely, it raises a build error before the `--check-clean` gate is reached, and only when a file the lock lists is missing on disk (`bundle/packer.py`, `pack_bundle`). apm "removes a server on its next update": this holds, and it holds for `apm install` as well (`install/mcp/integration.py`, `MCPIntegrator.remove_stale`). The amendment now says both precisely.
- **ADR-0022 amendment.** The amendment's placement and the validator names it cites are fixed.

12
apm.yml
View File

@@ -75,37 +75,25 @@ marketplace:
packages:
- name: kyberforge
description: Skills and agents for creating, maintaining, and managing a Claude Code / Copilot CLI plugin marketplace.
source: ./plugins/kyberforge
version: 2.0.0
category: Developer Tools
- name: bin
description: Skills for everyday AI-assisted development work that is not tied to a single tool, forge or language, and has not yet been split into a focused plugin.
source: ./plugins/bin
version: 1.1.7
category: Utilities
- name: git
description: Skills and agents for working with a local Git clone over the git wire protocol, and for authoring and running the pre-commit hooks that guard it.
source: ./plugins/git
version: 1.3.7
category: Version Control
- name: gitea
description: Skills and agents for working with a Gitea forge through its HTTP API — the forge's own objects, as distinct from the local git clone.
source: ./plugins/gitea
version: 1.3.8
category: Version Control
- name: core
description: Skills for authoring and auditing a repo's AGENTS.md and the provider adapter files that defer to it.
source: ./plugins/core
version: 1.1.2
category: Productivity
- name: lint
description: Skills and agents for configuring and running linters.
source: ./plugins/lint
version: 1.1.7
category: Developer Tools

View File

@@ -27,6 +27,16 @@ them by name — and the argument-free `entry:` contract is untouched. Read the
sync-check paragraph, and the `tests/test-vale-wrap.sh` Consequences bullet below as the state this
ADR established, not as current layout.
**Amended (2026-09-16): the `.pre-commit-hooks.yaml` export and its release tags are retired.**
The runtime half of this ADR — Vale config, styles and wrapper bundled inside the skill (now
`factory-audit`), self-located from `${BASH_SOURCE[0]}` — stands. The external git-hook/CI half does
not: the manifest, `check-release-needed` and the tag-cutting consequence are gone. See
[the amendment at the end of this file](#amendment-2026-09-16-the-external-hook-contract-is-retired)
before reading any paragraph above or below that names `.pre-commit-hooks.yaml`, a `rev:` tag,
`check-release-needed`, case 33, or the two exported hook IDs as current. That includes the
ADR-0025 amendment directly above: case 33 is deleted (its one-plugin narrowing guard is now a
property of case 32), and no external consumer pins the exported hook IDs any more.
`skill-audit`/`agent-audit`'s Step 1 called
`"$(git rev-parse --show-toplevel)/scripts/vale-wrap.sh" --config "$(git rev-parse --show-toplevel)/.vale.ini"`
— which resolves to whichever repo the skill happens to be running in. Inside `ai-development`
@@ -219,3 +229,51 @@ only when the original span is already two or more lines, so the pad count stays
`tests/test-vale-wrap.sh` case 20 asserts an apostrophe-bearing token actually fires on a flattened
description in all three apostrophe-carrying branches, and case 20b pins the pad arithmetic against
a body line's true line number.
## Amendment (2026-09-16): the external hook contract is retired
Root `.pre-commit-hooks.yaml`, `scripts/check-release-needed.sh`,
`tests/test-check-release-needed.sh` and `tests/test-vale-hooks-consumer.sh` are deleted, and the
`check-release-needed` pre-push hook is removed from `.pre-commit-config.yaml`. The three exported
hook IDs — `kyberforge-vale-audit-skill`, `kyberforge-vale-audit-agent` and
`kyberforge-skill-size-check` — no longer exist, and no new `vX.Y.Z` tag is cut when hook files
change. (Simplification audit finding 36.)
Three reasons, any one of which would have been enough to ask the question:
- **No consumer was found.** The Gitea instance holds two repos. The other one pins seven hook
repos, and none of them is this one. None of the 13 commits that touched the mechanism came from a
consumer report; all were found by this repo's own tests. Clones outside the instance cannot be
counted, but ADR-0024 accepted the same standard when it deleted the mirror.
- **The mechanism was already failing at its one job.** `scripts/skill-size-check.sh` changed on
`main` after `v2.0.1`, and no tag was cut, so a consumer pinning `rev: v2.0.1` already ran a
stale hook. The gate could not have caught it. It acted only when pre-commit reported a push to
`refs/heads/main`, and PRs here merge through Gitea's server-side merge button, which runs no
local hook. The script's own header said that closing the gap needed a server-side CI job the
repo does not have.
- **The README already contradicted it.** Its "For external consumers" section says apm is the only
supported install path and never mentions `.pre-commit-hooks.yaml` or `rev:` pinning.
**What is unaffected.** This repo's `repo: local` hooks — `skill-size-check`,
`vale-audit-prefilter-skill` and `vale-audit-prefilter-agent` — were always wired separately from
the export, so no internal lint coverage is lost. The two prefilter hook IDs stay separate for the
file-scope reason in "One hook per file-scope" above, not for an external contract.
`tests/test-vale-wrap.sh` case 33, the cross-manifest `files:` drift check that ADR-0025 ported,
went with the manifest it compared against. Its one guard that did not need a second manifest, a
local regex narrowed to a single plugin, is now a third property of case 32. The `v1.0.0`, `v2.0.0`
and `v2.0.1` tags are left in place. They are inert: nothing reads them, and apm's `per_package`
versioning never consults `tagPattern`.
**What is preserved for a return.** The `entry[0]`-only constraint in the Consequences above, and
its incident records at `LESSONS.md:101` and `:105`, stay as written. That constraint says a
published entry is a bare script path, with every bundled file located from `${BASH_SOURCE[0]}`,
and it took three review rounds to find. Both hook scripts still meet it: `vale-wrap.sh` takes no
`--config`, and `skill-size-check.sh` keeps its embedded resolver copy. If a consumer appears,
restore the manifest under that constraint, and restore `test-vale-hooks-consumer.sh` with it: it
was the only test that exercised the entry-resolution path that once shipped broken. Restore a
release gate only once a server-side job can run it on merge.
**Superseded statements elsewhere.** ADR-0022's notes that the version-bump gate "is not exported
through `.pre-commit-hooks.yaml`" and that it shares its gaps with `check-release-needed`, and
ADR-0025's point 5 ("Both exported Vale hook IDs survive unchanged") and its case-33 port, describe
the state before this amendment.

View File

@@ -124,6 +124,10 @@ was below the 360 s the script can legitimately take. A test asserts the invaria
literal — it parses every `timeout N` out of the script, sums them, and requires the `hooks.json`
value to be larger — so changing either side without the other fails the suite.
> **Amendment (2026-09-16) — the refresh is slower than first measured, still inside the budget.**
> Re-measured: ~24–26 s for the same six-behind refresh, warm, on a LAN remote — well inside the
> 380 s above.
**Reading a human-readable CLI for a control decision cost a silent failure, again.** `apm outdated`
has no `--json` or other machine-readable flag (confirmed against 0.28.0), so the hook must match
its prose. The first attempt matched `outdated dependencies found` — plural only. apm emits
@@ -145,6 +149,37 @@ to plural-only fails it.
deploy until this change is merged and `apm update` has run once against the new default branch.
Until then the repo has the mechanism in source and not in effect.
> **Amendment (2026-09-16) — on a feature branch, the refresh installs `main`, not the branch.**
> Recorded after it happened. The dependencies resolve against the remote default branch, so a
> session opened on a branch that changes `plugins/` loads `main`'s content, refreshed or not — a
> branch's own `.apm/` edits are live only once they are on the remote's `main`. Content the
> branch *removes* comes back in the deployed install: on `docs/simplification-audit` a refresh
> redeployed `main`'s `skill-audit` and `agent-audit` over the branch's merged `factory-audit`, and
> re-materialised `main`'s `plugins/bin/.mcp.json` into `apm_modules/`, so the gitignored root
> `.mcp.json` regained the `obsidian` server the branch deleted — invisible to `git status`.
> Skipping the refresh off the default branch was considered and rejected: it would not make the
> branch live, only freeze the session on an older `main` — the silent staleness this ADR exists to
> prevent. The redeployed content goes away once the branch merges. For the server, the next
> `apm update` or `apm install` that resolves a tree no longer declaring it removes it from
> `.mcp.json`: both commands call `MCPIntegrator.remove_stale` for every server listed under the
> lock's `mcp_servers` that no dependency declares any more (`apm_cli/install/mcp/integration.py`).
>
> The rewritten `apm.lock.yaml` is a separate matter. It records `main`'s current tip, but the
> branch's committed lock records a `main` commit too, just an older one, so committing the
> rewrite would not swap the branch for `main`. On a feature branch, discard it anyway
> (`git checkout -- apm.lock.yaml`, then `apm install`), for two reasons. First, it keeps lock
> churn that has nothing to do with the branch out of the branch's diff. Second, it keeps the
> deployed tree consistent with the committed lock that the `apm-pack-check-clean` pre-push hook
> reads. In this repo `apm pack` builds a bundle from the lock before its `--check-clean` gate
> runs, and it stops with a build error ("deployed files are missing on disk -- run 'apm
> install'") when a file the lock lists is absent (`apm_cli/bundle/packer.py`, `pack_bundle`).
> A refresh leaves the tree in that state whenever the newer `main` dropped a file the older lock
> still lists. Under `--dry-run` it checks only that each file exists, not its content hash. The
> cost of discarding is real: the session then runs the older `main` that the lock records, which
> is the staleness the rejected skip would have caused. That cost is accepted on a feature branch,
> and it does not last. At the next session start the hook finds the restored lock behind `main`
> and refreshes again.
**`.claude/settings.json` stops being `{"hooks": {}}`.** apm merges the hook into it and tracks
ownership in a `.claude/apm-hooks.json` sidecar, with the script copied to
`.claude/hooks/<pkg>/`. The sidecar and the script directory are gitignored install output; the

View File

@@ -18,6 +18,9 @@ file, `.claude-plugin/marketplace.json`. Read the "four generated files" in Cont
generated files" in Consequences as historical counts, true when written. The blast radius shrank;
the staleness hazard that motivated this ADR did not.
**Amended 2026-09-16:** the root `marketplace.packages[]` copy of each description was removed; the
package `apm.yml` is now the single source. See the amendment before Consequences.
## Context
A plugin's published description is one string authored twice — in `plugins/<name>/apm.yml` and in
@@ -203,6 +206,32 @@ sit inside ADR-0020's tiers; the tier would have been silent through all three f
string, not a link — and the README's own plugin list carries the same enumeration with the same
staleness, so this relocates the defect rather than fixing it.
## Amendment (2026-09-16): the root copy is removed — the package `apm.yml` is the single source
The Decision's rule that "the two copies … stay identical" is retired by removing the second copy.
The six `description:` lines under root `apm.yml`'s `marketplace.packages[]` are deleted, the same way
`2def060` deleted the six `version:` lines beside them. `plugins/<name>/apm.yml`'s `description:` is
now the only place a package's published description is authored.
The rule's own justification — "the root entry is what reaches the compiled marketplace" — was true
only while the root entry set the field. apm's Claude marketplace mapper resolves a local-path entry's
`description` curator-first: the entry's value wins when present, and when it is absent the value is
read from the package's own `apm.yml` (`apm_cli/marketplace/output_mappers.py`, the `is_local` branch
calling `_apply_field_with_precedence` with `source_label="package apm.yml"`). The root copy was
therefore an override, not a mirror. Nothing enforced the identity rule, and on drift apm silently
published the root value. Removing the copy removes the drift rather than leaving it unchecked.
All six root copies were byte-identical to their package's `apm.yml` when they were removed. After
the removal, `apm pack` regenerated `.claude-plugin/marketplace.json` with every `description` unchanged,
and `apm pack --check-versions --check-clean --dry-run` passes. The consequence for the "unbounded
obligation" in Context is that a description edit is now one edit, not two. The package version bump
and the catalog patch bump it earns are unchanged
(`plugins/kyberforge/.apm/skills/apm-workflow/references/configure.md`).
This applies to local-path (`source: ./…`) entries only. A remote entry has no local package
`apm.yml` to fall back to. Its `description:`, when set, is still the published text, and when it
is absent apm uses whatever its best-effort remote metadata fetch returns.
## Consequences
**Three descriptions are rewritten and the compiled output regenerated.** Eight generated files

View File

@@ -67,6 +67,91 @@ it's asked, not one that varies by plugin domain.
create/improve pass owes a bump, so the 12 skills carrying it are not tracking dead weight — removing
it discards real revision signal for no gain.
## Amendment (2026-09-16): the bump is enforced at push, not only required to exist
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 —
`scripts/skill-size-check.sh` (the pre-commit hook) and `factory-audit`'s
`scripts/lib-checks-skill.sh` — 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.)
`scripts/check-skill-version-bump.sh` now runs as a pre-push hook on every push, whatever the
target branch. It takes its baseline from the merge-base of the pushed commit with `origin/main`
(local `main` if `origin/main` does not resolve). For each skill directory under
`plugins/*/.apm/skills/` that differs between the pushed commit and that merge-base, ignoring
`tests/`, the pushed `metadata.version` must be strictly greater than the version the skill had at
the merge-base — not the version on `main`'s current tip. (The amendment below reverses that last
choice: the pushed version must now also exceed `main`'s tip.)
The baseline is the merge-base, 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.
The rule is "greater", not "exactly one patch higher", so a second `skill-author` pass on the same
branch that bumps again still passes. `tests/` is excluded because 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, and 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`.
The gate fails closed rather than passing when it has no trustworthy baseline: when neither
`origin/main` nor `main` resolves, when the pushed commit shares no merge-base with it, and when
only local `main` resolves and the pushed commit is that merge-base, since a local `main` the
pushed commit already contains is no independent record of what shipped. It also fails closed when
the pushed ref does not resolve to a commit, and when a `SKILL.md` the tree names cannot be read by
`git show` or parsed by `python3` — a read failure is reported as such, never as a missing version.
It reads versions with `python3` and PyYAML and fails with a clear message if either is missing.
Three alternatives were rejected. Declaring the field advisory is the cheapest, but concedes the
field cannot do its job. Dropping the field was rejected by this ADR already, and costs more now.
Checking at commit time against `HEAD` was rejected for the inflation described above.
The 17 unbumped skills took a patch bump in the commit that added the gate. A typo fix in a skill
now costs a version bump; that is the rule working, not noise.
**Amended by ADR-0014 (2026-09-16).** `check-release-needed` is retired, so the comparison below
records the state when this ADR was written, not a hook that still runs. See
[ADR-0014's amendment](0014-vale-prefilter-ships-from-the-plugin.md#amendment-2026-09-16-the-external-hook-contract-is-retired).
The gate differs from `check-release-needed` in when it runs: that hook acts only when pre-commit
reports a push to `main`, so a manual `pre-commit run --hook-stage pre-push` skips it, while this
gate runs there too and checks `HEAD`. The two hooks share both known gaps. A merge made with
Gitea's merge button runs no local hooks, so it is not checked. And pre-commit's pre-push
integration checks only one ref of a multi-ref push (`git push origin a b`, `git push --all`).
In pre-commit 4.6.1, `_pre_push_ns` in `hook_impl.py` skips delete lines and returns on the first
remaining ref whose remote sha is non-zero and present locally; a ref whose remote sha is zero or
unknown locally is returned only if it has commits that no remote-tracking ref of that remote has.
Every later ref is pushed unchecked. When the returned ref's unpushed history reaches a root
commit, pre-commit sets no `PRE_COMMIT_TO_REF` at all, so the gate checks `HEAD`, which is the
pushed ref only if it is checked out.
## Amendment (2026-09-16): the pushed version must also exceed `main`'s tip
This reverses the choice above that the baseline is the merge-base "not the version on `main`'s
current tip". A changed skill's pushed `metadata.version` must now be strictly greater than **both**
its version at the merge-base and its version at the tip of `origin/main` (local `main` under the
same fallback, with the same fail-closed rules).
The merge-base alone lets two branches ship two different changes under one version. Branches A and
B both start from a skill at `1.0.0`, change it differently, and bump it to `1.0.1`. A merges. B's
merge-base is still the `1.0.0` commit, so B passes, and the two `1.0.1` bumps are the same line
change, so git merges B without a conflict. `main` then carries two different `1.0.1` contents, and
the field again fails to answer "did this change since I last read it". Checking against the tip as
well makes B fail until it bumps past `1.0.1`.
- **A skill absent at the tip** (deleted on `main` since the branch started) is held to the
merge-base rule alone. **A skill absent at both** is new and stays exempt.
- **When `main` has not moved since the merge-base**, the two baselines are the same commit and the
skill is checked once.
- **The failure names the baseline it missed**: `(not above merge-base)` or
`(not above origin/main tip)`, one line per baseline missed.
The cost is that a branch behind `main` may have to bump again after another branch lands a bump
on the same skill. That is the case the rule exists for, and rebasing onto or merging `main` first
shows the version to beat. The rule reads `origin/main` as last fetched, so a tip that moved since
the last fetch is not seen until the next one.
## Consequences
27 SKILL.md files gain `metadata.version: "1.0.0"`, and a 28th — `bin/write-docs` — reaches the same

View File

@@ -5,6 +5,11 @@ exact pair, scoped itself to them, and then deferred the work as issue #101. The
here. `skill-author` and `agent-author` stay separate — ADR-0020 excluded the author pair
deliberately, and nothing in this change touches that exclusion.
**Amended by ADR-0014 (2026-09-16).** The published `.pre-commit-hooks.yaml` is retired. Point 5
below (both exported hook IDs survive) and the case 33 port no longer describe the repo. Case 33 is
deleted, and its one-plugin narrowing guard is now a property of case 32. See
[ADR-0014's amendment](0014-vale-prefilter-ships-from-the-plugin.md#amendment-2026-09-16-the-external-hook-contract-is-retired).
## Context
Every figure below was measured against the worktree on 2026-09-15. Re-derive rather than quote; the

View File

@@ -14,7 +14,7 @@ of the oddities documented here are load-bearing and have already been re-litiga
| Command | Scope |
|---|---|
| `pre-commit run --all-files` | the commit-stage hooks |
| `pre-commit run --hook-stage pre-push --all-files` | the push gate, one command — with one caveat below |
| `pre-commit run --hook-stage pre-push --all-files` | the push gate, one command |
| `pre-commit run skill-size-check --all-files` | just the ADR-0020 size/context gates |
Install hooks via `pc-run`, wiring **all three stages**. This repo's `.pre-commit-config.yaml` has no
@@ -25,16 +25,20 @@ The pre-push command reports **10** hooks, not 8. The extra two are pre-commit's
`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
is the count of hooks this repo authors itself.
is the count of hooks this repo authors itself, and `--hook-stage pre-push --all-files` is a full
rehearsal of all eight. A PR merged through Gitea's merge button runs none of them: no local push
happens at all.
**The caveat: one of those 8 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
a PR merged through Gitea's merge button, where no local push happens at all.
A real push has a gap of its own. When one `git push` carries several refs
(`git push origin a b`, `git push --all`), pre-commit runs the pre-push stage once, for one ref.
In pre-commit 4.6.1, `_pre_push_ns` in `hook_impl.py` skips delete lines and returns on the first
remaining ref whose remote sha is non-zero and present locally; a ref whose remote sha is zero or
unknown locally is returned only if it has commits that no remote-tracking ref of that remote has.
The one hook that reads the pushed ref, `check-skill-version-bump`, therefore checks only that
ref, and the others are pushed unchecked.
When that ref's unpushed history reaches a root commit, pre-commit runs with all files and sets no
`PRE_COMMIT_TO_REF`, so `check-skill-version-bump` checks `HEAD`, which is the pushed ref only if it
is checked out. Push one ref at a time when the gate matters.
## The pre-push gate
@@ -75,23 +79,69 @@ drift in generated text.
|---|---|
| `validate-marketplace` | `claude plugin validate --strict` on the root marketplace manifest |
**Release**
**Skill versioning** (every push, any branch)
| 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` | fails if a skill directory changed since the pushed commit's merge-base with `main` without its `metadata.version` rising above both the merge-base's and `main`'s tip's (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 says a skill change carries a bump;
`skill-size-check` only checks the field's presence and shape, so this hook holds the bump itself.
- **It runs on every push and under a manual `pre-commit run --hook-stage pre-push`.** It does not
read `PRE_COMMIT_REMOTE_BRANCH`, so the manual rehearsal really checks it. The pushed commit is `PRE_COMMIT_TO_REF`, or `HEAD` when that is unset.
- **"Changed" is measured from the merge-base of the pushed commit with `origin/main`** (local
`main` if `origin/main` does not resolve). Readers install from `main`, so "changed" means
changed against the `main` the branch started from. The remote branch tip is not the baseline:
a second push would excuse an unbumped change the first push already carried.
- **A changed skill's version must beat two baselines**: its version at that merge-base *and* its
version at the tip of the same `main` ref (ADR-0022's second 2026-09-16 amendment). The tip
check stops two branches that make the same bump (`1.0.0` → `1.0.1`) with different content from
both landing, since the identical version lines merge without a conflict. A skill absent at the
tip is held to the merge-base alone; when `main` has not moved, the two are the same commit. Each
failure line names the baseline it missed: `(not above merge-base)` or
`(not above origin/main tip)`. The tip is `origin/main` as last fetched.
- **It fails closed when it has no trustworthy baseline:** neither `origin/main` nor `main`
resolves; there is no merge-base (shallow clone, unrelated history); or only local `main`
resolves and the pushed commit *is* the merge-base, so local `main` already contains the pushed
commit and says nothing independent about what shipped.
- **A changed skill must end strictly above each 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, and so does a skill directory left without its `SKILL.md`. Each part is ASCII
digits, at most nine of them, with no leading zero (`1.0.08` is malformed) — the same shape
`skill-size-check` enforces, so a Unicode digit, an overflowing part or an octal-looking part
cannot pass as a bump. A leading UTF-8 BOM is ignored. A baseline with no valid version accepts
any valid version. Skills absent at both baselines (new, renamed, merged) or at the pushed ref
(deleted, or replaced by a symlink) are exempt. A file moved between skills counts as a change
to both: renames are diffed as delete plus add. A mode-only change counts too.
- **It also fails closed on read errors:** a pushed ref that does not resolve to a commit
(including a tag on a tree), or a `SKILL.md` that the tree names but `git show` or `python3`
cannot read, stops the push with a read-failure message rather than being reported as a missing
version or treated as an absent skill. Presence is read from the tree, so a blob missing from a
corrupt or partial clone cannot make a skill look new. Frontmatter that reads but does not parse
counts as an invalid version.
- **`<skill>/tests/` is excluded**: no agent loads it, so a test-only change ships nothing.
- **It needs `python3` and PyYAML** to read the frontmatter, and fails with a clear message if
either is missing, for the reasons in
[`python3` and PyYAML are hard requirements](#python3-and-pyyaml-are-hard-requirements).
- **Known gaps:** a PR merged through Gitea's merge button
runs no local hook; and a multi-ref push checks only the one ref pre-commit selects, and a push
reaching a root commit gets no `PRE_COMMIT_TO_REF`, so `HEAD` is checked (see
[Running the gates](#running-the-gates)).
- **An all-zeros `PRE_COMMIT_TO_REF` (a branch delete) exits 0.** The branch is defensive:
pre-commit 4.6.1 skips delete lines before it sets the variable.
## Skill and agent context gates (ADR-0020)
The `skill-size-check` pre-commit hook, scoped to `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$`,
runs `scripts/skill-size-check.sh`. It is also shipped to external repos as
`kyberforge-skill-size-check` (see
[External consumers](#external-consumers-the-root-pre-commit-hooksyaml)). Besides the ADR-0020
runs `scripts/skill-size-check.sh`. Besides the ADR-0020
gates below, it also asserts required frontmatter is present: `name`, a non-empty `description`, and
a `metadata.version` matching three-part semver (`1.0.0`) — folded in from a formerly standalone
`skill-frontmatter` hook that parsed the same fields with a shell script.
@@ -398,6 +448,8 @@ which is the exact vacuous-green failure the `python3` check exists to avoid. `p
**Neither requirement generalises to every hook in this repo.** `check-rtk-prefix` needs `python3`
but **not** PyYAML: it reads the markdown body and never touches frontmatter, so it has no scalar to
fold.
`check-skill-version-bump` needs both, for the same reason as `skill-size-check`: it parses
`metadata.version` out of frontmatter.
## Agent files take the description gates, not the body gate
@@ -654,27 +706,20 @@ rule at a blocking bare `YES`/`error`. It is not redundant with the probes above
(`DescriptionOpener`, `PaddingPhrase`, `SentenceOpenerThereIs`, `CompositionNote`) can each be
overridden out of `error` underneath a passing probe. That gap is closed.
Two cases cover the hook manifests.
**Case 32** covers the prefilter hooks' own scope in `.pre-commit-config.yaml`, with three
properties. Each vale hook's `files:` regex must still match at least one tracked file; every path it
matches must be in that hook's own artifact class; and it must match **every** tracked file of that
class under `plugins/*/.apm/`. A hook narrowed to zero files never runs, and pre-commit reports no
error. A hook narrowed to one plugin still matches files of the right class, which is why the third
property exists: narrowing `vale-audit-prefilter-skill` from `^plugins/[^/]+/...` to
`^plugins/kyberforge/...` once left 6 of 38 skills prefiltered and the whole suite green. Part B
narrows both regexes to zero files and Part C narrows both to one plugin, each in a copy of the
config, and requires Part A to fail by hook name.
**Case 33** is the original's cross-manifest `files:` drift check, ported. It extracts each vale
hook's `files:` regex from `.pre-commit-hooks.yaml` and from `.pre-commit-config.yaml`
*independently*, compares them per hook and never as a union, and asserts that each shared probe path
is in scope of both or neither. The original selected each hook's record by matching `entry:`
against the owning skill's `vale-wrap.sh` path. After the merge both hook IDs share one `entry:`, so
the port pairs them by `id:` from an explicit table: `kyberforge-vale-audit-skill` ↔
`vale-audit-prefilter-skill`, and `kyberforge-vale-audit-agent` ↔ `vale-audit-prefilter-agent`. A
missing hook id or a class with no shared probe fails by name. Part B requires three mutations to
fail: the skill hook narrowed to one plugin, the agent hook narrowed the same way, and a renamed
local hook id.
This was briefly a real hole. Narrowing `vale-audit-prefilter-skill` from `^plugins/[^/]+/...` to
`^plugins/kyberforge/...` left 6 of 38 skills prefiltered, and the whole suite green, before case 33
existed.
**Case 32** covers the separate zero-match question on the local manifest alone. Each
`.pre-commit-config.yaml` vale hook's `files:` regex must still match at least one tracked file, and
every path it matches must be in that hook's own artifact class. A hook narrowed to zero files never
runs, and pre-commit reports no error.
Case 32 used to have a partner, case 33, which compared each hook's `files:` regex against the
published `.pre-commit-hooks.yaml`. It went with that manifest (ADR-0014's 2026-09-16 amendment),
and its one guard that did not need a second manifest, the one-plugin narrowing, is case 32's third
property.
**Case 34** asks, statically and with no Vale binary, whether the shipped `.vale.ini` could load a
style at all. Four assertions: every `[glob]` section declares a **non-empty** `BasedOnStyles`; every
@@ -683,8 +728,8 @@ absolute**; and at least one `[glob]` section exists, so the check cannot pass v
with nothing in it. A section whose `BasedOnStyles` is empty is the silent case — Vale lints every
file that glob matches with no rule loaded, prints `0 errors` and exits 0. The absolute-path clause
is the one that is not obvious: an absolute `StylesPath` passes on the machine that wrote it and
hard-fails for every external consumer of `.pre-commit-hooks.yaml`, which is the only reason those
styles ship at all. Part B is a mutation self-test against the same function Part A calls — it empties
hard-fails for every repo that installs `factory-audit`, which is the only reason those styles ship
with the skill at all. Part B is a mutation self-test against the same function Part A calls — it empties
each section's `BasedOnStyles` in a copy of the assets, and absolutizes `StylesPath` in another
pointed at that copy's own real `styles/` directory, and requires each to fail by name.
@@ -735,27 +780,6 @@ from the hook definitions. Under this model they are no-ops; adding one is not a
The `verbose: true` escape hatch that makes `skill-size-check`'s SUGGESTION tier audible has no
analogue here — Vale has no tier to make audible.
### External consumers: the root `.pre-commit-hooks.yaml`
The root `.pre-commit-hooks.yaml` exposes two Vale hook IDs (`kyberforge-vale-audit-skill`,
`kyberforge-vale-audit-agent`) plus `kyberforge-skill-size-check`, so any external repo can enforce
the same rules with `repo: <this-repo-url>, rev: <tag>` in its own `.pre-commit-config.yaml`.
pre-commit clones the pinned rev into its own cache, independent of whether Claude Code or the
`kyberforge` plugin is installed at all; the same mechanism covers CI via `pre-commit run
--all-files`. `skill-size-check` has no external asset dependency, so it needed no relocation under
ADR-0014 — only exposure.
**The two IDs survive the merge even though they now point at the same wrapper.** Both
`kyberforge-vale-audit-skill` and `kyberforge-vale-audit-agent` keep their IDs and their `files:`
regexes, because an external repo pins them by name in its own `.pre-commit-config.yaml` and
collapsing them to one would break every such consumer silently. What changed is only the `entry:`
target: both now name `factory-audit/scripts/vale-wrap.sh`.
This repo's own `vale-audit-prefilter-skill` / `-agent` hooks consume the **identical**
plugin-bundled copy via `repo: local`. Deliberately not a second root copy, and deliberately **not a
pinned self-reference** — a pinned self-reference would lint working-tree edits against the last
tagged release rather than against the change being made.
### Pre-commit
Two prefilter hooks, with `.apm/`-scoped `files:` patterns:
@@ -775,9 +799,10 @@ authors. Without the binary the hooks fail with a bare "command not found" and n
reason was mechanical: with a config per skill, a single hook could point at only one copy and would
silently 0-file-skip the other file shape (see
[A 0-file Vale run is NOT RUN](#a-0-file-vale-run-is-not-run)). One `.vale.ini` carrying all three
sections removes that constraint. The split stays anyway because the two IDs are an exported
contract external consumers pin by name, and because the `files:` regexes still have to differ —
each hook hands Vale only the file shape it is scoped to.
sections removes that constraint. The split stays anyway because the `files:` regexes still have to
differ — each hook hands Vale only the file shape it is scoped to. Both hooks name the same
plugin-bundled `factory-audit/scripts/vale-wrap.sh` through `repo: local`; there is no second root
copy.
### The `.vale.ini` globs do no scoping
@@ -786,15 +811,12 @@ The `.vale.ini`'s section globs are **path-agnostic** — `[**/SKILL.md]`, `[**/
location: Vale's `*` crosses `/`. A `SKILL.md` outside `plugins/` (a project-scope
`.claude/skills/foo/SKILL.md`, say) still matches `[**/SKILL.md]` and gets linted normally.
All scoping therefore comes from the pre-commit hook's own `files:` regex and from `factory-audit`
passing one explicit file per invocation. The two manifests scope **differently on purpose**:
All scoping therefore comes from the pre-commit hooks' own `files:` regexes, which pin this repo's
layout (see [Pre-commit](#pre-commit)), and from `factory-audit` passing one explicit file per
invocation — in this repo or in any repo that installs it, whatever that repo's layout.
| Manifest | `-skill` | `-agent` |
|---|---|---|
| `.pre-commit-config.yaml` (pins this repo's layout) | `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$` | `^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$` |
| `.pre-commit-hooks.yaml` (layout-agnostic for consumers) | `(^\|/)SKILL\.md$` | `(^\|/)agents/[^/]+\.md$\|\.agent\.md$` |
Narrowing a `.vale.ini` glob to a `plugins/`-shaped path to "tighten" it breaks the consumer case.
Narrowing a `.vale.ini` glob to a `plugins/`-shaped path to "tighten" it breaks the consumer case:
`factory-audit` run against a project-scope `.claude/skills/` tree would lint nothing.
`check-vale-style-sync`'s probe set was built to catch exactly that; it moved to
`tests/test-vale-wrap.sh` with the hook's deletion, and two of the six probes exist specifically to
pin this location independence — see [One copy, one config](#one-copy-one-config).
@@ -827,10 +849,6 @@ config declines to match, which is the [0-file NOT RUN](#a-0-file-vale-run-is-no
green run that measured nothing. **Issue #117** records the style-scope half; the hook half has to
land in the same change or the fix is cosmetic.
The consumer manifest is a third axis and does not rescue this either: `.pre-commit-hooks.yaml`'s
`(^|/)SKILL\.md$` is layout-agnostic but still filename-shaped, so an external repo running
`kyberforge-vale-audit-skill` has the same gap.
### `vale-wrap.sh`, never bare `vale`
`factory-audit`'s Step 1 and both pre-commit hooks call
@@ -855,19 +873,19 @@ wrapper a bad description in any of the three broken forms sailed straight throu
### The `--config` argv defect
Handed **no `--config` at all**, the wrapper falls back to its own sibling `assets/vale/.vale.ini`,
located from `${BASH_SOURCE[0]}` rather than from the cwd. That is why both manifests' `entry:` is
now the bare script path with **no argument after it**.
located from `${BASH_SOURCE[0]}` rather than from the cwd. That is why both hooks' `entry:` is
the bare script path with **no argument after it**.
pre-commit prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]),
*cmd[1:])`), so every later argument resolves against the **consuming** repo's root. A `--config` in
`.pre-commit-hooks.yaml` therefore pointed at a path no consumer has and hard-failed every external
run with `E100 [--config] Runtime error`.
the since-retired `.pre-commit-hooks.yaml` therefore pointed at a path no consumer has and
hard-failed every external run with `E100 [--config] Runtime error`.
`.pre-commit-config.yaml` drops the argument too, deliberately keeping the two entries identical.
`.pre-commit-config.yaml` drops the argument too, deliberately matching that entry.
The local `repo: local` hook resolved its `--config` correctly only because the consuming repo *was*
this repo — and that divergence is why three review rounds exercised a path no external consumer
takes and missed the defect. **Do not reintroduce a `--config` to either manifest to make the local
run "explicit".**
takes and missed the defect. **Do not reintroduce a `--config` to either hook to make the local run
"explicit"**, and keep a restored published manifest to `entry[0]` alone (ADR-0014).
An explicit `--config` from any other caller still wins, in all three argv forms (`--config X`,
`--config=/abs`, `--config=rel`), and a relative one resolves against the caller's cwd — matching
@@ -903,12 +921,12 @@ clean.
`CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1` opt-out downgraded them audibly rather than skipping the
hook — is deleted with the second Vale copy (ADR-0025). The six glob probes survive it inside
`test-vale-wrap.sh`, so `run-tests --strict` is now the gate that runs them. That is also what keeps
`vale` a pre-push requirement: `test-vale-hooks-consumer.sh` exits 77 without the binary, and so does
`test-vale-wrap.sh` once its static cases pass, and a skip fails the push.
`vale` a pre-push requirement: `test-vale-wrap.sh` exits 77 without the binary once its static cases
pass, and a skip fails the push.
`test-vale-wrap.sh` without Vale skips only its Vale-dependent cases, not the whole suite. The cases
that are plain greps and awk over the config and the two hook manifests still run: case 0, 16, 26,
27, the static halves of 28, 31 Parts A and B, 32, 33 and 34. A static failure exits 1, because a
that are plain greps and awk over the Vale config and `.pre-commit-config.yaml` still run: case 0, 16, 26,
27, the static half of 28, 31 Parts A and B, 32 and 34. A static failure exits 1, because a
real defect is not a setup error. Only an all-static-pass run exits 77.
### Mentioning banned phrasing without tripping the rule
@@ -935,10 +953,11 @@ run. The pre-push hook invokes the same script as `--strict` (`RUN_TESTS_STRICT=
where a skip **does** fail the push: at pre-push a skip means one of the documented dependencies is
absent on this machine, so the gate would otherwise report success having run fewer suites than it
appears to. Without `--strict` the gate once went green having verified 15 of 17 suites on a
vale-less PATH, with the skip list swallowed. Without vale, two suites skip —
`test-vale-hooks-consumer.sh` and `test-vale-wrap.sh` — and the strict failure names each one and
what to install. (It was three until `test-check-vale-style-sync.sh` was deleted with its hook; see
[One copy, one config](#one-copy-one-config).)
vale-less PATH, with the skip list swallowed. Without vale, one suite skips — `test-vale-wrap.sh` —
and the strict failure names it and what to install. (It was three until
`test-check-vale-style-sync.sh` was deleted with its hook — see
[One copy, one config](#one-copy-one-config) — and `test-vale-hooks-consumer.sh` with the published
hook manifest.)
**Output assertions use a here-string, never a pipe.** Write `grep -q PATTERN <<< "$OUT"`, not
`echo "$OUT" | grep -q PATTERN`. Under `set -o pipefail` the pipe form fails depending on timing:

View File

@@ -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.

View File

@@ -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.

View File

@@ -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"
---
<what-to-do>

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -1,5 +1,5 @@
name: bin
version: 1.1.7
version: 1.1.8
description: Skills for everyday AI-assisted development work that is not tied to a single tool, forge or language, and has not yet been split into a focused plugin.
author:
name: Defame1297

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -1,5 +1,5 @@
name: core
version: 1.1.2
version: 1.1.3
description: Skills for authoring and auditing a repo's AGENTS.md and the provider adapter files that defer to it.
author:
name: Defame1297

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -1,5 +1,5 @@
name: git
version: 1.3.7
version: 1.3.8
description: Skills and agents for working with a local Git clone over the git wire protocol, and for authoring and running the pre-commit hooks that guard it.
author:
name: Defame1297

View File

@@ -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

View File

@@ -1,5 +1,5 @@
name: gitea
version: 1.3.8
version: 1.3.9
description: Skills and agents for working with a Gitea forge through its HTTP API — the forge's own objects, as distinct from the local git clone.
author:
name: Defame1297

View File

@@ -51,8 +51,25 @@ emit() {
printf '{"hookSpecificOutput":{"hookEventName":"SessionStart","reloadSkills":%s,"additionalContext":"%s"}}\n' "$1" "$2"
}
# What to do with the rewritten lock depends on the branch (ADR-0019): on the
# default branch it is a real update to commit or discard; on a feature branch it
# is churn unrelated to the branch and should be discarded. The branch name only
# selects between fixed strings and is never interpolated. Outside a git checkout,
# or on a detached HEAD, the neutral advice stands.
lock_advice="commit it or discard it deliberately."
current_branch="$(git symbolic-ref --short -q HEAD 2> /dev/null || true)"
if [[ -n "$current_branch" ]]; then
default_branch="$(git symbolic-ref --short -q refs/remotes/origin/HEAD 2> /dev/null || true)"
default_branch="${default_branch#origin/}"
if [[ "$current_branch" == "${default_branch:-main}" ]]; then
lock_advice="this is the default branch, so commit it or discard it deliberately."
else
lock_advice="this is a feature branch, so discard it: git checkout -- apm.lock.yaml && apm install"
fi
fi
if timeout 300 apm update --yes > /dev/null 2>&1; then
emit true "apm install was ${stale_count} package(s) behind the remote default branch and has been refreshed automatically; skills and agents were redeployed and re-scanned. apm.lock.yaml has been rewritten and is now a modified file in the working tree - commit it or discard it deliberately."
emit true "apm install was ${stale_count} package(s) behind the remote default branch and has been refreshed automatically; skills and agents were redeployed and re-scanned. apm.lock.yaml has been rewritten and is now a modified file in the working tree - ${lock_advice}"
else
emit false "apm install is ${stale_count} package(s) behind the remote default branch and the automatic refresh failed. Deployed skills and agents may be stale. Run: apm update --yes"
fi

View File

@@ -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

View File

@@ -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

View File

@@ -53,12 +53,19 @@ output changes.** Two triggers, not one:
The version belongs to the package, not to the repo: editing `plugins/foo/.apm/` never bumps
`plugins/bar/apm.yml`.
Under a `per_package` strategy the same number is also carried in the catalog's
`marketplace.packages[]` entry, so both copies move together in the same commit. The catalog's own
version follows a separate rule — see `references/marketplace.md`. `apm pack --check-versions`
fails the push when a package's version disagrees with the configured strategy, so a bump applied
in only one of the two places is caught, but a bump skipped in both is not: nothing infers intent
from a content diff.
Under a `per_package` strategy this `version:` is the single source for a local-path
(`source: ./…`) catalog entry: when that `marketplace.packages[]` entry omits `version:`,
`apm pack` reads it from the package's `apm.yml`. Do not restate it there. On a local entry a
`version:` is an override, not a copy — it silently wins in the compiled `marketplace.json`, and
`apm pack --check-versions` still reports `[matches]` when it disagrees with the package's own
number, so drift between the two is never caught. Set one only when an override is the intent.
`description:` behaves the same way: a local entry that omits it publishes the package `apm.yml`'s
`description`, and one that sets it silently overrides it. A remote entry is different. There,
`version:` is the semver range that selects which git tag to resolve, and a remote entry must carry
`version:` or `ref:`. Its `description:`, when set, is the published text.
A skipped bump is not caught either: nothing infers intent from a content diff. The catalog's own
version follows a separate rule — see `references/marketplace.md`.
## Dependency reference forms

View File

@@ -35,7 +35,6 @@ marketplace:
- name: plugin-a
description: ...
source: ./packages/plugin-a
version: 1.0.0
```
This local-path `source:` form IS valid — `apm marketplace check` and `apm pack` both accept it — even though the `package add` CLI subcommand cannot create it for you. Use `apm marketplace package add` only for packages hosted at a remote git ref; for local packages, edit the YAML directly.
@@ -63,12 +62,20 @@ The local-filesystem and `file://` forms need no hosted registry or network acce
marketplace:
versioning: { strategy: per_package }
packages:
- { name: plugin-a, source: ./packages/plugin-a, version: 2.0.0 }
- { name: plugin-b, source: ./packages/plugin-b, version: 0.1.0 }
- { name: plugin-a, source: ./packages/plugin-a }
- { name: plugin-b, source: ./packages/plugin-b }
```
Without this block, the default versioning strategy ties every listed package to the marketplace/root version.
With it, each local-path (`source: ./…`) entry's version comes from its package's own
`apm.yml`, and so does its `description`. A local entry may also set `version:` or `description:`,
but either one is an override: it silently wins in the compiled `marketplace.json`, and
`apm pack --check-versions` does not flag a version mismatch. Omit both unless an override is
intended — see `references/configure.md`. A remote entry is different: its `version:` is the semver
range that selects the git tag to resolve (matched through the entry's `tag_pattern`, else
`build.tagPattern`), not an override, and the entry must set `version:` or `ref:`.
## Bumping the catalog's own version (repo policy)
The section above is apm's *mechanic* — how per-package versions are declared and how
@@ -79,11 +86,13 @@ earned it:
- **Minor** when a `marketplace.packages[]` entry is added or removed. The catalog's contents
changed — a consumer resolving it now gets a different set of installable packages.
- **Patch** when only `marketplace:`-block fields change and the set of packages is unchanged: the
catalog description, owner, `build:`/`outputs:` config, or an existing entry's `version:`,
description, or category. The catalog describes the same packages; only its metadata moved. An
entry's `version:` is the most frequent of these by far — under `per_package` it moves here every
time any package bumps (see `references/configure.md`), and that alone earns the catalog patch.
- **Patch** when the set of packages is unchanged but what the catalog publishes moved: a
`marketplace:`-block field (the catalog description, owner, `build:`/`outputs:` config, or an
existing entry's description or category), or any listed package's own `apm.yml` `version:`. The
most frequent trigger by far is the package bump. When entries omit `version:` (the recommended
local form), the compiled `marketplace.json` still publishes each package's version, so a package
bump alone changes the catalog and alone earns the patch — made in the same commit (see
`references/configure.md`).
Keep the root `apm.yml`'s top-level `version:` in step with `marketplace.version`. They are separate
keys — the top-level one is not inherited into the compiled `marketplace.json`, but `apm audit`

View File

@@ -45,8 +45,8 @@ IFS='' read -r -d '' KYBERFORGE_RESOLVER_PY <<'KYBERFORGE_ADR0020_RESOLVER_PY' |
# plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-boundary-resolver.sh
# The block between these markers must stay byte-identical in both. It is copied
# rather than imported because a cache-installed plugin's scripts cannot read
# files outside their own plugin directory, and this repo-root hook resolves via
# .pre-commit-hooks.yaml, where entry[0] is the only token pre-commit rewrites --
# files outside their own plugin directory, and this repo-root hook is kept fit for
# a published hook manifest (retired; ADR-0014), where only entry[0] is rewritten --
# so no single file is reachable by both (the same constraint that duplicates the
# ADR-0020 constants). Edit one copy, then paste it over the other.
#

View File

@@ -235,7 +235,9 @@ else:
# --- ADR-0022: metadata.version is mandatory -------------------------------
# FAIL, not SUGGESTION, and the tier is set by the gate rather than by taste.
# `.pre-commit-config.yaml`'s `skill-size-check` hook REJECTS a SKILL.md with
# no `metadata.version`, and rejects a value that is not three-part semver.
# no `metadata.version`, and rejects a value that is not three-part semver:
# ASCII digits only, no leading zero (semver 2.0.0 item 2), at most nine digits
# per part (bash arithmetic in check-skill-version-bump.sh), whole-value match.
# skill-author's Step 4 says to run this audit and "resolve every FAIL", so any
# tier below FAIL lets that step report done on a skill the commit gate then
# refuses — the same audit-disagrees-with-the-gate failure the MAX_LINES note
@@ -246,8 +248,10 @@ else:
# The rule is DUPLICATED from that hook for the same cache-isolation reason as
# every other constant here — an installed plugin's scripts cannot read the
# repo-root config. Keep the two in step: this check must accept exactly what
# the hook accepts.
SEMVER_RE = re.compile(r'^\d+\.\d+\.\d+$')
# the hook accepts. Used with fullmatch(), never match() with ^...$ anchors:
# `$` also matches before a trailing newline, and `\d` also matches non-ASCII
# Unicode digits — both of which the hook rejects.
SEMVER_RE = re.compile(r'(0|[1-9][0-9]{0,8})\.(0|[1-9][0-9]{0,8})\.(0|[1-9][0-9]{0,8})')
try:
fm_data = yaml.safe_load(fm)
@@ -268,13 +272,15 @@ else:
# spelling is exactly the two-part value the hook rejects — coercing and
# then matching keeps this check and the hook agreeing on that case.
version_text = version_value if isinstance(version_value, str) else str(version_value)
version_text = version_text.strip()
if SEMVER_RE.match(version_text):
# Same normalisation as the hook: surrounding whitespace, then quotes.
version_text = version_text.strip().strip('\'"')
if SEMVER_RE.fullmatch(version_text):
ok(f"metadata.version present: '{version_text}' (ADR-0022)")
else:
fail(f"metadata.version '{version_text}' is not three-part semver — the "
f"skill-size-check pre-commit hook rejects it. Use MAJOR.MINOR.PATCH, "
f"e.g. \"1.0.0\"")
f"skill-size-check pre-commit hook rejects it. Use MAJOR.MINOR.PATCH "
f"with ASCII digits, no leading zeros and at most nine digits per "
f"part, e.g. \"1.0.0\"")
# SKILL.md size ceilings (agentskills.io skill-authoring.md: 500 lines,
# ~5,000 tokens). Both constants are DUPLICATED from the repo-root pre-commit

View File

@@ -29,10 +29,11 @@ set -euo pipefail
#
# Divergence 1: with no `--config` at all, this script's own sibling
# `assets/vale/.vale.ini` is used instead of vale's upward search. pre-commit
# prefixes only `entry[0]` with the hook-repo clone path, so a `--config` in
# `.pre-commit-hooks.yaml` would resolve against the *consuming* repo and
# hard-fail (E100) for every external consumer. The manifest therefore passes the
# script alone, and an explicit `--config` from any other caller still wins.
# prefixes only `entry[0]` with the hook-repo clone path, so a `--config` in a
# published `.pre-commit-hooks.yaml` would resolve against the *consuming* repo
# and hard-fail (E100) for every external consumer. That manifest is retired
# (ADR-0014, 2026-09-16 amendment), but the script still needs no `--config` so it
# can return; an explicit `--config` from any other caller still wins.
#
# Divergence 2: a path-shaped argument that does not exist is a hard error
# (exit 2). Bare vale drops it, falls back to reading stdin, and prints

View File

@@ -1989,7 +1989,7 @@ EOF
# NEW with the merge and additive: pre-merge, handing the SKILL.md itself to
# skill-audit's validate-provenance.sh hit the "not a directory" precondition
# and died. It matters because pre-commit `files:` hooks match FILES — the
# exported kyberforge-vale-audit-skill hook's regex is (^|/)SKILL\.md$ — so
# vale-audit-prefilter-skill hook's regex ends in /SKILL\.md$ — so
# every hook-driven invocation hands over a SKILL.md path, never its
# directory. The entry point rewrites the token to the directory in place.
local skill="$TMPDIR/my-skill"

View File

@@ -377,7 +377,7 @@ SH
# ---------------------------------------------------------------------------
# ADR-0022 — metadata.version is mandatory. FAIL tier, matching the
# skill-frontmatter pre-commit hook: an audit that graded this lower would
# skill-size-check pre-commit hook: an audit that graded this lower would
# report ready-to-ship on a file the commit gate rejects.
# ---------------------------------------------------------------------------
@@ -437,6 +437,62 @@ PY
assert_output --partial "metadata.version present: '0.1.3'"
}
@test "ADR-0022: a leading zero in the patch part FAILs (1.0.08)" {
local skill="$TMPDIR/my-skill"
make_valid_skill "$skill"
python3 - "$skill/SKILL.md" <<'PY'
import sys
p = sys.argv[1]
s = open(p).read().replace(' version: "1.0.0"\n', ' version: "1.0.08"\n')
open(p, 'w').write(s)
PY
run bash "$SCRIPT" "$skill"
assert_failure
assert_output --partial "three-part semver"
}
@test "ADR-0022: a leading zero in the major part FAILs (01.0.1)" {
local skill="$TMPDIR/my-skill"
make_valid_skill "$skill"
python3 - "$skill/SKILL.md" <<'PY'
import sys
p = sys.argv[1]
s = open(p).read().replace(' version: "1.0.0"\n', ' version: "01.0.1"\n')
open(p, 'w').write(s)
PY
run bash "$SCRIPT" "$skill"
assert_failure
assert_output --partial "three-part semver"
}
@test "ADR-0022: a multi-digit part with no leading zero passes (1.0.10)" {
local skill="$TMPDIR/my-skill"
make_valid_skill "$skill"
python3 - "$skill/SKILL.md" <<'PY'
import sys
p = sys.argv[1]
s = open(p).read().replace(' version: "1.0.0"\n', ' version: "1.0.10"\n')
open(p, 'w').write(s)
PY
run bash "$SCRIPT" "$skill"
assert_success
assert_output --partial "metadata.version present: '1.0.10'"
}
@test "ADR-0022: a zero major part passes (0.1.0)" {
local skill="$TMPDIR/my-skill"
make_valid_skill "$skill"
python3 - "$skill/SKILL.md" <<'PY'
import sys
p = sys.argv[1]
s = open(p).read().replace(' version: "1.0.0"\n', ' version: "0.1.0"\n')
open(p, 'w').write(s)
PY
run bash "$SCRIPT" "$skill"
assert_success
assert_output --partial "metadata.version present: '0.1.0'"
}
@test "fails when name contains consecutive hyphens" {
local skill="$TMPDIR/my--skill"
make_valid_skill "$skill"
@@ -960,8 +1016,8 @@ EOF
# NEW with the merge and additive rather than ported: pre-merge, handing the
# SKILL.md itself to skill-audit's validate.sh hit the directory precondition
# and gave a useless exit 1. It matters because pre-commit `files:` hooks
# match FILES — the exported kyberforge-vale-audit-skill hook's regex is
# (^|/)SKILL\.md$ — so every hook-driven invocation hands over a SKILL.md
# match FILES — the vale-audit-prefilter-skill hook's regex ends in
# /SKILL\.md$ — so every hook-driven invocation hands over a SKILL.md
# path, never the directory above it. The entry point resolves the file to
# its directory before dispatching.
local skill="$TMPDIR/my-skill"

View File

@@ -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

View File

@@ -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

View File

@@ -1,5 +1,5 @@
name: lint
version: 1.1.7
version: 1.1.8
description: Skills and agents for configuring and running linters.
author:
name: Defame1297

View File

@@ -1,242 +0,0 @@
#!/usr/bin/env bash
set -euo pipefail
# Hard-fails only when pushing to main: if any file covered by .pre-commit-hooks.yaml
# (the external git-hook/CI contract, see ADR-0014) changed since the last tag,
# a release must be cut before landing on main, or external consumers pinning
# `rev: <tag>` silently miss the change. Pre-commit sets PRE_COMMIT_REMOTE_BRANCH
# for pre-push hooks; on every other branch (feature work mid-review) this is a
# silent no-op — pushing WIP commits there must not be blocked on cutting a
# premature tag (see ADR-0014's repo: local vs pinned self-reference decision).
#
# Known gap: this only fires on a local `git push` through pre-commit's pre-push
# hook. A PR merged via Gitea's merge button (server-side, no local push) or a
# CI runner invoking `pre-commit run --hook-stage pre-push` directly does not set
# PRE_COMMIT_REMOTE_BRANCH and will not trigger this check — closing that
# requires a server-side CI job, which this repo does not have yet.
TARGET_BRANCH="refs/heads/main"
if [[ "${PRE_COMMIT_REMOTE_BRANCH:-}" != "$TARGET_BRANCH" ]]; then
exit 0
fi
# What is actually being pushed, which is only HEAD for the common
# `git push <remote> <current-branch>` case. pre-commit's pre-push hook-impl
# exports the local sha of each pushed ref as PRE_COMMIT_TO_REF; a
# `git push <remote> topic:main` from a different checkout would otherwise be
# gated on the wrong tip — a false negative when HEAD is behind the pushed ref
# (unreleased changes sail through), a false positive when it is ahead.
# PRE_COMMIT_FROM_REF, the *remote's* current tip, is deliberately not used
# anywhere here: the baseline is the last release tag, not what the remote
# already has. Diffing from the remote tip would let an untagged
# release-relevant commit already on main excuse the next push from cutting a
# tag, which is precisely the drift this gate exists to catch.
PUSHED_REF="${PRE_COMMIT_TO_REF:-HEAD}"
# pre-commit passes an all-zeros sha (40 hex zeros under sha1, 64 under sha256)
# as the "to" ref when the push deletes a branch. Nothing is being shipped, and
# every rev-taking command below would fail on an unresolvable sha, so bail out
# rather than turning a branch deletion into a confusing "could not diff".
if [[ "$PUSHED_REF" =~ ^0+$ ]]; then
exit 0
fi
REPO_ROOT="$(git rev-parse --show-toplevel)"
cd "$REPO_ROOT"
HOOKS_MANIFEST=".pre-commit-hooks.yaml"
if [[ ! -f "$HOOKS_MANIFEST" ]]; then
exit 0
fi
# Only vX.Y.Z release tags count as a baseline — an incidental checkpoint or
# experiment tag reachable from the pushed ref must not shift the diff baseline.
# The tag is resolved from $PUSHED_REF, not HEAD, for the same reason the diff
# is: a tag reachable only from HEAD is not part of the history being pushed.
# --match is a shell glob, not a regex: its trailing `*`s match any suffix, so
# without --exclude a pre-release/checkpoint tag like v1.2.3-checkpoint or
# v1.2.3-rc1 also satisfies 'v[0-9]*.[0-9]*.[0-9]*' and could be picked over the
# true last release tag. --exclude is glob syntax too, so '*-*' is what actually
# rules out any tag carrying a hyphenated suffix, leaving only bare vMAJOR.MINOR.PATCH.
LAST_TAG="$(git describe --tags --abbrev=0 --match 'v[0-9]*.[0-9]*.[0-9]*' --exclude '*-*' "$PUSHED_REF" 2>/dev/null || true)"
if [[ -z "$LAST_TAG" ]]; then
echo "FAIL: no release tag exists yet, but .pre-commit-hooks.yaml already exposes hooks to external consumers." >&2
echo " Fix: cut the first release tag (e.g. v1.0.0) before this lands on main." >&2
exit 1
fi
# Derive release-relevant paths from .pre-commit-hooks.yaml's own entry: lines
# instead of hand-maintaining a parallel list — the manifest is the single
# source of truth for what external consumers actually pull at a pinned rev,
# so a hook added/removed/renamed there can't silently drift out of sync here.
# Everything is derived from tokens[0], the hook's script: pre-commit prefixes
# only entry[0] with the hook-repo clone path, so any later token that looks
# like a path resolves against the *consuming* repo and can never name a file
# this repo ships. A hook's bundled data therefore has to be self-located
# relative to the script — vale-wrap.sh reads its own
# <script-dir>/../assets/vale/.vale.ini plus the sibling styles/ tree — which
# makes <script-dir>/../assets release-relevant alongside the script itself.
# The ../ is normalised by stripping a path component rather than with
# `realpath -m`, which is a GNU-only extension. Two guards keep the derivation
# from inventing paths: a bundle root of "." is skipped, because a script in a
# top-level directory (scripts/skill-size-check.sh) would derive the repo's own
# shared assets/, which no hook owns and whose churn must not demand a release;
# and the assets/ directory is added only where it is known to exist, since a
# hook that bundles nothing must not contribute a pathspec matching nothing.
RELEASE_PATHS=("$HOOKS_MANIFEST")
add_release_path() {
local candidate="$1" existing
for existing in "${RELEASE_PATHS[@]}"; do
[[ "$existing" == "$candidate" ]] && return 0
done
RELEASE_PATHS+=("$candidate")
}
# Emits one "<hook id><TAB><entry value>" line per hook so a rejected entry can
# name the hook a human has to go fix. The id sits on its own line above its
# entry: in YAML, so it is carried forward and then cleared; a hook that somehow
# has no id still reports something printable rather than an empty name. Kept in
# bash rather than awk: matching `[[:space:]]` inside a bracket expression is
# reliable in bash's own globs but not in the BWK awk macOS ships. `read -r` with
# a single variable is the trimmer — it strips leading and trailing whitespace
# while preserving anything in between, so a multi-token entry survives intact
# for the error message to quote back.
manifest_entries() {
local line id="" value
while IFS= read -r line; do
# Drop the indentation and the optional list dash, so that `- id: x` and
# ` entry: y` both reduce to the same bare "key: value" shape.
line="${line#"${line%%[![:space:]]*}"}"
if [[ "$line" == -* ]]; then
line="${line#-}"
line="${line#"${line%%[![:space:]]*}"}"
fi
case "$line" in
id:*)
read -r id <<< "${line#id:}"
;;
entry:*)
read -r value <<< "${line#entry:}"
printf '%s\t%s\n' "${id:-(unnamed hook)}" "$value"
id=""
;;
esac
done
}
# A hook's script is legitimate if it exists in the working tree *or* at
# $LAST_TAG — the same union the pathspec itself spans. Checking per-scope
# instead would reject exactly the case this gate exists to flag: a script
# deleted since the tag while its entry survives (see the no -e filtering note
# further down) is a real deletion to report, not a malformed manifest.
entry_path_exists() {
local candidate="$1"
[[ -e "$candidate" ]] && return 0
git cat-file -e "$LAST_TAG:$candidate" 2>/dev/null && return 0
return 1
}
# $1 selects where the "does this hook bundle an assets/ tree?" guard looks:
# "worktree" probes the filesystem, anything else is a rev whose tree is probed
# with git plumbing. Reading entry lines from stdin keeps one derivation for
# both the tagged manifest and the current one.
collect_release_paths() {
local scope="$1" line hook_id entry bundle_root where
local -a tokens
if [[ "$scope" == "worktree" ]]; then
where="the working tree's $HOOKS_MANIFEST"
else
where="$HOOKS_MANIFEST at $scope"
fi
while IFS= read -r line; do
hook_id="${line%%$'\t'*}"
entry="${line#*$'\t'}"
read -ra tokens <<< "$entry"
[[ ${#tokens[@]} -eq 0 ]] && continue
# ADR-0014 binds every entry to a bare script path and nothing else, because
# pre-commit rewrites only entry[0] into the hook-repo clone. That is a
# constraint nothing else enforces, and the sibling .pre-commit-config.yaml
# already ships the multi-token `bash <script>` shape one copy-paste away —
# so an entry like `bash scripts/foo.sh` would add "bash" as a pathspec that
# matches nothing and derive a bundle root of ".", dropping that hook's
# entire surface out of the gate silently. Both malformed shapes below fail
# loudly instead: silent degradation here is the same class of defect as the
# --config token already recorded in LESSONS.md.
if [[ ${#tokens[@]} -gt 1 ]]; then
echo "FAIL: hook '$hook_id' in $where has a multi-token entry: $entry" >&2
echo " Why: pre-commit rewrites only entry[0] into the hook-repo clone, so every later" >&2
echo " token resolves against the *consuming* repo and can never name a file this" >&2
echo " repo ships — and this gate would derive its release paths from '${tokens[0]}'." >&2
echo " Fix: make the entry a bare script path and have the script self-locate anything" >&2
echo " else from \${BASH_SOURCE[0]} (see ADR-0014, 'Consequences')." >&2
exit 1
fi
if ! entry_path_exists "${tokens[0]}"; then
echo "FAIL: hook '$hook_id' in $where names a path that exists neither in the working tree nor at $LAST_TAG: ${tokens[0]}" >&2
echo " Why: this gate derives its release-relevant pathspec from that path, so a name" >&2
echo " that resolves to no file silently drops the hook's whole surface from the diff." >&2
echo " Fix: point the entry at a script path this repo actually ships (see ADR-0014," >&2
echo " 'Consequences'); a bare command name is not a valid entry here." >&2
exit 1
fi
add_release_path "${tokens[0]}"
bundle_root="$(dirname "$(dirname "${tokens[0]}")")"
[[ "$bundle_root" == "." ]] && continue
if [[ "$scope" == "worktree" ]]; then
[[ -d "$bundle_root/assets" ]] && add_release_path "$bundle_root/assets"
else
git cat-file -e "$scope:$bundle_root/assets" 2>/dev/null && add_release_path "$bundle_root/assets"
fi
done
return 0
}
# The worktree alone is not enough: a path is release-relevant if it was part of
# the contract at $LAST_TAG *or* is part of it now, so both trees have to be
# derived and unioned. Deriving only from the worktree meant that deleting a
# hook's entire assets/ tree made the `-d` guard drop the path from the pathspec
# altogether, and the deletion — which breaks every consumer at the next rev —
# diffed clean. The two manifests can genuinely disagree (an entry added,
# removed, or renamed since the tag), and the union is the conservative side of
# that disagreement: a path the tag exposed and HEAD no longer does is a removal
# consumers must be told about, and a path only HEAD exposes is new contract
# surface they cannot reach without a new tag. The union never over-fires on its
# own, either — any manifest edit that makes the two disagree already changes
# $HOOKS_MANIFEST, which is itself a release-relevant path.
collect_release_paths worktree < <(manifest_entries < "$HOOKS_MANIFEST")
# A missing manifest at the tag is legitimate (the manifest was added since) but
# is indistinguishable from an unreadable tagged tree by its exit status alone,
# so the tag's root tree is verified separately. An absent tree object — a
# shallow clone, a truncated fetch — fails closed exactly like a `git diff`
# failure does, rather than silently degrading to worktree-only derivation.
if MANIFEST_AT_TAG="$(git cat-file -p "$LAST_TAG:$HOOKS_MANIFEST" 2>/dev/null)"; then
collect_release_paths "$LAST_TAG" < <(printf '%s\n' "$MANIFEST_AT_TAG" | manifest_entries)
elif ! git cat-file -e "$LAST_TAG^{tree}" 2>/dev/null; then
echo "FAIL: could not read the tree at $LAST_TAG to determine which paths that release exposed." >&2
echo " Fix: ensure full tag history is available (e.g. git fetch --unshallow) and retry." >&2
exit 1
fi
# No -e/existence filtering on the pathspec: a path deleted since $LAST_TAG is
# exactly the case that must be caught (external consumers pinning the old tag
# would hit a missing file), and `git diff` reports deletions fine without it
# existing at the pushed ref. A git failure (e.g. a shallow clone missing
# $LAST_TAG's history) must fail closed, not be swallowed into an empty,
# falsely-clean diff.
if ! CHANGED="$(git diff --name-only "$LAST_TAG".."$PUSHED_REF" -- "${RELEASE_PATHS[@]}")"; then
echo "FAIL: could not diff $LAST_TAG..$PUSHED_REF to check for release-relevant changes (see git error above)." >&2
echo " Fix: ensure full tag history is available (e.g. git fetch --unshallow) and retry." >&2
exit 1
fi
if [[ -n "$CHANGED" ]]; then
echo "FAIL: files covered by .pre-commit-hooks.yaml changed since $LAST_TAG:" >&2
echo "$CHANGED" | sed 's/^/ /' >&2
echo " Fix: cut a new release tag — external consumers pinning rev: $LAST_TAG would miss this change." >&2
exit 1
fi

View File

@@ -0,0 +1,278 @@
#!/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 <main> <pushed commit>`, where <main> is origin/main
# when it resolves and the local `main` branch otherwise. 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. The check runs on every push whatever the target
# branch — nothing here reads PRE_COMMIT_REMOTE_BRANCH — so it also runs under
# a manual `pre-commit run --hook-stage pre-push` (against HEAD, since no
# PRE_COMMIT_TO_REF is set). A missing bump is cheapest to fix on the branch,
# before review.
#
# Second baseline: the tip of that same <main> ref. A changed skill's pushed
# version must exceed its version there too (ADR-0022, second 2026-09-16
# amendment). Two branches bumping 1.0.0 -> 1.0.1 with different content merge
# without a conflict, so the merge-base alone would let main ship both under
# one version. When <main> has not moved since the merge-base, the two
# baselines are one commit and the skill is checked once. The tip is read as
# last fetched.
#
# Pushing main itself: with origin/main as the baseline, a push of main diffs
# the new commits against what the remote already has, so it is covered. A
# pushed commit that is already an ancestor of origin/main (merge-base equals
# the pushed commit) changes nothing relative to main and passes. With only the
# local `main` fallback, pushing main makes the merge-base the pushed commit
# itself — the diff is empty by construction, not because nothing changed — so
# that combination FAILS closed rather than passing unchecked.
#
# Scope: every skill directory plugins/<plugin>/.apm/skills/<skill>/, bin
# included. Anything under <skill>/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
# commit. Paths are read NUL-delimited (`git diff -z`), so core.quotePath never
# hides a non-ASCII path. Renames are diffed as delete + add (--no-renames), so:
# - a skill absent at both baselines (new, renamed-to, merged-into) is
# exempt; it has no prior version to exceed. Absent at the tip only
# (deleted on main since): the merge-base rule alone applies.
# - a file moved from one skill to another changes both.
# - a skill directory absent at the pushed commit (deleted, renamed-from) is
# exempt; there is nothing left to version. A directory replaced by a
# symlink is no longer a tree, so it counts as deleted (apm drops symlinks
# under .apm/, ADR-0017). A directory that survives without its SKILL.md
# is NOT exempt: it fails as "SKILL.md missing".
# Presence is read from the tree (rev-parse <commit>:<path>), not from the
# blob, so a blob missing from a corrupt or partial clone is a read failure,
# never a skill that looks new.
# A changed skill must carry a three-part semver `metadata.version` at the
# pushed commit that is numerically greater than each baseline's. The shape
# rule matches skill-size-check.sh (str()-coerce, strip whitespace and quotes,
# so `1.0` and `1.0.0-rc1` are rejected): each part is ASCII digits (Python's
# `\d` also matches e.g. U+FF11), at most 9 of them so it fits bash
# arithmetic, with no leading zero so bash never reads it as octal. A leading
# UTF-8 BOM is ignored. A baseline with no valid version (a skill predating
# ADR-0022) accepts any valid version. Versions are read from git objects,
# never the working tree.
#
# Fails closed: if neither origin/main nor main resolves, if no merge-base
# exists (shallow clone, unrelated history), if the pushed ref does not resolve
# to a commit (an unknown sha, a tag on a tree), if python3 or PyYAML is
# unavailable, or if a SKILL.md the tree names cannot be read. Passing in any
# of those would make that environment the one place the rule is silently off.
#
# Known gaps:
# - Only one pushed ref is gated. pre-commit (4.6.1, hook_impl.py
# `_pre_push_ns`) consumes the pre-push stdin itself and walks the ref
# lines in order: it skips deletes, returns on a ref whose remote sha is
# non-zero and exists locally, and otherwise returns on the ref only if it
# has commits no remote-tracking ref of that remote has. Every other ref in
# the same `git push` (e.g. `git push origin a b`, `--all`, `--tags`) is
# never seen. When the selected ref's unpushed history reaches a root
# commit, pre-commit runs with all_files and sets no PRE_COMMIT_TO_REF at
# all, so this script checks HEAD — the pushed ref only if checked out. The
# script cannot recover either case: the ref list is gone by the time it
# runs. Push refs one at a time to be sure each is checked.
# - A PR merged via Gitea's merge button runs no local hook at all.
# Closing it requires a server-side CI job, which this repo does not have
# yet.
# Byte-wise regex matching and messages: path bytes are matched against
# SKILL_PATH_RE below and must not depend on the caller's locale.
export LC_ALL=C
# PRE_COMMIT_TO_REF is the local object actually being pushed, which is only
# HEAD for the common case. For a tag push it is the tag object, so it is
# peeled to a commit below before use.
PUSHED_REF="${PRE_COMMIT_TO_REF:-HEAD}"
# All-zeros sha: the push deletes a branch, so nothing ships. Defensive only:
# pre-commit 4.6.1's `_pre_push_ns` already skips delete lines and never passes
# one here. Kept so a different caller cannot turn a delete into a rev-parse
# failure.
if [[ "$PUSHED_REF" =~ ^0+$ ]]; then
exit 0
fi
REPO_ROOT="$(git rev-parse --show-toplevel)"
cd "$REPO_ROOT"
if ! command -v python3 > /dev/null 2>&1; then
echo "FAIL: python3 is required to read SKILL.md metadata.version but was not found on PATH." >&2
echo " Fix: install python3 (pre-commit itself is a Python application, so it is almost certainly already present)." >&2
exit 1
fi
if ! python3 -c 'import yaml' > /dev/null 2>&1; then
echo "FAIL: PyYAML is required to read SKILL.md metadata.version but is not importable by python3." >&2
echo " Fix: python3 -m pip install PyYAML (or your distro's python3-yaml package)." >&2
exit 1
fi
if ! PUSHED_COMMIT="$(git rev-parse --verify -q "$PUSHED_REF^{commit}")"; then
echo "FAIL: pushed ref $PUSHED_REF does not resolve to a commit." >&2
exit 1
fi
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_COMMIT" 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 [[ "$MAIN_REF" == "main" && "$BASELINE" == "$PUSHED_COMMIT" ]]; then
echo "FAIL: origin/main does not resolve and $PUSHED_REF is already contained in local main, so local main cannot serve as an independent baseline — the diff would be empty by construction." >&2
echo " Fix: git fetch origin main and retry." >&2
exit 1
fi
CHANGED_FILE="$(mktemp)"
trap 'rm -f "$CHANGED_FILE"' EXIT
if ! git diff -z --no-renames --name-only "$BASELINE" "$PUSHED_COMMIT" -- plugins > "$CHANGED_FILE"; 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 -d '' 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_FILE"
[[ ${#SKILL_DIRS[@]} -eq 0 ]] && exit 0
# Reads SKILL.md bytes on stdin and prints exactly one line: `OK <version>`
# when metadata.version is a valid three-part semver, `INVALID` when it is
# missing or malformed (including unparseable frontmatter). Any other outcome —
# python3 crashing, PyYAML failing to import — is a non-zero exit with no OK /
# INVALID line, which the caller reports as a read failure, never as a missing
# version. Bytes are decoded explicitly so the caller's locale cannot turn a
# non-ASCII SKILL.md into a crash; `\s*` before each `\n` absorbs CRLF.
read_version() {
python3 -c '
import re, sys, yaml
text = sys.stdin.buffer.read().decode("utf-8-sig", errors="replace")
m = re.match(r"---[ \t\r]*\n(.*?)\n---[ \t\r]*(\n|\Z)", text, re.S)
data = None
if m:
try:
data = yaml.safe_load(m.group(1))
except yaml.YAMLError:
data = None
meta = data.get("metadata") if isinstance(data, dict) else None
ver = meta.get("version") if isinstance(meta, dict) else None
ver = None if ver is None else str(ver).strip().strip("\x27\"")
if ver is not None and re.fullmatch(r"(0|[1-9][0-9]{0,8})\.(0|[1-9][0-9]{0,8})\.(0|[1-9][0-9]{0,8})", ver):
print("OK " + ver)
else:
print("INVALID")
'
}
# version_at <commit> <path>: sets VERSION to the valid version or "" when
# invalid. Exits the script on a read failure.
version_at() {
local out
if ! out="$(git show "$1:$2" | read_version)" || [[ "$out" != OK\ * && "$out" != INVALID ]]; then
echo "FAIL: could not read metadata.version from $1:$2 (see error above)." >&2
exit 1
fi
VERSION=""
[[ "$out" == OK\ * ]] && VERSION="${out#OK }"
return 0
}
# Exit 0 when $1 > $2, both MAJOR.MINOR.PATCH with parts of at most 9 ASCII
# digits and no leading zero (so bash never reads a part as octal), compared
# numerically so 1.0.10 > 1.0.9.
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 (( a[i] > b[i] )); then return 0; fi
if (( a[i] < b[i] )); then return 1; fi
done
return 1
}
MAIN_TIP="$(git rev-parse --verify -q "$MAIN_REF^{commit}")"
# in_tree <commit> <path>: the tree names <path>. Unlike `git cat-file -e`, it
# does not need the blob itself, so a blob a corrupt or partial clone lacks is a
# read failure in version_at, not a skill that silently looks absent.
in_tree() {
git rev-parse --verify -q "$1:$2" > /dev/null
}
OFFENDERS=()
for dir in ${SKILL_DIRS[@]+"${SKILL_DIRS[@]}"}; do
at_base=false
at_tip=false
in_tree "$BASELINE" "$dir/SKILL.md" && at_base=true
# When main has not moved since the merge-base, the tip is the same baseline.
[[ "$MAIN_TIP" != "$BASELINE" ]] && in_tree "$MAIN_TIP" "$dir/SKILL.md" && at_tip=true
# Absent at both baselines: new, renamed-to, or merged-into. Exempt.
$at_base || $at_tip || continue
# Directory absent at pushed commit: deleted or renamed-from. Exempt.
[[ "$(git cat-file -t "$PUSHED_COMMIT:$dir" 2>/dev/null)" == "tree" ]] || continue
base_ver=""
tip_ver=""
if $at_base; then version_at "$BASELINE" "$dir/SKILL.md"; base_ver="$VERSION"; fi
if $at_tip; then version_at "$MAIN_TIP" "$dir/SKILL.md"; tip_ver="$VERSION"; fi
if ! in_tree "$PUSHED_COMMIT" "$dir/SKILL.md"; then
OFFENDERS+=("$dir: SKILL.md missing at $PUSHED_REF (baseline: ${base_ver:-none})")
continue
fi
version_at "$PUSHED_COMMIT" "$dir/SKILL.md"
cur_ver="$VERSION"
if [[ -z "$cur_ver" ]]; then
OFFENDERS+=("$dir: metadata.version missing or not MAJOR.MINOR.PATCH at $PUSHED_REF (baseline: ${base_ver:-none})")
continue
fi
if [[ -n "$base_ver" ]] && ! semver_gt "$cur_ver" "$base_ver"; then
OFFENDERS+=("$dir: $base_ver -> $cur_ver (not above merge-base)")
fi
if [[ -n "$tip_ver" ]] && ! semver_gt "$cur_ver" "$tip_ver"; then
OFFENDERS+=("$dir: $tip_ver -> $cur_ver (not above $MAIN_REF tip)")
fi
done
if [[ ${#OFFENDERS[@]} -gt 0 ]]; then
echo "FAIL: skills changed since merge-base with $MAIN_REF without a metadata.version above both that merge-base and the $MAIN_REF tip (ADR-0022):" >&2
printf ' %s\n' ${OFFENDERS[@]+"${OFFENDERS[@]}"} >&2
echo " Fix: raise metadata.version in each SKILL.md above the baseline named — bump PATCH at minimum." >&2
exit 1
fi

View File

@@ -109,10 +109,9 @@ FAIL=0
# ZERO ARGUMENTS IS A USAGE ERROR, exit 2 — not a clean run.
#
# This hook is `pass_filenames: true` in both .pre-commit-config.yaml and
# .pre-commit-hooks.yaml, and pre-commit skips a filename-passing hook entirely
# when nothing matches its `files:` pattern, so it never invokes this script
# with an empty argument list. Every no-argument invocation therefore comes from
# This hook is `pass_filenames: true` in .pre-commit-config.yaml, and
# pre-commit skips a filename-passing hook entirely when nothing matches its
# `files:` pattern, so it never invokes this script with an empty argument list. Every no-argument invocation therefore comes from
# somewhere else — a hand-run command, a wrapper, or a `files:` pattern edited
# into matching nothing — and printing nothing and exiting 0 made all three
# indistinguishable from a clean corpus. A mis-scoped pattern would have
@@ -236,8 +235,8 @@ def info(msg):
# plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-boundary-resolver.sh
# The block between these markers must stay byte-identical in both. It is copied
# rather than imported because a cache-installed plugin's scripts cannot read
# files outside their own plugin directory, and this repo-root hook resolves via
# .pre-commit-hooks.yaml, where entry[0] is the only token pre-commit rewrites --
# files outside their own plugin directory, and this repo-root hook is kept fit for
# a published hook manifest (retired; ADR-0014), where only entry[0] is rewritten --
# so no single file is reachable by both (the same constraint that duplicates the
# ADR-0020 constants). Edit one copy, then paste it over the other.
#
@@ -1374,7 +1373,10 @@ for path in files:
if version_val is None:
error("%s: metadata.version field is missing (required frontmatter "
"field, e.g. \"1.0.0\")." % path)
elif not re.match(r'^\d+\.\d+\.\d+$', str(version_val).strip().strip('\'"')):
# Same shape as check-skill-version-bump.sh: ASCII digits, at most nine per
# part (bash arithmetic), no leading zero (semver 2.0.0 item 2).
elif not re.fullmatch(r'(0|[1-9][0-9]{0,8})\.(0|[1-9][0-9]{0,8})\.(0|[1-9][0-9]{0,8})',
str(version_val).strip().strip('\'"')):
error("%s: metadata.version is malformed (%r) -- expected a "
"three-part semver, e.g. \"1.0.0\"." % (path, version_val))

View File

@@ -9,10 +9,10 @@
# ADR-0025 merged skill-audit and agent-audit, which dropped the count from
# three copies to two: factory-audit now holds ONE copy in a sourced
# lib-boundary-resolver.sh, and scripts/skill-size-check.sh keeps its
# embedded copy because it is a repo-root hook consumed through
# .pre-commit-hooks.yaml, where entry[0] is the only token pre-commit
# rewrites — it cannot reach a file inside the plugin at a path any consumer
# has. Nothing but this file asserts the two copies are still identical, and
# embedded copy because it is a repo-root hook kept fit for a published
# hook manifest (retired; ADR-0014), where entry[0] is the only token
# pre-commit rewrites — it could not reach a file inside the plugin at a
# path any consumer has. Nothing but this file asserts the two copies are still identical, and
# a one-line edit to a single copy is invisible: every constant-agreement
# assertion in tests/test-skill-size-check.sh still passes, because the
# CONSTANTS are not what drifted.
@@ -590,14 +590,14 @@ for probe in "scripts/skill-size-check.sh|$HOOK|$SUBJECT_SKILL_DIR/SKILL.md" \
done
# ---------------------------------------------------------------------------
# 3. verbose: true on the skill-size-check hook, in BOTH manifests
# 3. verbose: true on the skill-size-check hook
# ---------------------------------------------------------------------------
# .pre-commit-config.yaml governs this repo; .pre-commit-hooks.yaml is what a
# CONSUMER repo gets when it points at this one. Dropping the flag from either
# silences the SUGGESTION tier for that audience alone, which is the hardest
# version of the defect to notice.
# pre-commit prints nothing for a passing hook, so dropping the flag silences
# the SUGGESTION tier without failing anything. The published
# .pre-commit-hooks.yaml that once carried a second copy of this hook was
# retired (ADR-0014, 2026-09-16 amendment); if it returns, assert it here too.
echo ""
echo "--- the skill-size-check hook declares verbose: true in both manifests ---"
echo "--- the skill-size-check hook declares verbose: true ---"
VERBOSE_REPORT="$(python3 - "$REPO_ROOT" <<'PY'
import os
import sys
@@ -632,27 +632,6 @@ else:
emit('FAIL', '.pre-commit-config.yaml: skill-size-check has verbose=%r — '
'pre-commit prints nothing for a passing hook, so every '
'ADR-0020 SUGGESTION is swallowed' % (found.get('verbose'),))
# Consumer manifest: a flat list of hooks.
path = os.path.join(root, '.pre-commit-hooks.yaml')
try:
with open(path, encoding='utf-8') as fh:
hooks = yaml.safe_load(fh) or []
except Exception as exc:
emit('FAIL', '.pre-commit-hooks.yaml did not parse: %s' % exc)
hooks = []
found = None
for hook in hooks:
if isinstance(hook, dict) and hook.get('id') == 'kyberforge-skill-size-check':
found = hook
if found is None:
emit('FAIL', '.pre-commit-hooks.yaml declares no hook with id kyberforge-skill-size-check')
elif found.get('verbose') is True:
emit('PASS', '.pre-commit-hooks.yaml: kyberforge-skill-size-check is verbose: true')
else:
emit('FAIL', '.pre-commit-hooks.yaml: kyberforge-skill-size-check has verbose=%r — '
'a consumer repo would never see the SUGGESTION tier'
% (found.get('verbose'),))
PY
)"
while IFS=$'\t' read -r status msg; do

View File

@@ -5,7 +5,7 @@
# and agent-audit behind one auto-detecting entry point; every fixture here is a
# skill directory, so every invocation below runs the skill flow. The agent flow
# has no counterpart hook to differ from — there is no agent-file size gate in
# .pre-commit-hooks.yaml — so it is out of this suite's scope, not dropped from it.
# .pre-commit-config.yaml — so it is out of this suite's scope, not dropped from it.
#
# Why this exists as a separate suite. tests/test-skill-size-check.sh already
# asserts the two agree on their CONSTANTS, and that assertion is necessary but

View File

@@ -21,6 +21,11 @@ command -v python3 > /dev/null 2>&1 || { echo "python3 required"; exit 77; }
FAKE_BIN="$(mktemp -d)"
WORK="$(mktemp -d)"
trap 'rm -rf "$FAKE_BIN" "$WORK"' EXIT
# The hook asks git which branch it is on. Stop git's discovery at the temp
# root so a $TMPDIR that happens to sit inside a checkout cannot leak a branch
# into the fixtures that are meant to be outside one.
export GIT_CEILING_DIRECTORIES
GIT_CEILING_DIRECTORIES="$(dirname "$WORK")"
# Mock `apm`. $1 chooses what `apm outdated` reports; $2 the exit code of
# `apm update`. Sentinel files record whether update was actually invoked and
@@ -124,6 +129,55 @@ else
fail "emits valid JSON"
fi
# ---------------------------------------------------------------------------
echo ""
echo "--- lock advice follows the branch ---"
# ---------------------------------------------------------------------------
# ADR-0019: on the default branch the rewritten lock is a real update to commit or
# discard; on a feature branch it is unrelated churn to discard. Outside a git
# checkout (the fixture above) the neutral advice stands.
advice_of() { json_field additionalContext <<< "$1"; }
grep -q "commit it or discard it deliberately" <<< "$(advice_of "$out")" \
&& pass "gives neutral lock advice outside a git checkout" \
|| fail "outside a git checkout the advice should stay neutral"
if command -v git > /dev/null 2>&1; then
REPO="$WORK/repo"
mkdir -p "$REPO"
git -C "$REPO" init -q -b main
git -C "$REPO" -c user.email=probe@example.invalid -c user.name=probe \
commit -q --allow-empty -m init
touch "$REPO/apm.lock.yaml"
out="$(run_hook_in "$REPO" "$REPO")"
grep -q "default branch, so commit it or discard it deliberately" <<< "$(advice_of "$out")" \
&& pass "on main, says to commit or discard the lock deliberately" \
|| fail "on main the advice should be commit-or-discard: $(advice_of "$out")"
git -C "$REPO" checkout -q -b feature/x
out="$(run_hook_in "$REPO" "$REPO")"
advice="$(advice_of "$out")"
grep -qF "feature branch, so discard it: git checkout -- apm.lock.yaml && apm install" <<< "$advice" \
&& pass "on a feature branch, says to discard the lock and reinstall" \
|| fail "on a feature branch the advice should be discard-and-install: $advice"
grep -q "commit it" <<< "$advice" \
&& fail "on a feature branch the advice must not suggest committing the lock" \
|| pass "on a feature branch, does not suggest committing the lock"
echo "$out" | python3 -m json.tool > /dev/null 2>&1 \
&& pass "feature-branch notice is valid JSON" || fail "feature-branch notice broke the JSON"
# A remote whose default branch is not `main` is honoured via origin/HEAD.
git -C "$REPO" update-ref refs/remotes/origin/feature/x HEAD
git -C "$REPO" symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/feature/x
out="$(run_hook_in "$REPO" "$REPO")"
grep -q "default branch, so commit it" <<< "$(advice_of "$out")" \
&& pass "reads the default branch from origin/HEAD when it is set" \
|| fail "should treat origin/HEAD's branch as the default: $(advice_of "$out")"
else
echo " (git not on PATH — branch-specific advice cases not run)"
fi
# ---------------------------------------------------------------------------
echo ""
echo "--- stale, refresh fails ---"

View File

@@ -1,449 +0,0 @@
#!/usr/bin/env bash
set -euo pipefail
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
SCRIPT="$REPO_ROOT/scripts/check-release-needed.sh"
PASS=0
FAIL=0
pass() { echo " PASS: $1"; PASS=$((PASS + 1)); }
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
# Output assertions are `grep -q PATTERN <<< "$OUT"`, never `echo "$OUT" | grep -q`.
# Under pipefail the pipe form is scheduling-dependent: bash's echo writes a
# multi-line value one line at a time, `grep -q` exits on its first match, and a
# later line then hits a closed pipe. echo dies of SIGPIPE, pipefail reports the
# pipeline as failed, and a correct output reads as a missing match. The
# here-string has no writer process to race.
# Both entry shapes the real .pre-commit-hooks.yaml ships: a bare script with no
# bundled data, and a bare script whose sibling assets/ tree it self-locates at
# runtime. Neither carries arguments — pre-commit only rewrites entry[0] to the
# hook-repo clone path, so an argument path would resolve against the consuming
# repo. RELEASE_PATHS is derived from the manifest rather than hand-maintained,
# so it has to cope with both.
HOOK_DIR="plugins/demo/skills/demo-audit"
write_manifest() {
local dir="$1"
cat > "$dir/.pre-commit-hooks.yaml" <<EOF
- id: fake-size-check
entry: scripts/skill-size-check.sh
language: script
- id: fake-vale-check
entry: $HOOK_DIR/scripts/vale-wrap.sh
language: script
EOF
}
# Helper: writes the files both manifest entries expose — the two hook scripts
# plus the bundled Vale config and style rule the second one self-locates.
write_release_paths() {
local dir="$1"
mkdir -p "$dir/scripts" "$dir/$HOOK_DIR/scripts" "$dir/$HOOK_DIR/assets/vale/styles/Kyberforge"
echo "v1" > "$dir/scripts/skill-size-check.sh"
echo "v1" > "$dir/$HOOK_DIR/scripts/vale-wrap.sh"
echo "cfg" > "$dir/$HOOK_DIR/assets/vale/.vale.ini"
echo "rule: v1" > "$dir/$HOOK_DIR/assets/vale/styles/Kyberforge/DemoRule.yml"
}
# Helper: a fixture repo with a manifest and every release-relevant path it
# exposes, committed and tagged v1.0.0.
make_tagged_fixture() {
local dir
dir="$(mktemp -d)"
(cd "$dir" && git init -q && git config user.email t@t.t && git config user.name t)
write_manifest "$dir"
write_release_paths "$dir"
(cd "$dir" && git add -A && git commit -q -m "initial" && git tag v1.0.0)
echo "$dir"
}
# Helper: a fixture whose manifest carries one malformed entry: at the tag *and*
# at HEAD, plus a post-tag change to the file that entry was meant to cover.
# Committing the bad entry before the tag is what makes the assertion sharp — an
# edited manifest is itself release-relevant, so the gate would fail for the
# wrong reason and hide a parser that degrades silently.
make_malformed_fixture() {
local entry="$1" dir
dir="$(mktemp -d)"
(cd "$dir" && git init -q && git config user.email t@t.t && git config user.name t)
write_release_paths "$dir"
cat > "$dir/.pre-commit-hooks.yaml" <<EOF
- id: fake-size-check
entry: $entry
language: script
EOF
(cd "$dir" && git add -A && git commit -q -m "initial" && git tag v1.0.0)
echo "v2" > "$dir/scripts/skill-size-check.sh"
(cd "$dir" && git add -A && git commit -q -m "change the file the malformed entry should cover")
echo "$dir"
}
# $3 is optional: pre-commit's PRE_COMMIT_TO_REF, the local sha being pushed.
# Left off entirely, the variable stays unset and the script falls back to HEAD,
# exactly as a plain `git push <remote> <current-branch>` behaves.
# The fixture repo is the subject under test, so every PRE_COMMIT_* input must
# come from this function and nowhere else. Any such variable already in the
# environment belongs to the *caller's* repo: run under the pre-push hook this
# suite guards, PRE_COMMIT_TO_REF holds a sha of the real repo, which does not
# exist in the fixture, and the script resolves against the wrong rev. Clearing
# them is what makes a standalone run and a pre-push run the same test — this
# suite passed everywhere except under the hook it exists to protect.
run_check() {
local dir="$1" branch="$2"
if [[ $# -ge 3 ]]; then
(cd "$dir" && unset PRE_COMMIT_FROM_REF \
&& PRE_COMMIT_REMOTE_BRANCH="$branch" PRE_COMMIT_TO_REF="$3" bash "$SCRIPT" 2>&1)
else
(cd "$dir" && unset PRE_COMMIT_FROM_REF PRE_COMMIT_TO_REF \
&& PRE_COMMIT_REMOTE_BRANCH="$branch" bash "$SCRIPT" 2>&1)
fi
}
CLEANUP_DIRS=()
trap 'rm -rf ${CLEANUP_DIRS[@]+"${CLEANUP_DIRS[@]}"}' EXIT
track() { CLEANUP_DIRS+=("$1"); }
# --- 1. Not targeting main: silent no-op regardless of state ---
echo ""
echo "--- exits 0 when not pushing to main, even with no tags ---"
FIXTURE1="$(mktemp -d)"; track "$FIXTURE1"
(cd "$FIXTURE1" && git init -q)
if run_check "$FIXTURE1" "refs/heads/feature-branch" > /dev/null; then
pass "exits 0 when target branch isn't main"
else
fail "exited non-zero on a non-main target branch"
fi
# --- 2. Targeting main, no tag exists at all: hard fail ---
echo ""
echo "--- exits 1 when targeting main and no tag exists ---"
FIXTURE2="$(mktemp -d)"; track "$FIXTURE2"
(cd "$FIXTURE2" && git init -q && git config user.email t@t.t && git config user.name t)
write_manifest "$FIXTURE2"
write_release_paths "$FIXTURE2"
(cd "$FIXTURE2" && git add -A && git commit -q -m "initial")
if run_check "$FIXTURE2" "refs/heads/main" > /dev/null; then
fail "exited 0 when targeting main with no tag — expected exit 1"
else
pass "exits non-zero when targeting main and no tag exists yet"
fi
# --- 3. Targeting main, tag exists, no release-relevant changes since: passes ---
echo ""
echo "--- exits 0 when targeting main and nothing release-relevant changed since the tag ---"
FIXTURE3="$(make_tagged_fixture)"; track "$FIXTURE3"
echo "unrelated" > "$FIXTURE3/README.md"
(cd "$FIXTURE3" && git add -A && git commit -q -m "unrelated change")
if run_check "$FIXTURE3" "refs/heads/main" > /dev/null; then
pass "exits 0 when only unrelated files changed since the tag"
else
fail "exited non-zero despite no release-relevant changes since the tag"
fi
# --- 4. Targeting main, tag exists, a release-relevant file changed since: hard fail ---
echo ""
echo "--- exits 1 when a release-relevant file changed since the tag ---"
FIXTURE4="$(make_tagged_fixture)"; track "$FIXTURE4"
echo "v2" > "$FIXTURE4/scripts/skill-size-check.sh"
(cd "$FIXTURE4" && git add -A && git commit -q -m "update release-relevant script")
OUT4=$(run_check "$FIXTURE4" "refs/heads/main" || true)
if grep -q "skill-size-check.sh" <<< "$OUT4"; then
pass "exits non-zero and names the changed file when a release-relevant path changed since the tag"
else
fail "did not flag the release-relevant file that changed since the tag"
fi
# --- 5. Not targeting main even with release-relevant changes and a tag: still a no-op ---
echo ""
echo "--- exits 0 on a feature branch even with release-relevant changes since the tag ---"
FIXTURE5="$(make_tagged_fixture)"; track "$FIXTURE5"
echo "v2" > "$FIXTURE5/scripts/skill-size-check.sh"
(cd "$FIXTURE5" && git add -A && git commit -q -m "update release-relevant script")
if run_check "$FIXTURE5" "refs/heads/some-feature" > /dev/null; then
pass "exits 0 on a feature branch regardless of un-tagged release-relevant changes"
else
fail "hard-failed on a feature branch — should only ever fail when targeting main"
fi
# --- 6. A release-relevant path deleted since the tag is still flagged ---
echo ""
echo "--- exits 1 when a release-relevant path was deleted since the tag, not just modified ---"
FIXTURE6="$(make_tagged_fixture)"; track "$FIXTURE6"
rm -f "$FIXTURE6/$HOOK_DIR/assets/vale/.vale.ini"
(cd "$FIXTURE6" && git add -A && git commit -q -m "delete the bundled vale config")
OUT6=$(run_check "$FIXTURE6" "refs/heads/main" || true)
if grep -q "assets/vale/.vale.ini" <<< "$OUT6"; then
pass "flags a deleted release-relevant path instead of silently dropping it from the diff"
else
fail "did not flag deletion of a release-relevant path since the tag"
fi
# --- 7. A git diff failure hard-fails instead of reading as a clean pass ---
echo ""
echo "--- exits 1 (not a silent pass) when the underlying git diff errors out ---"
FIXTURE7="$(make_tagged_fixture)"; track "$FIXTURE7"
TAG_TREE="$(cd "$FIXTURE7" && git rev-parse 'v1.0.0^{tree}')"
echo "v2" > "$FIXTURE7/scripts/skill-size-check.sh"
(cd "$FIXTURE7" && git add -A && git commit -q -m "advance past the tag")
rm -f "$FIXTURE7/.git/objects/${TAG_TREE:0:2}/${TAG_TREE:2}"
if run_check "$FIXTURE7" "refs/heads/main" > /dev/null; then
fail "silently exited 0 when the underlying git diff failed"
else
pass "hard-fails instead of silently passing when git diff can't be computed"
fi
# --- 8. A non-version tag reachable from HEAD does not become the diff baseline ---
echo ""
echo "--- ignores a non-vX.Y.Z tag and still flags a change since the real release tag ---"
FIXTURE8="$(make_tagged_fixture)"; track "$FIXTURE8"
echo "checkpoint" > "$FIXTURE8/scripts/skill-size-check.sh"
(cd "$FIXTURE8" && git add -A && git commit -q -m "checkpoint work" && git tag checkpoint-1)
echo "v2" > "$FIXTURE8/scripts/skill-size-check.sh"
(cd "$FIXTURE8" && git add -A && git commit -q -m "real release-relevant change")
OUT8=$(run_check "$FIXTURE8" "refs/heads/main" || true)
if grep -q "skill-size-check.sh" <<< "$OUT8"; then
pass "still flags the release-relevant change since v1.0.0, ignoring the non-version checkpoint tag"
else
fail "an incidental non-version tag shifted the baseline and hid a real release-relevant change"
fi
# --- 9. A file outside every manifest entry does not trigger a fail ---
echo ""
echo "--- exits 0 when a changed file sits near, but isn't referenced by, a manifest entry ---"
FIXTURE9="$(make_tagged_fixture)"; track "$FIXTURE9"
echo "irrelevant" > "$FIXTURE9/scripts/unrelated-helper.sh"
(cd "$FIXTURE9" && git add -A && git commit -q -m "add an unrelated script alongside the exposed one")
if run_check "$FIXTURE9" "refs/heads/main" > /dev/null; then
pass "exits 0 for a file that lives alongside, but isn't referenced by, any manifest entry"
else
fail "flagged a file that no .pre-commit-hooks.yaml entry actually exposes"
fi
# --- 10. A change confined to a hook's bundled styles/ tree is release-relevant ---
# The manifest entry names only the wrapper script; the Vale rules it enforces
# live in the sibling assets/ tree it self-locates at runtime. If that tree is
# not covered, editing a rule and landing it on main demands no new tag, and a
# consumer pinned to the old rev keeps the stale rules forever.
echo ""
echo "--- exits 1 when only a bundled Vale style rule changed since the tag ---"
FIXTURE10="$(make_tagged_fixture)"; track "$FIXTURE10"
echo "rule: v2" > "$FIXTURE10/$HOOK_DIR/assets/vale/styles/Kyberforge/DemoRule.yml"
(cd "$FIXTURE10" && git add -A && git commit -q -m "tighten a vale rule")
OUT10=$(run_check "$FIXTURE10" "refs/heads/main" || true)
if grep -q "assets/vale/styles/Kyberforge/DemoRule.yml" <<< "$OUT10"; then
pass "flags a change confined to a hook's bundled assets/vale/styles/ tree"
else
fail "a bundled Vale style rule changed since the tag without demanding a release"
fi
# --- 11. The assets/ derivation must not invent a path for a bundle-less hook ---
# scripts/skill-size-check.sh has no sibling assets/ tree, so its derived
# candidate normalises to a bare top-level assets/ — a directory this repo does
# not ship. Adding it unconditionally would make any unrelated repo-root
# assets/ file falsely demand a release.
echo ""
echo "--- exits 0 when a top-level assets/ file changed but no hook bundles one ---"
FIXTURE11="$(make_tagged_fixture)"; track "$FIXTURE11"
mkdir -p "$FIXTURE11/assets"
echo "unrelated" > "$FIXTURE11/assets/logo.txt"
(cd "$FIXTURE11" && git add -A && git commit -q -m "add an unrelated top-level assets file")
if run_check "$FIXTURE11" "refs/heads/main" > /dev/null; then
pass "exits 0 for a top-level assets/ file that no manifest entry bundles"
else
fail "invented a bogus assets/ path for a hook script with no bundled tree"
fi
# --- 12. Deleting a hook's entire bundled assets/ tree is release-relevant ---
# The worktree-only derivation guarded the assets/ path on the directory still
# existing, so wiping the whole tree removed the path from the pathspec instead
# of diffing it: the single most consumer-breaking change possible diffed clean.
# The path list therefore has to be unioned with what $LAST_TAG exposed.
echo ""
echo "--- exits 1 when a hook's entire bundled assets/ tree was deleted since the tag ---"
FIXTURE12="$(make_tagged_fixture)"; track "$FIXTURE12"
rm -rf "${FIXTURE12:?}/$HOOK_DIR/assets"
(cd "$FIXTURE12" && git add -A && git commit -q -m "delete the whole bundled assets tree")
OUT12=$(run_check "$FIXTURE12" "refs/heads/main" || true)
if grep -q "assets/vale/.vale.ini" <<< "$OUT12"; then
pass "flags a wholesale deletion of a hook's bundled assets/ tree"
else
fail "a hook's entire bundled assets/ tree vanished since the tag without demanding a release"
fi
# --- 13. A hook script deleted while its manifest entry survives is flagged ---
# Characterisation test, not a bug fix: tokens[0] is added to the pathspec
# unconditionally (no existence guard), so this case was already covered. It is
# pinned here so the tagged-tree union can't accidentally introduce an existence
# guard on tokens[0] and reopen the hole its assets/ sibling had.
echo ""
echo "--- exits 1 when a hook script was deleted but its manifest entry remains ---"
FIXTURE13="$(make_tagged_fixture)"; track "$FIXTURE13"
rm -f "$FIXTURE13/$HOOK_DIR/scripts/vale-wrap.sh"
(cd "$FIXTURE13" && git add -A && git commit -q -m "delete a hook script, keep its manifest entry")
OUT13=$(run_check "$FIXTURE13" "refs/heads/main" || true)
if grep -q "vale-wrap.sh" <<< "$OUT13"; then
pass "flags a hook script deleted out from under a surviving manifest entry"
else
fail "a manifest entry's script vanished since the tag without demanding a release"
fi
# --- 14. Retiring a whole hook names what the tag exposed, not just the manifest ---
# Removing the entry and everything it shipped changes $HOOKS_MANIFEST, so the
# gate fires either way — but a derivation that only reads the current manifest
# can no longer name the retired script or its assets, and the failure message
# understates the breakage to consumers pinned at the old rev. The tagged
# manifest is what makes those paths reportable.
echo ""
echo "--- names the retired hook's own paths when an entry and its files are removed together ---"
FIXTURE14="$(make_tagged_fixture)"; track "$FIXTURE14"
cat > "$FIXTURE14/.pre-commit-hooks.yaml" <<'EOF'
- id: fake-size-check
entry: scripts/skill-size-check.sh
language: script
EOF
rm -rf "${FIXTURE14:?}/$HOOK_DIR"
(cd "$FIXTURE14" && git add -A && git commit -q -m "retire the vale hook entirely")
OUT14=$(run_check "$FIXTURE14" "refs/heads/main" || true)
if grep -q "vale-wrap.sh" <<< "$OUT14" && grep -q "assets/vale/.vale.ini" <<< "$OUT14"; then
pass "names the retired hook's script and bundled assets, not just the manifest edit"
else
fail "reported only the manifest change and hid which shipped paths the retirement removed"
fi
# --- 15. A multi-token entry: is rejected loudly, not silently mis-parsed ---
# ADR-0014 binds entries to a bare script path, but nothing enforced it, and the
# sibling .pre-commit-config.yaml already ships `entry: bash <script>`. Under the
# old parser tokens[0] became "bash": a pathspec matching nothing (which git diff
# accepts in silence) and a bundle root of "." (skipped), so the hook's whole
# surface dropped out of the gate and the post-tag change below diffed clean.
echo ""
echo "--- exits 1 naming the hook when an entry: carries more than one token ---"
# The entry is quoted back verbatim, not just its first token: that is what makes
# the diagnostic point at the argument the author has to remove, and what
# distinguishes this from the unresolvable-path rejection test 16 covers.
FIXTURE15="$(make_malformed_fixture "bash scripts/skill-size-check.sh")"; track "$FIXTURE15"
OUT15=$(run_check "$FIXTURE15" "refs/heads/main" || true)
if run_check "$FIXTURE15" "refs/heads/main" > /dev/null; then
fail "silently exited 0 on a multi-token entry, dropping that hook's paths from the gate"
elif grep -q "fake-size-check" <<< "$OUT15" \
&& grep -q "bash scripts/skill-size-check.sh" <<< "$OUT15" \
&& grep -q "ADR-0014" <<< "$OUT15"; then
pass "rejects a multi-token entry, quoting it back and naming the hook and ADR-0014"
else
fail "rejected the multi-token entry without naming the hook, the entry, and ADR-0014"
fi
# --- 16. An entry naming no file this repo ships is rejected loudly ---
# The token-count guard alone still lets a single bare command name (`entry:
# vale`, valid for language: system) through as a pathspec matching nothing.
# Existence is checked against the union of the worktree and $LAST_TAG, so this
# cannot misfire on the deletion cases tests 12-14 pin.
echo ""
echo "--- exits 1 naming the hook when an entry: names no file in the worktree or at the tag ---"
FIXTURE16="$(make_malformed_fixture "vale")"; track "$FIXTURE16"
OUT16=$(run_check "$FIXTURE16" "refs/heads/main" || true)
if run_check "$FIXTURE16" "refs/heads/main" > /dev/null; then
fail "silently exited 0 on an entry that names no shipped file"
elif grep -q "fake-size-check" <<< "$OUT16" && grep -q "ADR-0014" <<< "$OUT16"; then
pass "rejects an entry that resolves to no file, naming the hook and the ADR-0014 constraint"
else
fail "rejected the unresolvable entry without naming the hook and the ADR-0014 constraint"
fi
# --- 17. The pushed ref, not HEAD, is what gets gated ---
# pre-commit exports the local sha of each pushed ref as PRE_COMMIT_TO_REF.
# `git push <remote> pushed-tip:main` from a checkout sitting on an older commit
# is the false-negative direction: HEAD is still at the tag and diffs clean while
# the branch actually landing on main carries an untagged, release-relevant
# change. HEAD is reset back to the tag so the two genuinely differ.
echo ""
echo "--- exits 1 on a release-relevant change reachable only from PRE_COMMIT_TO_REF ---"
FIXTURE17="$(make_tagged_fixture)"; track "$FIXTURE17"
echo "v2" > "$FIXTURE17/scripts/skill-size-check.sh"
(cd "$FIXTURE17" && git add -A && git commit -q -m "release-relevant change" \
&& git branch pushed-tip && git reset -q --hard v1.0.0)
OUT17=$(run_check "$FIXTURE17" "refs/heads/main" "pushed-tip" || true)
if grep -q "skill-size-check.sh" <<< "$OUT17"; then
pass "gates the pushed ref's tip, not HEAD, when HEAD is behind it"
else
fail "diffed HEAD instead of PRE_COMMIT_TO_REF and missed a release-relevant change"
fi
# --- 18. Neither the diff tip nor the tag baseline may come from a newer HEAD ---
# The false-positive direction: HEAD has moved past a v2.0.0 that the pushed ref
# never saw. Reading either end of the diff off HEAD fails a push that is clean
# since its own baseline — diffing v2.0.0..HEAD flags HEAD's untagged commit, and
# resolving the tag from HEAD while diffing pushed-tip flags v2.0.0's change.
echo ""
echo "--- exits 0 when the pushed ref is clean since its own tag but HEAD has moved on ---"
FIXTURE18="$(make_tagged_fixture)"; track "$FIXTURE18"
(cd "$FIXTURE18" && git branch pushed-tip)
echo "v2" > "$FIXTURE18/scripts/skill-size-check.sh"
(cd "$FIXTURE18" && git add -A && git commit -q -m "released change" && git tag v2.0.0)
echo "v3" > "$FIXTURE18/scripts/skill-size-check.sh"
(cd "$FIXTURE18" && git add -A && git commit -q -m "unreleased change on HEAD's line")
if run_check "$FIXTURE18" "refs/heads/main" "pushed-tip" > /dev/null; then
pass "exits 0 for a pushed ref clean since the tag reachable from it, ignoring HEAD's line"
else
fail "gated HEAD's tag or tip and falsely demanded a release for a clean pushed ref"
fi
# --- 19. A branch deletion is a no-op, not a confusing git failure ---
# pre-commit sets PRE_COMMIT_TO_REF to an all-zeros sha when the push deletes a
# branch. Nothing is being shipped, and the sha resolves to nothing, so without
# an explicit guard the gate reports "could not diff" on an unrelated operation.
echo ""
echo "--- exits 0 when PRE_COMMIT_TO_REF is the all-zeros branch-deletion sha ---"
FIXTURE19="$(make_tagged_fixture)"; track "$FIXTURE19"
echo "v2" > "$FIXTURE19/scripts/skill-size-check.sh"
(cd "$FIXTURE19" && git add -A && git commit -q -m "release-relevant change")
if run_check "$FIXTURE19" "refs/heads/main" "0000000000000000000000000000000000000000" > /dev/null; then
pass "treats an all-zeros PRE_COMMIT_TO_REF as a branch deletion and exits 0"
else
fail "turned a branch deletion into a failure instead of a no-op"
fi
# --- 20. The repo's own .pre-commit-hooks.yaml satisfies the entry constraints ---
# The parser guards above are only safe to ship if the manifest actually in tree
# passes them. It is replayed into a fixture (with the paths its entries name
# created) rather than run against the real repo, which has no release tag yet.
echo ""
echo "--- accepts the real .pre-commit-hooks.yaml this repo ships ---"
FIXTURE20="$(mktemp -d)"; track "$FIXTURE20"
(cd "$FIXTURE20" && git init -q && git config user.email t@t.t && git config user.name t)
cp "$REPO_ROOT/.pre-commit-hooks.yaml" "$FIXTURE20/.pre-commit-hooks.yaml"
while IFS= read -r real_entry; do
mkdir -p "$FIXTURE20/$(dirname "$real_entry")"
echo "v1" > "$FIXTURE20/$real_entry"
done < <(sed -n 's/^[[:space:]]*entry:[[:space:]]*//p' "$REPO_ROOT/.pre-commit-hooks.yaml")
(cd "$FIXTURE20" && git add -A && git commit -q -m "initial" && git tag v1.0.0)
OUT20=$(run_check "$FIXTURE20" "refs/heads/main" || true)
if [[ -z "$OUT20" ]]; then
pass "parses every entry in the repo's real .pre-commit-hooks.yaml without complaint"
else
fail "the repo's own .pre-commit-hooks.yaml no longer satisfies the entry constraints: $OUT20"
fi
# --- 21. A vX.Y.Z-suffixed checkpoint tag must not satisfy the release gate ---
# git describe --match uses shell-glob semantics, not regex: the trailing `*` in
# 'v[0-9]*.[0-9]*.[0-9]*' matches any suffix, so a pre-release/checkpoint tag like
# v1.0.1-checkpoint also satisfies the glob and can be picked as LAST_TAG instead
# of the true last release tag — hiding a real release-relevant change that landed
# before the checkpoint tag from the diff.
echo ""
echo "--- ignores a vX.Y.Z-checkpoint tag and still flags the change since the real release tag ---"
FIXTURE21="$(make_tagged_fixture)"; track "$FIXTURE21"
echo "v2" > "$FIXTURE21/scripts/skill-size-check.sh"
(cd "$FIXTURE21" && git add -A && git commit -q -m "real release-relevant change" && git tag v1.0.1-checkpoint)
OUT21=$(run_check "$FIXTURE21" "refs/heads/main" || true)
if grep -q "skill-size-check.sh" <<< "$OUT21"; then
pass "still flags the release-relevant change since v1.0.0, ignoring the vX.Y.Z-checkpoint tag"
else
fail "a vX.Y.Z-checkpoint tag satisfied the glob and hid a real release-relevant change"
fi
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]]

View File

@@ -738,8 +738,8 @@ expect_gate "a fixture with no authoring root reports DID NOT RUN and exits 0" \
# provider-adapter-author's validate-adapter.sh and the one vale-wrap.sh already
# used: {0,1} are verdicts, 2 is "you invoked this wrong".
#
# SAFE FOR THE HOOK. Both manifests declare pass_filenames: true and neither
# sets always_run, and pre-commit skips a filename-passing hook outright when
# SAFE FOR THE HOOK. The skill-size-check hook declares pass_filenames: true
# and does not set always_run, and pre-commit skips a filename-passing hook outright when
# its `files:` pattern matches nothing, so pre-commit never invokes this script
# with an empty argument list. That claim is asserted below rather than left in
# prose, so a config edit that turns it false fails here.
@@ -773,7 +773,7 @@ if [[ $CLEAN_RC -eq 0 && $FINDING_RC -eq 1 && $USAGE_RC -eq 2 ]]; then
else
fail "exit codes collide — clean=$CLEAN_RC findings=$FINDING_RC usage=$USAGE_RC"
fi
# The hook contract the usage exit depends on. If either manifest ever stops
# The hook contract the usage exit depends on. If the hook ever stops
# passing filenames, or starts always_run, pre-commit could invoke the script
# with no paths and exit 2 would break the hook rather than diagnose a caller.
HOOK_CONTRACT="$(python3 - "$REPO_ROOT" <<'PYHOOK'
@@ -805,19 +805,11 @@ for repo in cfg.get('repos') or []:
found = hook
check('.pre-commit-config.yaml skill-size-check', found)
with open(os.path.join(root, '.pre-commit-hooks.yaml'), encoding='utf-8') as fh:
hooks = yaml.safe_load(fh) or []
found = None
for hook in hooks:
if isinstance(hook, dict) and hook.get('id') == 'kyberforge-skill-size-check':
found = hook
check('.pre-commit-hooks.yaml kyberforge-skill-size-check', found)
print('; '.join(problems))
PYHOOK
)"
if [[ -z "$HOOK_CONTRACT" ]]; then
pass "both manifests pass filenames and neither is always_run, so pre-commit never invokes the script with no paths"
pass "the hook passes filenames and is not always_run, so pre-commit never invokes the script with no paths"
else
fail "the usage exit would break the hook: $HOOK_CONTRACT"
fi
@@ -899,6 +891,45 @@ else
pass "the SUGGESTION survives LC_ALL=C, streams pinned to UTF-8"
fi
# ---------------------------------------------------------------------------
# metadata.version shape: no leading zeros, matching check-skill-version-bump
# ---------------------------------------------------------------------------
echo ""
echo "--- metadata.version with a leading zero is malformed ---"
for version_case in "1.0.08:malformed" "01.0.1:malformed" "1.0.10:valid" "0.1.0:valid"; do
version="${version_case%%:*}"
expected="${version_case##*:}"
VERSION_SKILL="$TMPDIR/version-$version"
mkdir -p "$VERSION_SKILL"
cat > "$VERSION_SKILL/SKILL.md" <<VERSIONEOF
---
name: version-skill
description: A valid skill description that is well within the limit.
metadata:
version: "$version"
---
## Step 1
Do the thing.
VERSIONEOF
set +e
VERSION_OUT="$("$SCRIPT" "$VERSION_SKILL/SKILL.md" 2>&1)"
VERSION_STATUS=$?
set -e
if [[ "$expected" == malformed ]]; then
if [[ $VERSION_STATUS -ne 0 && "$VERSION_OUT" == *"metadata.version is malformed ('$version')"* ]]; then
pass "'$version' is rejected as malformed"
else
fail "'$version' was not rejected as malformed (exit $VERSION_STATUS): ${VERSION_OUT:-<empty>}"
fi
elif [[ "$VERSION_OUT" == *"metadata.version is malformed"* ]]; then
fail "'$version' was wrongly rejected as malformed: $VERSION_OUT"
else
pass "'$version' is accepted"
fi
done
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]]

572
tests/test-skill-version-bump.sh Executable file
View File

@@ -0,0 +1,572 @@
#!/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 docs/spec/gates.md, Tests).
CLEANUP_DIRS=()
trap 'rm -rf ${CLEANUP_DIRS[@]+"${CLEANUP_DIRS[@]}"}' EXIT
# write_skill <repo> <plugin> <skill> <version-line|""> [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 <desc> <dir> [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 <desc> <pattern> <dir> [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)
echo "unrelated" > "$F/README.md"; commit "$F"
CLEAN="$(cd "$F" && git rev-parse HEAD)"
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" "$CLEAN"
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. pushed version must also exceed 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_fail "bump over the merge-base fails when main's tip is higher" \
"alpha: 1\.0\.5 -> 1\.0\.1 \(not above main tip\)" "$F"
OUT="$(run_check "$F" || true)"
if grep -q "not above merge-base" <<< "$OUT"; then
fail "merge-base reported as failed although 1.0.1 > 1.0.0: $OUT"
else
pass "only the baseline actually failed is named"
fi
write_skill "$F" demo alpha 'version: "1.0.6"' "branch change 2"; commit "$F"
expect_pass "bump above both the merge-base and main's tip passes" "$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"
expect_fail "the report names alpha and the versions read from origin/main's merge-base" \
"demo/\.apm/skills/alpha: 1\.0\.1 -> 1\.0\.1 \(not above merge-base\)" "$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 "--- 20. non-ASCII paths are not hidden by core.quotePath ---"
F="$(make_fixture)"
mkdir -p "$F/plugins/demo/.apm/skills/alpha/references"
echo "ref" > "$F/plugins/demo/.apm/skills/alpha/references/résumé.md"; commit "$F"
expect_fail "non-ASCII file under references/ without bump fails" "alpha: 1\.0\.0 -> 1\.0\.0" "$F"
F="$(make_fixture)"
(cd "$F" && git checkout -q main)
write_skill "$F" demo "café" 'version: "2.0.0"'; commit "$F" "add café"
(cd "$F" && git checkout -q feature && git merge -q main)
write_skill "$F" demo "café" 'version: "2.0.0"' "changed"; commit "$F"
expect_fail "non-ASCII skill dir without bump fails" "skills/café: 2\.0\.0 -> 2\.0\.0" "$F"
write_skill "$F" demo "café" 'version: "2.0.1"' "changed again"; commit "$F"
expect_pass "non-ASCII skill dir with bump passes" "$F"
echo ""
echo "--- 21. local main fallback: pushed commit already in main fails closed ---"
F="$(make_fixture)"
MAIN_SHA="$(cd "$F" && git rev-parse main)"
expect_fail "pushing main's sha without origin/main fails" \
"origin/main does not resolve.*already contained in local main" "$F" "$MAIN_SHA"
(cd "$F" && git checkout -q main)
expect_fail "HEAD on main without origin/main fails" \
"origin/main does not resolve and HEAD is already contained in local main" "$F"
echo ""
echo "--- 22. pushing main itself with origin/main present ---"
F="$(make_fixture)"
(cd "$F" && git checkout -q main && git update-ref refs/remotes/origin/main main)
expect_pass "main equal to origin/main passes (nothing changed vs main)" "$F"
write_skill "$F" demo alpha 'version: "1.0.1"' "x"; commit "$F"
expect_pass "main ahead of origin/main with a bump passes" "$F"
write_skill "$F" demo beta "version: 1.0.9" "x"; commit "$F"
expect_fail "main ahead of origin/main without a bump fails" "beta: 1\.0\.9 -> 1\.0\.9" "$F"
(cd "$F" && git update-ref refs/remotes/origin/main main)
expect_pass "already-merged content (merge-base == pushed) passes against origin/main" "$F" \
"$(cd "$F" && git rev-parse main~1)"
echo ""
echo "--- 23. version shape is ASCII-only, bounded, and has no leading zeros ---"
for v in 'version: "1.0.1"' 'version: "1.0.1"' 'version: "1.0.9999999999"' \
'version: "99999999999999999999.0.0"' 'version: "1.0.08"' 'version: "01.0.1"' \
'version: "1.00.1"'; do
F="$(make_fixture)"
write_skill "$F" demo alpha "$v" "new body"; commit "$F"
OUT="$(run_check "$F" || true)"
if grep -q "alpha: metadata\.version missing or not" <<< "$OUT" \
&& ! grep -qiE "integer|syntax error|value too great" <<< "$OUT"; then
pass "'$v' is rejected as invalid without a bash arithmetic error"
else
fail "'$v' not cleanly rejected: $OUT"
fi
done
F="$(make_fixture)"
write_skill "$F" demo alpha 'version: "1.0.999999999"' "new body"; commit "$F"
expect_pass "nine-digit part is accepted and compared" "$F"
F="$(make_fixture)"
write_skill "$F" demo alpha 'version: "1.0.10"' "new body"; commit "$F"
expect_pass "a zero inside a part (1.0.10) is not a leading zero" "$F"
echo ""
echo "--- 24. python3 / PyYAML failures are never reported as a missing version ---"
REAL_PYTHON="$(command -v python3)"
SHIM_ROOT="$(mktemp -d)"; CLEANUP_DIRS+=("$SHIM_ROOT")
mkdir -p "$SHIM_ROOT/shadow" "$SHIM_ROOT/noyaml" "$SHIM_ROOT/crash"
printf 'raise ImportError("PyYAML deliberately unavailable in this fixture")\n' \
> "$SHIM_ROOT/shadow/yaml.py"
cat > "$SHIM_ROOT/noyaml/python3" <<EOF
#!/bin/sh
PYTHONPATH="$SHIM_ROOT/shadow\${PYTHONPATH:+:\$PYTHONPATH}" exec "$REAL_PYTHON" "\$@"
EOF
# Passes the up-front import probe, crashes on the real read.
cat > "$SHIM_ROOT/crash/python3" <<EOF
#!/bin/sh
[ "\$1" = "-c" ] && [ "\$2" = "import yaml" ] && exec "$REAL_PYTHON" "\$@"
echo "Traceback: simulated interpreter failure" >&2
exit 1
EOF
chmod +x "$SHIM_ROOT/noyaml/python3" "$SHIM_ROOT/crash/python3"
if PATH="$SHIM_ROOT/noyaml:$PATH" python3 -c 'import yaml' 2>/dev/null; then
fail "fixture check: the no-PyYAML shim still imports yaml — the next assertion would be vacuous"
else
pass "fixture check: the no-PyYAML shim makes 'import yaml' fail"
fi
F="$(make_fixture)"
write_skill "$F" demo alpha 'version: "1.0.1"' "new body"; commit "$F"
OUT="$(cd "$F" && unset PRE_COMMIT_FROM_REF PRE_COMMIT_TO_REF PRE_COMMIT_REMOTE_BRANCH \
&& PATH="$SHIM_ROOT/noyaml:$PATH" bash "$SCRIPT" 2>&1)" && RC=0 || RC=$?
if [[ $RC -ne 0 ]] && grep -q "PyYAML is required" <<< "$OUT" && grep -q "Fix: python3 -m pip install PyYAML" <<< "$OUT" \
&& ! grep -q "Traceback" <<< "$OUT"; then
pass "missing PyYAML fails with FAIL + Fix, no traceback"
else
fail "missing PyYAML not reported cleanly (rc=$RC): $OUT"
fi
OUT="$(cd "$F" && unset PRE_COMMIT_FROM_REF PRE_COMMIT_TO_REF PRE_COMMIT_REMOTE_BRANCH \
&& PATH="$SHIM_ROOT/crash:$PATH" bash "$SCRIPT" 2>&1)" && RC=0 || RC=$?
if [[ $RC -ne 0 ]] && grep -q "could not read metadata.version" <<< "$OUT" \
&& ! grep -q "missing or not" <<< "$OUT"; then
pass "python3 crash during the read is a read failure, not a missing version"
else
fail "python3 crash misreported (rc=$RC): $OUT"
fi
echo ""
echo "--- 25. SKILL.md deleted but skill dir kept ---"
F="$(make_fixture)"
mkdir -p "$F/plugins/demo/.apm/skills/alpha/references"
echo "ref" > "$F/plugins/demo/.apm/skills/alpha/references/x.md"
rm "$F/plugins/demo/.apm/skills/alpha/SKILL.md"; commit "$F"
expect_fail "missing SKILL.md is reported as such" "alpha: SKILL\.md missing at HEAD \(baseline: 1\.0\.0\)" "$F"
echo ""
echo "--- 26. baseline without a valid version accepts any valid version ---"
F="$(make_fixture)"
(cd "$F" && git checkout -q main)
write_skill "$F" demo legacy ""; commit "$F" "add legacy skill"
(cd "$F" && git checkout -q feature && git merge -q main)
write_skill "$F" demo legacy 'version: "0.0.1"' "changed"; commit "$F"
expect_pass "invalid baseline + valid current passes" "$F"
echo ""
echo "--- 27. no merge-base (unrelated histories) fails closed ---"
F="$(make_fixture)"
(cd "$F" && git checkout -q --orphan unrelated && git rm -rq --cached . && rm -rf plugins)
write_skill "$F" demo alpha 'version: "1.0.0"' "orphan"; commit "$F" "orphan root"
expect_fail "unrelated history fails at the merge-base check" "no merge-base between main and HEAD" "$F"
echo ""
echo "--- 28. annotated tag objects as PRE_COMMIT_TO_REF ---"
F="$(make_fixture)"
write_skill "$F" demo alpha 'version: "1.0.0"' "unbumped"; commit "$F"
(cd "$F" && git tag -a v9 -m "tag" && git checkout -q main)
TAG_OBJ="$(cd "$F" && git rev-parse v9)"
if [[ "$(cd "$F" && git cat-file -t "$TAG_OBJ")" == "tag" ]]; then
pass "fixture check: TO_REF is a tag object, not a commit"
else
fail "fixture check: v9 is not an annotated tag object"
fi
expect_fail "unbumped change behind an annotated tag fails" "alpha: 1\.0\.0 -> 1\.0\.0" "$F" "$TAG_OBJ"
# Peeling is what makes the local-main fallback's "pushed commit is the
# merge-base" test see through a tag: compared unpeeled, the tag's own sha
# never equals the merge-base and the empty diff would pass.
(cd "$F" && git tag -a on-main -m "tag" main)
expect_fail "a tag on local main's commit fails closed like the commit itself" \
"origin/main does not resolve and [0-9a-f]+ is already contained in local main" \
"$F" "$(cd "$F" && git rev-parse on-main)"
(cd "$F" && git tag -a tree-tag -m "tag" "main^{tree}")
expect_fail "a tag on a tree fails closed" "pushed ref [0-9a-f]+ does not resolve to a commit" \
"$F" "$(cd "$F" && git rev-parse tree-tag)"
echo ""
echo "--- 29. CRLF frontmatter is parsed ---"
# crlf <repo> <skill>: rewrite that skill's SKILL.md with CRLF line endings.
crlf() {
local f="$1/plugins/demo/.apm/skills/$2/SKILL.md"
sed 's/$/\r/' "$f" > "$f.tmp" && mv "$f.tmp" "$f"
}
F="$(make_fixture)"
(cd "$F" && git config core.autocrlf false && git checkout -q main)
crlf "$F" alpha; commit "$F" "alpha to CRLF"
(cd "$F" && git checkout -q feature && git merge -q main)
write_skill "$F" demo alpha 'version: "1.0.0"' "crlf body"; crlf "$F" alpha; commit "$F"
if grep -q $'\r' "$F/plugins/demo/.apm/skills/alpha/SKILL.md"; then
pass "fixture check: SKILL.md carries CRLF"
else
fail "fixture check: SKILL.md has no CRLF"
fi
expect_fail "unbumped CRLF skill reports both parsed versions" "alpha: 1\.0\.0 -> 1\.0\.0" "$F"
write_skill "$F" demo alpha 'version: "1.0.1"' "crlf body 2"; crlf "$F" alpha; commit "$F"
expect_pass "bumped CRLF skill passes" "$F"
echo ""
echo "--- 30. identical bump already merged to main fails ---"
# Branches A and B both bump alpha 1.0.0 -> 1.0.1 with different content. The
# bumps do not conflict at merge, so without the tip rule main would ship two
# changes under one version.
F="$(make_fixture)"
(cd "$F" && git checkout -q -b branch-a main)
write_skill "$F" demo alpha 'version: "1.0.1"' "change A"; commit "$F"
(cd "$F" && git checkout -q main && git merge -q --no-ff -m "merge A" branch-a \
&& git update-ref refs/remotes/origin/main main && git checkout -q feature)
write_skill "$F" demo alpha 'version: "1.0.1"' "change B"; commit "$F"
expect_fail "B's 1.0.1 fails against A's 1.0.1 on origin/main" \
"alpha: 1\.0\.1 -> 1\.0\.1 \(not above origin/main tip\)" "$F"
write_skill "$F" demo alpha 'version: "1.0.2"' "change B 2"; commit "$F"
expect_pass "B at 1.0.2 passes" "$F"
echo ""
echo "--- 31. skill deleted on main's tip: only the merge-base rule applies ---"
F="$(make_fixture)"
(cd "$F" && git checkout -q main)
rm -rf "$F/plugins/demo/.apm/skills/alpha"; commit "$F" "drop alpha on main"
(cd "$F" && git checkout -q feature)
write_skill "$F" demo alpha 'version: "1.0.1"' "branch change"; commit "$F"
expect_pass "bump over the merge-base passes when main's tip lacks the skill" "$F"
(cd "$F" && git checkout -q -b unbumped main~1)
write_skill "$F" demo alpha 'version: "1.0.0"' "unbumped"; commit "$F"
expect_fail "unbumped change still fails against the merge-base" \
"alpha: 1\.0\.0 -> 1\.0\.0 \(not above merge-base\)" "$F"
echo ""
echo "--- 32. UTF-8 BOM before the frontmatter is parsed ---"
# bom <repo> <skill>: prefix that skill's SKILL.md with a UTF-8 byte-order mark.
bom() {
local f="$1/plugins/demo/.apm/skills/$2/SKILL.md"
{ printf '\xef\xbb\xbf'; cat "$f"; } > "$f.tmp" && mv "$f.tmp" "$f"
}
F="$(make_fixture)"
write_skill "$F" demo alpha 'version: "1.0.0"' "bom body"; bom "$F" alpha; commit "$F"
if [[ "$(head -c 3 "$F/plugins/demo/.apm/skills/alpha/SKILL.md" | od -An -tx1 | tr -d ' ')" == "efbbbf" ]]; then
pass "fixture check: SKILL.md starts with a BOM"
else
fail "fixture check: SKILL.md has no BOM"
fi
expect_fail "unbumped BOM skill reports its parsed version, not a missing one" \
"alpha: 1\.0\.0 -> 1\.0\.0 \(not above merge-base\)" "$F"
write_skill "$F" demo alpha 'version: "1.0.1"' "bom body 2"; bom "$F" alpha; commit "$F"
expect_pass "bumped BOM skill passes" "$F"
echo ""
echo "--- 33. a file moved from one skill to another flags both ---"
# --no-renames: with rename detection, --name-only lists only the new path and
# alpha would lose a file without anyone noticing.
F="$(make_fixture)"
(cd "$F" && git checkout -q main)
mkdir -p "$F/plugins/demo/.apm/skills/alpha/references"
printf 'ref line %s\n' 1 2 3 4 5 > "$F/plugins/demo/.apm/skills/alpha/references/x.md"
commit "$F" "alpha reference"
(cd "$F" && git checkout -q feature && git merge -q main)
mkdir -p "$F/plugins/demo/.apm/skills/beta/references"
(cd "$F" && git mv plugins/demo/.apm/skills/alpha/references/x.md plugins/demo/.apm/skills/beta/references/x.md)
write_skill "$F" demo beta "version: 1.0.10"; commit "$F"
expect_fail "alpha is flagged although only beta was bumped" \
"alpha: 1\.0\.0 -> 1\.0\.0 \(not above merge-base\)" "$F"
echo ""
echo "--- 34. comparison is ordered major first ---"
F="$(make_fixture)"
(cd "$F" && git checkout -q main)
write_skill "$F" demo alpha 'version: "2.0.0"'; commit "$F" "alpha 2.0.0"
(cd "$F" && git checkout -q feature && git merge -q main)
write_skill "$F" demo alpha 'version: "1.9.0"' "new body"; commit "$F"
expect_fail "2.0.0 -> 1.9.0 fails although minor rose" "alpha: 2\.0\.0 -> 1\.9\.0" "$F"
echo ""
echo "--- 35. unresolvable PRE_COMMIT_TO_REF fails closed ---"
F="$(make_fixture)"
expect_fail "a sha absent from the repo fails" \
"pushed ref 1234567890abcdef1234567890abcdef12345678 does not resolve to a commit" \
"$F" "1234567890abcdef1234567890abcdef12345678"
echo ""
echo "--- 36. a SKILL.md git cannot read fails closed ---"
# drop_blob <repo> <rev:path>: delete that blob's loose object, as a corrupt or
# partial clone would lack it. The tree still names the file.
drop_blob() {
local sha
sha="$(cd "$1" && git rev-parse "$2")"
rm -f "$1/.git/objects/${sha:0:2}/${sha:2}"
}
F="$(make_fixture)"
write_skill "$F" demo alpha 'version: "1.0.1"' "new body"; commit "$F"
drop_blob "$F" "HEAD:plugins/demo/.apm/skills/alpha/SKILL.md"
OUT="$(run_check "$F")" && RC=0 || RC=$?
if [[ $RC -ne 0 ]] && grep -q "could not read metadata.version from [0-9a-f]*:plugins/demo/.apm/skills/alpha/SKILL.md" <<< "$OUT" \
&& grep -q "fatal: bad object" <<< "$OUT" && ! grep -qE "missing or not|SKILL\.md missing" <<< "$OUT"; then
pass "an unreadable pushed SKILL.md is a read failure carrying git's error"
else
fail "unreadable pushed SKILL.md misreported (rc=$RC): $OUT"
fi
F="$(make_fixture)"
write_skill "$F" demo alpha 'version: "1.0.1"' "new body"; commit "$F"
drop_blob "$F" "main:plugins/demo/.apm/skills/alpha/SKILL.md"
OUT="$(run_check "$F")" && RC=0 || RC=$?
if [[ $RC -ne 0 ]] && grep -q "could not read metadata.version" <<< "$OUT"; then
pass "an unreadable merge-base SKILL.md fails closed instead of exempting the skill"
else
fail "unreadable merge-base SKILL.md not caught (rc=$RC): $OUT"
fi
echo ""
echo "--- 37. a mode-only change counts as a change ---"
F="$(make_fixture)"
(cd "$F" && git config core.fileMode true)
chmod +x "$F/plugins/demo/.apm/skills/alpha/SKILL.md"; commit "$F"
if [[ "$(cd "$F" && git diff --summary main HEAD)" == *"mode change 100644 => 100755"* ]]; then
pass "fixture check: the commit changes only the file mode"
else
fail "fixture check: no mode change recorded"
fi
expect_fail "chmod +x without a bump fails" "alpha: 1\.0\.0 -> 1\.0\.0" "$F"
echo ""
echo "--- 38. skill directory replaced by a symlink ---"
# Current behaviour, pinned: the path is no longer a tree at the pushed commit,
# so the skill is exempt as deleted. apm drops symlinks under .apm/ (ADR-0017),
# so readers do lose the skill.
F="$(make_fixture)"
rm -rf "$F/plugins/demo/.apm/skills/alpha"
ln -s beta "$F/plugins/demo/.apm/skills/alpha"; commit "$F"
if [[ "$(cd "$F" && git ls-tree HEAD plugins/demo/.apm/skills/alpha)" == 120000* ]]; then
pass "fixture check: alpha is committed as a symlink"
else
fail "fixture check: alpha is not a symlink in the commit"
fi
expect_pass "a skill replaced by a symlink is exempt as deleted" "$F"
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]]

View File

@@ -1,276 +0,0 @@
#!/usr/bin/env bash
# Integration test for .pre-commit-hooks.yaml as an EXTERNAL hook repo — the
# contract ADR-0014 exists to provide, and the one thing running pre-commit
# inside this repo can never exercise: `repo: local` makes pre-commit's clone
# prefix equal to the consuming repo's root, so a hook entry that only works
# because those two coincide passes here and hard-fails everywhere else.
# (It did: every argument after entry[0] resolves against the CONSUMING repo,
# so a `--config plugins/.../.vale.ini` argument gave external consumers
# `E100 [--config] Runtime error ... does not exist`, exit 2, on both Vale hooks.)
#
# The hook repo is built from the WORKING TREE, not from HEAD, so an uncommitted
# change to the manifest or the wrapper is what gets tested.
set -euo pipefail
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
PASS=0
FAIL=0
pass() { echo " PASS: $1"; PASS=$((PASS + 1)); }
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
for bin in pre-commit vale git; do
if ! command -v "$bin" &>/dev/null; then
echo "SKIP: $bin is not installed — cannot stand up a consumer repo"
exit 77
fi
done
WORK="$(mktemp -d)"
trap 'rm -rf "$WORK"' EXIT
HOOK_REPO="$WORK/hookrepo"
CONSUMER="$WORK/consumer"
export PRE_COMMIT_HOME="$WORK/pc-home"
mkdir -p "$HOOK_REPO/plugins/kyberforge/.apm/skills" "$HOOK_REPO/scripts"
cp "$REPO_ROOT/.pre-commit-hooks.yaml" "$HOOK_REPO/"
cp "$REPO_ROOT/scripts/skill-size-check.sh" "$HOOK_REPO/scripts/"
# One skill since ADR-0025 merged skill-audit and agent-audit into factory-audit,
# and one vale-wrap.sh with it. Both Vale hook IDs still ship and both are still
# registered by the consumer below — they now point at the same entry and differ
# only in their `files:` scope, which is exactly what the per-hook attribution in
# case 1 exists to prove is still true.
skill=factory-audit
mkdir -p "$HOOK_REPO/plugins/kyberforge/.apm/skills/$skill"
cp -R "$REPO_ROOT/plugins/kyberforge/.apm/skills/$skill/scripts" \
"$REPO_ROOT/plugins/kyberforge/.apm/skills/$skill/assets" \
"$HOOK_REPO/plugins/kyberforge/.apm/skills/$skill/"
git -C "$HOOK_REPO" init -q
git -C "$HOOK_REPO" add -A
git -C "$HOOK_REPO" -c user.email=test@example.invalid -c user.name=test commit -qm "hook repo"
HOOK_REV="$(git -C "$HOOK_REPO" rev-parse HEAD)"
# Every hook scopes by filename, so the consumer needs one file of each shape:
# a hook with nothing to match reports `Skipped` and proves nothing. All three
# hooks .pre-commit-hooks.yaml ships are registered — an unregistered one would
# let a regression (a lost `100755` bit, a bad entry path) reach every external
# consumer while this repo's own `repo: local` runs stayed green.
mkdir -p "$CONSUMER/skills/demo" "$CONSUMER/agents"
git -C "$CONSUMER" init -q
cat > "$CONSUMER/.pre-commit-config.yaml" <<EOF
repos:
- repo: file://$HOOK_REPO
rev: $HOOK_REV
hooks:
- id: kyberforge-vale-audit-skill
- id: kyberforge-vale-audit-agent
- id: kyberforge-skill-size-check
EOF
# The two fixtures carry DIFFERENT flagged tokens so an alert can never be
# credited to the hook that did not raise it. Both bodies land mid-sentence in a
# folded block scalar that still spans two physical lines, which is the
# flattening the wrapper exists to do.
write_fixtures() {
local skill_body="$1"
local agent_body="${2:-$1}"
cat > "$CONSUMER/skills/demo/SKILL.md" <<EOF
---
name: demo
description: >
Use when the caller wants a demonstration skill $skill_body across two
physical lines of one folded block scalar.
metadata:
version: "1.0.0"
---
Body.
EOF
cat > "$CONSUMER/agents/demo.md" <<EOF
---
name: demo
description: >
Use when the caller wants a demonstration agent $agent_body across two
physical lines of one folded block scalar.
---
Body.
EOF
git -C "$CONSUMER" add -A
}
# Vale prints each linted path as its own header line with that file's alerts
# indented beneath it, so an alert belongs to the nearest preceding path line.
# Reads a hook log on stdin and prints only the alert lines filed under `$1`.
# The `sed` strips vale's ANSI colouring, which it emits into pre-commit's pipe
# too, so the header lines compare as plain paths.
alerts_for() {
sed $'s/\033\\[[0-9;]*m//g' | awk -v want="$1" '
/^[^[:space:]].*\.md$/ { cur = $0; next }
/^[[:space:]]*[0-9]+:[0-9]+[[:space:]]/ { if (cur == want) print }
'
}
# --- 1. Each Vale hook resolves its config and gates its own file shape ---
# Asserted per hook, against that hook's own fixture path and its own token. An
# aggregate alert count over both hooks' combined output does not prove this:
# one fixture description carries every flagged token, so ONE working hook
# already clears a `>= 2` threshold. And a hook whose .vale.ini globs match
# nothing reaches neither of the guards below — it still MATCHES the file via
# its `files:` regex, so pre-commit does not report `Skipped`; vale simply lints
# nothing, prints `0 errors ... in 1 file` and exits 0, and the hook shows
# `Passed`. Attribution is the only thing that catches it.
echo ""
echo "--- each Vale hook flags its own fixture in an external consumer repo ---"
write_fixtures "that helps with things" "that will utilize things"
while IFS='|' read -r HOOK_ID FIXTURE TOKEN; do
[[ -n "$HOOK_ID" ]] || continue
LOG="$WORK/$HOOK_ID.log"
set +e
(cd "$CONSUMER" && pre-commit run "$HOOK_ID" --all-files > "$LOG" 2>&1)
RC_HOOK=$?
set -e
if grep -q "does not exist" "$LOG"; then
fail "$HOOK_ID hard-errored on a path resolved against the consumer repo (E100) — the bug this test guards against"
sed 's/^/ /' "$LOG"
elif grep -q "Skipped" "$LOG"; then
fail "$HOOK_ID matched no files, so it proved nothing"
sed 's/^/ /' "$LOG"
elif [[ $RC_HOOK -eq 0 ]]; then
fail "$HOOK_ID passed $FIXTURE despite its flagged '$TOKEN' — a .vale.ini glob matching nothing lints zero files and exits 0"
sed 's/^/ /' "$LOG"
elif alerts_for "$FIXTURE" < "$LOG" | grep -qF "'$TOKEN'"; then
pass "$HOOK_ID flattens $FIXTURE and flags its '$TOKEN' in a consumer repo"
else
fail "$HOOK_ID failed, but no alert quoting '$TOKEN' was filed under $FIXTURE"
sed 's/^/ /' "$LOG"
fi
done <<'EOF'
kyberforge-vale-audit-skill|skills/demo/SKILL.md|helps with
kyberforge-vale-audit-agent|agents/demo.md|utilize
EOF
# --- 1b. Every rule in the shipped style is asserted to FIRE, not merely to
# exist. A Vale rule can be well-formed, load without a diagnostic, and match
# nothing at all: `extends: existence` CONCATENATES multiple `raw:` entries
# rather than alternating them, so a rule written as a list of alternatives
# silently becomes one impossible expression, lints every file clean and exits
# 0 — indistinguishable from a corpus with no violations. `Kyberforge.CompositionNote`
# was written that way first and passed all 43 skill and agent files before the
# defect was found by hand. Each rule gets its own fixture pass, with the token
# it must quote attributed to the file that raised it, so one rule's alert can
# never stand in for another's.
echo ""
echo "--- each Kyberforge description rule fires through both shipped hooks ---"
write_desc_fixtures() {
local skill_desc="$1" agent_desc="$2"
cat > "$CONSUMER/skills/demo/SKILL.md" <<EOF
---
name: demo
description: >
$skill_desc across two
physical lines of one folded block scalar.
---
Body.
EOF
cat > "$CONSUMER/agents/demo.md" <<EOF
---
name: demo
description: >
$agent_desc across two
physical lines of one folded block scalar.
---
Body.
EOF
git -C "$CONSUMER" add -A
}
run_rule_case() {
local label="$1" hook_id="$2" fixture="$3" token="$4"
local log="$WORK/rule-$label.log"
set +e
(cd "$CONSUMER" && pre-commit run "$hook_id" --all-files > "$log" 2>&1)
local rc=$?
set -e
if grep -q "Skipped" "$log"; then
fail "$hook_id matched no files for $label, so it proved nothing"
sed 's/^/ /' "$log"
elif [[ $rc -eq 0 ]]; then
fail "$hook_id passed $fixture despite its flagged '$token' — $label matches nothing"
sed 's/^/ /' "$log"
elif alerts_for "$fixture" < "$log" | grep -qF "'$token'"; then
pass "$label fires through $hook_id and quotes '$token' under $fixture"
else
fail "$hook_id failed, but no $label alert quoting '$token' was filed under $fixture"
sed 's/^/ /' "$log"
fi
}
# CompositionNote: a distinct banned token per file shape.
write_desc_fixtures \
"Use when the caller wants a demo skill that composes other skills" \
"Use when the caller wants a cross-cutting demo agent"
run_rule_case "Kyberforge.CompositionNote" kyberforge-vale-audit-skill skills/demo/SKILL.md "composes"
run_rule_case "Kyberforge.CompositionNote" kyberforge-vale-audit-agent agents/demo.md "cross-cutting"
# DescriptionOpener: the widened pattern catches every non-imperative "This..."
# opener, not only the literal "This skill"/"This agent" pair it was anchored to
# before. Both fixtures open with "This is", the form two shipped descriptions
# used mid-sentence and which the old pattern could not express.
write_desc_fixtures \
"This is a demo skill for callers who want one" \
"This is a demo agent for callers who want one"
run_rule_case "Kyberforge.DescriptionOpener" kyberforge-vale-audit-skill skills/demo/SKILL.md "This"
run_rule_case "Kyberforge.DescriptionOpener" kyberforge-vale-audit-agent agents/demo.md "This"
# --- 2. Clean files pass — the hooks gate, they don't just always fail ---
echo ""
echo "--- all three hooks pass clean files in an external consumer repo ---"
write_fixtures "of the packaged hook contract"
set +e
(cd "$CONSUMER" && pre-commit run --all-files > "$WORK/clean.log" 2>&1)
RC_CLEAN=$?
set -e
if grep -q "Skipped" "$WORK/clean.log"; then
fail "a hook matched no files on the clean run, so it proved nothing"
sed 's/^/ /' "$WORK/clean.log"
elif [[ $RC_CLEAN -eq 0 ]]; then
pass "all three hooks exit 0 on clean files"
else
fail "hooks failed on clean files (rc=$RC_CLEAN)"
sed 's/^/ /' "$WORK/clean.log"
fi
# --- 3. The size hook gates too. It ran clean above, which is what proves it
# is executable and its entry path resolves; this half proves it still fails a
# file that breaks the ceiling rather than passing everything. ---
echo ""
echo "--- kyberforge-skill-size-check fails an oversized SKILL.md in an external consumer repo ---"
mkdir -p "$CONSUMER/skills/oversized"
{
echo "---"
echo "name: oversized"
echo "description: Use when the caller wants an oversized fixture."
echo "---"
for ((i = 1; i <= 600; i++)); do
echo "word"
done
} > "$CONSUMER/skills/oversized/SKILL.md"
git -C "$CONSUMER" add -A
set +e
(cd "$CONSUMER" && pre-commit run kyberforge-skill-size-check --all-files > "$WORK/size.log" 2>&1)
RC_SIZE=$?
set -e
if [[ $RC_SIZE -ne 0 ]] && grep -q "500-line ceiling" "$WORK/size.log"; then
pass "kyberforge-skill-size-check exits non-zero and names the ceiling it broke"
else
fail "kyberforge-skill-size-check did not gate an oversized SKILL.md (rc=$RC_SIZE)"
sed 's/^/ /' "$WORK/size.log"
fi
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]]

View File

@@ -21,8 +21,8 @@ fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
# Vale absent skips the Vale-DEPENDENT cases, not the suite. An early `exit 77`
# here used to skip everything, including the checks that are plain greps and
# awk over the config and the two hook manifests (cases 16, 26-28's static
# halves, 31 Parts A/B, 32, 33) -- so a machine without vale reported a skip
# awk over the config and the hook config (cases 16, 26-28's static
# halves, 31 Parts A/B, 32) -- so a machine without vale reported a skip
# while never looking at a manifest it could have read. Those still run; the
# suite exits 77 at the end only if they all passed, so run-tests.sh keeps
# reporting SKIPPED and `--strict` keeps turning that skip into a failure. A
@@ -452,10 +452,11 @@ else
fi
# --- 12. With no --config at all, the wrapper falls back to its own sibling
# assets/vale/.vale.ini. `.pre-commit-hooks.yaml` relies on this: pre-commit
# prefixes only entry[0] with the hook-repo clone path, so a --config argument
# there resolves against the consuming repo and hard-errors (E100) for every
# external consumer.
# assets/vale/.vale.ini. factory-audit's Step 1 and both prefilter hooks rely
# on this: they pass no --config. A published hook manifest would too, were it
# restored (ADR-0014): pre-commit prefixes only entry[0] with the hook-repo clone
# path, so a --config argument there resolves against the consuming repo and
# hard-errors (E100) for every external consumer.
echo ""
echo "--- defaults --config to the wrapper's own sibling assets/vale/.vale.ini ---"
FIXTURE12="$(make_fixture 2)"
@@ -464,7 +465,7 @@ OUT12=$(run_wrap "$FIXTURE12" plugins/testplugin/skills/zzzskill/SKILL.md)
if grep -q "VagueWording" <<< "$OUT12"; then
pass "a --config-less invocation uses the wrapper's bundled config"
else
fail "a --config-less invocation found no config — external pre-commit consumers get E100, the bug this test guards against"
fail "a --config-less invocation found no config — factory-audit's Step 1 and both prefilter hooks pass no --config, so they would get E100"
fi
# --- 13. No GNU-only `realpath -m`. macOS ships the BSD realpath, which has no
@@ -1080,7 +1081,7 @@ elif [[ "$WRAPPED21_FILES" != "$BARE21_FILES" ]]; then
fail "the directory walk dropped a symlinked file: wrapper saw '$WRAPPED21_FILES', bare vale '$BARE21_FILES'"
# A here-string, not `echo "$WRAPPED21" | grep -q`: the match sits on line 3 of
# 8, and under pipefail grep -q exiting early can SIGPIPE echo mid-write and
# fail this branch on correct output (see tests/test-check-release-needed.sh).
# fail this branch on correct output (see docs/spec/gates.md, Tests).
elif grep -q "VagueWording" <<< "$WRAPPED21"; then
pass "a symlinked file under a directory argument is mirrored, flattened and flagged"
else
@@ -1341,7 +1342,7 @@ fi
# exits 0. Every gate in this repo reads that as a pass.
#
# ADR-0014's reason for two hook IDs was this same problem, and .pre-commit-
# hooks.yaml still carries both IDs after the merge for that reason.
# config.yaml still carries both IDs after the merge for that reason.
# One representative path per file shape the prefilter is supposed to cover,
# tagged with the `.vale.ini` section that is supposed to cover it and with
@@ -1365,8 +1366,15 @@ fi
#
# `demo.md` (bare, no `.agent.md` suffix) exercises `[**/agents/*.md]` in
# isolation, not because any current `.apm/agents/*` file has that shape -- per
# ADR-0016 they are all `*.agent.md`. `.pre-commit-hooks.yaml`'s agent regex
# still covers the bare shape, which is what keeps the row honest.
# ADR-0016 they are all `*.agent.md`. factory-audit's agent flow can still be
# handed that shape in a consuming repo, which is what keeps the row honest.
#
# These rows describe what factory-audit's own Vale call can be handed at
# runtime in any repo, not what this repo's hooks select: most of them sit
# outside `.pre-commit-config.yaml`'s `^plugins/`-anchored `files:` regexes on
# purpose. The hook-scope half that once held each row to a hook regex read the
# published `.pre-commit-hooks.yaml`, which is retired (ADR-0014, 2026-09-16
# amendment); case 32 owns the local hooks' scope.
PROBE_TABLE28="$(cat <<'EOF_PROBE28'
plugins/demo/.apm/skills/demo/SKILL.md|[**/SKILL.md]|isolating
.claude/skills/demo/SKILL.md|[**/SKILL.md]|isolating
@@ -1428,12 +1436,11 @@ files_scanned28() {
| tail -1
}
# Prints `<id>|<files regex>` for every hook in the given pre-commit manifest
# whose entry is factory-audit's vale-wrap.sh -- either manifest, since the two
# carry the same two hooks in the same shape. Records are delimited by their
# Prints `<id>|<files regex>` for every hook in the given pre-commit config
# whose entry is factory-audit's vale-wrap.sh. Records are delimited by their
# `- id:` line, so this does not depend on `entry:` preceding `files:` within a
# record. Case 28 wants the regexes alone and case 32 needs to know which hook
# each belongs to, so the id is carried here and dropped by the wrapper below.
# record. Case 32 needs to know which hook each regex belongs to, so the id is
# carried here.
hook_records28() {
local manifest="$1" id raw
[[ -f "$manifest" ]] || return 0
@@ -1456,29 +1463,8 @@ hook_records28() {
done
}
# Prints the `files:` regex of every hook in the given manifest whose entry is
# factory-audit's vale-wrap.sh.
hook_file_regexes28() {
hook_records28 "$1" | cut -d'|' -f2-
}
matches_any_regex28() {
local rel="$1" regexes="$2" re
[[ -n "$regexes" ]] || return 1
while IFS= read -r re; do
[[ -n "$re" ]] || continue
if grep -Eq "$re" <<< "$rel"; then
return 0
fi
done <<EOF_RE28
$regexes
EOF_RE28
return 1
}
TREE28="$(build_probe_tree28)"
new_fixture "$TREE28"
HOOK_REGEXES28="$(hook_file_regexes28 "$REPO_ROOT/.pre-commit-hooks.yaml")"
echo ""
echo "--- every .vale.ini glob section actually scans a real file shape ---"
@@ -1526,24 +1512,13 @@ fi
# One `PASS|<rel>|<message>` or `FAIL|<rel>|<message>` line per probe row, for
# the config at $1. A function rather than an inline loop so Part B can hold a
# mutated copy to this exact logic -- a second, "equivalent" loop for the
# fixture would prove nothing about the live check. With $2 = false only the
# hook-scope half runs: that half reads .pre-commit-hooks.yaml, not vale, so a
# machine without vale still gets it.
# fixture would prove nothing about the live check. With $2 = false it prints
# nothing: every remaining half of it needs vale.
probe_coverage28() {
local cfg="$1" with_vale="$2" rel sec report count
[[ "$with_vale" == true ]] || return 0
while IFS='|' read -r rel sec _; do
[[ -n "$rel" ]] || continue
if ! matches_any_regex28 "$rel" "$HOOK_REGEXES28"; then
# Original wording: the probe path is stale, or the hook was rescoped away
# from a shape it still needs to lint. Either way the row below stops
# describing anything the push gate actually hands to vale.
echo "FAIL|$rel|$rel matches no 'files:' regex of any factory-audit vale hook in .pre-commit-hooks.yaml — the probe path is stale, or the hook was rescoped away from a shape it still needs to lint"
continue
fi
if [[ "$with_vale" != true ]]; then
echo "PASS|$rel|$rel is in scope of a published factory-audit vale hook (glob coverage not checked: Vale-dependent half held back)"
continue
fi
report="$(vale_report28 "$cfg" "$TREE28" "$rel")"
count="$(files_scanned28 "$report")"
if [[ -z "$count" ]]; then
@@ -1995,25 +1970,29 @@ fi
# which worked only because the two skills gave the two hooks two distinct entry
# paths. After the merge both hooks share one `entry:`, so that selector can no
# longer tell them apart and a faithful port would have to key on hook `id:`
# instead. Case 33 is that port; this case covers the separate question of
# whether each local hook selects a live corpus at all. The zero-match half of the hole stands on its own, and nothing
# else in the repo covers it: tests/test-vale-hooks-consumer.sh synthesises its
# own consumer config out of `.pre-commit-hooks.yaml` and never reads the local
# one, and cases 28-30 read `.pre-commit-hooks.yaml` too. This repo's OWN
# prefilter regexes -- `.pre-commit-config.yaml`'s vale-audit-prefilter-skill and
# vale-audit-prefilter-agent -- are therefore asserted by no test at all. Narrow
# either one to match zero files and every gate still passes: pre-commit does not
# instead. That port was case 33, deleted with the published
# `.pre-commit-hooks.yaml` it compared against (ADR-0014, 2026-09-16
# amendment); its one guard that did not depend on the second manifest -- a
# local regex narrowed to a single plugin -- is property 3 below. Nothing else
# in the repo asserts this repo's OWN prefilter regexes --
# `.pre-commit-config.yaml`'s vale-audit-prefilter-skill and
# vale-audit-prefilter-agent. Narrow either one to match zero files and every gate still passes: pre-commit does not
# error on a hook that matches nothing, it simply never runs it. That is the same
# silent-zero failure mode case 28 guards on the vale side of this pipeline, one
# layer up -- there the glob scans 0 files and exits 0, here the hook is handed 0
# files and never starts.
#
# Two properties, because matching SOMETHING is not the same as matching the
# Three properties, because matching SOMETHING is not the same as matching the
# right thing: a regex loosened to `^plugins/` would match hundreds of files and
# clear a bare non-emptiness check while handing vale a corpus it has no glob
# for. So each hook must also select only its own artifact class -- ADR-0014's
# reason for two hook IDs, carried across the merge by ADR-0025's comment in the
# config, is precisely that the two scopes stay independently addressable.
# And each hook must select ALL of its class's authoring source: a regex
# narrowed from `^plugins/[^/]+/...` to `^plugins/kyberforge/...` still matches
# tracked files, all of the right class, while silently dropping every other
# plugin out of the prefilter. Measured before any case caught it: that exact
# narrowing left 6 of 38 skills prefiltered and the whole suite green.
echo ""
echo "--- each .pre-commit-config.yaml vale prefilter hook matches a real, correctly-classed file ---"
@@ -2023,6 +2002,11 @@ PC_CONFIG32="$REPO_ROOT/.pre-commit-config.yaml"
# repository paths, so a regex matching no tracked path matches nothing the gate
# will ever hand the hook.
REPO_FILES32="$(cd "$REPO_ROOT" && git ls-files)"
# Each class's full authoring-source corpus, per the layout AGENTS.md fixes
# (`plugins/<name>/.apm/` is the only authoring source). Globals, not locals,
# so Part D can point them at a layout that no longer exists.
CORPUS_RE_SKILL32='^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$'
CORPUS_RE_AGENT32='^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$'
# Prints one failure token per defect; empty output means every prefilter hook in
# $1 is scoped to a live corpus of its own artifact class. The config path is an
@@ -2031,7 +2015,7 @@ REPO_FILES32="$(cd "$REPO_ROOT" && git ls-files)"
# the live check.
prefilter_scope_failures32() {
local config="$1" files="$2"
local records id re class matched count offenders offending m
local records id re class matched count offenders offending m corpus missing nmissing corpus_re
local seen_skill=false seen_agent=false bad=""
records="$(hook_records28 "$config")"
if [[ -z "$records" ]]; then
@@ -2082,6 +2066,27 @@ EOF_MATCHED32
if [[ "$offending" -gt 0 ]]; then
bad+="[$id: 'files: $re' selects $count file(s), $offending of them outside the $class artifact class, so the two prefilter scopes are no longer independently addressable and vale is handed files no glob in its config covers — first: $offenders] "
fi
# Every path in the class's corpus (CORPUS_RE_*32 above) must be
# selected, or part of the corpus is silently unprefiltered.
if [[ "$class" == skill ]]; then
corpus_re="$CORPUS_RE_SKILL32"
else
corpus_re="$CORPUS_RE_AGENT32"
fi
corpus="$(printf '%s\n' "$files" | { grep -E "$corpus_re" || true; })"
# An empty corpus makes the comparison below vacuous: nothing can be
# missing from nothing. The corpus regex is hard-coded to today's layout,
# so a layout move would silently turn this property into a pass. Fail
# instead, the same way property 1 fails on zero matches.
if [[ -z "$(printf '%s\n' "$corpus" | grep . || true)" ]]; then
bad+="[$id: the $class corpus regex '$corpus_re' matches no tracked file, so this case cannot check that 'files: $re' selects the whole $class corpus; update the regex to the current layout] "
continue
fi
missing="$(comm -23 <(printf '%s\n' "$corpus" | grep . | sort) <(printf '%s\n' "$matched" | grep . | sort) || true)"
nmissing="$(printf '%s\n' "$missing" | grep -c . || true)"
if [[ "$nmissing" -gt 0 ]]; then
bad+="[$id: 'files: $re' misses $nmissing tracked $class file(s) under plugins/*/.apm/, so part of the corpus is never prefiltered while every gate still reports a pass — first: $(printf '%s\n' "$missing" | head -3 | tr '\n' ' ')] "
fi
done <<EOF_RECORDS32
$records
EOF_RECORDS32
@@ -2129,205 +2134,54 @@ else
pass "narrowing either hook's 'files:' regex to match zero files is caught by Part A, which is what makes its pass mean something"
fi
# --- 33. The local and published vale hooks agree on every shared file shape --
#
# The cross-manifest `files:` agreement check of the deleted
# scripts/check-vale-style-sync.sh (ADR-0025), ported. Case 32 does not cover
# it: a local regex narrowed from `^plugins/[^/]+/...` to
# `^plugins/kyberforge/...` still matches tracked files, all of them SKILL.md,
# so it clears both of 32's properties while silently dropping every other
# plugin's skills out of this repo's prefilter. Measured before this case
# existed: that exact narrowing left the whole suite green.
#
# The original's comment on why the two manifests are compared per hook rather
# than unioned, verbatim in substance: "A union here previously let a probe that
# matched only the older, looser .pre-commit-hooks.yaml pattern read as 'in
# scope' even after .pre-commit-config.yaml's copy of the same hook had been
# narrowed away from it -- silently masking exactly the kind of hook-rescoping
# drift this script exists to catch."
#
# What changed in the port is the selector and nothing else. The original found
# each manifest's hook by `entry ~ skill "/scripts/vale-wrap.sh"`, which told the
# two hooks apart only because two skills gave them two entry paths; after the
# merge both hooks share one entry. They are paired by `id:` instead, from an
# explicit table. The pairing is explicit rather than inferred from an id suffix
# so that renaming either id fails here by name instead of quietly dropping a
# class out of the comparison. The probe table and its shared/hooks-only scopes
# are the original's rows, unchanged:
#
# shared -- a shape this repo's own layout has, so both manifests must
# agree on it. This is what catches the narrowing above.
# hooks-only -- a shape only the layout-agnostic published manifest has to
# cover. `.pre-commit-config.yaml` pinning this repo's own
# `plugins/*/.apm/` layout is by design, not drift; bare
# `agents/demo.md` is hooks-only because per ADR-0016 every
# `.apm/agents/` file is `*.agent.md`.
#
# A probe in scope of neither manifest fails too, exactly as in the original:
# the probe path is stale, or both hooks were rescoped away from it.
HOOK_PAIRS33="$(cat <<'EOF_PAIRS33'
skill|kyberforge-vale-audit-skill|vale-audit-prefilter-skill
agent|kyberforge-vale-audit-agent|vale-audit-prefilter-agent
EOF_PAIRS33
)"
PROBES33="$(cat <<'EOF_PROBES33'
skill|plugins/demo/.apm/skills/demo/SKILL.md|shared
skill|.claude/skills/demo/SKILL.md|hooks-only
agent|plugins/demo/.apm/agents/demo.md|hooks-only
agent|plugins/demo/.apm/agents/demo.agent.md|shared
agent|.claude/agents/demo.md|hooks-only
agent|copilot/demo.agent.md|hooks-only
EOF_PROBES33
)"
# Prints the `files:` regex of the hook whose id is exactly $2 in manifest $1,
# or nothing. Records are delimited by their `- id:` line, as in
# hook_records28, but selected by id alone: after the merge `entry:` no longer
# distinguishes them.
hook_regex_by_id33() {
local manifest="$1" want="$2" raw
[[ -f "$manifest" ]] || return 0
raw="$(WANT="$want" awk '
function flush() {
if (id == ENVIRON["WANT"] && files != "") print files
id = ""; files = ""
}
/^[ \t]*-[ \t]*id:/ { flush(); id = $0; sub(/^[ \t]*-[ \t]*id:[ \t]*/, "", id); sub(/[ \t]+$/, "", id) }
/^[ \t]*files:/ { files = $0; sub(/^[ \t]*files:[ \t]*/, "", files) }
END { flush() }
' "$manifest" | head -1)"
raw="${raw%\'}"; raw="${raw#\'}"
raw="${raw%\"}"; raw="${raw#\"}"
printf '%s' "$raw"
}
# Prints one failure token per defect for published manifest $1 against local
# manifest $2; empty output means every probe is in scope of at least one of its
# class's hooks and every shared probe is in scope of both. Manifest paths are
# arguments so Part B runs this exact function against mutated copies.
cross_manifest_failures33() {
local published="$1" local_cfg="$2"
local class pub_id loc_id rel scope pub_re loc_re in_hooks in_config
local bad="" checked=0 shared_classes="" missing_classes=""
while IFS='|' read -r class pub_id loc_id; do
[[ -n "$class" ]] || continue
if [[ -z "$(hook_regex_by_id33 "$published" "$pub_id")" ]]; then
bad+="[no hook with id '$pub_id' and a files: regex in ${published##*/}, so the $class class is compared against nothing] "
missing_classes+="$class "
fi
if [[ -z "$(hook_regex_by_id33 "$local_cfg" "$loc_id")" ]]; then
bad+="[no hook with id '$loc_id' and a files: regex in ${local_cfg##*/}, so the $class class is compared against nothing] "
missing_classes+="$class "
fi
done <<EOF_PAIRCHK33
$HOOK_PAIRS33
EOF_PAIRCHK33
while IFS='|' read -r class rel scope; do
[[ -n "$class" ]] || continue
pub_id="$(printf '%s\n' "$HOOK_PAIRS33" | awk -F'|' -v c="$class" '$1 == c { print $2 }')"
loc_id="$(printf '%s\n' "$HOOK_PAIRS33" | awk -F'|' -v c="$class" '$1 == c { print $3 }')"
if [[ -z "$pub_id" || -z "$loc_id" ]]; then
bad+="[probe $rel names class '$class', which has no row in the hook pair table] "
continue
fi
pub_re="$(hook_regex_by_id33 "$published" "$pub_id")"
loc_re="$(hook_regex_by_id33 "$local_cfg" "$loc_id")"
# A missing hook was reported above; comparing against it would add a
# second message for the same defect.
[[ -n "$pub_re" && -n "$loc_re" ]] || continue
checked=$((checked + 1))
[[ "$scope" != shared ]] || shared_classes+="$class "
in_hooks=false
matches_any_regex28 "$rel" "$pub_re" && in_hooks=true
in_config=false
matches_any_regex28 "$rel" "$loc_re" && in_config=true
if [[ "$in_hooks" == false && "$in_config" == false ]]; then
bad+="[$rel matches neither $pub_id's nor $loc_id's 'files:' regex — the probe path is stale, or the hook was rescoped away from a shape it still needs to lint] "
elif [[ "$scope" == shared && "$in_hooks" != "$in_config" ]]; then
bad+="[$rel is in scope of $pub_id in ${published##*/} (hooks=$in_hooks) but not of $loc_id in ${local_cfg##*/} (config=$in_config), or vice versa — the local and published 'files:' regexes have drifted, and the narrower one silently stops prefiltering that shape] "
fi
done <<EOF_PROBECHK33
$PROBES33
EOF_PROBECHK33
# The original's PROBES_CHECKED floor, plus one it lacked: a class with no
# shared probe is never compared across manifests at all.
if [[ "$checked" -eq 0 && -z "$missing_classes" ]]; then
bad+="[no probe row was checked, so the agreement check verified nothing] "
fi
while IFS='|' read -r class _ _; do
[[ -n "$class" ]] || continue
# A class whose hook is missing was already reported, and its probes were
# skipped for that reason, not for want of a shared row.
[[ " $missing_classes" != *" $class "* ]] || continue
[[ " $shared_classes" == *" $class "* ]] \
|| bad+="[the $class class has no shared probe row, so its two hooks are never compared] "
done <<EOF_CLASSCHK33
$HOOK_PAIRS33
EOF_CLASSCHK33
printf '%s' "$bad"
}
echo ""
echo "--- the local and published vale hooks agree on every shared file shape ---"
PUBLISHED33="$REPO_ROOT/.pre-commit-hooks.yaml"
# Part A: the live assertion, against the two real manifests.
AGREE_FAILS33="$(cross_manifest_failures33 "$PUBLISHED33" "$PC_CONFIG32")"
if [[ -n "$AGREE_FAILS33" ]]; then
fail ".pre-commit-config.yaml and .pre-commit-hooks.yaml disagree on the vale hooks' scope: $AGREE_FAILS33"
else
pass "every probe is in scope of its class's hooks, and both manifests agree on every shared shape"
fi
# Part B: proof that Part A can fail. Each mutation lands in a COPY of
# .pre-commit-config.yaml, never the real file.
#
# skill -- the exact narrowing that slipped through before this case existed.
# agent -- the same narrowing applied to the agent hook.
# id -- the local skill hook renamed, which must fail by name rather than
# drop the skill class out of the comparison.
#
# Replacement is fixed-string, via ENVIRON, so the regex's backslashes reach awk
# literally; `awk -v` would process them as escape sequences.
narrow_config33() {
OLD33="$2" NEW33="$3" awk '
$0 ~ ENVIRON["LINE33"] {
i = index($0, ENVIRON["OLD33"])
if (i) $0 = substr($0, 1, i - 1) ENVIRON["NEW33"] substr($0, i + length(ENVIRON["OLD33"]))
# Part C: proof that property 3 can fail. Each hook's regex is narrowed to one
# plugin in a COPY of the config -- the exact narrowing that once slipped
# through. Replacement is fixed-string, via ENVIRON, so the regex's backslashes
# reach awk literally; `awk -v` would process them as escape sequences.
narrow_config32() {
OLD32="$2" NEW32="$3" awk '
/^[ \t]*files:/ {
i = index($0, ENVIRON["OLD32"])
if (i) $0 = substr($0, 1, i - 1) ENVIRON["NEW32"] substr($0, i + length(ENVIRON["OLD32"]))
}
{ print }
' "$1"
}
MUT33="$(mktemp -d)"
new_fixture "$MUT33"
LINE33='^[ \t]*files:' narrow_config33 "$PC_CONFIG32" \
'^plugins/[^/]+/\.apm/skills/' '^plugins/kyberforge/\.apm/skills/' > "$MUT33/skill.yaml"
LINE33='^[ \t]*files:' narrow_config33 "$PC_CONFIG32" \
'^plugins/[^/]+/\.apm/agents/' '^plugins/kyberforge/\.apm/agents/' > "$MUT33/agent.yaml"
LINE33='^[ \t]*-[ \t]*id:' narrow_config33 "$PC_CONFIG32" \
'vale-audit-prefilter-skill' 'vale-audit-prefilter-skill-renamed' > "$MUT33/id.yaml"
SKILL_FAILS33="$(cross_manifest_failures33 "$PUBLISHED33" "$MUT33/skill.yaml")"
AGENT_FAILS33="$(cross_manifest_failures33 "$PUBLISHED33" "$MUT33/agent.yaml")"
ID_FAILS33="$(cross_manifest_failures33 "$PUBLISHED33" "$MUT33/id.yaml")"
if ! grep -qF '^plugins/kyberforge/\.apm/skills/' "$MUT33/skill.yaml" \
|| ! grep -qF '^plugins/kyberforge/\.apm/agents/' "$MUT33/agent.yaml" \
|| ! grep -qF 'vale-audit-prefilter-skill-renamed' "$MUT33/id.yaml"; then
fail "a mutation never reached its copied config, so Part B mutated nothing and proves nothing about Part A"
elif ! grep -qF "[plugins/demo/.apm/skills/demo/SKILL.md is in scope of kyberforge-vale-audit-skill" <<< "$SKILL_FAILS33"; then
fail "narrowing the local skill hook to ^plugins/kyberforge/ did not fail Part A, so the prefilter can drop every other plugin's skills with every gate green: ${SKILL_FAILS33:-<no failure>}"
elif [[ -z "$AGREE_FAILS33" ]] && grep -qF "kyberforge-vale-audit-agent" <<< "$SKILL_FAILS33"; then
# Only meaningful against a clean base: when Part A already failed, the copy
# inherits that defect, and reporting it again here would be one defect twice.
fail "narrowing only the skill hook also reported an agent-class defect, so the comparison is not confined to its class: $SKILL_FAILS33"
elif ! grep -qF "[plugins/demo/.apm/agents/demo.agent.md is in scope of kyberforge-vale-audit-agent" <<< "$AGENT_FAILS33"; then
fail "narrowing the local agent hook to ^plugins/kyberforge/ did not fail Part A: ${AGENT_FAILS33:-<no failure>}"
elif ! grep -qF "[no hook with id 'vale-audit-prefilter-skill' and a files: regex in id.yaml" <<< "$ID_FAILS33"; then
fail "renaming the local skill hook's id did not fail Part A by name, so the skill class could fall out of the comparison silently: ${ID_FAILS33:-<no failure>}"
narrow_config32 "$PC_CONFIG32" \
'^plugins/[^/]+/\.apm/skills/' '^plugins/kyberforge/\.apm/skills/' > "$MUT32/skill.yaml"
narrow_config32 "$PC_CONFIG32" \
'^plugins/[^/]+/\.apm/agents/' '^plugins/kyberforge/\.apm/agents/' > "$MUT32/agent.yaml"
NARROW_SKILL32="$(prefilter_scope_failures32 "$MUT32/skill.yaml" "$REPO_FILES32")"
NARROW_AGENT32="$(prefilter_scope_failures32 "$MUT32/agent.yaml" "$REPO_FILES32")"
if cmp -s "$PC_CONFIG32" "$MUT32/skill.yaml" || cmp -s "$PC_CONFIG32" "$MUT32/agent.yaml"; then
fail "a one-plugin narrowing left the copied config unchanged, so Part C narrowed nothing and proves nothing about property 3"
elif ! grep -qF "vale-audit-prefilter-skill: 'files: ^plugins/kyberforge/" <<< "$NARROW_SKILL32" \
|| ! grep -qF "tracked skill file(s)" <<< "$NARROW_SKILL32"; then
fail "narrowing the skill hook to ^plugins/kyberforge/ did not fail Part A as an incomplete corpus: ${NARROW_SKILL32:-<no failure>}"
elif ! grep -qF "vale-audit-prefilter-agent: 'files: ^plugins/kyberforge/" <<< "$NARROW_AGENT32" \
|| ! grep -qF "tracked agent file(s)" <<< "$NARROW_AGENT32"; then
fail "narrowing the agent hook to ^plugins/kyberforge/ did not fail Part A as an incomplete corpus: ${NARROW_AGENT32:-<no failure>}"
else
pass "narrowing either local hook to one plugin, or renaming one, is caught by Part A"
pass "narrowing either hook's 'files:' regex to one plugin is caught by Part A as an incomplete corpus"
fi
# Part D: proof that property 3 cannot pass vacuously. Each corpus regex is
# pointed at a layout no tracked file has -- what a future move of
# plugins/*/.apm/ would do to the hard-coded regexes -- and the live config must
# then fail as an empty corpus rather than pass with nothing to compare.
SAVED_SKILL_RE32="$CORPUS_RE_SKILL32"
SAVED_AGENT_RE32="$CORPUS_RE_AGENT32"
CORPUS_RE_SKILL32='^zzz-no-such-path/SKILL\.md$'
CORPUS_RE_AGENT32='^zzz-no-such-path/[^/]+\.agent\.md$'
EMPTY_FAILS32="$(prefilter_scope_failures32 "$PC_CONFIG32" "$REPO_FILES32")"
CORPUS_RE_SKILL32="$SAVED_SKILL_RE32"
CORPUS_RE_AGENT32="$SAVED_AGENT_RE32"
if ! grep -qF "vale-audit-prefilter-skill: the skill corpus regex" <<< "$EMPTY_FAILS32"; then
fail "a skill corpus regex matching no tracked file did not fail Part A, so property 3 would pass vacuously after a layout move: ${EMPTY_FAILS32:-<no failure>}"
elif ! grep -qF "vale-audit-prefilter-agent: the agent corpus regex" <<< "$EMPTY_FAILS32"; then
fail "an agent corpus regex matching no tracked file did not fail Part A, so property 3 would pass vacuously after a layout move: ${EMPTY_FAILS32:-<no failure>}"
else
pass "a corpus regex matching no tracked file fails Part A instead of passing vacuously"
fi
# --- 34. Every glob section loads a real style, asserted without vale --------
@@ -2342,7 +2196,7 @@ fi
# file the section matches with NO rule, prints `0 errors ... in 1 file` and
# exits 0, and every gate in this repo reads that as a pass. That is the
# silent-pass class ADR-0013 exists to prevent, and `--strict` -- which a
# consumer's clone does not run -- is the only thing standing in front of it.
# consuming repo's install does not run -- is the only thing standing in front of it.
#
# So this case asks the same question of the config TEXT, with no dependency on
# vale being installed. It is a separate case rather than an addition to either
@@ -2354,9 +2208,9 @@ fi
# `[[ "$sp" == /* ]] || sp="$dir/$sp"`, which ACCEPTS an absolute StylesPath --
# a path that resolves on the machine that wrote it and on no other. Relative
# resolution against the config's own directory is the only reason the bundled
# styles are found under a CONSUMING repo's clone prefix, so an absolute one
# passes every check here and hard-fails every external consumer of
# .pre-commit-hooks.yaml. It is asserted here, not in case 0, to keep case 0's
# styles are found under a CONSUMING repo's install of factory-audit, so an
# absolute one passes every check here and hard-fails every repo that installs
# the plugin. It is asserted here, not in case 0, to keep case 0's
# scope the one its comment describes.
VALE_ASSETS34="$FACTORY_AUDIT/assets/vale"
@@ -2383,7 +2237,7 @@ section_styles34() {
# One failure token per defect in the config at $1; empty output means every
# section would load at least one real style for a consumer. The section floor
# is here for the same reason case 28 Part A and case 33 carry theirs: a config
# is here for the same reason case 28 Part A carries its own: a config
# whose sections were all deleted lints nothing at all, and without a floor this
# function would report it clean.
style_load_defects34() {
@@ -2402,7 +2256,7 @@ style_load_defects34() {
return 0
fi
if [[ "$sp" == /* ]]; then
printf '%s' "[${cfg##*/} sets the ABSOLUTE StylesPath '$sp'; it resolves only on the machine that wrote it, and a consuming repo's clone -- which is the only reason .pre-commit-hooks.yaml ships these styles -- gets 'path does not exist'] "
printf '%s' "[${cfg##*/} sets the ABSOLUTE StylesPath '$sp'; it resolves only on the machine that wrote it, and a consuming repo's install of factory-audit -- which is the only reason these styles ship -- gets 'path does not exist'] "
return 0
fi
if [[ ! -d "$dir/$sp" ]]; then
@@ -2490,7 +2344,7 @@ EOF_MUT34
ABS34="$(mktemp -d)"
new_fixture "$ABS34"
cp -r "$VALE_ASSETS34/." "$ABS34/"
# Written through ENVIRON, as in narrow_config33, so the path reaches awk
# Written through ENVIRON, as in narrow_config32, so the path reaches awk
# literally rather than through `-v`'s escape processing.
ABS_SP34="$ABS34/styles" awk '
/^[ \t]*StylesPath[ \t]*=/ { print "StylesPath = " ENVIRON["ABS_SP34"]; next }