diff --git a/CONTEXT.md b/CONTEXT.md index 3667991..143bf74 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -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: , rev: ` 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 `` (HTML-comment form; the MDX `{/* */}` form does not work in plain Markdown) is the fallback only where backticking is impossible. diff --git a/docs/adr/0014-vale-prefilter-ships-from-the-plugin.md b/docs/adr/0014-vale-prefilter-ships-from-the-plugin.md index 10ac2c0..ccdb972 100644 --- a/docs/adr/0014-vale-prefilter-ships-from-the-plugin.md +++ b/docs/adr/0014-vale-prefilter-ships-from-the-plugin.md @@ -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 /.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. diff --git a/plugins/lint/docs/research/docs/vale/cli-reference.md b/plugins/lint/docs/research/docs/vale/cli-reference.md index 7ed35fb..16b6fdb 100644 --- a/plugins/lint/docs/research/docs/vale/cli-reference.md +++ b/plugins/lint/docs/research/docs/vale/cli-reference.md @@ -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=