test(gates): pin hook wiring and enforce the bats TAP plan
The repo's gates were not pinned to their wiring. Deleting the check-skill-version-bump block from .pre-commit-config.yaml left the whole suite green; deleting eight blocks at once, run-tests among them, also left it green. Only 4 of 20 hook ids had their wiring pinned anywhere, so a merge conflict resolved badly could stop the suite running at pre-push forever while every test still reported green. test-adr0020-contract now derives the repo-authored hooks from the repo: local entries and pins each one's id, entry and stages against an explicit expected set, both directions, with the same non-vacuity guards the file already applies to its own fixtures. Upstream hooks and their rev: values are untouched, so a rev bump does not churn the test. Mutation-checked: a removed block, a repointed entry and a hook moved off pre-push each go red; a rev bump, a comment edit and reordering stay green. 29 -> 44 assertions. run-bats computed each file's TAP plan and then discarded it, so a process printing "1..10", three ok lines and exit 0 was counted as "3 tests, 0 failures" with seven tests silently gone. That is exactly the wrapper-swallows-the-status case the runner's own comment puts in its threat model, and the plan was the only surviving signal. The plan is now enforced in both directions when a file emits exactly one. Also: test-no-pipefail-early-exit-grep's live-tree floor goes from 20 to 50 against an actual 57, matching test-vale-wrap's per-glob discipline, and test-vale-wrap's header names the real path to vale-wrap.sh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2
This commit is contained in:
@@ -41,6 +41,12 @@
|
||||
# passing hook, and a SUGGESTION deliberately does not fail, so dropping
|
||||
# one word from the config silences the tier ADR-0020 depends on while
|
||||
# every test and every hook still reports green.
|
||||
# 4. The WIRING of every repo-authored hook — its id, its `entry:` and its
|
||||
# `stages:`. Same failure shape as 3, one level up: the tests drive these
|
||||
# scripts by path, so nothing noticed whether .pre-commit-config.yaml
|
||||
# still invoked them. On a scratch copy, deleting eight local hook blocks
|
||||
# at once — `run-tests` among them — left every suite green. Upstream
|
||||
# hooks and every `rev:` are deliberately out of scope.
|
||||
set -euo pipefail
|
||||
|
||||
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
||||
@@ -680,6 +686,199 @@ while IFS=$'\t' read -r status msg; do
|
||||
fi
|
||||
done <<< "$VERBOSE_REPORT"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 4. Every repo-authored hook is still WIRED, at the stage it claims
|
||||
# ---------------------------------------------------------------------------
|
||||
# Same failure mode as assertion 3, one level up. Every gate this repo owns is
|
||||
# driven in tests by PATH -- tests/test-skill-version-bump.sh runs
|
||||
# scripts/check-skill-version-bump.sh directly -- so the script staying correct
|
||||
# and the hook still existing are independent facts, and only the first one was
|
||||
# pinned. Deleting the `run-tests` block from .pre-commit-config.yaml (a routine
|
||||
# merge-conflict casualty) stops the entire suite from running at pre-push
|
||||
# forever, and `bash tests/run-tests.sh --strict` still prints all green: the
|
||||
# suite cannot notice that nothing invokes it. Verified on a scratch copy --
|
||||
# eight repo-authored hook blocks were deleted at once and every suite stayed
|
||||
# green.
|
||||
#
|
||||
# So: id + entry + stages, for the repo-authored hooks only. "Repo-authored"
|
||||
# is derived from the config (every hook under a `repo: local` entry), not
|
||||
# hardcoded, and the derived set is then compared against the expected list
|
||||
# below -- a new local hook that nobody pinned is itself a failure.
|
||||
#
|
||||
# Deliberately NOT pinned: the stock upstream hooks (gitleaks, check-yaml,
|
||||
# pretty-format-json, shellcheck, conventional-pre-commit, the `repo: meta`
|
||||
# pair) and every `rev:`. Those are somebody else's contract; a rev bump must
|
||||
# not churn this test.
|
||||
echo ""
|
||||
echo "--- every repo-authored hook is wired, with the entry and stages it claims ---"
|
||||
WIRING_REPORT="$(python3 - "$REPO_ROOT" <<'PY'
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
||||
import yaml
|
||||
|
||||
root = sys.argv[1]
|
||||
|
||||
|
||||
def emit(status, msg):
|
||||
print("%s\t%s" % (status, msg))
|
||||
|
||||
|
||||
# The expected wiring of every hook this repo authors: id -> (entry, stages).
|
||||
# ADDING OR REMOVING A REPO-AUTHORED HOOK DELIBERATELY REQUIRES EDITING THIS
|
||||
# LIST. That is the point -- the list is the second party to the agreement, so a
|
||||
# hook cannot leave .pre-commit-config.yaml without someone saying so here.
|
||||
# Upstream hooks are absent on purpose and must stay absent.
|
||||
EXPECTED = {
|
||||
# pre-push
|
||||
'run-tests': (
|
||||
'bash tests/run-tests.sh --strict', ['pre-push']),
|
||||
'check-executables-allow-sync': (
|
||||
'bash scripts/check-executables-allow-sync.sh', ['pre-push']),
|
||||
'apm-audit-ci': (
|
||||
'bash -c \'for d in . plugins/*/; do (cd "$d" && apm audit --ci) || '
|
||||
'{ echo "apm audit --ci failed in $d" >&2; exit 1; }; done\'',
|
||||
['pre-push']),
|
||||
'check-apm-agents-valid': (
|
||||
'bash scripts/check-apm-agents-valid.sh', ['pre-push']),
|
||||
'apm-pack-check-clean': (
|
||||
'apm pack --check-versions --check-clean --dry-run', ['pre-push']),
|
||||
'check-scope-walkup-sync': (
|
||||
'bash scripts/check-scope-walkup-sync.sh', ['pre-push']),
|
||||
'check-skill-version-bump': (
|
||||
'bash scripts/check-skill-version-bump.sh', ['pre-push']),
|
||||
'validate-marketplace': (
|
||||
'claude plugin validate --strict .claude-plugin/marketplace.json',
|
||||
['pre-push']),
|
||||
# pre-commit
|
||||
'skill-size-check': (
|
||||
'scripts/skill-size-check.sh', ['pre-commit']),
|
||||
'check-rtk-prefix': (
|
||||
'scripts/check-rtk-prefix.sh', ['pre-commit']),
|
||||
'vale-audit-prefilter-skill': (
|
||||
'plugins/kyberforge/.apm/skills/factory-audit/scripts/vale-wrap.sh',
|
||||
['pre-commit']),
|
||||
'vale-audit-prefilter-agent': (
|
||||
'plugins/kyberforge/.apm/skills/factory-audit/scripts/vale-wrap.sh',
|
||||
['pre-commit']),
|
||||
}
|
||||
|
||||
path = os.path.join(root, '.pre-commit-config.yaml')
|
||||
try:
|
||||
with open(path, encoding='utf-8') as fh:
|
||||
cfg = yaml.safe_load(fh) or {}
|
||||
except Exception as exc:
|
||||
emit('FAIL', '.pre-commit-config.yaml did not parse: %s' % exc)
|
||||
sys.exit(0)
|
||||
|
||||
local = {}
|
||||
duplicates = []
|
||||
for repo in cfg.get('repos') or []:
|
||||
if repo.get('repo') != 'local':
|
||||
continue
|
||||
for hook in (repo.get('hooks') or []):
|
||||
hid = hook.get('id')
|
||||
if hid in local:
|
||||
duplicates.append(hid)
|
||||
local[hid] = hook
|
||||
|
||||
# Non-vacuity, first. Every assertion below iterates the derived set, so an
|
||||
# empty one would report nothing and the section would pass having checked
|
||||
# nothing at all -- the same shape of silent hole this whole file exists to
|
||||
# close.
|
||||
if not EXPECTED:
|
||||
emit('FAIL', 'the expected repo-authored hook list is empty — '
|
||||
'every wiring assertion below is vacuous')
|
||||
sys.exit(0)
|
||||
if not local:
|
||||
emit('FAIL', '.pre-commit-config.yaml declares no `repo: local` hooks at '
|
||||
'all — every gate this repo authors has been unwired, or the '
|
||||
'config moved and this assertion is now measuring nothing')
|
||||
sys.exit(0)
|
||||
emit('PASS', '.pre-commit-config.yaml declares %d repo-authored (`repo: local`) '
|
||||
'hooks — the wiring assertions below are not vacuous' % len(local))
|
||||
|
||||
if duplicates:
|
||||
emit('FAIL', '.pre-commit-config.yaml declares duplicate local hook ids '
|
||||
'(%s) — pre-commit runs one of them and the other is dead '
|
||||
'config' % ', '.join(sorted(set(duplicates))))
|
||||
|
||||
# The set, before the per-hook detail: a deleted block shows up here as a
|
||||
# missing id even if nothing else in the file changed.
|
||||
missing = sorted(set(EXPECTED) - set(local))
|
||||
extra = sorted(set(local) - set(EXPECTED))
|
||||
if missing or extra:
|
||||
parts = []
|
||||
if missing:
|
||||
parts.append('NOT WIRED in .pre-commit-config.yaml: %s' % ', '.join(missing))
|
||||
if extra:
|
||||
parts.append('wired but not pinned in this test: %s' % ', '.join(extra))
|
||||
emit('FAIL', 'the repo-authored hook set has drifted — %s. A hook that '
|
||||
'leaves the config stops running at push time while every '
|
||||
'test stays green; a hook that arrives unpinned can leave '
|
||||
'again unnoticed. Fix the config, or update EXPECTED in '
|
||||
'tests/test-adr0020-contract.sh deliberately.' % '; '.join(parts))
|
||||
else:
|
||||
emit('PASS', '.pre-commit-config.yaml wires exactly the %d expected '
|
||||
'repo-authored hooks, no more and no fewer' % len(EXPECTED))
|
||||
|
||||
# Per hook: the entry that runs and the stage it runs at. Both are single
|
||||
# tokens whose loss is invisible -- an entry repointed at a path that no longer
|
||||
# exists makes pre-commit fail loudly, but an entry repointed at a DIFFERENT
|
||||
# real script does not, and a hook moved off pre-push simply never fires.
|
||||
entry_paths = []
|
||||
for hid in sorted(EXPECTED):
|
||||
hook = local.get(hid)
|
||||
if hook is None:
|
||||
continue # already reported as missing above
|
||||
exp_entry, exp_stages = EXPECTED[hid]
|
||||
got_entry = hook.get('entry')
|
||||
got_stages = hook.get('stages')
|
||||
problems = []
|
||||
if got_entry != exp_entry:
|
||||
problems.append('entry is %r, expected %r' % (got_entry, exp_entry))
|
||||
if list(got_stages or []) != exp_stages:
|
||||
problems.append('stages is %r, expected %r — a hook at the wrong stage '
|
||||
'(or at none) never fires' % (got_stages, exp_stages))
|
||||
if problems:
|
||||
emit('FAIL', 'hook %s: %s' % (hid, '; '.join(problems)))
|
||||
else:
|
||||
emit('PASS', 'hook %s runs `%s` at %s'
|
||||
% (hid, exp_entry, ','.join(exp_stages)))
|
||||
# Collect the in-repo script/manifest paths the pinned entry names, so the
|
||||
# pin cannot agree with a config that points at nothing.
|
||||
for token in re.split(r'\s+', exp_entry):
|
||||
token = token.strip('\'"')
|
||||
if re.fullmatch(r'[A-Za-z0-9_.\-/]+\.(sh|json)', token):
|
||||
entry_paths.append((hid, token))
|
||||
|
||||
# Every pinned entry path exists on disk. Without this, EXPECTED and the config
|
||||
# could agree perfectly on a script that was deleted.
|
||||
if not entry_paths:
|
||||
emit('FAIL', 'no pinned entry named an in-repo script or manifest — the '
|
||||
'existence check below iterated zero times and proved nothing')
|
||||
else:
|
||||
broken = ['%s -> %s' % (hid, token)
|
||||
for hid, token in entry_paths
|
||||
if not os.path.exists(os.path.join(root, token))]
|
||||
if broken:
|
||||
emit('FAIL', 'pinned hook entries point at files that do not exist: %s'
|
||||
% ', '.join(broken))
|
||||
else:
|
||||
emit('PASS', 'all %d in-repo files named by a pinned hook entry exist '
|
||||
'on disk' % len(entry_paths))
|
||||
PY
|
||||
)"
|
||||
while IFS=$'\t' read -r status msg; do
|
||||
[[ -n "$status" ]] || continue
|
||||
if [[ "$status" == PASS ]]; then
|
||||
pass "$msg"
|
||||
else
|
||||
fail "$msg"
|
||||
fi
|
||||
done <<< "$WIRING_REPORT"
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
|
||||
Reference in New Issue
Block a user