feat(lint): wire Vale as deterministic prefilter for skill-audit/agent-audit #85

Merged
Defame1297 merged 45 commits from feat/84-vale-audit-prefilter into main 2026-08-10 16:46:59 +00:00
4 changed files with 50 additions and 11 deletions
Showing only changes of commit e9234f6d8a - Show all commits

View File

@@ -72,7 +72,7 @@ A standalone, repo-agnostic plugin (`plugins/lint/`) for configuring and running
### Vale audit prefilter (skill-audit / agent-audit)
Wiring Vale as a deterministic prefilter for `skill-audit`/`agent-audit`'s Description dimension (ADR motivation: issue #84) is repo-specific, not part of the generic `lint` plugin, so it doesn't live in `plugins/lint/` — but per ADR-0014 it also doesn't live at the repo root anymore. Two copies live inside `plugins/kyberforge/`, one per skill, since a plugin's cache-install only copies each skill's own files (no cross-skill sharing): `plugins/kyberforge/skills/agent-audit/assets/vale/` is canonical (`.vale.ini` plus a custom `Kyberforge` style covering description-opener banning ("This skill/agent..."), vague-capability wording ("helps with", "utilize", ...), and generic "see references/ for details" padding — and a `KyberforgeCopilot` style scoped only to `.agent.md` files for the Copilot-only "Use proactively has no effect" check), and `plugins/kyberforge/skills/skill-audit/assets/vale/` is a smaller duplicate (`Kyberforge` only, scoped to `SKILL.md`) kept in sync by `scripts/check-vale-style-sync.sh` (pre-push). A root-level `.pre-commit-hooks.yaml` exposes both copies (plus `skill-size-check`) so any external repo can enforce the same rules via `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, and the same mechanism covers CI (`pre-commit run --all-files`). This repo's own `vale-audit-prefilter-skill`/`-agent` pre-commit hooks consume the identical plugin-bundled copies via `repo: local` (not a third root copy, and not a pinned self-reference — a pinned self-reference would lint working-tree edits against the last tagged release rather than the change being made). Every rule is `level: error` and every alert is a FAIL — no ignorable tier, same 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 `warning`/`suggestion` rules exit 0 and pre-commit swallows the output of a passing hook, leaving them invisible and blocking nothing. `MinAlertLevel` and `--minAlertLevel` are correspondingly absent from `.vale.ini` and the hook, being no-ops under this model. Vale covers the pattern-matchable sub-checks named in issue #84 (imperative opener, vague filler, `Use proactively`, generic reference-pointer padding) plus, per ADR-0013, one body-wide prose-pattern check ("There is/are" sentence openers) — everything else about body discipline (defaults-vs-menus, why-rationale, non-pattern-matchable judgment calls), near-miss exclusion strength, and control calibration stays LLM judgment.
Both skills' Step 1, and the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks, call each copy's own `scripts/vale-wrap.sh` rather than `vale` directly — a workaround for a confirmed Vale 3.15.2 limitation (see `vale-config`'s Gotchas): `text.frontmatter.description` silently stops matching once the description is a YAML block scalar (`>`/`|`) spanning 2+ physical lines, which is how most skills/agents in this repo write it. The wrapper flattens the description to one physical line in a scratch copy (padding with blank lines so every other line number is unchanged) before handing off to real `vale`; single-line descriptions pass through untouched; a relative `--config` path resolves against the caller's cwd, matching bare `vale`, not against the repo root. `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` has that glob section). Each `.vale.ini`'s section globs are path-agnostic (`[**/SKILL.md]` for skill-audit's copy; `[**/agents/*.md]`/`[**/*.agent.md]` for agent-audit's) and do no scoping on their own: Vale's `*` crosses `/`. Scoping comes from each pre-commit hook's own `files:` regex — `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` for `-skill`, `^plugins/[^/]+/agents/[^/]+\.md$` for `-agent` (split into two hooks precisely because one combined hook pointed at only one copy would silently 0-file-skip the other file type) — and from the audit skills passing one explicit file per invocation. A skill outside `plugins/` (e.g. project-scope `.claude/skills/foo/SKILL.md`) matches no glob section, so Vale reports 0 files and exits 0 — which the audits read as clean. Both audits treat a 0-file Vale run as NOT RUN and fall back to full LLM judgment.
Both skills' Step 1, and the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks, call each copy's own `scripts/vale-wrap.sh` rather than `vale` directly — a workaround for a confirmed Vale 3.15.2 limitation (see `vale-config`'s Gotchas): `text.frontmatter.description` silently stops matching once the description is a YAML block scalar (`>`/`|`) spanning 2+ physical lines, which is how most skills/agents in this repo write it. The wrapper flattens the description to one physical line in a scratch copy (padding with blank lines so every other line number is unchanged) before handing off to real `vale`; single-line descriptions pass through untouched. 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 — which 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` 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. An explicit `--config` still wins, in all three argv forms (`--config X`, `--config=/abs`, `--config=rel`), and a relative one still resolves against the caller's cwd, matching bare `vale`, not the repo root; both audit skills' Step 1 still passes `--config assets/vale/.vale.ini` and is unaffected. `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` has that glob section). Each `.vale.ini`'s section globs are path-agnostic (`[**/SKILL.md]` for skill-audit's copy; `[**/agents/*.md]`/`[**/*.agent.md]` for agent-audit's) and do no scoping on their own: Vale's `*` crosses `/`. Scoping comes from each 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: this repo's `.pre-commit-config.yaml` pins its own layout — `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` for `-skill`, `^plugins/[^/]+/agents/[^/]+\.md$` for `-agent` — while the shipped `.pre-commit-hooks.yaml` stays layout-agnostic for external consumers whose skills live anywhere, using `(^|/)SKILL\.md$` and `(^|/)agents/[^/]+\.md$|\.agent\.md$`. Both manifests split the prefilter into two hooks precisely because one combined hook pointed at only one copy would silently 0-file-skip the other file type. A `SKILL.md` outside `plugins/` (e.g. project-scope `.claude/skills/foo/SKILL.md`) still matches `[**/SKILL.md]` and gets linted normally — the globs constrain filename shape, not location. 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, so both audits treat a 0-file Vale run as NOT RUN and fall back to full LLM judgment.
This scope expands per ADR-0013: one cherry-picked low-noise `write-good`/`alex` rule landed in `styles/Kyberforge`, `Kyberforge.SentenceOpenerThereIs` (22 held-out hits, both in-corpus hits clean rewrites, zero suppressions). A second, `Kyberforge.VagueQualifier`, was cherry-picked and then deleted: 2 hits across the 41 skill/agent files, one marginal and one an unfixable false positive (`caveman/SKILL.md` quotes `of course` as an example of filler — a mention, not a use) that forced the repo's only Vale suppression comments. Also new is a sibling pre-commit hook, `skill-size-check` (`scripts/skill-size-check.sh`), enforcing agentskills.io's 500-line/5,000-token `SKILL.md` ceiling — failing only above 500 lines, matching `skill-audit/scripts/validate.sh`'s `<= 500` pass — scoped to `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` only, same as `vale-audit-prefilter-skill`, so it never lints `docs/research/examples/` reference skills. It's also exposed in the root-level `.pre-commit-hooks.yaml` as `kyberforge-skill-size-check` — it has no external asset dependency, so it needed no relocation, only exposure to external consumers. File scope (`SKILL.md` + agent files) and enforcement model (rules land directly in `styles/Kyberforge`, blocking immediately, no trial tier) stay unchanged; governance.md/CONTROLS.md were evaluated and excluded as rule sources (nothing prose-pattern-matchable to mine). 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; inline `<!-- vale Rule = NO -->` (HTML-comment form; the MDX `{/* */}` form does not work in plain Markdown) is the fallback only where backticking is impossible.

View File

@@ -55,11 +55,27 @@ define separate `-skill`/`-agent` hook IDs, each with a `files:` regex matching
target copy's glob covers. (Confirmed empirically before deleting the root files: retargeting a
single hook at agent-audit's copy silently scanned 0 SKILL.md files.)
**The hook `entry:` is the wrapper alone; the wrapper self-locates its config.** pre-commit
prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]), *cmd[1:])`);
every later argument is handed to the process untouched and so resolves against the *consuming*
repo's root. A `--config plugins/kyberforge/skills/…/assets/vale/.vale.ini` in
`.pre-commit-hooks.yaml` therefore named a path no consumer has, and every external run died with
`E100 [--config] Runtime error`. The external-consumer contract this ADR exists to establish
cannot be expressed as a `--config` argument at all — the config path has to be derived inside
the process, from the script's own location. `vale-wrap.sh` accordingly defaults to its sibling
`assets/vale/.vale.ini`, resolved from `${BASH_SOURCE[0]}`, whenever no `--config` is supplied;
an explicit `--config` from any other caller still wins and still resolves against the caller's
cwd, so both audit skills' Step 1 (`--config assets/vale/.vale.ini`) is unaffected. Both
manifests now carry the identical argument-free `entry:`. Keeping them identical is part of the
decision: the local `repo: local` hook resolved its `--config` correctly only because the
consuming repo *was* this repo, and that one difference is why three review rounds exercised a
code path no external consumer ever takes.
**Vale's `StylesPath` resolves relative to the `.vale.ini` file's own location**, confirmed
against `docs.vale.sh/keys/stylespath` — so `--config <path-into-plugin>/.vale.ini` correctly
finds that ini's sibling `styles/` regardless of the caller's cwd, with no extra path-juggling
needed beyond what `vale-wrap.sh` already does for its cwd-relative `--config`/file-argument
handling.
against `docs.vale.sh/keys/stylespath` — so a config path into the plugin finds that ini's
sibling `styles/` regardless of the caller's cwd, whether it arrives as an explicit `--config` or
as the wrapper's self-located default. No extra path-juggling is needed beyond `vale-wrap.sh`'s
cwd-relative `--config`/path-argument handling and that fallback.
**A sync-check catches drift between the two copies.** `scripts/check-vale-style-sync.sh` diffs
`scripts/vale-wrap.sh` and `assets/vale/styles/Kyberforge/` between skill-audit and agent-audit
@@ -91,8 +107,18 @@ doesn't wonder if it was overlooked.
- Root `.vale.ini`, `styles/`, `scripts/vale-wrap.sh` are deleted. Two copies remain:
`plugins/kyberforge/skills/agent-audit/assets/vale/` (canonical, superset) and
`plugins/kyberforge/skills/skill-audit/assets/vale/` (subset, `Kyberforge` only).
- `plugins/kyberforge`'s `plugin.json` and `.claude-plugin/plugin.json` both patch-bump to
`1.2.5` for the shipped content change (per ADR-0006's version-parity invariant).
- `plugins/kyberforge`'s `plugin.json` and `.claude-plugin/plugin.json` both patch-bump for every
shipped content change (per ADR-0006's version-parity invariant): `1.2.5` for the relocation
itself, `1.2.6` for the self-locating `vale-wrap.sh` that followed.
- **`.pre-commit-hooks.yaml` entries are a bare script path and nothing else — a constraint, not a
house style, and it binds every future hook here, not just the Vale two.** Since pre-commit
rewrites only `entry[0]` into the hook-repo clone, no argument token in any entry can reference
a file this repo ships: a relative path resolves against the *consuming* repo and hard-fails,
and the absolute path is unknowable at author time. A hook that needs one of its own bundled
files must have the script self-locate it from `$0`/`${BASH_SOURCE[0]}`, exactly as
`vale-wrap.sh` now does for `.vale.ini`. Anything else rediscovers this as another `E100`.
`.pre-commit-config.yaml` stays byte-identical to the shipped manifest on those `entry:` lines
so the local gate keeps exercising the same resolution path a consumer does.
- `tests/test-vale-wrap.sh` now exercises skill-audit's copy specifically — its fixtures are all
`SKILL.md`-shaped, and only skill-audit's `.vale.ini` has the matching glob section.
- The first `vX.Y.Z` tag is cut once this change and its tests pass, giving external
@@ -117,3 +143,14 @@ doesn't wonder if it was overlooked.
as a separate piece of infrastructure, not fixed here. `RELEASE_PATHS` is derived from
`.pre-commit-hooks.yaml`'s own `entry:` lines rather than hand-maintained, so at least the set of
paths it checks can't drift from the manifest on its own.
- **Dropping `--config` moved the release gate's path derivation too.** `check-release-needed.sh`
used to reach each hook's bundled assets through the `dirname` of its `--config` target. With
no `--config` token left, that loop went dead and silently dropped both `assets/vale/` trees
from release coverage — a Vale *rule* change could then land on `main` without demanding a tag,
leaving consumers pinned to an old `rev:` running stale rules while the gate stayed green. The
script now derives the bundle's `assets/` tree from `tokens[0]` instead (double-`dirname`,
guarded on the candidate existing and on not resolving to `.`), which is the only derivation
compatible with the argument-free `entry:` contract above.
- **Accepted residual in the release gate:** deleting a hook's *entire* `assets/` tree is not
flagged — the derived candidate path stops existing, so the guard drops it before it reaches
the pathspec. Deleting individual files inside a surviving tree is flagged, and tested.

View File

@@ -19,11 +19,11 @@ Lints the given file(s)/glob against the styles configured in `.vale.ini`.
| `vale sync` | Downloads and installs packages/styles declared in `.vale.ini`. Run after install and whenever `Packages` changes. |
| `vale ls-config` | Prints the currently active, fully-resolved configuration as JSON. Useful for debugging what settings actually apply to a file. |
| `--output=<style>` | Sets the output format/template: `line`, `JSON`, `CLI` (default), or a custom template. |
| `--no-exit` | Suppresses the non-zero exit code Vale normally returns when alerts are found — useful in CI pipelines that shouldn't hard-fail on lint output. |
| `--no-exit` | Suppresses the non-zero exit code Vale normally returns when `error`-level alerts are found — useful in CI pipelines that shouldn't hard-fail on lint output. |
| `--ignore-syntax` | Treats input as plain, unformatted text, skipping syntax-aware parsing (Markdown/HTML/etc). |
| `--minAlertLevel=<level>` | Overrides `MinAlertLevel` from the config for this run (`suggestion`, `warning`, `error`). |
| `--minAlertLevel=<level>` | Overrides `MinAlertLevel` from the config for this run (`suggestion`, `warning`, `error`). Filters what is displayed; does not affect the exit code. |
| `--version` | Prints the Vale binary version. |
## Exit Codes
By default, `vale` exits non-zero when it finds any alert at or above `MinAlertLevel` — this is what makes it usable as a CI gate. Pass `--no-exit` to always exit `0` regardless of findings.
By default, `vale` exits non-zero only when it finds at least one `error`-level alert — this is what makes it usable as a CI gate. `warning` and `suggestion` alerts are printed but still exit `0`, so a rule that must gate CI or a commit hook has to be `level: error`. `MinAlertLevel` and `--minAlertLevel` filter which alerts are displayed and never affect the exit code. Pass `--no-exit` to always exit `0` regardless of findings.

View File

@@ -53,4 +53,6 @@ If a file's syntax-aware parsing produces noisy/incorrect results (e.g. an unsup
## CI Failing Unexpectedly
If a CI job fails solely because Vale returns a non-zero exit code on found alerts (not because the content is actually wrong for that pipeline stage), add `--no-exit` rather than suppressing the rule itself — this preserves the lint output while not gating the build on it.
If a CI job fails solely because Vale returns a non-zero exit code on `error`-level alerts (not because the content is actually wrong for that pipeline stage), add `--no-exit` rather than suppressing the rule itself — this preserves the lint output while not gating the build on it. Raising `MinAlertLevel` is not an alternative: it only filters which alerts print, so an `error`-level alert still exits non-zero.
The mirror-image failure is a Vale gate that never fails. Only `error`-level alerts drive the exit code, so a `warning`- or `suggestion`-level rule prints its alert and still exits `0` — invisible in any CI stage that hides passing output. If a rule must block, give it `level: error`.