Why: the relocation into gates.md and architecture.md moved text faster than it
verified it, and a review found eight assertions that do not survive contact
with the repo.
- gates.md cited docs/research/examples/, which does not exist; the tree is at
plugins/kyberforge/docs/research/examples/. The same wrong string survives at
ADR-0013:98, where it originated, and is tracked separately.
- gates.md's description-budget arithmetic was built on 23,427, a figure
ADR-0020 measured at f9b919d and pins there, and compared a name-inclusive
total against a description-only ceiling. Dropped rather than re-measured,
following the precedent this branch set for CONTEXT.md's token figure: a
second live copy is a second thing to go stale.
- gates.md listed "every boundary-clause routing target must resolve" as a hard
FAIL. skill-size-check.sh emits unresolved targets as SUGGESTION by default,
promotes to ERROR only when terminal and either route-notated or corroborated
by a resolving target in the same sentence, and reports INFO DID NOT RUN when
no universe resolves. Verified with five fixtures; replaced with a
three-verdict table.
- gates.md and README called `pre-commit run --hook-stage pre-push --all-files`
the whole push gate. check-release-needed.sh exits 0 unless
PRE_COMMIT_REMOTE_BRANCH is refs/heads/main, which only the real git hook
exports -- its own header comment titles this a "Known gap".
- "repo-defined" was loose: the two meta hooks are also declared here, and what
separates them is repo: meta from repo: local.
- architecture.md claimed `when:` appears in two of 39 skill sources. Exactly
one SKILL.md carries it; the second hit is a META.md. The likely origin is
skill-implementation-workflow.md:233, which claims META.md sits alongside
every skill when one exists in the whole tree.
- architecture.md said core/AGENTS.md is "deployed beside" CLAUDE.md.
deploy-manifest.sh sends them to ~/.claude/ and ~/.agents/ respectively; the
relation is import, not adjacency.
- gates.md's see-also cited ADRs as bare stems where its neighbours use paths.
Implementation notes: architecture.md's closing pointer sent readers to the
published description "for what a consumer actually gets", which was true
against an enumeration and is not after ADR-0021. It now names the boundary
relationship and points at the skills tree and README for an inventory. README's
bin bullet gains caveman and zoom-out, since ADR-0021 leaves it the only place
an inventory lives.
Impact: 16/16 pre-push hooks pass. Deliberately not fixed here and tracked
separately: the 12 pre-commit-stage hooks gates.md does not document, its own
SentenceOpenerThereIs violation, and README's missing scripts/install.sh step.
Refs: #105
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TmFqzpuExLJv3m114XVE9w
681 lines
39 KiB
Markdown
681 lines
39 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 **16** hooks, not 14. 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`. Fourteen
|
||
is the count of hooks this repo authors itself.
|
||
|
||
**The caveat: one of those 14 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 13 hooks and a skip of the fourteenth. 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
|
||
|
||
Fourteen 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)) |
|
||
| `check-manifests` | `marketplace.json` and `plugin.json` paths resolve (needs `jq`) |
|
||
|
||
**Generated-content drift gates**
|
||
|
||
| Hook | Guards |
|
||
|---|---|
|
||
| `check-plugin-content-sync` | each plugin's flat `skills/agents/commands/hooks` mirror matches `.apm/` (issue #90) |
|
||
| `check-marketplace-mirror-sync` | `.github/plugin/marketplace.json` is byte-identical to `.claude-plugin/marketplace.json` — no apm output profile targets that path |
|
||
| `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-marketplace-check` | every `marketplace.packages[]` entry resolves, including network reachability of remote refs |
|
||
| `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** (both need the `claude` CLI on PATH)
|
||
|
||
| Hook | Guards |
|
||
|---|---|
|
||
| `validate-plugins` | `claude plugin validate --strict` on every plugin directory |
|
||
| `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)) |
|
||
|
||
Four of these shell out to `apm`: `apm-marketplace-check`, `apm-audit-ci`, `apm-pack-check-clean`,
|
||
and `check-plugin-content-sync` (via `scripts/sync-plugin-content.sh`, which wraps `apm pack`). The
|
||
first and third are bare `apm …` entries and the second 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`. `jq` is
|
||
needed by `scripts/check-manifests.sh` and `scripts/sync-plugin-content.sh` — those at least fail
|
||
loudly (`Error: jq is required but not installed`).
|
||
|
||
## 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`. That scope means it never lints the
|
||
`plugins/kyberforge/docs/research/examples/` reference skills. It is also shipped to external repos
|
||
as `kyberforge-skill-size-check` (see
|
||
[External consumers](#external-consumers-the-root-pre-commit-hooksyaml)).
|
||
|
||
### 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 **terminal** (not a compound modifier) **and** either written in route notation (`/name`, `-> name`) **or** 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.
|
||
|
||
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 fixed: 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 — **26 description
|
||
FAILs, 9 body FAILs, 2 dangling targets, 0 missing references, 58 SUGGESTIONs**.
|
||
|
||
### SUGGESTION-only checks
|
||
|
||
Three more, deterministic to measure but judgment to act on:
|
||
|
||
- a description with **no boundary clause at all**;
|
||
- a `## Gotchas` section with **more than five entries**;
|
||
- a `## Gotchas` section over **25% of the body**.
|
||
|
||
### `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.
|
||
|
||
## 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` (933), `gitea-orchestrate` (1,199) and
|
||
`apm-orchestrate` (1,080). 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 recording each
|
||
non-compliant skill's current numbers was considered and rejected in favour of hot gates.
|
||
|
||
Two independent hot gates are currently red, and the first will not warn you about the second.
|
||
|
||
| Gate | Current findings |
|
||
|---|---|
|
||
| `skill-size-check` | **26 of 39** descriptions and **9 of 39** bodies exceed their FAIL tier; 2 dangling targets; 58 SUGGESTIONs |
|
||
| `Kyberforge.CompositionNote` (Vale) | **10 errors across four skills**: `gitea-issues`, `gitea-labels-milestones`, `gitea-prs`, `gitea-workflow` |
|
||
|
||
`Kyberforge.CompositionNote` is the ADR-0020 Vale rule banning composition and architecture prose
|
||
from a description. Every Vale rule here is `level: error` with no ignorable tier, so touching any of
|
||
those four skills means fixing its prose findings as well as its size findings.
|
||
|
||
Consequence: editing a non-compliant skill *for any reason* means retrofitting it to the contract
|
||
first — a one-line fix to `gitea-prs` cannot be committed until that skill complies. This is
|
||
deliberate; it guarantees convergence and avoids a half-state. Tracked as Gitea issue **#99**.
|
||
|
||
Check where a skill stands before starting, and check **both** gates:
|
||
|
||
```
|
||
pre-commit run skill-size-check --all-files # size/context only
|
||
pre-commit run --all-files # size AND Vale
|
||
```
|
||
|
||
Scoping a retrofit off `skill-size-check` output alone leaves you blocked at the second gate.
|
||
|
||
## 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. A `SKILL.md` in the generated flat mirror matches
|
||
neither pattern, so prose findings surface only when you edit the file you are supposed to be
|
||
editing. 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.
|
||
|
||
### `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 **sixth 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: **six alternations,
|
||
expanding to sixteen real files** — 3 root marketplace manifests, 2 per plugin × 6 plugins, 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
|
||
|
||
Exactly **two** pre-push hooks need the network, for one shared reason: root `apm.yml`'s
|
||
`marketplace.packages[]` contains exactly one remote entry — `mattpocock-skills`,
|
||
`source: mattpocock/skills` — and resolving it needs a `git ls-remote`.
|
||
|
||
| Hook | Offline failure |
|
||
|---|---|
|
||
| `apm-marketplace-check` (`always_run`, resolves every entry) | `No cached refs (offline)` |
|
||
| `apm-pack-check-clean` (re-resolves the same entry) | `Error: Git network timeout during ls-remote` |
|
||
|
||
Pinning the entry to an exact version does **not** remove the call — an exact pin still ls-remotes.
|
||
`--offline` rescues neither.
|
||
|
||
To push without a network, skip both using pre-commit's own mechanism:
|
||
|
||
```
|
||
SKIP=apm-marketplace-check,apm-pack-check-clean git push
|
||
```
|
||
|
||
**Skip those two alone.** Verified under `unshare -rn`: the other twelve pre-push hooks pass offline
|
||
because they are real local checks. (`check-executables-allow-sync` landed after that run, but reads
|
||
two local manifests and makes no network call.) Adding any other hook to `SKIP` disarms it silently.
|
||
|
||
`apm-audit-ci` calls `apm` too but stays local: its org-policy discovery resolves nothing on this
|
||
remote *before* any network call, so it does not join the pair above.
|
||
|
||
---
|
||
|
||
## 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/0017-plugin-content-mirror-bridges-apm-to-host-discovery.md`,
|
||
`docs/adr/0015-apm-replaces-plugin-marketplace-authoring.md`,
|
||
`docs/adr/0014-vale-prefilter-ships-from-the-plugin.md` — plugin content sync, 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
|