A five-agent review of718c79aandd2480b8found no skill, agent or hook regressions (39 skills before and after) and confirmed both hook removals are genuinely moot -- verified against the tree, not taken on the commit's word. It did find one functional regression (fixed separately) and this documentation drift. Counting errors, all from a git pathspec `*` crossing `/`: - 17 .bats files shipped to consumers is really 10; 17 counted tracked paths merely containing /tests/, one of them a template asset - "roughly 88s off every push" is ~92.4s; 88 omitted validate-plugins - "roughly 70% of each plugin remains live" holds only for kyberforge; the real spread is 44.3% (bin) to 70.6%, now a table - the pre-push enforcement row was half-corrected: 33 entries stood unstruck (now 27) and 14 -> 11 switched counting basis mid-sentence - the root .claude-plugin/plugin.json was described as "kept"; it has never been tracked gates.md said "Ten hooks" above a nine-row table (11 was decremented for one removal, not two), and "both need the claude CLI" for one remaining validator. Its pretty-format-json exclude rationale claimed six alternations expanding to sixteen files in a passage headed "Mind which number you are quoting" -- four alternations, two live files; the two dead ones are dropped from the pattern. check-useless-excludes could not catch this: it only flags an exclude matching nothing at all. ADR-0024 cited ADR-0006 for a patch-bump rule it does not contain and which ADR-0015 explicitly retired; stated apm's marketplace probe order backwards (.claude-plugin/ is the last candidate, not the first, so the earlier .github/plugin/ deletion only demoted resolution); undercounted apm's skill-deploying targets as seven when there are fifteen; and never recorded that validate-plugins was removed. The symlink hedge is resolved: apm_cli/security/gate.py's ignore_non_content() drops symlinks silently on deploy while apm_modules/ materialization dereferences them, so content survives that far and vanishes at install. Accepted with no replacement guard, per decision -- kyberforge/docs/hooks.md previously asserted a guard that had been deleted with its script. Four plugin READMEs still advertised `claude plugin install`; ADRs 0001, 0006, 0013, 0014, 0015 and 0019 described deleted machinery in the present tense, 0019 most consequentially as the live justification for the SessionStart hook's .apm/ path. CONTEXT.md's "apm package" entry forbade "plugin" while using it in its own body, and "Output profile" lost the antecedent for "one catalogue serves both". run-tests.sh gains the .claude/skills/ exclusion run-bats.sh already had. Latent today -- no test-*.sh lives under any .apm/skills/*/tests/ -- but apm now deploys those directories, so one would be discovered twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
909 lines
55 KiB
Markdown
909 lines
55 KiB
Markdown
# Enforcement gates
|
|
|
|
Reference for this repo's pre-commit and pre-push hooks: what each one guards, what its numbers
|
|
mean, and which shapes were tried and rejected. Read it when a gate fails, before changing anything
|
|
in `.pre-commit-config.yaml`, or before "fixing" something that looks like an inconsistency — several
|
|
of the oddities documented here are load-bearing and have already been re-litigated once.
|
|
|
|
`AGENTS.md` carries only the operative rules an agent needs in the moment. The reasoning lives here.
|
|
|
|
---
|
|
|
|
## Running the gates
|
|
|
|
| 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 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
|
|
`default_install_hook_types`, so a plain install silently skips `commit-msg` (Conventional Commits)
|
|
and `pre-push` (everything below).
|
|
|
|
The pre-push command reports **11** hooks, not 9. The extra two are pre-commit's own `meta` hooks,
|
|
`check-hooks-apply` and `check-useless-excludes`: they declare no `stages:`, so they run at every
|
|
stage including this one. Both are declared in this repo's `.pre-commit-config.yaml` like everything
|
|
else — what separates them is `repo: meta` (pre-commit's own built-ins) from `repo: local`. Nine
|
|
is the count of hooks this repo authors itself.
|
|
|
|
**The caveat: one of those 9 is a silent no-op under that invocation.**
|
|
`check-release-needed` exits 0 immediately unless `PRE_COMMIT_REMOTE_BRANCH` equals
|
|
`refs/heads/main`, and pre-commit exports that variable only from the real pre-push git hook during
|
|
an actual `git push`. Running the stage by hand — or from a CI runner — therefore reports it
|
|
`Passed` having checked nothing. That is by design for feature branches — pushing WIP must not be
|
|
blocked on cutting a premature tag — but it means `--hook-stage pre-push --all-files` is a full
|
|
rehearsal of 8 hooks and a skip of the ninth. The script's own header records the same gap for
|
|
a PR merged through Gitea's merge button, where no local push happens at all.
|
|
|
|
## The pre-push gate
|
|
|
|
Nine hooks, grouped below by what they guard rather than by the order `.pre-commit-config.yaml` declares them in.
|
|
|
|
**Core checks**
|
|
|
|
| Hook | Guards |
|
|
|---|---|
|
|
| `run-tests` | `bash tests/run-tests.sh --strict` — the whole suite, skips fatal (see [Tests](#tests)) |
|
|
|
|
**Generated-content drift gates**
|
|
|
|
| Hook | Guards |
|
|
|---|---|
|
|
| `check-vale-style-sync` | skill-audit's Vale copy matches agent-audit's canonical copy, plus six glob-coverage probes (see [Vale](#vale)) |
|
|
| `check-scope-walkup-sync` | `validate.sh`, `validate-provenance.sh`, `new-agent.sh` and `new-skill.sh`'s four independent `$HOME`/`.git`/`apm.yml` walk-up ports still agree behaviorally |
|
|
| `check-executables-allow-sync` | root `apm.yml`'s `executables.allow` key names kyberforge's actual version (see [apm gates](#apm-gates)) |
|
|
|
|
`check-executables-allow-sync` is the odd one in this group: it guards a *silent failure* rather than
|
|
drift in generated text.
|
|
|
|
**Artifact validators**
|
|
|
|
| Hook | Guards |
|
|
|---|---|
|
|
| `check-apm-agents-valid` | runs agent-audit's `validate.sh` over every real `plugins/*/.apm/agents/*.agent.md` (see [Agent files](#agent-files-take-the-description-gates-not-the-body-gate)) |
|
|
|
|
**apm's own gates**
|
|
|
|
| Hook | Guards |
|
|
|---|---|
|
|
| `apm-audit-ci` | `apm audit --ci` once per manifest — root plus each of the six plugin packages |
|
|
| `apm-pack-check-clean` | `apm pack --check-versions --check-clean --dry-run` — the compiled marketplace still matches what `apm.yml` + `.apm/` would generate, and per-package versions agree with the `per_package` strategy |
|
|
|
|
**Host validators** (needs the `claude` CLI on PATH)
|
|
|
|
| Hook | Guards |
|
|
|---|---|
|
|
| `validate-marketplace` | `claude plugin validate --strict` on the root marketplace manifest |
|
|
|
|
**Release**
|
|
|
|
| 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)) |
|
|
|
|
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`.
|
|
|
|
## 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
|
|
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.
|
|
|
|
**Two things fall outside that scope, both deliberately.** The `[^/]+/SKILL\.md$` tail admits only a
|
|
`SKILL.md` sitting directly in a skill directory under `.apm/skills/`:
|
|
|
|
- the `plugins/kyberforge/docs/research/examples/` reference skills, which are vendored upstream
|
|
corpus and not this repo's to gate;
|
|
- `plugins/kyberforge/.apm/skills/skill-author/assets/templates/SKILL.md` — inside `.apm/skills/`,
|
|
but two directories deeper. It is the `FILL IN:` scaffold `skill-author` copies, so its
|
|
`description: >` is a comment block rather than a description and every ADR-0020 measurement over
|
|
it would be meaningless. A reader adjusting the pattern needs to know it is there.
|
|
|
|
Everything else it matches exactly, with nothing over- or under-caught. Re-derive both halves:
|
|
|
|
```
|
|
git ls-files | grep -cE '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$' # the real skills
|
|
git ls-files | grep -E '^plugins/[^/]+/\.apm/skills/.*SKILL\.md$' \
|
|
| grep -vE '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$' # the scaffold only
|
|
```
|
|
|
|
The first count equals the number of skill directories (`ls -d plugins/*/.apm/skills/*/ | wc -l`);
|
|
the second returns exactly the template. Every other tracked `SKILL.md` in the tree is one of the
|
|
four vendored `plugins/kyberforge/docs/research/examples/skill-write/` corpus files, excluded by the
|
|
`.apm/skills/` segment — the first of the two deliberate exclusions above.
|
|
|
|
### Two independent gate families, neither replaced the other
|
|
|
|
**Family 1 — agentskills.io spec backstop** (unchanged, conformance not quality):
|
|
|
|
| Constant | Value | Measured over |
|
|
|---|---|---|
|
|
| `MAX_LINES` | 500 | whole file, **frontmatter included** |
|
|
| `MAX_WORDS` | 2,770 | whole file, **frontmatter included** |
|
|
|
|
**Family 2 — ADR-0020 context budget** (measured differently, on purpose):
|
|
|
|
| Check | SUGGESTION | FAIL | Measured over |
|
|
|---|---|---|---|
|
|
| `description` characters | 250 | 400 | the YAML-**folded** value |
|
|
| body words | 600 | 900 | **body only** — everything after the frontmatter's closing `---` |
|
|
|
|
Plus two hard FAILs with no suggestion tier:
|
|
|
|
- **A missing, valueless or `null` `description:`.** Not a skip. The description is the one field
|
|
preloaded into every session, so a gate that declines to measure it reports green. (This is not
|
|
hypothetical: `description:` with no value followed by `model: sonnet` let a line regex capture the
|
|
*next* key, which looked non-empty, so the "missing or empty" branch never fired and every gate
|
|
below early-returned on the genuinely empty folded value — exit 0, zero output, on a blocking gate.)
|
|
- **Every `references/<file>.md` a body names must exist** on disk. A dispatch table pointing at a
|
|
file that was never written is a silently dead branch, and nothing else in the gate/audit/vale
|
|
stack notices it.
|
|
|
|
A file can sit well inside one family and fail the other. 2,770 whole-file words is a conformance
|
|
backstop; 900 body-only words is a quality gate. Conflating them is what produced the current state.
|
|
|
|
### An unresolved routing target is not automatically a FAIL
|
|
|
|
A boundary-clause target that resolves to no skill or agent has **three** possible verdicts, not one
|
|
(`unresolved_targets()` in `scripts/skill-size-check.sh`):
|
|
|
|
| Verdict | When |
|
|
|---|---|
|
|
| **SUGGESTION** — the default | the target does not resolve and neither promotion condition below holds |
|
|
| **blocking ERROR** | the target is written in **route notation** — `/name` for any name, or any arrow form (a bare `-> name` only when the name is hyphenated, a backticked `` -> `name` `` for any — see the gap below); **or** it is a bare **terminal** name (not a compound modifier) **corroborated** by another target in the same sentence that *does* resolve |
|
|
| **INFO, "DID NOT RUN"** | no skill universe could be determined for the path at all — the targets are named and left unchecked, exit 0 |
|
|
|
|
The default is deliberately soft because a hyphenated word in a boundary clause is as likely to be a
|
|
tool, a file format or an English compound as a route: "pre-commit hooks" is prose about a tool and
|
|
never reaches the check at all, being a compound modifier rather than a terminal name. The
|
|
SUGGESTION text says how to opt in — write it as `/name` or `-> name` and it gets checked properly.
|
|
|
|
**The two promotion conditions are not symmetric, and the order matters.** `_add()` decides
|
|
**notation first**: when the name is written `/name`, or reached through any arrow form, the target
|
|
is marked error-eligible there and the terminal test is never run. Terminality gates only the *bare*
|
|
path — a name in prose earns its error from corroboration, and a compound modifier can never dangle.
|
|
Reading the row as "terminal AND (notation OR corroborated)" gets the notation half backwards: it
|
|
predicts that `` … Do not use for Y — use /no-such-skill afterwards. `` is a SUGGESTION, because
|
|
`afterwards` is a follower outside `FOLLOWER_OK`. It exits 1. That was the defect — `-> name` reached
|
|
`_add()` with `strict=True` from both its call sites and `/name` did not, so the one spelling
|
|
ADR-0020 offers an author who wants a route checked unconditionally was the one spelling a stray
|
|
follower could silence.
|
|
|
|
**Known gap: a BARE arrow target must be hyphenated.** Target extraction is built on `NAME_HYPH` in
|
|
`scripts/skill-size-check.sh`, which requires at least one hyphen, and `ARROW_BOUNDARY` inherits
|
|
that. So `Not X -> gitea-prs` is extracted and checked, while `Not X -> triage` yields no target.
|
|
The exclusion is deliberate, not an oversight: `research`, `triage`, `forge`, `prototype` and `tdd`
|
|
are all real skill names *and* ordinary English, so a bare single-word rule would flag most of the
|
|
corpus. The marked spellings carry no such restriction — `` `triage` `` and `/triage` are both
|
|
extracted — and are the forms to prefer. **Both arrow spellings are recognised:** `ARROW_MARKED`,
|
|
`ARROW_BOUNDARY` and `BOUNDARY_ARROW` are each built from `(?:->|→)`, so the unicode arrow `→`
|
|
behaves exactly like `->` in every case below. Cite these constants by symbol name, never by line
|
|
number: the script moves often enough that a pinned line lands a reader in an unrelated comment
|
|
block and reads as plausible.
|
|
|
|
**The gap is no longer silent.** It used to be exactly that — no ERROR, no SUGGESTION, exit 0 — which
|
|
made the dangling-target SUGGESTION's own advice unsafe for a single-word skill: taking it silenced
|
|
the finding instead of checking it. `boundary_clause_status()` now separates the case out and
|
|
reports it as `unparsed` (see below), naming the parse failure and the two spellings that fix it.
|
|
The target is still not *resolved*; the author is now told so rather than left with a green gate.
|
|
`tests/test-adr0020-targets.sh` covers both directions (`arrow-single-word-target` and the silent
|
|
control `arrow-single-word-marked`).
|
|
|
|
Corroboration is what makes the soft default safe: a sentence whose *other* target resolves is
|
|
demonstrably a routing sentence, so a sibling that does not resolve is a typo rather than a noun, and
|
|
gets promoted.
|
|
|
|
### Target resolution walk
|
|
|
|
Resolution walks up **from the file being checked** — never from the script's own location. Deriving
|
|
it from `${BASH_SOURCE}` leaked holocron's 39-skill universe into every consumer repo running the
|
|
hook through pre-commit, so a consumer skill routing to `skill-audit` resolved against a plugin it
|
|
had never installed.
|
|
|
|
The walk finds an **authoring root**: the nearest ancestor holding `plugins/*/.apm/skills` or
|
|
`plugins/*/.apm/agents`, falling back to the nearest ancestor holding `.git`. **Two passes, not one
|
|
interleaved walk**, so a nested `.git` (a submodule, a sub-package worktree) cannot beat a real
|
|
monorepo root further up.
|
|
|
|
The universe is then:
|
|
|
|
1. every skill and agent under `<root>/plugins/*/` — sibling plugins resolve, which is what a
|
|
monorepo means;
|
|
2. the checked file's own apm package;
|
|
3. the packages that package declares in **its own** `apm.yml` `dependencies.apm`.
|
|
|
|
The **root** manifest's `dependencies:` block is not read, and no plugin here declares a cross-plugin
|
|
apm dependency — none needs to.
|
|
|
|
Deployed `.claude/` / `.agents/` trees are consulted **only** when the walk found no plugin monorepo
|
|
root, whether it landed on a bare `.git` ancestor or on nothing at all. That is the consumer case.
|
|
|
|
**The gate keys on which of the two passes matched, never on whether the root contributed a new
|
|
name.** A name-count delta looks equivalent and is not: `_collect_authoring_root()` re-collects the
|
|
checked file's own plugin, whose names the earlier steps already added, so a single-plugin monorepo
|
|
shows a delta of zero and would wrongly reach for the deployed trees — including the user's global
|
|
`~/.claude/skills`, making the verdict depend on what happens to be installed.
|
|
|
|
Why it matters: those trees are gitignored `apm install` output, present only on a machine that has
|
|
run it. Four cross-plugin targets here (`gitea-branches` → `git-branches`, `gitea-branches` →
|
|
`git-history`, `gitea-issues` → `git-branches`, `gitea-workflow` → `git-workflow`) once resolved
|
|
through `.claude/skills/` alone, so **the same commit measured 2 dangling targets on a developer
|
|
machine and 6 on a fresh clone**. A gate shipping hot with no baseline cannot give two answers.
|
|
|
|
Verified: running the hook over a tree holding only `plugins/` and the root `apm.yml`, with no
|
|
`.claude/` or `.agents/` anywhere, produces findings identical to the working tree — confirming the
|
|
two trees agree on the current corpus, independent of what happens to be installed locally.
|
|
|
|
### Boundary-clause detection: three outcomes, not two
|
|
|
|
`boundary_clause_status()` returns one of three values, and the two findings get separate messages:
|
|
|
|
| Status | When | Reported as |
|
|
|---|---|---|
|
|
| `present` | a prose marker (`do not`, `instead`, `rather than`, `not for`) or an arrow clause was found | nothing |
|
|
| `absent` | neither was found | SUGGESTION: add a boundary clause, in either form |
|
|
| `unparsed` | an arrow clause was found and **no target could be read out of it** | SUGGESTION: the clause is present — this is a *parse* failure, not a missing clause |
|
|
|
|
The third had to be split out. Collapsing it into `absent` is a **wrong** finding, not a strict one:
|
|
it sends the author to add a clause that is already there. Three of them instead reworded a correct
|
|
clause until the regex accepted it, one stripping the very filename that discriminates the skill
|
|
from its neighbour (**#110**).
|
|
|
|
`unparsed` is narrow and certain on purpose. It fires only on the arrow form, which *always* names a
|
|
target, so zero targets means the name is written in a shape the extractor cannot see — in practice
|
|
a bare single-word target, per the known gap above, and the message says to write it `` `name` `` or
|
|
`/name`. A **prose** clause yielding no target is not reported at all: "Do not use for anything else"
|
|
is a complete and legitimate boundary clause that names nowhere to go.
|
|
|
|
**One arrow, one target.** An arrow clause naming two or more targets draws its own SUGGESTION,
|
|
quoting both names and asking for a split, because only the first is ever resolved: the conjunction
|
|
continuation (`CONT_MARKED` / `CONT_ANY`) is wired to the prose route verbs and never to arrows. So
|
|
`Not X -> a or b` resolved `a`, left `b` resolved by nothing and reported by nothing, and then let
|
|
the audit print "1 of 1 boundary target(s) resolve" on a clause naming two — a gate under-reporting
|
|
its own coverage, which is the one failure mode ADR-0020 says a gate must not have (**#107**). The
|
|
clause is **rejected rather than the arrow scan extended**: extending it would widen the resolver's
|
|
deliberately conservative false-positive tuning across every arrow in the corpus, where splitting
|
|
costs the author one full stop. The convention is one arrow per target — `Not X -> a. Not Y -> b.` —
|
|
already what every retrofitted `gitea-*` skill does in practice, now stated in
|
|
`skill-author`'s `references/contract.md` instead of being folklore.
|
|
|
|
**Dotted filenames in a boundary clause now parse.** `CLAUSE_BODY` — what may sit between `Not` and
|
|
the arrow — used to be `[^.;]`, a class that cannot cross a `.`, so every clause naming a dotted
|
|
filename between the two (`AGENTS.md`, `.vale.ini`, `.pre-commit-config.yaml`) was invisible to both
|
|
`BOUNDARY_ARROW` and `ARROW_BOUNDARY`. The two resulting failures were different sizes (**#110**):
|
|
|
|
- with a **backticked** target the clause was *misdiagnosed*. The backtick sweep still extracted the
|
|
target, so the route was checked, but the gate reported "no boundary clause" on a clause that was
|
|
present and working. That is the misdiagnosis the three rewordings above came from.
|
|
- with a **bare** target the clause was *unchecked*. `ARROW_BOUNDARY` is the only extractor for a
|
|
bare arrow target, so `Not AGENTS.md -> no-such-skill` produced no target, no dangling report and
|
|
no missing-clause SUGGESTION. Silence, not noise — the worse of the two.
|
|
|
|
`CLAUSE_BODY` is now `(?:[^.;]|\.(?=\S))`: a dot inside a filename is followed by a non-space, a
|
|
sentence-ending dot by whitespace or end of string, so the class crosses `AGENTS.md` and still stops
|
|
at a real sentence end. **Read the second bullet forward as well as back:** a bare target sitting
|
|
after a dotted filename is now extracted, resolved, and a blocking ERROR when it dangles, where the
|
|
same clause used to pass unchecked in silence.
|
|
|
|
### SUGGESTION-only checks
|
|
|
|
Deterministic to measure, judgment to act on:
|
|
|
|
- a description with **no boundary clause at all** (`absent`);
|
|
- an **arrow clause whose target could not be read** (`unparsed`);
|
|
- an **arrow clause naming more than one target**;
|
|
- a `## Gotchas` section with **more than five entries**;
|
|
- a `## Gotchas` section over **25% of the body**.
|
|
|
|
### Hand-invoked skills are exempt from the routing rules, and only those
|
|
|
|
A skill or agent whose frontmatter carries `disable-model-invocation: true` skips three checks:
|
|
|
|
- the boundary-clause check, `absent` and `unparsed` alike;
|
|
- the multi-target arrow check;
|
|
- the 250-character description **target** (`hand_invoked()` in `scripts/skill-size-check.sh`).
|
|
|
|
It keeps the 400-character description FAIL and **both** body word tiers, and if its description
|
|
does happen to name a target, that target is still resolved and can still dangle.
|
|
|
|
Why the exemption is right: `disable-model-invocation: true` removes the skill from the
|
|
model-visible listing entirely — it is not preloaded, and the Skill tool refuses to call it — so its
|
|
description is never matched against user intent. ADR-0020 and `skill-author`'s contract therefore
|
|
give such a skill **one plain human-facing sentence**: no trigger list, no boundary clause. No
|
|
validator knew the field existed (**#108**), so the boundary-clause SUGGESTION fired on exactly the
|
|
shape the contract mandates, and its remedy — "so the router knows where NOT to send this skill" —
|
|
was addressed to a router that cannot see the skill at all. An author who followed the advice made
|
|
the file worse. There is no router to inform.
|
|
|
|
The half that does **not** lift is the point. The body is still loaded on invocation and still
|
|
competes with the caller's live conversation, so neither body tier moves. The 400-character ceiling
|
|
stands too: a hand-invoked description is not preloaded, but it is still the one line the user reads
|
|
when choosing from the `/` menu, and that ceiling is an outlier stop rather than a routing-quality
|
|
budget — which is precisely why the 250-character target is the tier that lifts.
|
|
|
|
The field is read as a **boolean**, not as a mention of the key. PyYAML already resolves the
|
|
unquoted YAML 1.1 booleans, so the extra handling catches a quoted `"true"`, which a host reads as
|
|
truthy; `disable-model-invocation: false` is the model-invoked case written out longhand and buys
|
|
nothing. A frontmatter parse failure returns false rather than raising — the flag is a *modifier* on
|
|
other checks, and `description_value()` on the same text already reports the broken frontmatter, so
|
|
raising here would diagnose one file twice two different ways.
|
|
|
|
`caveman` and `zoom-out` are the two carriers here. `tests/test-skill-size-check.sh` pins both
|
|
halves — what the carve-out lifts, each with a flag-removed control, and what it must not.
|
|
|
|
### `verbose: true` is load-bearing
|
|
|
|
The hook is declared `verbose: true` so the SUGGESTION tier is audible. pre-commit prints nothing at
|
|
all for a passing hook, and a SUGGESTION deliberately does not fail — without verbose every
|
|
suggestion is swallowed, which is exactly the invisibility ADR-0013 records for Vale warnings.
|
|
ADR-0020's preload arithmetic depends on it: writing to the 400-char FAIL delivers roughly half the
|
|
cut that writing to the 250-char SUGGESTION does, so the intended saving depends entirely on that
|
|
tier being visible. The numbers, and the measurement method behind them, are not restated here —
|
|
they live in ADR-0020's Consequences section, under "A ceiling does not produce an average", whose
|
|
figures are pinned to the base commit the decision was taken on (`f9b919d`). Quoting them here would
|
|
just create a second copy to go stale. It costs nothing on a clean file — the script prints only
|
|
findings.
|
|
|
|
### Duplicated constants
|
|
|
|
`skill-audit`'s `validate.sh` holds a second copy of the four ADR-0020 constants
|
|
(`DESC_SUGGEST_CHARS` / `DESC_MAX_CHARS` / `BODY_SUGGEST_WORDS` / `BODY_MAX_WORDS`), and
|
|
`agent-audit`'s `validate.sh` holds a third copy of the two description constants. They are copied
|
|
rather than imported because a cache-installed plugin's scripts cannot read files outside their own
|
|
plugin directory. `tests/test-skill-size-check.sh` asserts the copies agree, so drift fails CI rather
|
|
than silently letting an audit bless a skill the commit hook then rejects. The shared boundary
|
|
resolver block is embedded verbatim in all three scripts between `BEGIN`/`END ADR-0020 SHARED
|
|
BOUNDARY RESOLVER` markers and must stay byte-identical.
|
|
|
|
### `python3` and PyYAML are hard requirements
|
|
|
|
Both, and neither is a best-effort accelerator.
|
|
|
|
`python3` because the script measures the **folded** `description` value. Most descriptions here are
|
|
`>`-block scalars, so a regex over the raw lines measures indentation and newlines instead of the
|
|
value. Missing it fails the hook with an install pointer rather than skipping the ADR-0020 checks,
|
|
which would be a vacuous green. In practice it is already present — pre-commit is itself a Python
|
|
application.
|
|
|
|
**PyYAML** because the hand-rolled fallback frontmatter reader has been **removed deliberately**. It
|
|
disagreed with a real parser across the FAIL boundary — one corpus description measured 270
|
|
characters parsed and 412 unparsed — and a quoted `"description"` key or an explicit
|
|
`description: null` returned empty from it, silently skipping the description *and* routing checks. A
|
|
reader that mis-parses an unfamiliar scalar shape reports a clean pass on a file it never measured,
|
|
which is the exact vacuous-green failure the `python3` check exists to avoid. `pip install pyyaml`
|
|
(or `python3 -m pip install PyYAML`, or the distro's `python3-yaml`) if the hook reports it missing.
|
|
|
|
**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.
|
|
|
|
## Agent files take the description gates, not the body gate
|
|
|
|
`check-apm-agents-valid` runs agent-audit's `validate.sh` over every real
|
|
`plugins/*/.apm/agents/*.agent.md`. It derives its expected file set from `git ls-files` — the pattern
|
|
`tests/run-bats.sh` established — so an agent file deleted from the worktree but still tracked fails
|
|
the run, and **discovering zero agent files is an error, not a pass**. An untracked *new* agent file
|
|
is still validated: the derivation is one-directional on purpose, so uncommitted work is not blocked
|
|
but also cannot bypass the gate.
|
|
|
|
The hook exists because `validate.sh` was previously exercised only by `check-scope-walkup-sync`,
|
|
against synthetic `mktemp` fixtures — it had never run against the agent files it governs. That is
|
|
how ADR-0016 could be amended to bless a `disallowedTools` frontmatter field while `validate.sh`'s
|
|
allowlist still rejected it: spec and enforcer disagreed and every gate stayed green.
|
|
|
|
Agents take the ADR-0020 **description** gates (agent-audit's `validate.sh` holds its own copy of
|
|
those two constants) and, deliberately, **no body word gate**. A skill body is loaded into the
|
|
caller's context and competes with the live conversation; an agent body becomes the system prompt of
|
|
a *fresh* context. The rationale for the 900-word FAIL does not transfer. A bats test pins that
|
|
absence in agent-audit's validator — adding a body gate there contradicts the ADR rather than fixing
|
|
an inconsistency.
|
|
|
|
**Be precise about the scope of that guarantee: it holds for the *validator*, not for the shared
|
|
script.** `scripts/skill-size-check.sh` applies its body gate to whatever path it is handed, and
|
|
|
|
```
|
|
bash scripts/skill-size-check.sh plugins/*/.apm/agents/*.agent.md
|
|
```
|
|
|
|
exits 1 today with 900-word body FAILs on `git-orchestrate` and `gitea-orchestrate`. (Counts are
|
|
deliberately not pinned here — agent bodies are edited like any other file, and a figure in this
|
|
paragraph goes stale the moment one is trimmed. Run the command.) Agent files escape only because
|
|
the hook definitions filter on `SKILL.md`
|
|
— a file-pattern accident that happens to implement the design, not the design itself. **Do not
|
|
"extend" that hook's `files:` pattern to cover agents** on the assumption that the script already
|
|
knows the difference; doing so silently enforces a gate ADR-0020 declines to set.
|
|
|
|
## Current retrofit status
|
|
|
|
The ADR-0020 gates ship hot, with no baseline file — a shrinking baseline was considered and
|
|
rejected. The corpus is currently clean on both: 0 of 39 descriptions/bodies exceed their FAIL tier,
|
|
0 dangling targets, 0 `Kyberforge.CompositionNote` (Vale) errors. History: issue #99.
|
|
|
|
Nothing is grandfathered — a new skill, or an edit that crosses a FAIL tier, is blocked on first
|
|
commit. SUGGESTION counts are not pinned here; they move with every edit. Measure and check both
|
|
gates before starting work on a skill:
|
|
|
|
```
|
|
bash scripts/skill-size-check.sh plugins/*/.apm/skills/*/SKILL.md | grep -c '^SUGGESTION'
|
|
pre-commit run --all-files # size AND Vale — skill-size-check alone can pass while Vale still blocks
|
|
```
|
|
|
|
## The `rtk` prefix gate (ADR-0023)
|
|
|
|
`check-rtk-prefix` is a `repo: local` pre-commit hook running `scripts/check-rtk-prefix.sh` over
|
|
`^plugins/[^/]+/\.apm/(skills/.*\.md|agents/.*\.agent\.md)$`, with `README.md` excluded. It enforces
|
|
**ADR-0023 clause 1 and nothing else**: an executable, instructed local git command in plugin skill
|
|
or agent content is written `rtk git`.
|
|
|
|
It is wider in file scope than the ADR-0020 hooks — every markdown file under a plugin's
|
|
`.apm/skills/` and `.apm/agents/`, not `SKILL.md` alone — because the rule it enforces is about
|
|
commands an agent runs, and most of those live in `references/`, which the ADR-0020 gates do not
|
|
reach ([the `references/` blind spot](#the-blind-spot-references-is-unlinted-for-two-independent-reasons)).
|
|
|
|
### What it can decide, and what it declines to
|
|
|
|
ADR-0023 has three clauses and only the first is a pattern:
|
|
|
|
| Clause | Rule | Gated |
|
|
|---|---|---|
|
|
| 1 | executable + instructed → `rtk git` | yes |
|
|
| 2 | illustrative / referential → bare `git` | no — undecidable |
|
|
| 3 | machine-parsed or interactive → bare `git` | no — opt-out marker |
|
|
|
|
Clause 2 is a judgement about what a sentence is *doing*. "Run `git switch <branch>`" and "`git
|
|
switch` refuses rather than clobbering local edits" are the same token sequence. A gate that guessed
|
|
would fire on correct prose, and **a gate that fires on correct content gets added to `SKIP`** —
|
|
which disarms clause 1 along with it. So the hook looks only at the two contexts where a `git`
|
|
mention is unambiguously an instruction to execute:
|
|
|
|
- a line inside a fenced code block whose info string names a shell — `bash`, `sh`, `shell`, `zsh`,
|
|
`console`, `shell-session`. Fences tagged `text`, `yaml`, `json`, or tagged with nothing, are **not**
|
|
checked;
|
|
- the **opening** backticked span of a "Run" column cell in a markdown dispatch table, and only the
|
|
opening span.
|
|
|
|
That last narrowing is not fussiness. A Run cell routinely carries a command followed by prose about
|
|
it, and the prose is clause 2. `git-worktrees/SKILL.md` has both shapes on adjacent rows — one cell
|
|
reading `` `rtk git worktree add --track …` `` — always correct. `` `git worktree add <path>
|
|
<branch>` `` expands to exactly this (instruction, then reference), and a `**Never** …` row whose Run
|
|
cell is entirely explanation containing a bare `git push`. Checking every backticked span flags both;
|
|
checking only a leading span flags neither, and still catches the ordinary
|
|
`` | List | `git worktree list -v` | `` case the gate exists for.
|
|
|
|
### The clause-3 opt-out
|
|
|
|
A command that is deliberately bare — because rtk rewrites the output the skill parses, or because
|
|
the command is interactive — is exempted by putting the literal string `ADR-0023` **on the same
|
|
line**: in a shell comment for a code line, in the cell text for a table row.
|
|
|
|
Per line, never per block. A fenced procedure routinely mixes `rtk git` steps with one deliberately
|
|
bare command (`git-remotes/references/push.md` does exactly that), and a block-level marker would
|
|
silently disarm every checked line around the marked one. The cost is a repeated `# bare per
|
|
ADR-0023` in the three blocks of `git-log-format.md` where every line is deliberately bare; that
|
|
repetition is the price of the marked line being the only line the marker speaks for.
|
|
|
|
The marker is a plain substring match, so a line that mentions `ADR-0023` for an unrelated reason is
|
|
also exempt. Accepted deliberately: the marker records an author's opt-out, it is not a security
|
|
boundary, and a stricter form would only move the same trust to a different string.
|
|
|
|
### What it deliberately does not cover
|
|
|
|
- **Clause 2.** Nothing checks that an illustrative mention stayed bare. A sweep that re-prefixes a
|
|
referential `git` passes this gate. The inline reasons ADR-0023 requires on clause-3 sites are the
|
|
only defence, and they are prose.
|
|
- **Prose bullets.** Most of `branch-operations.md`, `merging.md` and `rewrite-history.md` instruct
|
|
in list items, not fences. Those are clause-1 sites the gate cannot see, because it cannot
|
|
distinguish them from clause-2 mentions in the same list.
|
|
- **`README.md`, excluded by pattern.** A skill-directory README is consumer-facing prose no agent
|
|
loads, and the `git clone https://github.com/bats-core/…` lines in the seven `tests/README.md`
|
|
files are setup instructions for a third party who has no `rtk`. Prefixing those would be actively
|
|
wrong, not merely noisy — see ADR-0023's consumer section.
|
|
- **Quoting.** The line splitter breaks on `;`, `|`, `&&`, `||`, `$(` and backticks without tracking
|
|
quotes, so a git command inside a quoted argument is decided by accident.
|
|
`rtk git submodule foreach 'git pull origin main || :'` passes because the segment holding the
|
|
inner command begins with `rtk` — the right answer for the wrong reason. Write
|
|
`foreach 'git a; git b'` and the second inner command is a false positive needing the marker.
|
|
ADR-0023 records this shape as one the rule itself does not decide.
|
|
- **Non-git commands.** Only `git` is checked. `rtk` fronts `gh`, `docker`, `kubectl` and others; no
|
|
gate covers those, and the corpus does not currently instruct them.
|
|
|
|
`tests/test-check-rtk-prefix.sh` pins all of it, including the false-positive cases. Its first case
|
|
reconstructs the plugin corpus as it stood on `main` before the #113 sweep and asserts the gate
|
|
fails there with at least 20 findings, one of them the `gitea-*` `git remote get-url origin` drift
|
|
the sweep missed — a gate that only passes on the already-fixed tree proves nothing about the drift
|
|
it was written for.
|
|
|
|
## Vale
|
|
|
|
Install the `vale` binary — `brew install vale` (macOS), `snap install vale` (Linux),
|
|
`choco install vale` (Windows), or see <https://vale.sh/docs/vale-cli/installation/>. No `vale sync`
|
|
is needed: the `Kyberforge` styles are **committed** under
|
|
`plugins/kyberforge/.apm/skills/{skill-audit,agent-audit}/assets/vale/styles/`, not downloaded
|
|
packages (ADR-0014).
|
|
|
|
### Two copies, one canonical
|
|
|
|
Wiring Vale as a deterministic prefilter for `skill-audit`/`agent-audit`'s Description dimension
|
|
(motivation: issue #84) is repo-specific, not part of the generic `lint` plugin, so it does not live
|
|
in `plugins/lint/` — and per ADR-0014 it no longer lives at the repo root either. It lives **twice**,
|
|
one copy per skill, both under `plugins/kyberforge/.apm/skills/`:
|
|
|
|
| Copy | Styles | `.vale.ini` sections |
|
|
|---|---|---|
|
|
| `agent-audit/assets/vale/` — **canonical** | `Kyberforge`, `KyberforgeCopilot` | `[**/agents/*.md]`, `[**/*.agent.md]` |
|
|
| `skill-audit/assets/vale/` — smaller duplicate | `Kyberforge` | `[**/SKILL.md]` |
|
|
|
|
Duplicated rather than shared because a plugin's cache-install copies only each skill's own files —
|
|
there is no cross-skill sharing to point at. `check-vale-style-sync` at pre-push is what keeps them
|
|
from drifting; `KyberforgeCopilot` is the one deliberate inequality, being scoped only to `.agent.md`
|
|
files for the Copilot-only "`Use proactively` has no effect" check.
|
|
|
|
### What Vale owns, and what stays LLM judgment
|
|
|
|
Eleven rule files across the two copies, six distinct rules:
|
|
|
|
| Rule | Vale scope | Bans | From |
|
|
|---|---|---|---|
|
|
| `Kyberforge.DescriptionOpener` | `text.frontmatter.description` | non-imperative openers ("This skill/agent…") | issue #84 |
|
|
| `Kyberforge.VagueWording` | `text.frontmatter.description` | vague capability wording ("helps with", "utilize", …) | issue #84 |
|
|
| `Kyberforge.PaddingPhrase` | `text` | generic "see `references/` for details" padding | issue #84 |
|
|
| `KyberforgeCopilot.ProactivePhrase` | `text.frontmatter.description` | `Use proactively` (no effect in Copilot) | issue #84 |
|
|
| `Kyberforge.SentenceOpenerThereIs` | `sentence` | "There is/are" sentence openers | ADR-0013 |
|
|
| `Kyberforge.CompositionNote` | `text.frontmatter.description` | architecture and composition prose in a description | ADR-0020 |
|
|
|
|
Vale covers the **pattern-matchable** sub-checks named in issue #84 plus, per ADR-0013, one
|
|
cherry-picked body-wide prose-pattern rule. Everything else stays LLM judgment: defaults-vs-menus,
|
|
why-rationale, the non-pattern-matchable body-discipline calls, near-miss exclusion strength, and
|
|
control calibration. New rules land directly in `styles/Kyberforge` and block immediately — there is
|
|
no trial tier.
|
|
|
|
The cherry-pick record, so it is not re-litigated:
|
|
|
|
- `Kyberforge.SentenceOpenerThereIs` **landed** — 22 held-out hits, both in-corpus hits clean
|
|
rewrites, zero suppressions needed.
|
|
- `Kyberforge.VagueQualifier` was cherry-picked and then **deleted**. 2 hits across the corpus as it
|
|
stood on 2026-08-08 (before the `.apm/` restructure): one marginal, and one unfixable false
|
|
positive — `caveman/SKILL.md` quotes `of course` as an example of filler, a mention rather than a
|
|
use — which forced the repo's only Vale suppression comments.
|
|
- `governance.md` and `CONTROLS.md` were evaluated as rule sources and **excluded**: nothing
|
|
prose-pattern-matchable to mine.
|
|
|
|
### Why every rule is `level: error`
|
|
|
|
Every alert is a FAIL, with no ignorable tier — same all-or-nothing model as shellcheck, the test
|
|
suite, and conventional-pre-commit. Graded severities do not work here: **Vale's exit code keys on
|
|
`error` alerts alone**, so a `warning` or `suggestion` rule exits 0, and pre-commit swallows a
|
|
passing hook's output. Such a rule would be invisible and would block nothing.
|
|
|
|
`MinAlertLevel` and `--minAlertLevel` are correspondingly **absent** from both `.vale.ini` files and
|
|
from the hook definitions. Under this model they are no-ops; adding one is not a missing knob.
|
|
|
|
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 both Vale copies (`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.
|
|
|
|
This repo's own `vale-audit-prefilter-skill` / `-agent` hooks consume the **identical**
|
|
plugin-bundled copies via `repo: local`. Deliberately not a third 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:
|
|
|
|
| Hook | Pattern |
|
|
|---|---|
|
|
| `vale-audit-prefilter-skill` | `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$` |
|
|
| `vale-audit-prefilter-agent` | `^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$` |
|
|
|
|
Only the **authoring source** triggers them. The `skill-author` scaffold template and the four
|
|
vendored research-corpus `SKILL.md` files match neither pattern (see
|
|
[Skill and agent context gates](#skill-and-agent-context-gates-adr-0020), which uses the same scope
|
|
for `skill-size-check`), so prose findings surface only when you edit a file this repo actually
|
|
authors. Without the binary the hooks fail with a bare "command not found" and no install pointer.
|
|
|
|
**Two hooks, not one combined hook.** Both manifests split the prefilter in two precisely because a
|
|
single hook can point at only one copy, and that copy 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)).
|
|
|
|
### The `.vale.ini` globs do no scoping
|
|
|
|
Each `.vale.ini`'s section globs are **path-agnostic** — `[**/SKILL.md]` for skill-audit's copy,
|
|
`[**/agents/*.md]` and `[**/*.agent.md]` for agent-audit's — and constrain filename *shape*, not
|
|
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 the audit skills
|
|
passing one explicit file per invocation. The two manifests scope **differently on purpose**:
|
|
|
|
| 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,
|
|
and `check-vale-style-sync`'s probe set is built to catch exactly that.
|
|
|
|
### The blind spot: `references/` is unlinted, for two independent reasons
|
|
|
|
Every `references/*.md` file in the corpus is outside the prose gate. Count them with
|
|
`git ls-files | grep -cE '^plugins/[^/]+/\.apm/skills/[^/]+/references/.*\.md$'` rather than reading
|
|
a figure here; it moves with every retrofit. This is the gap that matters most, because the context
|
|
contract's own remedy for an over-long body is to move prose **into** `references/` — the gate pushes
|
|
text across its own boundary and then stops watching it.
|
|
|
|
**Closing either cause alone changes nothing.** There are two, and they are independent:
|
|
|
|
| Cause | Where | Effect on a `references/` file |
|
|
|---|---|---|
|
|
| the `Kyberforge` style is scoped `[**/SKILL.md]` | `skill-audit/assets/vale/.vale.ini` | matches no section, so Vale lints 0 files and exits 0 |
|
|
| the hook's `files:` regex is `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$` | `vale-audit-prefilter-skill` in `.pre-commit-config.yaml` | the file is never handed to Vale at all |
|
|
|
|
Verified both ways. Handing skill-audit's `vale-wrap.sh` a reference file directly — bypassing
|
|
pre-commit entirely, so only the style scope is in play — prints `0 errors … in 0 files` and exits 0,
|
|
where the same wrapper on a `SKILL.md` reports `in 1 file`. And the hook's `files:` regex, applied to
|
|
`git ls-files`, selects only the skill-directory `SKILL.md` files scoped at the top of this page, so
|
|
pre-commit never hands Vale a reference file to begin with. Widening the glob to `[**/*.md]` would
|
|
still lint nothing through the hook; widening the hook's `files:` alone would hand Vale files its own
|
|
config declines to match, which is the [0-file NOT RUN](#a-0-file-vale-run-is-not-run) shape — a
|
|
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`
|
|
|
|
Both audit skills' Step 1 and both pre-commit hooks call **each copy's own**
|
|
`scripts/vale-wrap.sh`, not `vale`. It works around a confirmed **Vale 3.15.2** limitation:
|
|
`text.frontmatter.description` silently stops matching on most — not all — multi-line descriptions.
|
|
|
|
Verified by reproduction on a deliberately-bad fixture, not assumed:
|
|
|
|
| Description scalar spanning 2+ lines | Vale's behaviour |
|
|
|---|---|
|
|
| `>` folded block | 0 alerts, exit 0 — **broken** |
|
|
| plain (unquoted) continuation lines | 0 alerts, exit 0 — **broken** |
|
|
| single- or double-quoted, wrapped | 0 alerts, exit 0 — **broken** |
|
|
| `\|` literal block | alerts fire, exit 1 — lints normally |
|
|
|
|
The wrapper flattens the three broken forms to a single-line scalar in a scratch copy — or, for the
|
|
rare value no inline scalar can spell verbatim, a `|-` block with one content line — padding with
|
|
blank lines so **every other line number is unchanged**. `|` literal blocks and single-line
|
|
descriptions pass through untouched. Most descriptions in this repo are `>` blocks, so before the
|
|
wrapper a bad description in any of the three broken forms sailed straight through the prefilter.
|
|
|
|
### 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**.
|
|
|
|
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`.
|
|
|
|
`.pre-commit-config.yaml` drops the argument too, deliberately keeping the two entries identical.
|
|
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".**
|
|
|
|
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
|
|
bare `vale`, not the repo root.
|
|
|
|
Both audit skills' Step 1 passes no `--config` either. Step 1 resolves the script relative to the
|
|
skill's own directory so the call works from an installed plugin cache; a relative `--config`
|
|
alongside it would resolve against the cwd instead, yielding `E100 Runtime error … does not exist`
|
|
and exit 2 — which both skills' fallback misreads as "vale unavailable" and silently downgrades to
|
|
full LLM judgment.
|
|
|
|
`tests/test-vale-wrap.sh` regression-tests this against **skill-audit's** copy specifically: its
|
|
fixtures are all `SKILL.md`-shaped, and only skill-audit's `.vale.ini` carries that glob section.
|
|
|
|
### A 0-file Vale run is NOT RUN
|
|
|
|
Vale reports 0 files only when the path it is handed matches **no glob section at all** — a
|
|
differently-named file, or a directory argument holding nothing that matches. That run prints
|
|
|
|
```
|
|
✔ 0 errors ... in 0 files.
|
|
```
|
|
|
|
and exits 0, indistinguishable from a clean pass. Both audits therefore treat a 0-file Vale run as
|
|
**NOT RUN** and fall back to full LLM judgment rather than reporting the Description dimension
|
|
clean.
|
|
|
|
### Pre-push
|
|
|
|
`vale` is a **pre-push** dependency too, not only pre-commit. `check-vale-style-sync` runs **six
|
|
glob-coverage probes** by invoking `vale --config` — one representative path per file shape the
|
|
prefilter is supposed to cover. They are the only assertions in the script that catch a `.vale.ini`
|
|
glob typo (`[**/SKILL.md]` → `[**/SKILLS.md]`), the failure mode where every text-level check stays
|
|
clean while vale lints zero files. As a warning this self-disabled on exactly that mutation and
|
|
exited 0, and since pre-commit swallows a passing hook's output the stderr line was never seen — the
|
|
hook reported `Passed`. Missing `vale` is therefore a hard failure here.
|
|
|
|
The opt-out is `CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1`, and **it is not `SKIP=`**: the hook
|
|
still runs and still asserts everything verifiable from file text, but the six probes do not, and its
|
|
summary says so explicitly —
|
|
|
|
```
|
|
Vale style sync check passed (text-level only, vale unavailable): … 0 glob probe(s) verified.
|
|
```
|
|
|
|
Use it only on a machine that genuinely cannot install `vale`, and read that line as "the glob axis
|
|
was not checked", not as a pass. The hook is `verbose: true` for exactly that reason — its clean
|
|
output is a single line, so it costs one line per push.
|
|
|
|
### Mentioning banned phrasing without tripping the rule
|
|
|
|
House convention: banned phrasing that must be **mentioned** rather than used goes in backticks or a
|
|
fenced code block. Vale skips code spans and fences, so no suppression is needed — which is why this
|
|
document quotes `Use proactively` and "There is/are" the way it does.
|
|
|
|
Inline `<!-- vale Rule = NO -->` is the fallback **only** where backticking is impossible. Use the
|
|
HTML-comment form; the MDX `{/* */}` form does not work in plain Markdown. The one time a rule forced
|
|
suppression comments, the rule was deleted instead (see the `VagueQualifier` entry above).
|
|
|
|
## Tests
|
|
|
|
```
|
|
bash tests/run-tests.sh # every test-*.sh plus the bats suite
|
|
bash tests/run-tests.sh --bats-only # just bats
|
|
```
|
|
|
|
First run auto-initializes the bats submodules; no manual `git submodule update` needed.
|
|
|
|
**Exit 77 = SKIPPED.** A suite that skips because a dependency is missing does **not** fail an ad-hoc
|
|
run. The pre-push hook invokes the same script as `--strict` (`RUN_TESTS_STRICT=1` is equivalent),
|
|
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, three suites skip —
|
|
`test-check-vale-style-sync.sh`, `test-vale-hooks-consumer.sh`, `test-vale-wrap.sh` — and the strict
|
|
failure names each one and what to install.
|
|
|
|
`tests/run-bats.sh` derives the set of `.bats` files it expects from `git ls-files`, so a `.bats`
|
|
file deleted from the worktree but still tracked in the index fails the run rather than silently
|
|
shrinking the suite. Remove one with `git rm` (or stage the deletion) when intentional; an untracked
|
|
new `.bats` file is picked up and needs no ceremony.
|
|
|
|
Both discovery walks (`tests/run-bats.sh` and `tests/run-tests.sh`) exclude `apm_modules/`:
|
|
`apm install` materializes a full copy of every plugin there, and running a dependency's copy of a
|
|
`.bats` file breaks its relative path to the bats helpers — **167 spurious failures** before the
|
|
exclusion landed.
|
|
|
|
## apm gates
|
|
|
|
### `apm-audit-ci`
|
|
|
|
Runs `apm audit --ci` **once per manifest** — the root one and each of the six plugin packages —
|
|
because the root-only invocation audits the marketplace manifest and **nothing else**, and
|
|
`apm-pack-check-clean` does not parse plugin `dependencies:` blocks either. Verified: a malformed
|
|
dependency entry passes `apm pack --check-versions --check-clean --dry-run` and fails
|
|
`apm audit --ci` in that package's directory. Costs ~0.5s per package.
|
|
|
|
It verifies **exactly two things** per manifest and claims no more:
|
|
|
|
- **manifest-parse** — each `apm.yml` parses as a valid APM manifest. Unconditional; verified to fire
|
|
on a dependency entry missing its `git`/`path`/`registry` field (`Cannot parse apm.yml`).
|
|
- **lockfile-exists** — any package declaring dependencies has a consistent `apm.lock.yaml`.
|
|
Conditional, and vacuous while every plugin `apm.yml` declares `dependencies: {apm: [], mcp: []}`;
|
|
it arms itself the moment one does not (verified by adding a git dependency to
|
|
`plugins/lint/apm.yml`).
|
|
|
|
It does **not** enforce an org policy. apm discovers one from the git remote and only understands
|
|
github.com and Azure DevOps, so against this repo's self-hosted Gitea remote it prints:
|
|
|
|
```
|
|
No org policy found at unknown; enforcement skipped
|
|
```
|
|
|
|
**Do not "fix" that with `policy.fetch_failure_default: block` in `apm.yml`.** apm's own message
|
|
suggests it; it was tried on a scratch copy and **rejected**. With no reachable policy source it does
|
|
not make the check meaningful, it makes it permanently red — `apm audit --ci` exits 1 with
|
|
`No org policy found at unknown (policy.fetch_failure_default=block)` on every push, forever. A gate
|
|
that can never go green is not a gate. Revisit only if this repo gains a policy source apm can reach.
|
|
|
|
It also does not scan for hidden Unicode: that scan is plain `apm audit`, a different mode (`--ci`
|
|
refuses to combine with `--file`/`--strip`/`--dry-run`/`PACKAGE`), and plain `apm audit` here reports
|
|
`No apm.lock.yaml found -- nothing to scan` and exits 0. Adding it would buy a second vacuous check.
|
|
|
|
### `check-executables-allow-sync`
|
|
|
|
apm gates a package's `hooks/` and `bin/` on an **exact `<package>#<version>` dictionary lookup** in
|
|
root `apm.yml`'s `executables.allow` (`apm_cli/security/executables.py`, `is_package_approved`).
|
|
There is no wildcard and no version-less form.
|
|
|
|
So bumping `plugins/kyberforge/apm.yml`'s `version:` without bumping the key **errors nowhere**: the
|
|
entry simply stops matching, the gate blocks the hook, kyberforge's `SessionStart` hook stops
|
|
deploying, and the apm install goes quietly stale — the exact failure ADR-0019 exists to end,
|
|
reintroduced through the mechanism meant to secure it. ADR-0019 records this as a live failure mode;
|
|
the release that shipped the hook hit it immediately.
|
|
|
|
`scripts/check-executables-allow-sync.sh` parses `version:` out of `plugins/kyberforge/apm.yml` and
|
|
asserts root `apm.yml` carries the matching `kyberforge#<version>` key. A comment in the
|
|
`executables:` block stays as the human-facing pointer; the hook is what actually holds. It parses
|
|
with PyYAML where importable and falls back to a two-shape scan otherwise, so a missing pip package
|
|
cannot become the thing that blocks every push.
|
|
|
|
## `.claude/settings.json`
|
|
|
|
**apm owns this file. Nothing repo-authored goes in it.**
|
|
|
|
`apm audit --ci` replays the install into a scratch tree and diffs the result byte-for-byte, so
|
|
anything apm would not have written there — an `enabledPlugins` block, a real `hooks` entry — is
|
|
permanent drift that fails `apm-audit-ci`. A hook you want in this repo is authored in
|
|
`plugins/<name>/.apm/hooks/` and deployed by apm, never hand-written here.
|
|
|
|
Its committed content is whatever apm last wrote, which today is the merged `SessionStart` entry for
|
|
kyberforge's `check-apm-current.sh`. That is apm's own output and it belongs in the commit (ADR-0019;
|
|
ADR-0018's statement that the committed content is exactly `{"hooks": {}}` is superseded on that
|
|
point only). Machine-specific settings go in the gitignored `.claude/settings.local.json`, which apm
|
|
does not deploy and the replay does not compare; shared enforcement belongs in
|
|
`.pre-commit-config.yaml`.
|
|
|
|
### Why it is excluded from `pretty-format-json`
|
|
|
|
It is the **second and last alternation** in that hook's `exclude:` pattern, and the only one there
|
|
for a reason other than "generated manifest". Mind which number you are quoting: **two alternations,
|
|
expanding to two real files** — `.claude-plugin/marketplace.json`, plus this one.
|
|
|
|
`pretty-format-json --autofix` sorts object keys unless `--no-sort-keys` is passed, while apm's hook
|
|
integrator emits insertion order (`matcher` before `hooks`, `type` before `command`). Leaving the
|
|
file in that hook's scope therefore rewrites apm's output into a form apm would never produce on the
|
|
way into **every** commit, and `apm-audit-ci` then reports permanent drift on a file with an empty
|
|
`git diff` — exactly what happened when the `SessionStart` hook first landed in `2e395a4`. Re-running
|
|
`apm install` fixes the file; leaving it in scope would re-break it on the very commit carrying the
|
|
fix.
|
|
|
|
**Load-bearing. Do not tidy it out of that list** (see `LESSONS.md`, 2026-08-14).
|
|
|
|
## Pushing without a network
|
|
|
|
No pre-push hook needs the network. Every entry in root `apm.yml`'s `marketplace.packages[]`
|
|
resolves from a local `./plugins/<name>` path, so `apm-pack-check-clean` never calls `git ls-remote`.
|
|
|
|
`apm-audit-ci` calls `apm` too but was always local: its org-policy discovery resolves nothing on
|
|
this remote before any network call.
|
|
|
|
---
|
|
|
|
## See also
|
|
|
|
- `docs/adr/0020-skill-description-and-body-context-contract.md` — the context contract, its
|
|
enforcement table (deterministic vs. auditor judgment), and every rejected alternative
|
|
- `docs/adr/0019-session-start-hook-keeps-the-apm-install-current.md` — the `SessionStart` hook, the
|
|
executable-trust gate, and the version-pinned allow key
|
|
- `docs/adr/0024-apm-is-the-only-supported-install-path.md` — apm as the sole install path, and the
|
|
deletion of the flat content mirror and its `check-plugin-content-sync` gate. It supersedes
|
|
`docs/adr/0017-plugin-content-mirror-bridges-apm-to-host-discovery.md` (**superseded** — plugin
|
|
content sync, kept as the historical record)
|
|
- `docs/adr/0015-apm-replaces-plugin-marketplace-authoring.md`,
|
|
`docs/adr/0014-vale-prefilter-ships-from-the-plugin.md` — apm-generated manifests, committed Vale
|
|
styles
|
|
- `docs/spec/architecture.md` — directory structure, install pipeline, what is generated and what is
|
|
hand-authored
|
|
- `.pre-commit-config.yaml` — the hooks themselves, with inline rationale comments
|