Four repo gates reported success in states they exist to reject. `check-vale-style-sync.sh` passed while a Kyberforge lint rule was silenced. The check matched a blocklist of severity values, but Vale's semantic is an allowlist: anything that is not exactly YES/error/warning/suggestion disables the rule. So `= false`, `= 0`, `= garbage`, an empty value and — worst — a lowercase `= yes` all killed enforcement while reading as "enabled" to a human. Inverted to an allowlist. Two sibling holes: dropping `KyberforgeCopilot` from `BasedOnStyles` unloaded the Copilot-only check silently, and narrowing a section glob to a location made Vale lint zero files, which is the "0 files, hook Passed" failure the script's own comment says it exists to catch. `sync-marketplace-mirror.sh --check` failed open when its source was missing, while its sibling correctly errored in the same state. `check-scope-walkup-sync.sh` wrote to hardcoded `/tmp/fN.out` paths and read one back, making it non-reentrant — a concurrent instance can flip a verdict, and this branch made the test runner concurrent. Now per-run `mktemp -d`. `check-manifests.sh` had no disk-to-marketplace pass, so a plugin directory absent from `marketplace.json` passed every gate while the `validate-plugins` hook globbed it. The "listed" match is restricted to remote-source entry names; matching any entry name let a genuine orphan through on a name coincidence. `run-bats.sh` reported an empty TAP stream as `0 tests, 0 failures`, exit 0 — a total harness failure reading as a pass. The test-side changes are the larger half, because the guards were the real problem. `test-sync-marketplace-mirror.sh` could overwrite the live tracked mirror under an inherited GIT_DIR, which is precisely the git-hook context it runs in. The bash-3.2 scan hand-maintained its file list, omitting the new shared runner, and had no rule for `wait -n` or `nproc` — the two hazards the previous review round found live. It now derives 43 files across three globs with per-glob floors. Several assertions were decoration: the concurrency checks caught the reentrancy defect 0 times in 10, the leak fix was green either way, and two manifest fixtures passed with the code they claimed to cover deleted. Every assertion now has a revert it provably fails against. Refs: #90 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
325 lines
18 KiB
Bash
Executable File
325 lines
18 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
set -euo pipefail
|
|
|
|
# Kyberforge's Vale prefilter is duplicated into skill-audit and agent-audit's own
|
|
# scripts/assets (per plugins/kyberforge/.apm/skills/skill-author/references/deployment-modes.md's
|
|
# no-cross-skill-path rule: a plugin's cache-install copy only includes each skill's own files).
|
|
# agent-audit's copy is canonical — it's the superset (Kyberforge + KyberforgeCopilot) that the
|
|
# repo root's own pre-commit hook and .pre-commit-hooks.yaml both consume. This fails the build
|
|
# if skill-audit's copy has drifted from it, since nothing else would catch a rule fix landing in
|
|
# only one of the two. Run from repo root or pass REPO_ROOT as arg.
|
|
|
|
REPO_ROOT="${1:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"
|
|
# A nonexistent REPO_ROOT must fail loudly, not fall through to the "neither
|
|
# copy present" no-op below — that guard exists for a repo that legitimately
|
|
# has no kyberforge plugin installed, not for a typo'd or stale path, and a
|
|
# clean exit 0 here would read as "checked, in sync" when nothing ran at all.
|
|
if [[ ! -d "$REPO_ROOT" ]]; then
|
|
echo "Vale style sync check failed: REPO_ROOT '$REPO_ROOT' is not a directory." >&2
|
|
exit 1
|
|
fi
|
|
# Absolutized because the glob probe below `cd`s into a scratch tree, where a
|
|
# relative --config path would stop resolving.
|
|
REPO_ROOT="$(cd "$REPO_ROOT" && pwd)"
|
|
FAIL=0
|
|
|
|
err() { echo " FAIL: $1" >&2; FAIL=$((FAIL + 1)); }
|
|
|
|
SKILL_AUDIT="$REPO_ROOT/plugins/kyberforge/.apm/skills/skill-audit"
|
|
AGENT_AUDIT="$REPO_ROOT/plugins/kyberforge/.apm/skills/agent-audit"
|
|
|
|
if [[ ! -d "$SKILL_AUDIT" && ! -d "$AGENT_AUDIT" ]]; then
|
|
exit 0
|
|
fi
|
|
|
|
# Exactly one present is drift, not absence: the missing copy can't be in sync
|
|
# with the surviving one, and treating it as a no-op is how a deleted or
|
|
# renamed copy would slip through silently.
|
|
if [[ ! -d "$SKILL_AUDIT" ]]; then
|
|
echo "Vale style sync check failed: $AGENT_AUDIT exists but $SKILL_AUDIT does not — run scripts/sync-vale-styles.sh to regenerate skill-audit's copy." >&2
|
|
exit 1
|
|
fi
|
|
if [[ ! -d "$AGENT_AUDIT" ]]; then
|
|
echo "Vale style sync check failed: $SKILL_AUDIT exists but $AGENT_AUDIT does not — agent-audit holds the canonical copy, so restore it before syncing." >&2
|
|
exit 1
|
|
fi
|
|
|
|
if ! diff -q "$SKILL_AUDIT/scripts/vale-wrap.sh" "$AGENT_AUDIT/scripts/vale-wrap.sh" >/dev/null 2>&1; then
|
|
err "scripts/vale-wrap.sh differs between skill-audit and agent-audit"
|
|
fi
|
|
|
|
if ! diff -rq "$SKILL_AUDIT/assets/vale/styles/Kyberforge" "$AGENT_AUDIT/assets/vale/styles/Kyberforge" >/dev/null 2>&1; then
|
|
err "assets/vale/styles/Kyberforge differs between skill-audit and agent-audit"
|
|
fi
|
|
|
|
# --- .vale.ini coverage ------------------------------------------------------
|
|
# The two .vale.ini files are deliberately NOT identical — agent-audit's carries
|
|
# an extra [**/*.agent.md] section and the KyberforgeCopilot style — so they
|
|
# cannot be diffed like the styles above. Nothing else in the repo read them at
|
|
# all, and that is what let a one-character glob typo silently disable the
|
|
# prefilter for a whole file type: the hook still MATCHES the file via its
|
|
# `files:` regex, so pre-commit reports neither `Skipped` nor an error; vale
|
|
# lints zero files, prints `0 errors ... in 1 file` and exits 0, and the hook
|
|
# shows `Passed`. So check the parts that must hold in both, not equality.
|
|
|
|
SKILL_INI="$SKILL_AUDIT/assets/vale/.vale.ini"
|
|
AGENT_INI="$AGENT_AUDIT/assets/vale/.vale.ini"
|
|
|
|
for ini in "$SKILL_INI" "$AGENT_INI"; do
|
|
rel_ini="${ini#"$REPO_ROOT"/}"
|
|
if [[ ! -f "$ini" ]]; then
|
|
err "$rel_ini is missing — without it vale falls back to an upward config search and lints with whatever it finds"
|
|
continue
|
|
fi
|
|
# Present but unreadable is its own case: every assertion below is a grep, and
|
|
# grep exits 2 on a read error. The override capture swallows that into an
|
|
# empty result, which would read as "no findings" rather than "not checked".
|
|
if [[ ! -r "$ini" ]]; then
|
|
err "$rel_ini is not readable — none of its assertions could run, and an unreadable file cannot be distinguished from a clean one downstream"
|
|
continue
|
|
fi
|
|
# StylesPath is resolved relative to the .vale.ini, which is the only reason
|
|
# the bundled styles are found from a consuming repo's clone prefix.
|
|
if ! grep -Eq '^[[:space:]]*StylesPath[[:space:]]*=[[:space:]]*styles[[:space:]]*$' "$ini"; then
|
|
err "$rel_ini has no 'StylesPath = styles' — the bundled styles/ directory would not be found"
|
|
fi
|
|
# Matches `Kyberforge` as a whole name, so `KyberforgeCopilot` alone does not
|
|
# satisfy it. Avoids \b, which is a GNU grep extension.
|
|
if ! grep -Eq '^[[:space:]]*BasedOnStyles[[:space:]]*=.*Kyberforge([[:space:],]|$)' "$ini"; then
|
|
err "$rel_ini has no section whose BasedOnStyles names Kyberforge — every rule the audit prefilters on lives in that style"
|
|
fi
|
|
# Per-rule overrides are the third way to retire a rule without touching a
|
|
# style file or a glob. CONTEXT.md's "Vale audit prefilter" entry: "Every rule
|
|
# is `level: error` and every alert is a FAIL — no ignorable tier". Vale's exit
|
|
# code keys on `error` alerts alone, so any override that leaves a rule at
|
|
# anything other than `error` still lints the file, still exits 0, and still
|
|
# shows `Passed` in pre-commit. The glob probe below cannot backstop this: it
|
|
# keys on one `Kyberforge.VagueWording` alert, so DescriptionOpener,
|
|
# PaddingPhrase, SentenceOpenerThereIs and ProactivePhrase can each be retired
|
|
# underneath a passing probe.
|
|
#
|
|
# Asserted as an ALLOWLIST, not a blocklist of `NO|warning|suggestion`, because
|
|
# that is vale 3.15.2's own semantic: only the exact tokens `YES` and `error`
|
|
# keep a rule blocking. `warning`/`suggestion` downgrade it (alert still
|
|
# printed, exit 0 — invisible, since pre-commit swallows a passing hook's
|
|
# output); every other value — `NO`, `false`, `0`, `off`, `n`, empty,
|
|
# `garbage`, and lowercase `yes`, `true`, `1`, `on` — silences the rule
|
|
# outright. Lowercase `yes` is the trap a blocklist cannot cover: it reads as
|
|
# "enabled" to a human and disables the rule. Verified by enumerating the
|
|
# value space against vale 3.15.2.
|
|
#
|
|
# The allowlist demands a BARE `YES`/`error` with nothing after it, which also
|
|
# rejects `error # note` and `error ; note`. Vale itself strips those — a
|
|
# whitespace-preceded `#` or `;` comment is removed and the rule stays live —
|
|
# so rejecting them is deliberately stricter than vale, not a workaround for
|
|
# it. Uniformity is worth more here than the ability to annotate a line that
|
|
# should not exist: no shipped `.vale.ini` has any override line at all, and
|
|
# the failure mode is a loud false positive rather than a silent pass. The
|
|
# genuine hazard is the no-space form — `error# note` and `error; note` are
|
|
# NOT stripped and silence the rule outright — and a rule that demands a bare
|
|
# token catches those without having to reimplement vale's comment parsing.
|
|
#
|
|
# `[A-Za-z0-9_-]` on both halves of the name, not `[A-Za-z]`: a rule named
|
|
# `Kyberforge.Vague2` is genuinely silenced by `= NO` (verified: 1 error ->
|
|
# 0 errors), so an alpha-only class would let a digit-bearing rule name slip
|
|
# past the gate. All five current rule names are pure alpha, so this is
|
|
# forward cover, not a live hole.
|
|
bad_overrides="$(
|
|
grep -E '^[[:space:]]*Kyberforge[A-Za-z0-9_-]*\.[A-Za-z0-9_-]+[[:space:]]*=' "$ini" \
|
|
| grep -Ev '^[[:space:]]*Kyberforge[A-Za-z0-9_-]*\.[A-Za-z0-9_-]+[[:space:]]*=[[:space:]]*(YES|error)[[:space:]]*$' \
|
|
|| true
|
|
)"
|
|
# No `grep -q` in that pipeline on purpose: `-q` exits on its first match, and
|
|
# under `set -o pipefail` the resulting SIGPIPE on the upstream grep would make
|
|
# the whole pipeline report 141 and read as "no findings".
|
|
if [[ -n "$bad_overrides" ]]; then
|
|
err "$rel_ini overrides a Kyberforge rule to something other than a bare YES or error (first: '${bad_overrides%%$'\n'*}') — every rule in this prefilter is level: error and every alert is a FAIL, and any other value downgrades or silences the rule while vale still exits 0. A trailing comment is rejected too: vale strips a spaced '# ...' but not 'error# ...', so this asks for the bare token rather than guessing which form you meant"
|
|
fi
|
|
done
|
|
|
|
# KyberforgeCopilot is agent-audit's alone — CONTEXT.md describes it as "scoped
|
|
# only to `.agent.md` files for the Copilot-only 'Use proactively has no effect'
|
|
# check". The loop above deliberately asserts only `Kyberforge`, since
|
|
# skill-audit's copy legitimately has no Copilot style, so dropping
|
|
# `, KyberforgeCopilot` from agent-audit's `[**/*.agent.md]` section unloaded the
|
|
# whole style silently: no glob broke, the styles/ diff above stayed clean (the
|
|
# style directory is still shipped, just never loaded), the two .vale.ini files
|
|
# are deliberately unequal so no equality check applies, and the probe below
|
|
# still passed because it keys on a Kyberforge alert. Assert the style is loaded
|
|
# whenever it is shipped.
|
|
if [[ -d "$AGENT_AUDIT/assets/vale/styles/KyberforgeCopilot" && -f "$AGENT_INI" ]]; then
|
|
if ! grep -Eq '^[[:space:]]*BasedOnStyles[[:space:]]*=.*KyberforgeCopilot([[:space:],]|$)' "$AGENT_INI"; then
|
|
err "${AGENT_INI#"$REPO_ROOT"/} ships a styles/KyberforgeCopilot style but no section's BasedOnStyles names it — the style is never loaded, so its Copilot-only rules lint nothing"
|
|
fi
|
|
fi
|
|
|
|
# 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.
|
|
#
|
|
# 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="$2" raw result idx=0
|
|
local cache_key="$skill|$manifest"
|
|
while [[ $idx -lt ${#HOOK_REGEX_CACHE_KEYS[@]} ]]; do
|
|
if [[ "${HOOK_REGEX_CACHE_KEYS[$idx]}" == "$cache_key" ]]; then
|
|
printf '%s' "${HOOK_REGEX_CACHE_VALS[$idx]}"
|
|
return
|
|
fi
|
|
idx=$((idx + 1))
|
|
done
|
|
result="$(
|
|
if [[ -f "$manifest" ]]; then
|
|
awk -v skill="$skill" '
|
|
function flush() {
|
|
if (entry ~ skill "/scripts/vale-wrap.sh" && files != "") print files
|
|
entry = ""; files = ""
|
|
}
|
|
/^[ \t]*-[ \t]*id:/ { flush() }
|
|
/^[ \t]*entry:/ { entry = $0 }
|
|
/^[ \t]*files:/ { files = $0; sub(/^[ \t]*files:[ \t]*/, "", files) }
|
|
END { flush() }
|
|
' "$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[@]}]="$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 <<EOF_RE
|
|
$regexes
|
|
EOF_RE
|
|
return 1
|
|
}
|
|
|
|
# Asks vale — the thing that actually applies these globs — whether a config
|
|
# covers a path, rather than reimplementing doublestar matching. The probe file
|
|
# carries a description with a token Kyberforge.VagueWording flags, so a config
|
|
# whose glob matches but whose BasedOnStyles lost Kyberforge fails too: it would
|
|
# lint the file and report nothing.
|
|
vale_flags_path() {
|
|
local cfg="$1" rel="$2" tmp out
|
|
tmp="$(mktemp -d)"
|
|
mkdir -p "$tmp/$(dirname "$rel")"
|
|
{
|
|
echo "---"
|
|
echo "name: probe"
|
|
echo "description: Use when the caller wants a probe that helps with things."
|
|
echo "---"
|
|
echo ""
|
|
echo "Body."
|
|
} > "$tmp/$rel"
|
|
out="$(cd "$tmp" && vale --config "$cfg" "$rel" 2>&1)" || true
|
|
rm -rf "$tmp"
|
|
printf '%s\n' "$out" | grep -qF "Kyberforge.VagueWording"
|
|
}
|
|
|
|
VALE_AVAILABLE=true
|
|
if ! command -v vale >/dev/null 2>&1; then
|
|
VALE_AVAILABLE=false
|
|
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,
|
|
# 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
|
|
|
|
hooks_regexes="$(hook_file_regexes "$skill" "$REPO_ROOT/.pre-commit-hooks.yaml")"
|
|
config_regexes="$(hook_file_regexes "$skill" "$REPO_ROOT/.pre-commit-config.yaml")"
|
|
in_hooks=false
|
|
matches_any_regex "$rel" "$hooks_regexes" && in_hooks=true
|
|
in_config=false
|
|
matches_any_regex "$rel" "$config_regexes" && in_config=true
|
|
|
|
if [[ "$in_hooks" == false && "$in_config" == 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"
|
|
elif [[ "$scope" == "shared" && "$in_hooks" != "$in_config" ]]; then
|
|
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"
|
|
fi
|
|
|
|
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"
|
|
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`.
|
|
#
|
|
# The two `.claude/`-prefixed probes carry the location-independence CONTEXT.md
|
|
# asserts: "A `SKILL.md` outside `plugins/` (e.g. project-scope
|
|
# `.claude/skills/foo/SKILL.md`) still matches `[**/SKILL.md]` and gets linted
|
|
# normally — the globs constrain filename shape, not location." Every other
|
|
# probe here starts with `plugins/`, so narrowing a glob to a `plugins/`-shaped
|
|
# path (`[**/SKILL.md]` -> `[**/.apm/skills/*/SKILL.md]`) left all of them
|
|
# matching while the project-scope shape started linting as `0 errors ... in 0
|
|
# files` — the exact "vale lints zero files, hook shows Passed" failure the
|
|
# comment at the top of this section describes. Both are `hooks-only`: only
|
|
# `.pre-commit-hooks.yaml` is layout-agnostic, and `.pre-commit-config.yaml`
|
|
# pinning this repo's own `plugins/**/.apm/` layout is by design, not drift.
|
|
done <<'EOF_PROBE'
|
|
skill-audit|plugins/demo/.apm/skills/demo/SKILL.md|shared
|
|
skill-audit|.claude/skills/demo/SKILL.md|hooks-only
|
|
agent-audit|plugins/demo/.apm/agents/demo.md|hooks-only
|
|
agent-audit|plugins/demo/.apm/agents/demo.agent.md|shared
|
|
agent-audit|.claude/agents/demo.md|hooks-only
|
|
agent-audit|copilot/demo.agent.md|hooks-only
|
|
EOF_PROBE
|
|
|
|
if [[ $FAIL -gt 0 ]]; then
|
|
echo "Vale style sync check failed: $FAIL error(s). For a drifted wrapper or style, agent-audit's copy is canonical — run scripts/sync-vale-styles.sh to regenerate skill-audit's copy, then commit both. A .vale.ini finding is not drift and sync-vale-styles.sh will not fix it: edit that file's own StylesPath, BasedOnStyles or glob sections." >&2
|
|
exit 1
|
|
fi
|