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
This commit is contained in:
@@ -83,33 +83,43 @@ for ini in "$SKILL_INI" "$AGENT_INI"; do
|
|||||||
fi
|
fi
|
||||||
done
|
done
|
||||||
|
|
||||||
# Prints the `files:` regex of every hook, in either manifest, whose entry is
|
# Prints the `files:` regex of every hook, in ONE manifest ($2), whose entry
|
||||||
# $1's vale-wrap.sh. Records are delimited by their `- id:` line, so the check
|
# is $1's vale-wrap.sh. Records are delimited by their `- id:` line, so the
|
||||||
# does not depend on `entry:` preceding `files:` within a record.
|
# check does not depend on `entry:` preceding `files:` within a record.
|
||||||
#
|
#
|
||||||
# Cached per skill (parallel HOOK_REGEX_CACHE_KEYS/_VALS arrays, populated
|
# Deliberately kept per-manifest rather than unioned across both files: the
|
||||||
# lazily) because the final validation loop below probes agent-audit twice —
|
# validation loop below needs to know whether a probe path is in scope of
|
||||||
# once for its CC agent-file shape, once for its Copilot .agent.md shape — and
|
# .pre-commit-hooks.yaml (the canonical, external-facing manifest) and
|
||||||
# both probes need the same regex set. Without the cache, that pair of calls
|
# .pre-commit-config.yaml (this repo's own dev-time copy of the same hook)
|
||||||
# would each re-parse both manifest files from scratch for no new information.
|
# *independently*. A union here previously let a probe that matched only the
|
||||||
# Plain indexed arrays, not `declare -A`: associative arrays are bash 4.0+ and
|
# older, looser .pre-commit-hooks.yaml pattern read as "in scope" even after
|
||||||
# this script must run on macOS's stock bash 3.2. Only ${#arr[@]} (always safe
|
# .pre-commit-config.yaml's copy of the same hook had been narrowed away from
|
||||||
# on an empty/unset array under `set -u`) and index access are used below —
|
# it — silently masking exactly the kind of hook-rescoping drift this script
|
||||||
# never a bare `${arr[@]}` expansion, which aborts on bash < 4.4 under nounset.
|
# 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_KEYS=()
|
||||||
HOOK_REGEX_CACHE_VALS=()
|
HOOK_REGEX_CACHE_VALS=()
|
||||||
hook_file_regexes() {
|
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
|
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]}"
|
printf '%s' "${HOOK_REGEX_CACHE_VALS[$idx]}"
|
||||||
return
|
return
|
||||||
fi
|
fi
|
||||||
idx=$((idx + 1))
|
idx=$((idx + 1))
|
||||||
done
|
done
|
||||||
result="$(
|
result="$(
|
||||||
for manifest in "$REPO_ROOT/.pre-commit-hooks.yaml" "$REPO_ROOT/.pre-commit-config.yaml"; do
|
if [[ -f "$manifest" ]]; then
|
||||||
[[ -f "$manifest" ]] || continue
|
|
||||||
awk -v skill="$skill" '
|
awk -v skill="$skill" '
|
||||||
function flush() {
|
function flush() {
|
||||||
if (entry ~ skill "/scripts/vale-wrap.sh" && files != "") print files
|
if (entry ~ skill "/scripts/vale-wrap.sh" && files != "") print files
|
||||||
@@ -119,19 +129,34 @@ hook_file_regexes() {
|
|||||||
/^[ \t]*entry:/ { entry = $0 }
|
/^[ \t]*entry:/ { entry = $0 }
|
||||||
/^[ \t]*files:/ { files = $0; sub(/^[ \t]*files:[ \t]*/, "", files) }
|
/^[ \t]*files:/ { files = $0; sub(/^[ \t]*files:[ \t]*/, "", files) }
|
||||||
END { flush() }
|
END { flush() }
|
||||||
' "$manifest"
|
' "$manifest" | while IFS= read -r raw; do
|
||||||
done | while IFS= read -r raw; do
|
# Strip the surrounding YAML quotes; the regex itself never carries them.
|
||||||
# Strip the surrounding YAML quotes; the regex itself never carries them.
|
raw="${raw%\'}"; raw="${raw#\'}"
|
||||||
raw="${raw%\'}"; raw="${raw#\'}"
|
raw="${raw%\"}"; raw="${raw#\"}"
|
||||||
raw="${raw%\"}"; raw="${raw#\"}"
|
printf '%s\n' "$raw"
|
||||||
printf '%s\n' "$raw"
|
done
|
||||||
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"
|
HOOK_REGEX_CACHE_VALS[${#HOOK_REGEX_CACHE_VALS[@]}]="$result"
|
||||||
printf '%s' "$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 <<EOF_RE
|
||||||
|
$regexes
|
||||||
|
EOF_RE
|
||||||
|
return 1
|
||||||
|
}
|
||||||
|
|
||||||
# Asks vale — the thing that actually applies these globs — whether a config
|
# Asks vale — the thing that actually applies these globs — whether a config
|
||||||
# covers a path, rather than reimplementing doublestar matching. The probe file
|
# covers a path, rather than reimplementing doublestar matching. The probe file
|
||||||
# carries a description with a token Kyberforge.VagueWording flags, so a config
|
# carries a description with a token Kyberforge.VagueWording flags, so a config
|
||||||
@@ -160,39 +185,53 @@ if ! command -v vale >/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
|
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
|
fi
|
||||||
|
|
||||||
# One representative path per file shape the prefilter is supposed to cover. Each
|
# One representative path per file shape the prefilter is supposed to cover,
|
||||||
# is cross-checked against the shipped hooks' `files:` regexes first, so a path
|
# tagged with whether that shape is expected to be in scope of BOTH manifests
|
||||||
# that goes stale because a hook was rescoped fails loudly here instead of
|
# ("shared") or only the external-facing .pre-commit-hooks.yaml ("hooks-only"
|
||||||
# quietly probing a shape nothing lints any more.
|
# — e.g. a Copilot .agent.md file living outside this repo's own plugins/.apm/
|
||||||
while IFS='|' read -r skill rel; do
|
# 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
|
[[ -n "$skill" ]] || continue
|
||||||
dir="$REPO_ROOT/plugins/kyberforge/.apm/skills/$skill"
|
dir="$REPO_ROOT/plugins/kyberforge/.apm/skills/$skill"
|
||||||
ini="$dir/assets/vale/.vale.ini"
|
ini="$dir/assets/vale/.vale.ini"
|
||||||
[[ -f "$ini" ]] || continue
|
[[ -f "$ini" ]] || continue
|
||||||
|
|
||||||
regexes="$(hook_file_regexes "$skill")"
|
hooks_regexes="$(hook_file_regexes "$skill" "$REPO_ROOT/.pre-commit-hooks.yaml")"
|
||||||
if [[ -n "$regexes" ]]; then
|
config_regexes="$(hook_file_regexes "$skill" "$REPO_ROOT/.pre-commit-config.yaml")"
|
||||||
in_scope=false
|
in_hooks=false
|
||||||
while IFS= read -r re; do
|
matches_any_regex "$rel" "$hooks_regexes" && in_hooks=true
|
||||||
[[ -n "$re" ]] || continue
|
in_config=false
|
||||||
if printf '%s\n' "$rel" | grep -Eq "$re"; then
|
matches_any_regex "$rel" "$config_regexes" && in_config=true
|
||||||
in_scope=true
|
|
||||||
fi
|
if [[ "$in_hooks" == false && "$in_config" == false ]]; then
|
||||||
done <<EOF_RE
|
err "$rel matches no 'files:' regex of any $skill hook — the probe path is stale, or the hook was rescoped away from a shape it still needs to lint"
|
||||||
$regexes
|
elif [[ "$scope" == "shared" && "$in_hooks" != "$in_config" ]]; then
|
||||||
EOF_RE
|
err "$rel is in scope of $skill's hook in .pre-commit-hooks.yaml but not .pre-commit-config.yaml (or vice versa: hooks=$in_hooks, config=$in_config) — the local and canonical 'files:' regexes have drifted out of sync for this hook"
|
||||||
if [[ "$in_scope" == false ]]; then
|
|
||||||
err "$rel matches no 'files:' regex of any $skill hook — the probe path is stale, or the hook was rescoped away from a shape it still needs to lint"
|
|
||||||
fi
|
|
||||||
fi
|
fi
|
||||||
|
|
||||||
if [[ "$VALE_AVAILABLE" == true ]] && ! vale_flags_path "$ini" "$rel"; then
|
if [[ "$VALE_AVAILABLE" == true ]] && ! vale_flags_path "$ini" "$rel"; then
|
||||||
err "$skill/assets/vale/.vale.ini raises no Kyberforge alert on $rel — its glob sections do not cover a path its own pre-commit hook is scoped to, so the hook passes that shape without linting it"
|
err "$skill/assets/vale/.vale.ini raises no Kyberforge alert on $rel — its glob sections do not cover a path its own pre-commit hook is scoped to, so the hook passes that shape without linting it"
|
||||||
fi
|
fi
|
||||||
|
# `demo.md` (bare, no `.agent.md` suffix) is `hooks-only` rather than
|
||||||
|
# `shared`: it exists only to exercise agent-audit's `[**/agents/*.md]` glob
|
||||||
|
# section in isolation from `[**/*.agent.md]` (test-check-vale-style-sync.sh's
|
||||||
|
# case 10), not because any real file under `.apm/agents/` still has that
|
||||||
|
# shape — per ADR-0016 every `.apm/agents/*` file is named `*.agent.md`, so
|
||||||
|
# `.pre-commit-config.yaml`'s regex correctly no longer matches it and that's
|
||||||
|
# not drift. `demo.agent.md` is the real, current shape and is `shared`.
|
||||||
done <<'EOF_PROBE'
|
done <<'EOF_PROBE'
|
||||||
skill-audit|plugins/demo/.apm/skills/demo/SKILL.md
|
skill-audit|plugins/demo/.apm/skills/demo/SKILL.md|shared
|
||||||
agent-audit|plugins/demo/.apm/agents/demo.md
|
agent-audit|plugins/demo/.apm/agents/demo.md|hooks-only
|
||||||
agent-audit|copilot/demo.agent.md
|
agent-audit|plugins/demo/.apm/agents/demo.agent.md|shared
|
||||||
|
agent-audit|copilot/demo.agent.md|hooks-only
|
||||||
EOF_PROBE
|
EOF_PROBE
|
||||||
|
|
||||||
if [[ $FAIL -gt 0 ]]; then
|
if [[ $FAIL -gt 0 ]]; then
|
||||||
|
|||||||
@@ -258,6 +258,31 @@ else
|
|||||||
pass "exits non-zero when a probe path is in no hook's scope any more"
|
pass "exits non-zero when a probe path is in no hook's scope any more"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# --- 11b. Exits 1 when the local config's files: regex narrows out of sync
|
||||||
|
# with the canonical .pre-commit-hooks.yaml regex ---
|
||||||
|
# hook_file_regexes() used to union the two manifests' `files:` regexes before
|
||||||
|
# checking probe coverage, so a probe that matched only the old, looser
|
||||||
|
# .pre-commit-hooks.yaml pattern still passed as "in scope" even after
|
||||||
|
# .pre-commit-config.yaml's copy of the same hook was narrowed away from it.
|
||||||
|
# That is exactly the shape of rescoping this repo's own agent hook went
|
||||||
|
# through (SKILL/agent `.md` -> `.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 ---
|
# --- 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
|
# 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
|
# probe fails on these same mutations, so it would mask them: only masking vale
|
||||||
|
|||||||
Reference in New Issue
Block a user