chore(gates): retire the external pre-commit hook contract
Why: .pre-commit-hooks.yaml and its release-tag gate served external consumers that do not exist. No repo on the Gitea instance pins these hooks, and the README names apm as the only supported install path. The mechanism was also already failing: skill-size-check.sh changed after v2.0.1 with no tag cut, and the gate cannot fire through Gitea's merge button. (Simplification audit finding 36.) Implementation Notes: - Delete .pre-commit-hooks.yaml, scripts/check-release-needed.sh, tests/test-check-release-needed.sh and tests/test-vale-hooks-consumer.sh, and remove the check-release-needed pre-push hook. The repo: local skill-size-check and vale-audit-prefilter-* hooks are unchanged. - ADR-0014 is amended, not retired: its runtime decision to bundle Vale inside factory-audit stands. The amendment keeps the entry[0]-only constraint (LESSONS.md:101,105) in case the export returns. ADR-0025 gets a pointer. - test-vale-wrap.sh: drop case 33 (the cross-manifest drift check) and case 28's hook-scope half, which read the published manifest. Case 32 now also requires each hook to select every tracked file of its class, which keeps case 33's one-plugin-narrowing guard, with a mutation test. - test-skill-size-check.sh and test-adr0020-contract.sh now assert the hook contract and verbose: true on .pre-commit-config.yaml only. - gates.md: pre-push count goes from 9 to 8 authored hooks (11 to 10 reported), and the Release table, the External consumers section and the two-manifest scope table are removed. README and script/test comments no longer describe the export as live. The resolver comment is edited identically in both copies. - The v1.0.0/v2.0.0/v2.0.1 tags are left in place; they are inert. ADR: 0014 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -27,6 +27,14 @@ them by name — and the argument-free `entry:` contract is untouched. Read the
|
||||
sync-check paragraph, and the `tests/test-vale-wrap.sh` Consequences bullet below as the state this
|
||||
ADR established, not as current layout.
|
||||
|
||||
**Amended (2026-09-16): the `.pre-commit-hooks.yaml` export and its release tags are retired.**
|
||||
The runtime half of this ADR — Vale config, styles and wrapper bundled inside the skill (now
|
||||
`factory-audit`), self-located from `${BASH_SOURCE[0]}` — stands. The external git-hook/CI half does
|
||||
not: the manifest, `check-release-needed` and the tag-cutting consequence are gone. See
|
||||
[the amendment at the end of this file](#amendment-2026-09-16-the-external-hook-contract-is-retired)
|
||||
before reading any paragraph below that names `.pre-commit-hooks.yaml`, a `rev:` tag, or
|
||||
`check-release-needed` as current.
|
||||
|
||||
`skill-audit`/`agent-audit`'s Step 1 called
|
||||
`"$(git rev-parse --show-toplevel)/scripts/vale-wrap.sh" --config "$(git rev-parse --show-toplevel)/.vale.ini"`
|
||||
— which resolves to whichever repo the skill happens to be running in. Inside `ai-development`
|
||||
@@ -219,3 +227,51 @@ only when the original span is already two or more lines, so the pad count stays
|
||||
`tests/test-vale-wrap.sh` case 20 asserts an apostrophe-bearing token actually fires on a flattened
|
||||
description in all three apostrophe-carrying branches, and case 20b pins the pad arithmetic against
|
||||
a body line's true line number.
|
||||
|
||||
## Amendment (2026-09-16): the external hook contract is retired
|
||||
|
||||
Root `.pre-commit-hooks.yaml`, `scripts/check-release-needed.sh`,
|
||||
`tests/test-check-release-needed.sh` and `tests/test-vale-hooks-consumer.sh` are deleted, and the
|
||||
`check-release-needed` pre-push hook is removed from `.pre-commit-config.yaml`. The three exported
|
||||
hook IDs — `kyberforge-vale-audit-skill`, `kyberforge-vale-audit-agent` and
|
||||
`kyberforge-skill-size-check` — no longer exist, and no new `vX.Y.Z` tag is cut when hook files
|
||||
change. (Simplification audit finding 36.)
|
||||
|
||||
Three reasons, any one of which would have been enough to ask the question:
|
||||
|
||||
- **No consumer was found.** The Gitea instance holds two repos. The other one pins seven hook
|
||||
repos, and none of them is this one. None of the 13 commits that touched the mechanism came from a
|
||||
consumer report; all were found by this repo's own tests. Clones outside the instance cannot be
|
||||
counted, but ADR-0024 accepted the same standard when it deleted the mirror.
|
||||
- **The mechanism was already failing at its one job.** `scripts/skill-size-check.sh` changed on
|
||||
`main` after `v2.0.1`, and no tag was cut, so a consumer pinning `rev: v2.0.1` already ran a
|
||||
stale hook. The gate could not have caught it. It acted only when pre-commit reported a push to
|
||||
`refs/heads/main`, and PRs here merge through Gitea's server-side merge button, which runs no
|
||||
local hook. The script's own header said that closing the gap needed a server-side CI job the
|
||||
repo does not have.
|
||||
- **The README already contradicted it.** Its "For external consumers" section says apm is the only
|
||||
supported install path and never mentions `.pre-commit-hooks.yaml` or `rev:` pinning.
|
||||
|
||||
**What is unaffected.** This repo's `repo: local` hooks — `skill-size-check`,
|
||||
`vale-audit-prefilter-skill` and `vale-audit-prefilter-agent` — were always wired separately from
|
||||
the export, so no internal lint coverage is lost. The two prefilter hook IDs stay separate for the
|
||||
file-scope reason in "One hook per file-scope" above, not for an external contract.
|
||||
`tests/test-vale-wrap.sh` case 33, the cross-manifest `files:` drift check that ADR-0025 ported,
|
||||
went with the manifest it compared against. Its one guard that did not need a second manifest, a
|
||||
local regex narrowed to a single plugin, is now a third property of case 32. The `v1.0.0`, `v2.0.0`
|
||||
and `v2.0.1` tags are left in place. They are inert: nothing reads them, and apm's `per_package`
|
||||
versioning never consults `tagPattern`.
|
||||
|
||||
**What is preserved for a return.** The `entry[0]`-only constraint in the Consequences above, and
|
||||
its incident records at `LESSONS.md:101` and `:105`, stay as written. That constraint says a
|
||||
published entry is a bare script path, with every bundled file located from `${BASH_SOURCE[0]}`,
|
||||
and it took three review rounds to find. Both hook scripts still meet it: `vale-wrap.sh` takes no
|
||||
`--config`, and `skill-size-check.sh` keeps its embedded resolver copy. If a consumer appears,
|
||||
restore the manifest under that constraint, and restore `test-vale-hooks-consumer.sh` with it: it
|
||||
was the only test that exercised the entry-resolution path that once shipped broken. Restore a
|
||||
release gate only once a server-side job can run it on merge.
|
||||
|
||||
**Superseded statements elsewhere.** ADR-0022's notes that the version-bump gate "is not exported
|
||||
through `.pre-commit-hooks.yaml`" and that it shares its gaps with `check-release-needed`, and
|
||||
ADR-0025's point 5 ("Both exported Vale hook IDs survive unchanged") and its case-33 port, describe
|
||||
the state before this amendment.
|
||||
|
||||
@@ -5,6 +5,11 @@ exact pair, scoped itself to them, and then deferred the work as issue #101. The
|
||||
here. `skill-author` and `agent-author` stay separate — ADR-0020 excluded the author pair
|
||||
deliberately, and nothing in this change touches that exclusion.
|
||||
|
||||
**Amended by ADR-0014 (2026-09-16).** The published `.pre-commit-hooks.yaml` is retired. Point 5
|
||||
below (both exported hook IDs survive) and the case 33 port no longer describe the repo. Case 33 is
|
||||
deleted, and its one-plugin narrowing guard is now a property of case 32. See
|
||||
[ADR-0014's amendment](0014-vale-prefilter-ships-from-the-plugin.md#amendment-2026-09-16-the-external-hook-contract-is-retired).
|
||||
|
||||
## Context
|
||||
|
||||
Every figure below was measured against the worktree on 2026-09-15. Re-derive rather than quote; the
|
||||
|
||||
@@ -21,37 +21,28 @@ Install hooks via `pc-run`, wiring **all three stages**. This repo's `.pre-commi
|
||||
`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,
|
||||
The pre-push command reports **10** hooks, not 8. 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.
|
||||
else — what separates them is `repo: meta` (pre-commit's own built-ins) from `repo: local`. Eight
|
||||
is the count of hooks this repo authors itself, and `--hook-stage pre-push --all-files` is a full
|
||||
rehearsal of all eight. A PR merged through Gitea's merge button runs none of them: no local push
|
||||
happens at all.
|
||||
|
||||
A real push has a gap of its own. When one `git push` carries several refs
|
||||
(`git push origin a b`, `git push --all`), pre-commit runs the pre-push stage once, for one ref.
|
||||
In pre-commit 4.6.1, `_pre_push_ns` in `hook_impl.py` skips delete lines and returns on the first
|
||||
remaining ref whose remote sha is non-zero and present locally; a ref whose remote sha is zero or
|
||||
unknown locally is returned only if it has commits that no remote-tracking ref of that remote has.
|
||||
The two hooks that read the pushed ref — `check-release-needed` and
|
||||
`check-skill-version-bump` — therefore check only that ref, and the others are pushed unchecked.
|
||||
The one hook that reads the pushed ref, `check-skill-version-bump`, therefore checks only that
|
||||
ref, and the others are pushed unchecked.
|
||||
When that ref's unpushed history reaches a root commit, pre-commit runs with all files and sets no
|
||||
`PRE_COMMIT_TO_REF`, so `check-skill-version-bump` checks `HEAD`, which is the pushed ref only if it
|
||||
is checked out.
|
||||
For `check-release-needed`, a multi-ref push whose first ref is not `main` never gates `main` at
|
||||
all. Push one ref at a time when the gate matters.
|
||||
is checked out. Push one ref at a time when the gate matters.
|
||||
|
||||
## The pre-push gate
|
||||
|
||||
Nine hooks, grouped below by what they guard rather than by the order `.pre-commit-config.yaml` declares them in.
|
||||
Eight hooks, grouped below by what they guard rather than by the order `.pre-commit-config.yaml` declares them in.
|
||||
|
||||
**Core checks**
|
||||
|
||||
@@ -94,12 +85,6 @@ drift in generated text.
|
||||
|---|---|
|
||||
| `check-skill-version-bump` | fails if a skill directory changed since the pushed commit's merge-base with `main` without its `metadata.version` rising above both the merge-base's and `main`'s tip's (see [below](#check-skill-version-bump)) |
|
||||
|
||||
**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
|
||||
@@ -111,8 +96,7 @@ ADR-0022 makes `metadata.version` mandatory and says a skill change carries a bu
|
||||
`skill-size-check` only checks the field's presence and shape, so this hook holds the bump itself.
|
||||
|
||||
- **It runs on every push and under a manual `pre-commit run --hook-stage pre-push`.** It does not
|
||||
read `PRE_COMMIT_REMOTE_BRANCH`, so unlike `check-release-needed` the manual rehearsal really
|
||||
checks it. The pushed commit is `PRE_COMMIT_TO_REF`, or `HEAD` when that is unset.
|
||||
read `PRE_COMMIT_REMOTE_BRANCH`, so the manual rehearsal really checks it. The pushed commit is `PRE_COMMIT_TO_REF`, or `HEAD` when that is unset.
|
||||
- **"Changed" is measured from the merge-base of the pushed commit with `origin/main`** (local
|
||||
`main` if `origin/main` does not resolve). Readers install from `main`, so "changed" means
|
||||
changed against the `main` the branch started from. The remote branch tip is not the baseline:
|
||||
@@ -147,7 +131,7 @@ ADR-0022 makes `metadata.version` mandatory and says a skill change carries a bu
|
||||
- **It needs `python3` and PyYAML** to read the frontmatter, and fails with a clear message if
|
||||
either is missing, for the reasons in
|
||||
[`python3` and PyYAML are hard requirements](#python3-and-pyyaml-are-hard-requirements).
|
||||
- **Known gaps, both shared with `check-release-needed`:** a PR merged through Gitea's merge button
|
||||
- **Known gaps:** a PR merged through Gitea's merge button
|
||||
runs no local hook; and a multi-ref push checks only the one ref pre-commit selects, and a push
|
||||
reaching a root commit gets no `PRE_COMMIT_TO_REF`, so `HEAD` is checked (see
|
||||
[Running the gates](#running-the-gates)).
|
||||
@@ -157,9 +141,7 @@ ADR-0022 makes `metadata.version` mandatory and says a skill change carries a bu
|
||||
## Skill and agent context gates (ADR-0020)
|
||||
|
||||
The `skill-size-check` pre-commit hook, scoped to `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$`,
|
||||
runs `scripts/skill-size-check.sh`. It is also shipped to external repos as
|
||||
`kyberforge-skill-size-check` (see
|
||||
[External consumers](#external-consumers-the-root-pre-commit-hooksyaml)). Besides the ADR-0020
|
||||
runs `scripts/skill-size-check.sh`. Besides the ADR-0020
|
||||
gates below, it also asserts required frontmatter is present: `name`, a non-empty `description`, and
|
||||
a `metadata.version` matching three-part semver (`1.0.0`) — folded in from a formerly standalone
|
||||
`skill-frontmatter` hook that parsed the same fields with a shell script.
|
||||
@@ -724,27 +706,20 @@ rule at a blocking bare `YES`/`error`. It is not redundant with the probes above
|
||||
(`DescriptionOpener`, `PaddingPhrase`, `SentenceOpenerThereIs`, `CompositionNote`) can each be
|
||||
overridden out of `error` underneath a passing probe. That gap is closed.
|
||||
|
||||
Two cases cover the hook manifests.
|
||||
**Case 32** covers the prefilter hooks' own scope in `.pre-commit-config.yaml`, with three
|
||||
properties. Each vale hook's `files:` regex must still match at least one tracked file; every path it
|
||||
matches must be in that hook's own artifact class; and it must match **every** tracked file of that
|
||||
class under `plugins/*/.apm/`. A hook narrowed to zero files never runs, and pre-commit reports no
|
||||
error. A hook narrowed to one plugin still matches files of the right class, which is why the third
|
||||
property exists: narrowing `vale-audit-prefilter-skill` from `^plugins/[^/]+/...` to
|
||||
`^plugins/kyberforge/...` once left 6 of 38 skills prefiltered and the whole suite green. Part B
|
||||
narrows both regexes to zero files and Part C narrows both to one plugin, each in a copy of the
|
||||
config, and requires Part A to fail by hook name.
|
||||
|
||||
**Case 33** is the original's cross-manifest `files:` drift check, ported. It extracts each vale
|
||||
hook's `files:` regex from `.pre-commit-hooks.yaml` and from `.pre-commit-config.yaml`
|
||||
*independently*, compares them per hook and never as a union, and asserts that each shared probe path
|
||||
is in scope of both or neither. The original selected each hook's record by matching `entry:`
|
||||
against the owning skill's `vale-wrap.sh` path. After the merge both hook IDs share one `entry:`, so
|
||||
the port pairs them by `id:` from an explicit table: `kyberforge-vale-audit-skill` ↔
|
||||
`vale-audit-prefilter-skill`, and `kyberforge-vale-audit-agent` ↔ `vale-audit-prefilter-agent`. A
|
||||
missing hook id or a class with no shared probe fails by name. Part B requires three mutations to
|
||||
fail: the skill hook narrowed to one plugin, the agent hook narrowed the same way, and a renamed
|
||||
local hook id.
|
||||
|
||||
This was briefly a real hole. Narrowing `vale-audit-prefilter-skill` from `^plugins/[^/]+/...` to
|
||||
`^plugins/kyberforge/...` left 6 of 38 skills prefiltered, and the whole suite green, before case 33
|
||||
existed.
|
||||
|
||||
**Case 32** covers the separate zero-match question on the local manifest alone. Each
|
||||
`.pre-commit-config.yaml` vale hook's `files:` regex must still match at least one tracked file, and
|
||||
every path it matches must be in that hook's own artifact class. A hook narrowed to zero files never
|
||||
runs, and pre-commit reports no error.
|
||||
Case 32 used to have a partner, case 33, which compared each hook's `files:` regex against the
|
||||
published `.pre-commit-hooks.yaml`. It went with that manifest (ADR-0014's 2026-09-16 amendment),
|
||||
and its one guard that did not need a second manifest, the one-plugin narrowing, is case 32's third
|
||||
property.
|
||||
|
||||
**Case 34** asks, statically and with no Vale binary, whether the shipped `.vale.ini` could load a
|
||||
style at all. Four assertions: every `[glob]` section declares a **non-empty** `BasedOnStyles`; every
|
||||
@@ -753,8 +728,8 @@ absolute**; and at least one `[glob]` section exists, so the check cannot pass v
|
||||
with nothing in it. A section whose `BasedOnStyles` is empty is the silent case — Vale lints every
|
||||
file that glob matches with no rule loaded, prints `0 errors` and exits 0. The absolute-path clause
|
||||
is the one that is not obvious: an absolute `StylesPath` passes on the machine that wrote it and
|
||||
hard-fails for every external consumer of `.pre-commit-hooks.yaml`, which is the only reason those
|
||||
styles ship at all. Part B is a mutation self-test against the same function Part A calls — it empties
|
||||
hard-fails for every repo that installs `factory-audit`, which is the only reason those styles ship
|
||||
with the skill at all. Part B is a mutation self-test against the same function Part A calls — it empties
|
||||
each section's `BasedOnStyles` in a copy of the assets, and absolutizes `StylesPath` in another
|
||||
pointed at that copy's own real `styles/` directory, and requires each to fail by name.
|
||||
|
||||
@@ -805,27 +780,6 @@ from the hook definitions. Under this model they are no-ops; adding one is not a
|
||||
The `verbose: true` escape hatch that makes `skill-size-check`'s SUGGESTION tier audible has no
|
||||
analogue here — Vale has no tier to make audible.
|
||||
|
||||
### External consumers: the root `.pre-commit-hooks.yaml`
|
||||
|
||||
The root `.pre-commit-hooks.yaml` exposes two Vale hook IDs (`kyberforge-vale-audit-skill`,
|
||||
`kyberforge-vale-audit-agent`) plus `kyberforge-skill-size-check`, so any external repo can enforce
|
||||
the same rules with `repo: <this-repo-url>, rev: <tag>` in its own `.pre-commit-config.yaml`.
|
||||
pre-commit clones the pinned rev into its own cache, independent of whether Claude Code or the
|
||||
`kyberforge` plugin is installed at all; the same mechanism covers CI via `pre-commit run
|
||||
--all-files`. `skill-size-check` has no external asset dependency, so it needed no relocation under
|
||||
ADR-0014 — only exposure.
|
||||
|
||||
**The two IDs survive the merge even though they now point at the same wrapper.** Both
|
||||
`kyberforge-vale-audit-skill` and `kyberforge-vale-audit-agent` keep their IDs and their `files:`
|
||||
regexes, because an external repo pins them by name in its own `.pre-commit-config.yaml` and
|
||||
collapsing them to one would break every such consumer silently. What changed is only the `entry:`
|
||||
target: both now name `factory-audit/scripts/vale-wrap.sh`.
|
||||
|
||||
This repo's own `vale-audit-prefilter-skill` / `-agent` hooks consume the **identical**
|
||||
plugin-bundled copy via `repo: local`. Deliberately not a second root copy, and deliberately **not a
|
||||
pinned self-reference** — a pinned self-reference would lint working-tree edits against the last
|
||||
tagged release rather than against the change being made.
|
||||
|
||||
### Pre-commit
|
||||
|
||||
Two prefilter hooks, with `.apm/`-scoped `files:` patterns:
|
||||
@@ -845,9 +799,10 @@ authors. Without the binary the hooks fail with a bare "command not found" and n
|
||||
reason was mechanical: with a config per skill, a single hook could point at only one copy and would
|
||||
silently 0-file-skip the other file shape (see
|
||||
[A 0-file Vale run is NOT RUN](#a-0-file-vale-run-is-not-run)). One `.vale.ini` carrying all three
|
||||
sections removes that constraint. The split stays anyway because the two IDs are an exported
|
||||
contract external consumers pin by name, and because the `files:` regexes still have to differ —
|
||||
each hook hands Vale only the file shape it is scoped to.
|
||||
sections removes that constraint. The split stays anyway because the `files:` regexes still have to
|
||||
differ — each hook hands Vale only the file shape it is scoped to. Both hooks name the same
|
||||
plugin-bundled `factory-audit/scripts/vale-wrap.sh` through `repo: local`; there is no second root
|
||||
copy.
|
||||
|
||||
### The `.vale.ini` globs do no scoping
|
||||
|
||||
@@ -856,15 +811,12 @@ The `.vale.ini`'s section globs are **path-agnostic** — `[**/SKILL.md]`, `[**/
|
||||
location: Vale's `*` crosses `/`. A `SKILL.md` outside `plugins/` (a project-scope
|
||||
`.claude/skills/foo/SKILL.md`, say) still matches `[**/SKILL.md]` and gets linted normally.
|
||||
|
||||
All scoping therefore comes from the pre-commit hook's own `files:` regex and from `factory-audit`
|
||||
passing one explicit file per invocation. The two manifests scope **differently on purpose**:
|
||||
All scoping therefore comes from the pre-commit hooks' own `files:` regexes, which pin this repo's
|
||||
layout (see [Pre-commit](#pre-commit)), and from `factory-audit` passing one explicit file per
|
||||
invocation — in this repo or in any repo that installs it, whatever that repo's layout.
|
||||
|
||||
| Manifest | `-skill` | `-agent` |
|
||||
|---|---|---|
|
||||
| `.pre-commit-config.yaml` (pins this repo's layout) | `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$` | `^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$` |
|
||||
| `.pre-commit-hooks.yaml` (layout-agnostic for consumers) | `(^\|/)SKILL\.md$` | `(^\|/)agents/[^/]+\.md$\|\.agent\.md$` |
|
||||
|
||||
Narrowing a `.vale.ini` glob to a `plugins/`-shaped path to "tighten" it breaks the consumer case.
|
||||
Narrowing a `.vale.ini` glob to a `plugins/`-shaped path to "tighten" it breaks the consumer case:
|
||||
`factory-audit` run against a project-scope `.claude/skills/` tree would lint nothing.
|
||||
`check-vale-style-sync`'s probe set was built to catch exactly that; it moved to
|
||||
`tests/test-vale-wrap.sh` with the hook's deletion, and two of the six probes exist specifically to
|
||||
pin this location independence — see [One copy, one config](#one-copy-one-config).
|
||||
@@ -897,10 +849,6 @@ config declines to match, which is the [0-file NOT RUN](#a-0-file-vale-run-is-no
|
||||
green run that measured nothing. **Issue #117** records the style-scope half; the hook half has to
|
||||
land in the same change or the fix is cosmetic.
|
||||
|
||||
The consumer manifest is a third axis and does not rescue this either: `.pre-commit-hooks.yaml`'s
|
||||
`(^|/)SKILL\.md$` is layout-agnostic but still filename-shaped, so an external repo running
|
||||
`kyberforge-vale-audit-skill` has the same gap.
|
||||
|
||||
### `vale-wrap.sh`, never bare `vale`
|
||||
|
||||
`factory-audit`'s Step 1 and both pre-commit hooks call
|
||||
@@ -925,19 +873,19 @@ wrapper a bad description in any of the three broken forms sailed straight throu
|
||||
### The `--config` argv defect
|
||||
|
||||
Handed **no `--config` at all**, the wrapper falls back to its own sibling `assets/vale/.vale.ini`,
|
||||
located from `${BASH_SOURCE[0]}` rather than from the cwd. That is why both manifests' `entry:` is
|
||||
now the bare script path with **no argument after it**.
|
||||
located from `${BASH_SOURCE[0]}` rather than from the cwd. That is why both hooks' `entry:` is
|
||||
the bare script path with **no argument after it**.
|
||||
|
||||
pre-commit prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]),
|
||||
*cmd[1:])`), so every later argument resolves against the **consuming** repo's root. A `--config` in
|
||||
`.pre-commit-hooks.yaml` therefore pointed at a path no consumer has and hard-failed every external
|
||||
run with `E100 [--config] Runtime error`.
|
||||
the since-retired `.pre-commit-hooks.yaml` therefore pointed at a path no consumer has and
|
||||
hard-failed every external run with `E100 [--config] Runtime error`.
|
||||
|
||||
`.pre-commit-config.yaml` drops the argument too, deliberately keeping the two entries identical.
|
||||
`.pre-commit-config.yaml` drops the argument too, deliberately matching that entry.
|
||||
The local `repo: local` hook resolved its `--config` correctly only because the consuming repo *was*
|
||||
this repo — and that divergence is why three review rounds exercised a path no external consumer
|
||||
takes and missed the defect. **Do not reintroduce a `--config` to either manifest to make the local
|
||||
run "explicit".**
|
||||
takes and missed the defect. **Do not reintroduce a `--config` to either hook to make the local run
|
||||
"explicit"**, and keep a restored published manifest to `entry[0]` alone (ADR-0014).
|
||||
|
||||
An explicit `--config` from any other caller still wins, in all three argv forms (`--config X`,
|
||||
`--config=/abs`, `--config=rel`), and a relative one resolves against the caller's cwd — matching
|
||||
@@ -973,12 +921,12 @@ clean.
|
||||
`CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1` opt-out downgraded them audibly rather than skipping the
|
||||
hook — is deleted with the second Vale copy (ADR-0025). The six glob probes survive it inside
|
||||
`test-vale-wrap.sh`, so `run-tests --strict` is now the gate that runs them. That is also what keeps
|
||||
`vale` a pre-push requirement: `test-vale-hooks-consumer.sh` exits 77 without the binary, and so does
|
||||
`test-vale-wrap.sh` once its static cases pass, and a skip fails the push.
|
||||
`vale` a pre-push requirement: `test-vale-wrap.sh` exits 77 without the binary once its static cases
|
||||
pass, and a skip fails the push.
|
||||
|
||||
`test-vale-wrap.sh` without Vale skips only its Vale-dependent cases, not the whole suite. The cases
|
||||
that are plain greps and awk over the config and the two hook manifests still run: case 0, 16, 26,
|
||||
27, the static halves of 28, 31 Parts A and B, 32, 33 and 34. A static failure exits 1, because a
|
||||
that are plain greps and awk over the Vale config and `.pre-commit-config.yaml` still run: case 0, 16, 26,
|
||||
27, the static half of 28, 31 Parts A and B, 32 and 34. A static failure exits 1, because a
|
||||
real defect is not a setup error. Only an all-static-pass run exits 77.
|
||||
|
||||
### Mentioning banned phrasing without tripping the rule
|
||||
@@ -1005,10 +953,11 @@ run. The pre-push hook invokes the same script as `--strict` (`RUN_TESTS_STRICT=
|
||||
where a skip **does** fail the push: at pre-push a skip means one of the documented dependencies is
|
||||
absent on this machine, so the gate would otherwise report success having run fewer suites than it
|
||||
appears to. Without `--strict` the gate once went green having verified 15 of 17 suites on a
|
||||
vale-less PATH, with the skip list swallowed. Without vale, two suites skip —
|
||||
`test-vale-hooks-consumer.sh` and `test-vale-wrap.sh` — and the strict failure names each one and
|
||||
what to install. (It was three until `test-check-vale-style-sync.sh` was deleted with its hook; see
|
||||
[One copy, one config](#one-copy-one-config).)
|
||||
vale-less PATH, with the skip list swallowed. Without vale, one suite skips — `test-vale-wrap.sh` —
|
||||
and the strict failure names it and what to install. (It was three until
|
||||
`test-check-vale-style-sync.sh` was deleted with its hook — see
|
||||
[One copy, one config](#one-copy-one-config) — and `test-vale-hooks-consumer.sh` with the published
|
||||
hook manifest.)
|
||||
|
||||
**Output assertions use a here-string, never a pipe.** Write `grep -q PATTERN <<< "$OUT"`, not
|
||||
`echo "$OUT" | grep -q PATTERN`. Under `set -o pipefail` the pipe form fails depending on timing:
|
||||
|
||||
Reference in New Issue
Block a user