refactor!: carry out the simplification audit across gates, tests, plugins and docs #135
@@ -167,7 +167,7 @@ repos:
|
||||
# assertions diffed skill-audit's Vale copy against agent-audit's; the
|
||||
# merge into factory-audit leaves one copy, so those are moot. The other
|
||||
# 11 moved into tests/test-vale-wrap.sh (case 0, cases 28-31, its
|
||||
# Vale-absent skip, and case 33 for cross-manifest files: agreement),
|
||||
# Vale-absent skip, and case 32 for the one-plugin narrowing guard),
|
||||
# which run-tests runs here at
|
||||
# pre-push, so do not re-add the hook to restore coverage. Do not
|
||||
# confuse its removal with check-scope-walkup-sync below, which survives:
|
||||
|
||||
@@ -152,6 +152,8 @@ This is the area you named as hardest to understand and slowest. Root cause: mos
|
||||
> **Superseded count (2026-09-15):** finding 14 deleted `check-vale-style-sync` with the merge into `factory-audit` (ADR-0025), so pre-push ~~is now~~ → was **8 repo-authored hooks** (10 reported). The dated note above is the state on 2026-09-14; see finding 14's note for the correction.
|
||||
>
|
||||
> > **Superseded count (2026-09-16, at HEAD `b426460`):** `8451169` added `check-skill-version-bump` (finding 33), so pre-push is **9 repo-authored hooks** (11 reported) — the eight above plus that one. Measured with `grep -c "stages: \[pre-push\]" .pre-commit-config.yaml`.
|
||||
>
|
||||
> > **Superseded count (2026-09-16, at `4de5b6b`):** `4de5b6b` removed `check-release-needed` (finding 36), so pre-push is **8 repo-authored hooks** (10 reported) — the nine above minus that one. Measured the same way.
|
||||
|
||||
Pre-commit stays roughly as is minus `skill-frontmatter`, and minus `check-ast` once finding 9 removes the only `.py` files. ~~Tests 26 files to about 10 (12,400 to about 5,000 lines).~~ Keep bats and its three submodules; the 351 bats tests ship inside plugins and are the right tool there. ~~Do not port the bash suites to bats; delete them instead.~~ **Struck (2026-09-16, grill):** see finding 8's closing note — the suites are regression coverage (findings 3 and 5; finding 16 found the same of the validators they test).
|
||||
|
||||
@@ -470,7 +472,7 @@ Not covered by the area audits above; found on a final sweep of the root config
|
||||
>
|
||||
> **Updated (2026-09-16, later):** **36** is decided and done (`4de5b6b`), so **15** is the only open finding. **22** stays deferred with `bin`.
|
||||
|
||||
> **Re-derived (2026-09-16, at HEAD):** this ordering was written before the findings were worked, and ~~seven of its entries are now closed~~ → all but two of its bucket-2 and bucket-3 entries are now closed (corrected later on 2026-09-16, after the grill). Struck above: **14** landed (`467bbd7`, ADR-0025); **7** was superseded then done (`718c79a`); **3**, **5** and **19** are not proceeding on refuted premises; ~~**34**,~~ **16** and **24** are refuted outright; **34** was refuted as a removal and then decided and done as documentation of the branch hazard in ADR-0019 (`afcf477`), with the hook kept; **33** was decided and done (enforce the bump, `8451169`); **8**, **20** and **28** closed at the grill; **17**, **23** and **25** were declined by the human; **11** was declined by the human. ~~**5** is left standing but is downstream of 16 by its own note, so it cannot be taken in this bucket's order.~~ **5** is closed with 16: its own note says it is downstream of 16, and 16 is refuted. Still open, per the Status note above: **15** alone, now that **36** is done (`4de5b6b`); **22** is deferred with `bin`. Read each finding's own marker, not this list — it is a plan of record, not a status board. Bucket 1 is left as written: every entry in it is marked `[x]` or carries a decision note at its own finding.
|
||||
> **Re-derived (2026-09-16, at HEAD):** this ordering was written before the findings were worked, and ~~seven of its entries are now closed~~ → ~~all but two~~ → all but one of its bucket-2 and bucket-3 entries are now closed (corrected later on 2026-09-16, after the grill, and again once **36** closed). Struck above: **14** landed (`467bbd7`, ADR-0025); **7** was superseded then done (`718c79a`); **3**, **5** and **19** are not proceeding on refuted premises; ~~**34**,~~ **16** and **24** are refuted outright; **34** was refuted as a removal and then decided and done as documentation of the branch hazard in ADR-0019 (`afcf477`), with the hook kept; **33** was decided and done (enforce the bump, `8451169`); **8**, **20** and **28** closed at the grill; **17**, **23** and **25** were declined by the human; **11** was declined by the human; **36** was done (`4de5b6b`). ~~**5** is left standing but is downstream of 16 by its own note, so it cannot be taken in this bucket's order.~~ **5** is closed with 16: its own note says it is downstream of 16, and 16 is refuted. Still open, per the Status note above: **15** alone, now that **36** is done (`4de5b6b`); **22** is deferred with `bin`. Read each finding's own marker, not this list — it is a plan of record, not a status board. Bucket 1 is left as written: every entry in it is marked `[x]` or carries a decision note at its own finding.
|
||||
|
||||
Findings 9, 10, 11, and 12 are coupled through the provenance validator and the audit criteria; land them together or the audit gates start reporting the removals.
|
||||
|
||||
|
||||
@@ -32,8 +32,10 @@ The runtime half of this ADR — Vale config, styles and wrapper bundled inside
|
||||
`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.
|
||||
before reading any paragraph above or below that names `.pre-commit-hooks.yaml`, a `rev:` tag,
|
||||
`check-release-needed`, case 33, or the two exported hook IDs as current. That includes the
|
||||
ADR-0025 amendment directly above: case 33 is deleted (its one-plugin narrowing guard is now a
|
||||
property of case 32), and no external consumer pins the exported hook IDs any more.
|
||||
|
||||
`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"`
|
||||
|
||||
@@ -110,6 +110,10 @@ Checking at commit time against `HEAD` was rejected for the inflation described
|
||||
The 17 unbumped skills took a patch bump in the commit that added the gate. A typo fix in a skill
|
||||
now costs a version bump; that is the rule working, not noise.
|
||||
|
||||
**Amended by ADR-0014 (2026-09-16).** `check-release-needed` is retired, so the comparison below
|
||||
records the state when this ADR was written, not a hook that still runs. See
|
||||
[ADR-0014's amendment](0014-vale-prefilter-ships-from-the-plugin.md#amendment-2026-09-16-the-external-hook-contract-is-retired).
|
||||
|
||||
The gate differs from `check-release-needed` in when it runs: that hook acts only when pre-commit
|
||||
reports a push to `main`, so a manual `pre-commit run --hook-stage pre-push` skips it, while this
|
||||
gate runs there too and checks `HEAD`. The two hooks share both known gaps. A merge made with
|
||||
|
||||
@@ -14,7 +14,7 @@ of the oddities documented here are load-bearing and have already been re-litiga
|
||||
| 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 --hook-stage pre-push --all-files` | the push gate, one command |
|
||||
| `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
|
||||
|
||||
@@ -2002,6 +2002,11 @@ PC_CONFIG32="$REPO_ROOT/.pre-commit-config.yaml"
|
||||
# repository paths, so a regex matching no tracked path matches nothing the gate
|
||||
# will ever hand the hook.
|
||||
REPO_FILES32="$(cd "$REPO_ROOT" && git ls-files)"
|
||||
# Each class's full authoring-source corpus, per the layout AGENTS.md fixes
|
||||
# (`plugins/<name>/.apm/` is the only authoring source). Globals, not locals,
|
||||
# so Part D can point them at a layout that no longer exists.
|
||||
CORPUS_RE_SKILL32='^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$'
|
||||
CORPUS_RE_AGENT32='^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$'
|
||||
|
||||
# Prints one failure token per defect; empty output means every prefilter hook in
|
||||
# $1 is scoped to a live corpus of its own artifact class. The config path is an
|
||||
@@ -2010,7 +2015,7 @@ REPO_FILES32="$(cd "$REPO_ROOT" && git ls-files)"
|
||||
# the live check.
|
||||
prefilter_scope_failures32() {
|
||||
local config="$1" files="$2"
|
||||
local records id re class matched count offenders offending m corpus missing nmissing
|
||||
local records id re class matched count offenders offending m corpus missing nmissing corpus_re
|
||||
local seen_skill=false seen_agent=false bad=""
|
||||
records="$(hook_records28 "$config")"
|
||||
if [[ -z "$records" ]]; then
|
||||
@@ -2061,13 +2066,21 @@ EOF_MATCHED32
|
||||
if [[ "$offending" -gt 0 ]]; then
|
||||
bad+="[$id: 'files: $re' selects $count file(s), $offending of them outside the $class artifact class, so the two prefilter scopes are no longer independently addressable and vale is handed files no glob in its config covers — first: $offenders] "
|
||||
fi
|
||||
# The class's full authoring-source corpus, per the layout AGENTS.md fixes
|
||||
# (`plugins/<name>/.apm/` is the only authoring source). Every path in it
|
||||
# must be selected, or part of the corpus is silently unprefiltered.
|
||||
# Every path in the class's corpus (CORPUS_RE_*32 above) must be
|
||||
# selected, or part of the corpus is silently unprefiltered.
|
||||
if [[ "$class" == skill ]]; then
|
||||
corpus="$(printf '%s\n' "$files" | { grep -E '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$' || true; })"
|
||||
corpus_re="$CORPUS_RE_SKILL32"
|
||||
else
|
||||
corpus="$(printf '%s\n' "$files" | { grep -E '^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$' || true; })"
|
||||
corpus_re="$CORPUS_RE_AGENT32"
|
||||
fi
|
||||
corpus="$(printf '%s\n' "$files" | { grep -E "$corpus_re" || true; })"
|
||||
# An empty corpus makes the comparison below vacuous: nothing can be
|
||||
# missing from nothing. The corpus regex is hard-coded to today's layout,
|
||||
# so a layout move would silently turn this property into a pass. Fail
|
||||
# instead, the same way property 1 fails on zero matches.
|
||||
if [[ -z "$(printf '%s\n' "$corpus" | grep . || true)" ]]; then
|
||||
bad+="[$id: the $class corpus regex '$corpus_re' matches no tracked file, so this case cannot check that 'files: $re' selects the whole $class corpus; update the regex to the current layout] "
|
||||
continue
|
||||
fi
|
||||
missing="$(comm -23 <(printf '%s\n' "$corpus" | grep . | sort) <(printf '%s\n' "$matched" | grep . | sort) || true)"
|
||||
nmissing="$(printf '%s\n' "$missing" | grep -c . || true)"
|
||||
@@ -2152,6 +2165,25 @@ else
|
||||
pass "narrowing either hook's 'files:' regex to one plugin is caught by Part A as an incomplete corpus"
|
||||
fi
|
||||
|
||||
# Part D: proof that property 3 cannot pass vacuously. Each corpus regex is
|
||||
# pointed at a layout no tracked file has -- what a future move of
|
||||
# plugins/*/.apm/ would do to the hard-coded regexes -- and the live config must
|
||||
# then fail as an empty corpus rather than pass with nothing to compare.
|
||||
SAVED_SKILL_RE32="$CORPUS_RE_SKILL32"
|
||||
SAVED_AGENT_RE32="$CORPUS_RE_AGENT32"
|
||||
CORPUS_RE_SKILL32='^zzz-no-such-path/SKILL\.md$'
|
||||
CORPUS_RE_AGENT32='^zzz-no-such-path/[^/]+\.agent\.md$'
|
||||
EMPTY_FAILS32="$(prefilter_scope_failures32 "$PC_CONFIG32" "$REPO_FILES32")"
|
||||
CORPUS_RE_SKILL32="$SAVED_SKILL_RE32"
|
||||
CORPUS_RE_AGENT32="$SAVED_AGENT_RE32"
|
||||
if ! grep -qF "vale-audit-prefilter-skill: the skill corpus regex" <<< "$EMPTY_FAILS32"; then
|
||||
fail "a skill corpus regex matching no tracked file did not fail Part A, so property 3 would pass vacuously after a layout move: ${EMPTY_FAILS32:-<no failure>}"
|
||||
elif ! grep -qF "vale-audit-prefilter-agent: the agent corpus regex" <<< "$EMPTY_FAILS32"; then
|
||||
fail "an agent corpus regex matching no tracked file did not fail Part A, so property 3 would pass vacuously after a layout move: ${EMPTY_FAILS32:-<no failure>}"
|
||||
else
|
||||
pass "a corpus regex matching no tracked file fails Part A instead of passing vacuously"
|
||||
fi
|
||||
|
||||
# --- 34. Every glob section loads a real style, asserted without vale --------
|
||||
#
|
||||
# Case 28 asks this behaviourally -- it lints a probe through the real config
|
||||
|
||||
Reference in New Issue
Block a user