From 07ea0aeb17a11a6aa7ddf980cbe2ce35e372e5a5 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Thu, 13 Aug 2026 22:07:22 +0000 Subject: [PATCH] fix(kyberforge): stop union-masking drift between vale-audit-prefilter manifests hook_file_regexes() unioned the `files:` regex from .pre-commit-hooks.yaml and .pre-commit-config.yaml before checking whether a probe path is in scope of a kyberforge vale-audit-prefilter hook. That union let a probe matching only the old, looser .pre-commit-hooks.yaml pattern pass even after .pre-commit-config.yaml's copy of the same hook had been narrowed (e.g. to require a `.agent.md` suffix) -- silently masking exactly the kind of hook-rescoping drift this check exists to catch. Per ADR-0014 the two manifests are meant to exercise the same resolution path an external consumer's hook would, so this divergence is real drift, not noise. hook_file_regexes() now takes the manifest path explicitly and caches per (skill, manifest) pair instead of per skill, so each manifest's regex set can be inspected on its own. The probe-validation loop computes in_hooks/in_config independently via a new matches_any_regex() helper. Probes carry a new third heredoc field, `shared` or `hooks-only`: `shared` probes (a file shape genuinely covered by both manifests, e.g. plugins/demo/.apm/agents/demo.agent.md) must agree between the two or the check now fails with a drift error; `hooks-only` probes (a Copilot .agent.md living outside this repo's own plugins/.apm/ layout, and the legacy bare-`.md`-under-agents/ shape kept only to exercise a distinct .vale.ini glob section in isolation) are exempt, since .pre-commit-hooks.yaml is deliberately broader there by design. The original "matches no regex in either manifest" staleness check is unchanged. Added case 11b to tests/test-check-vale-style-sync.sh: narrows a fixture's local config regex further while leaving .pre-commit-hooks.yaml untouched, and asserts the check now flags it. Confirmed red against the pre-fix script before applying the fix. Refs: #95 --- scripts/check-vale-style-sync.sh | 131 ++++++++++++++++++---------- tests/test-check-vale-style-sync.sh | 25 ++++++ 2 files changed, 110 insertions(+), 46 deletions(-) diff --git a/scripts/check-vale-style-sync.sh b/scripts/check-vale-style-sync.sh index 3876c3f..defd5f3 100755 --- a/scripts/check-vale-style-sync.sh +++ b/scripts/check-vale-style-sync.sh @@ -83,33 +83,43 @@ for ini in "$SKILL_INI" "$AGENT_INI"; do fi done -# Prints the `files:` regex of every hook, in either manifest, whose entry is -# $1's vale-wrap.sh. Records are delimited by their `- id:` line, so the check -# does not depend on `entry:` preceding `files:` within a record. +# Prints the `files:` regex of every hook, in ONE manifest ($2), whose entry +# is $1's vale-wrap.sh. Records are delimited by their `- id:` line, so the +# check does not depend on `entry:` preceding `files:` within a record. # -# Cached per skill (parallel HOOK_REGEX_CACHE_KEYS/_VALS arrays, populated -# lazily) because the final validation loop below probes agent-audit twice — -# once for its CC agent-file shape, once for its Copilot .agent.md shape — and -# both probes need the same regex set. Without the cache, that pair of calls -# would each re-parse both manifest files from scratch for no new information. -# Plain indexed arrays, not `declare -A`: associative arrays are bash 4.0+ and -# this script must run on macOS's stock bash 3.2. Only ${#arr[@]} (always safe -# on an empty/unset array under `set -u`) and index access are used below — -# never a bare `${arr[@]}` expansion, which aborts on bash < 4.4 under nounset. +# Deliberately kept per-manifest rather than unioned across both files: the +# validation loop below needs to know whether a probe path is in scope of +# .pre-commit-hooks.yaml (the canonical, external-facing manifest) and +# .pre-commit-config.yaml (this repo's own dev-time copy of the same hook) +# *independently*. A union here previously let a probe that matched only the +# older, looser .pre-commit-hooks.yaml pattern read as "in scope" even after +# .pre-commit-config.yaml's copy of the same hook had been narrowed away from +# it — silently masking exactly the kind of hook-rescoping drift this script +# exists to catch. +# +# Cached per (skill, manifest) pair (parallel HOOK_REGEX_CACHE_KEYS/_VALS +# arrays, populated lazily) because the final validation loop below probes +# agent-audit's two manifests across three probe shapes; without the cache, +# each repeated (skill, manifest) pairing would re-parse the same manifest +# file from scratch for no new information. Plain indexed arrays, not +# `declare -A`: associative arrays are bash 4.0+ and this script must run on +# macOS's stock bash 3.2. Only ${#arr[@]} (always safe on an empty/unset array +# under `set -u`) and index access are used below — never a bare `${arr[@]}` +# expansion, which aborts on bash < 4.4 under nounset. HOOK_REGEX_CACHE_KEYS=() HOOK_REGEX_CACHE_VALS=() hook_file_regexes() { - local skill="$1" manifest raw result idx=0 + local skill="$1" manifest="$2" raw result idx=0 + local cache_key="$skill|$manifest" while [[ $idx -lt ${#HOOK_REGEX_CACHE_KEYS[@]} ]]; do - if [[ "${HOOK_REGEX_CACHE_KEYS[$idx]}" == "$skill" ]]; then + if [[ "${HOOK_REGEX_CACHE_KEYS[$idx]}" == "$cache_key" ]]; then printf '%s' "${HOOK_REGEX_CACHE_VALS[$idx]}" return fi idx=$((idx + 1)) done result="$( - for manifest in "$REPO_ROOT/.pre-commit-hooks.yaml" "$REPO_ROOT/.pre-commit-config.yaml"; do - [[ -f "$manifest" ]] || continue + if [[ -f "$manifest" ]]; then awk -v skill="$skill" ' function flush() { if (entry ~ skill "/scripts/vale-wrap.sh" && files != "") print files @@ -119,19 +129,34 @@ hook_file_regexes() { /^[ \t]*entry:/ { entry = $0 } /^[ \t]*files:/ { files = $0; sub(/^[ \t]*files:[ \t]*/, "", files) } END { flush() } - ' "$manifest" - done | while IFS= read -r raw; do - # Strip the surrounding YAML quotes; the regex itself never carries them. - raw="${raw%\'}"; raw="${raw#\'}" - raw="${raw%\"}"; raw="${raw#\"}" - printf '%s\n' "$raw" - done + ' "$manifest" | while IFS= read -r raw; do + # Strip the surrounding YAML quotes; the regex itself never carries them. + raw="${raw%\'}"; raw="${raw#\'}" + raw="${raw%\"}"; raw="${raw#\"}" + printf '%s\n' "$raw" + done + fi )" - HOOK_REGEX_CACHE_KEYS[${#HOOK_REGEX_CACHE_KEYS[@]}]="$skill" + HOOK_REGEX_CACHE_KEYS[${#HOOK_REGEX_CACHE_KEYS[@]}]="$cache_key" HOOK_REGEX_CACHE_VALS[${#HOOK_REGEX_CACHE_VALS[@]}]="$result" printf '%s' "$result" } +# True if $1 matches at least one newline-delimited regex in $2. +matches_any_regex() { + local rel="$1" regexes="$2" re + [[ -n "$regexes" ]] || return 1 + while IFS= read -r re; do + [[ -n "$re" ]] || continue + if printf '%s\n' "$rel" | grep -Eq "$re"; then + return 0 + fi + done </dev/null 2>&1; then echo " WARNING: vale is not installed — .vale.ini glob coverage was NOT verified. Install it (https://vale.sh/docs/vale-cli/installation/) before trusting a clean run." >&2 fi -# One representative path per file shape the prefilter is supposed to cover. Each -# is cross-checked against the shipped hooks' `files:` regexes first, so a path -# that goes stale because a hook was rescoped fails loudly here instead of -# quietly probing a shape nothing lints any more. -while IFS='|' read -r skill rel; do +# One representative path per file shape the prefilter is supposed to cover, +# tagged with whether that shape is expected to be in scope of BOTH manifests +# ("shared") or only the external-facing .pre-commit-hooks.yaml ("hooks-only" +# — e.g. a Copilot .agent.md file living outside this repo's own plugins/.apm/ +# layout, which .pre-commit-config.yaml's repo-scoped regex has no reason to +# cover). Each probe is checked against the two manifests' `files:` regexes +# *separately*, not unioned: a path that goes stale because a hook was +# rescoped fails loudly here instead of quietly probing a shape nothing lints +# any more, and a "shared" path the two manifests disagree on fails loudly +# too — that disagreement is exactly how .pre-commit-config.yaml's regex can +# narrow out of sync with .pre-commit-hooks.yaml's without either manifest's +# own hook breaking (each still matches real files on its own), so nothing +# else would catch it. +while IFS='|' read -r skill rel scope; do [[ -n "$skill" ]] || continue dir="$REPO_ROOT/plugins/kyberforge/.apm/skills/$skill" ini="$dir/assets/vale/.vale.ini" [[ -f "$ini" ]] || continue - regexes="$(hook_file_regexes "$skill")" - if [[ -n "$regexes" ]]; then - in_scope=false - while IFS= read -r re; do - [[ -n "$re" ]] || continue - if printf '%s\n' "$rel" | grep -Eq "$re"; then - in_scope=true - fi - done < `.apm/.../*.agent.md`): the local hook quietly +# stopped linting a shape the shipped, external-facing manifest still claims +# to cover, and nothing caught it. Reproduce it directly: narrow only the +# fixture's local config regex (leave .pre-commit-hooks.yaml as shipped) and +# assert the check now flags the disagreement instead of passing silently. +echo "" +echo "--- exits 1 when .pre-commit-config.yaml's files: regex drifts out of sync with .pre-commit-hooks.yaml's ---" +FIXTURE16B="$(make_fixture)" +FIXTURES+=("$FIXTURE16B") +break_glob "$FIXTURE16B/.pre-commit-config.yaml" \ + "files: '^plugins/[^/]+/\\.apm/agents/[^/]+\\.agent\\.md\$'" \ + "files: '^plugins/kyberforge/\\.apm/agents/[^/]+\\.agent\\.md\$'" +if bash "$SCRIPT" "$FIXTURE16B" > /dev/null 2>&1; then + fail "exited 0 when the local config regex narrowed out of sync with .pre-commit-hooks.yaml — expected exit 1" +else + pass "exits non-zero when the local config regex narrows out of sync with the canonical .pre-commit-hooks.yaml regex" +fi + # --- 12. The text-level assertions hold on a machine without vale --- # They are the fallback when the glob probe cannot run. With vale on PATH the # probe fails on these same mutations, so it would mask them: only masking vale