Compare commits
5 Commits
55d956b298
...
0f0ac5821f
| Author | SHA1 | Date | |
|---|---|---|---|
| 0f0ac5821f | |||
| c442f7eb85 | |||
| 5a61b417c9 | |||
| 73393b9d01 | |||
| 49d21bcb4d |
@@ -36,7 +36,7 @@ Fall back to raw shell only when no skill covers it.
|
|||||||
- Install `jq` — required by `scripts/check-manifests.sh` and `scripts/sync-plugin-content.sh`, both pre-push. These at least fail loudly (`Error: jq is required but not installed`).
|
- Install `jq` — required by `scripts/check-manifests.sh` and `scripts/sync-plugin-content.sh`, both pre-push. These at least fail loudly (`Error: jq is required but not installed`).
|
||||||
- Install the `vale` binary — required by the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks. Their `files:` patterns are `.apm/`-scoped: `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$` and `^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$`. Only the authoring source triggers them — a `SKILL.md` in the generated mirror matches neither pattern, so prose findings surface only when you edit the file you are supposed to be editing. Without the binary the hooks fail with a bare "command not found" and no install pointer. `brew install vale` (macOS), `snap install vale` (Linux), `choco install vale` (Windows), or see https://vale.sh/docs/vale-cli/installation/. No `vale sync` needed — the `Kyberforge` styles are committed under `plugins/kyberforge/.apm/skills/{skill-audit,agent-audit}/assets/vale/styles/`, not downloaded packages (see ADR-0014).
|
- Install the `vale` binary — required by the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks. Their `files:` patterns are `.apm/`-scoped: `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$` and `^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$`. Only the authoring source triggers them — a `SKILL.md` in the generated mirror matches neither pattern, so prose findings surface only when you edit the file you are supposed to be editing. Without the binary the hooks fail with a bare "command not found" and no install pointer. `brew install vale` (macOS), `snap install vale` (Linux), `choco install vale` (Windows), or see https://vale.sh/docs/vale-cli/installation/. No `vale sync` needed — the `Kyberforge` styles are committed under `plugins/kyberforge/.apm/skills/{skill-audit,agent-audit}/assets/vale/styles/`, not downloaded packages (see ADR-0014).
|
||||||
- Run `bash tests/run-tests.sh` before considering any change done — it runs every `test-*.sh` script in the repo plus the bats suite (`--bats-only` for just bats). First run auto-initializes the bats submodules; no manual `git submodule update` needed.
|
- Run `bash tests/run-tests.sh` before considering any change done — it runs every `test-*.sh` script in the repo plus the bats suite (`--bats-only` for just bats). First run auto-initializes the bats submodules; no manual `git submodule update` needed.
|
||||||
- Pushing runs 12 pre-push hooks, not just the test suite — `run-tests` and `check-manifests`, plus generated-content drift gates (`check-plugin-content-sync`, `check-marketplace-mirror-sync`, `check-vale-style-sync`, `check-scope-walkup-sync`), apm's own gates (`apm-marketplace-check`, `apm-audit-ci`, `apm-pack-check-clean`), host validators (`validate-plugins`, `validate-marketplace`, both needing the `claude` CLI), and `check-release-needed`. Run `pre-commit run --hook-stage pre-push --all-files` locally — one command, the whole gate.
|
- Pushing runs 12 repo-defined pre-push hooks, not just the test suite — `run-tests` and `check-manifests`, plus generated-content drift gates (`check-plugin-content-sync`, `check-marketplace-mirror-sync`, `check-vale-style-sync`, `check-scope-walkup-sync`), apm's own gates (`apm-marketplace-check`, `apm-audit-ci`, `apm-pack-check-clean`), host validators (`validate-plugins`, `validate-marketplace`, both needing the `claude` CLI), and `check-release-needed`. Run `pre-commit run --hook-stage pre-push --all-files` locally — one command, the whole gate. That command reports **14**, not 12: pre-commit's own `meta` hooks, `check-hooks-apply` and `check-useless-excludes`, declare no `stages:` and so run at every stage including this one.
|
||||||
- `apm-marketplace-check` needs the network. It resolves every `marketplace.packages[]` entry including the remote `mattpocock-skills` ref, and it is `always_run`, so an unreachable network hard-fails the push. `--offline` is not an escape hatch — it still exits 1 on that entry (`No cached refs (offline)`). To push without a network, skip that one hook using pre-commit's own mechanism: `SKIP=apm-marketplace-check git push`. Skip that hook alone — it is the only one whose failure mode is "no network". Every other pre-push hook is a real local check, and adding it to `SKIP` disarms it silently.
|
- `apm-marketplace-check` needs the network. It resolves every `marketplace.packages[]` entry including the remote `mattpocock-skills` ref, and it is `always_run`, so an unreachable network hard-fails the push. `--offline` is not an escape hatch — it still exits 1 on that entry (`No cached refs (offline)`). To push without a network, skip that one hook using pre-commit's own mechanism: `SKIP=apm-marketplace-check git push`. Skip that hook alone — it is the only one whose failure mode is "no network". Every other pre-push hook is a real local check, and adding it to `SKIP` disarms it silently.
|
||||||
- Author commits with `git:git-commits` — it validates Conventional Commits (enforced at `commit-msg`) for you.
|
- Author commits with `git:git-commits` — it validates Conventional Commits (enforced at `commit-msg`) for you.
|
||||||
|
|
||||||
|
|||||||
@@ -92,8 +92,17 @@ parts=()
|
|||||||
[ -n "$vim_mode" ] && parts+=("${COLOR_MAGENTA}${vim_mode}${COLOR_RESET}")
|
[ -n "$vim_mode" ] && parts+=("${COLOR_MAGENTA}${vim_mode}${COLOR_RESET}")
|
||||||
|
|
||||||
# --- Join with ' · ' separator and print ---
|
# --- Join with ' · ' separator and print ---
|
||||||
|
# `parts` is seeded empty above and every append is conditional, so all seven can
|
||||||
|
# be false and the array can reach here with no elements. Two things had to both
|
||||||
|
# stay true for bare "${parts[@]}" to be safe: this file enabling no `set -u`
|
||||||
|
# (house style in every other script here is `set -euo pipefail`), and the shell
|
||||||
|
# being bash 4.4+, which stopped treating an empty-array expansion as unbound.
|
||||||
|
# On bash 3.2 -- macOS's system bash, and an explicit repo target -- adding
|
||||||
|
# `set -u` aborts here. The guarded form removes the tripwire instead of relying
|
||||||
|
# on both conditions holding. install.sh deploys this file to every user
|
||||||
|
# machine. See issue #96.
|
||||||
output=""
|
output=""
|
||||||
for part in "${parts[@]}"; do
|
for part in ${parts[@]+"${parts[@]}"}; do
|
||||||
[ -z "$output" ] && output="$part" || output="${output} · ${part}"
|
[ -z "$output" ] && output="$part" || output="${output} · ${part}"
|
||||||
done
|
done
|
||||||
|
|
||||||
|
|||||||
@@ -46,7 +46,8 @@ if [[ ! -f "$MARKETPLACE" ]]; then
|
|||||||
fi
|
fi
|
||||||
|
|
||||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||||
# shellcheck source=lib/marketplace-plugins.sh
|
# Repo-root-relative, not script-dir-relative -- see tests/run-tests.sh for why.
|
||||||
|
# shellcheck source=scripts/lib/marketplace-plugins.sh
|
||||||
source "$SCRIPT_DIR/lib/marketplace-plugins.sh"
|
source "$SCRIPT_DIR/lib/marketplace-plugins.sh"
|
||||||
|
|
||||||
# Every local plugin directory marketplace.json claimed, canonicalized, so the
|
# Every local plugin directory marketplace.json claimed, canonicalized, so the
|
||||||
|
|||||||
@@ -67,15 +67,31 @@ AGENT_INI="$AGENT_AUDIT/assets/vale/.vale.ini"
|
|||||||
|
|
||||||
for ini in "$SKILL_INI" "$AGENT_INI"; do
|
for ini in "$SKILL_INI" "$AGENT_INI"; do
|
||||||
rel_ini="${ini#"$REPO_ROOT"/}"
|
rel_ini="${ini#"$REPO_ROOT"/}"
|
||||||
if [[ ! -f "$ini" ]]; then
|
# `-e`, not `-f`: a path that exists but is not a readable regular file (a
|
||||||
|
# directory sitting where the file should be, say) is not "missing", and
|
||||||
|
# reporting it as missing sends you looking for a deleted file. It belongs to
|
||||||
|
# the unreadable case below, which is the one that describes what actually
|
||||||
|
# went wrong.
|
||||||
|
if [[ ! -e "$ini" ]]; then
|
||||||
err "$rel_ini is missing — without it vale falls back to an upward config search and lints with whatever it finds"
|
err "$rel_ini is missing — without it vale falls back to an upward config search and lints with whatever it finds"
|
||||||
continue
|
continue
|
||||||
fi
|
fi
|
||||||
# Present but unreadable is its own case: every assertion below is a grep, and
|
# 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
|
# 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".
|
# empty result, which would read as "no findings" rather than "not checked",
|
||||||
if [[ ! -r "$ini" ]]; then
|
# and the two greps above it report "has no StylesPath"/"names no Kyberforge"
|
||||||
err "$rel_ini is not readable — none of its assertions could run, and an unreadable file cannot be distinguished from a clean one downstream"
|
# for a file that may well have both — a misdiagnosis, not a missed one.
|
||||||
|
#
|
||||||
|
# Decided by ACTUALLY READING the file, not by `[[ -r ]]`. `-r` is access(2),
|
||||||
|
# which answers "would the permission bits allow it" — and for uid 0 that is
|
||||||
|
# yes even on a mode-000 file (verified). This hook runs at pre-push, and this
|
||||||
|
# repo's dev environment is root, so an `[[ ! -r ]]` guard could never fire in
|
||||||
|
# the one place it exists to fire: it was untestable because it was dead. A
|
||||||
|
# read attempt is also the stricter question, catching EISDIR and EIO, which
|
||||||
|
# access(2) reports on neither. `cat`, not a bare `< "$ini"` redirect: opening
|
||||||
|
# a directory for reading succeeds, only the read fails.
|
||||||
|
if ! cat "$ini" >/dev/null 2>&1; then
|
||||||
|
err "$rel_ini exists but could not be read — none of its assertions could run, and an unreadable file cannot be distinguished from a clean one downstream"
|
||||||
continue
|
continue
|
||||||
fi
|
fi
|
||||||
# StylesPath is resolved relative to the .vale.ini, which is the only reason
|
# StylesPath is resolved relative to the .vale.ini, which is the only reason
|
||||||
|
|||||||
@@ -46,6 +46,12 @@ set -euo pipefail
|
|||||||
# The merged hooks file is mirrored like the other MIRROR_DIRS content: synced when
|
# The merged hooks file is mirrored like the other MIRROR_DIRS content: synced when
|
||||||
# .apm/hooks/ produces one, and removed (real mode) / flagged as drift (--check) when
|
# .apm/hooks/ produces one, and removed (real mode) / flagged as drift (--check) when
|
||||||
# it no longer does but a mirrored copy is still sitting there from a prior sync.
|
# it no longer does but a mirrored copy is still sitting there from a prior sync.
|
||||||
|
# The hooks/ directory holding it is owned outright by the mirror the same way every
|
||||||
|
# MIRROR_DIRS destination is -- a real sync wipes and rebuilds it, so a stray file
|
||||||
|
# dropped inside, or the empty directory left behind once .apm/hooks/ stops producing
|
||||||
|
# a hooks.json, is cleaned up rather than preserved. (A stray at the PLUGIN ROOT is a
|
||||||
|
# different matter and stays out of scope by design: README.md, docs/, bin/, .mcp.json
|
||||||
|
# and friends are hand-authored there.)
|
||||||
# It lands at hooks/hooks.json, not at the plugin root: Claude Code convention-scans
|
# It lands at hooks/hooks.json, not at the plugin root: Claude Code convention-scans
|
||||||
# `hooks/hooks.json` "at the plugin root, not inside .claude-plugin/"
|
# `hooks/hooks.json` "at the plugin root, not inside .claude-plugin/"
|
||||||
# (plugins/kyberforge/docs/research/docs/claude-code-plugins/configuration.md's "Plugin
|
# (plugins/kyberforge/docs/research/docs/claude-code-plugins/configuration.md's "Plugin
|
||||||
@@ -106,17 +112,30 @@ if ! command -v jq &>/dev/null; then
|
|||||||
fi
|
fi
|
||||||
|
|
||||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||||
# shellcheck source=lib/marketplace-plugins.sh
|
# Both source= paths below are repo-root-relative, not script-dir-relative -- see
|
||||||
|
# tests/run-tests.sh for why the script-dir spelling silently fails to resolve.
|
||||||
|
# shellcheck source=scripts/lib/marketplace-plugins.sh
|
||||||
source "$SCRIPT_DIR/lib/marketplace-plugins.sh"
|
source "$SCRIPT_DIR/lib/marketplace-plugins.sh"
|
||||||
# shellcheck source=lib/batch-run.sh
|
# shellcheck source=scripts/lib/batch-run.sh
|
||||||
source "$SCRIPT_DIR/lib/batch-run.sh"
|
source "$SCRIPT_DIR/lib/batch-run.sh"
|
||||||
|
|
||||||
# Convention subdirectories apm's plugin exporter can populate from .apm/.
|
# Convention subdirectories apm's plugin exporter can populate from .apm/.
|
||||||
|
#
|
||||||
|
# These are DESTINATION directory names at the plugin root, not .apm/ source
|
||||||
|
# directory names -- the two lists are deliberately not the same, so do not "fix"
|
||||||
|
# this by pasting in the header comment's .apm/ list. In particular .apm/prompts/
|
||||||
|
# has no destination of its own: apm's exporter folds it into commands/ together
|
||||||
|
# with .apm/commands/, renaming *.prompt.md to *.md (ADR-0017's mapping table;
|
||||||
|
# re-verified empirically against apm 0.28.0, and pinned by the prompts test in
|
||||||
|
# tests/test-sync-plugin-content.sh). Adding `prompts` here would name an output
|
||||||
|
# directory apm never emits and no plugin host ever scans.
|
||||||
MIRROR_DIRS=(agents skills commands instructions extensions)
|
MIRROR_DIRS=(agents skills commands instructions extensions)
|
||||||
|
|
||||||
# Where the merged hooks file lands (Claude Code's convention-scanned path), and
|
# The generated hooks directory, the merged hooks file inside it (Claude Code's
|
||||||
# the pre-fix root-level path a real sync now cleans up as stale.
|
# convention-scanned path), and the pre-fix root-level path a real sync now cleans
|
||||||
HOOKS_REL="hooks/hooks.json"
|
# up as stale.
|
||||||
|
HOOKS_DIR_REL="hooks"
|
||||||
|
HOOKS_REL="$HOOKS_DIR_REL/hooks.json"
|
||||||
LEGACY_HOOKS_REL="hooks.json"
|
LEGACY_HOOKS_REL="hooks.json"
|
||||||
|
|
||||||
FAIL=0
|
FAIL=0
|
||||||
@@ -202,15 +221,24 @@ sync_hooks_json() {
|
|||||||
rm -f "$legacy"
|
rm -f "$legacy"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# hooks/ is generated output the mirror owns outright, exactly like every
|
||||||
|
# MIRROR_DIRS destination -- so wipe it wholesale and rebuild, rather than
|
||||||
|
# editing hooks.json in place and leaving whatever else happens to be in there.
|
||||||
|
# sync_dir gets this for free from its own rm -rf; hooks/ has to spell it out
|
||||||
|
# because its one legitimate occupant is written by name instead of copied as a
|
||||||
|
# tree. The wipe covers both a stray file someone dropped alongside hooks.json
|
||||||
|
# and the case where .apm/hooks/ stops producing a hooks.json at all: the
|
||||||
|
# directory then ends up absent, not present-and-empty.
|
||||||
|
#
|
||||||
|
# ${target_dir:?} rather than a bare expansion: `set -u` aborts on an UNSET
|
||||||
|
# variable but not an empty one, and an empty $target_dir would make this
|
||||||
|
# `rm -rf /hooks`. The other rm -rf calls here take a $dst built by their
|
||||||
|
# caller; this one is the only place a bare parameter is the whole prefix.
|
||||||
|
rm -rf "${target_dir:?}/$HOOKS_DIR_REL"
|
||||||
|
|
||||||
if [[ -f "$src" ]]; then
|
if [[ -f "$src" ]]; then
|
||||||
mkdir -p "$target_dir/hooks"
|
mkdir -p "$target_dir/$HOOKS_DIR_REL"
|
||||||
normalize_trailing_newline "$src" "$dst"
|
normalize_trailing_newline "$src" "$dst"
|
||||||
elif [[ -f "$dst" ]]; then
|
|
||||||
# .apm/hooks/ no longer produces a hooks.json, but one is still sitting at
|
|
||||||
# $dst from a prior sync -- that's stale mirrored output, not "no .apm/hooks/
|
|
||||||
# content" (which would mean $dst never existed in the first place).
|
|
||||||
rm -f "$dst"
|
|
||||||
rmdir "$target_dir/hooks" 2>/dev/null || true
|
|
||||||
fi
|
fi
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -290,6 +318,14 @@ path_manifest() {
|
|||||||
# symlink to identical content, both leave --check at exit 0 while a real sync
|
# symlink to identical content, both leave --check at exit 0 while a real sync
|
||||||
# silently repairs them -- check and sync disagreeing, which is the one thing this
|
# silently repairs them -- check and sync disagreeing, which is the one thing this
|
||||||
# gate exists to prevent. Compare an explicit type+exec-bit manifest as well.
|
# gate exists to prevent. Compare an explicit type+exec-bit manifest as well.
|
||||||
|
#
|
||||||
|
# The manifest is a full recursive listing, so it is also what makes an entry that
|
||||||
|
# exists on only one side visible. That is the sole coverage the generated hooks/
|
||||||
|
# directory gets for anything other than hooks.json itself (which check_file above
|
||||||
|
# handles by name): a stray file inside hooks/, or hooks/ still sitting there empty
|
||||||
|
# after .apm/hooks/ stopped producing anything, shows up here as a manifest line
|
||||||
|
# present on one side only. Both are things sync_hooks_json's rm -rf removes, so
|
||||||
|
# both have to be drift.
|
||||||
check_path_modes() {
|
check_path_modes() {
|
||||||
local plugin_dir="$1" expected_dir="$2"
|
local plugin_dir="$1" expected_dir="$2"
|
||||||
shift 2
|
shift 2
|
||||||
@@ -299,7 +335,7 @@ check_path_modes() {
|
|||||||
path_manifest "$expected_dir" "$@" >"$expected"
|
path_manifest "$expected_dir" "$@" >"$expected"
|
||||||
path_manifest "$plugin_dir" "$@" >"$actual"
|
path_manifest "$plugin_dir" "$@" >"$actual"
|
||||||
if ! diff -q "$expected" "$actual" >/dev/null 2>&1; then
|
if ! diff -q "$expected" "$actual" >/dev/null 2>&1; then
|
||||||
echo "DRIFT $plugin_dir: mirrored file types/modes differ from a fresh sync" >&2
|
echo "DRIFT $plugin_dir: mirrored paths/types/modes differ from a fresh sync" >&2
|
||||||
diff "$expected" "$actual" 2>&1 | sed 's/^/ /' >&2 || true
|
diff "$expected" "$actual" 2>&1 | sed 's/^/ /' >&2 || true
|
||||||
FAIL=1
|
FAIL=1
|
||||||
fi
|
fi
|
||||||
@@ -426,7 +462,10 @@ sync_one() {
|
|||||||
# not apm's own Copilot-ecosystem output (which omits it).
|
# not apm's own Copilot-ecosystem output (which omits it).
|
||||||
reinject_mcp_servers "$plugin_dir" "$pack_cwd"
|
reinject_mcp_servers "$plugin_dir" "$pack_cwd"
|
||||||
local -a checked_paths
|
local -a checked_paths
|
||||||
checked_paths=("${MIRROR_DIRS[@]}" "$HOOKS_REL" "$LEGACY_HOOKS_REL")
|
# $HOOKS_DIR_REL, not $HOOKS_REL: the manifest comparison has to see the whole
|
||||||
|
# generated directory (see check_path_modes), and listing it recursively already
|
||||||
|
# covers hooks/hooks.json.
|
||||||
|
checked_paths=("${MIRROR_DIRS[@]}" "$HOOKS_DIR_REL" "$LEGACY_HOOKS_REL")
|
||||||
for d in "${MIRROR_DIRS[@]}"; do
|
for d in "${MIRROR_DIRS[@]}"; do
|
||||||
check_dir "$plugin_dir" "$pack_cwd" "$d"
|
check_dir "$plugin_dir" "$pack_cwd" "$d"
|
||||||
done
|
done
|
||||||
|
|||||||
@@ -51,7 +51,9 @@ fi
|
|||||||
# than a rolling `wait -n` pool.
|
# than a rolling `wait -n` pool.
|
||||||
SCRATCH_ROOT="$(mktemp -d)"
|
SCRATCH_ROOT="$(mktemp -d)"
|
||||||
trap 'rm -rf "$SCRATCH_ROOT"' EXIT
|
trap 'rm -rf "$SCRATCH_ROOT"' EXIT
|
||||||
# shellcheck source=../scripts/lib/batch-run.sh
|
# Repo-root-relative -- see tests/run-tests.sh for why `../scripts/...` does not
|
||||||
|
# resolve here despite looking right.
|
||||||
|
# shellcheck source=scripts/lib/batch-run.sh
|
||||||
source "$REPO_ROOT/scripts/lib/batch-run.sh"
|
source "$REPO_ROOT/scripts/lib/batch-run.sh"
|
||||||
|
|
||||||
declare -a batch_args=()
|
declare -a batch_args=()
|
||||||
|
|||||||
@@ -58,7 +58,19 @@ done < <(
|
|||||||
# rolling `wait -n` pool.
|
# rolling `wait -n` pool.
|
||||||
SCRATCH_ROOT="$(mktemp -d)"
|
SCRATCH_ROOT="$(mktemp -d)"
|
||||||
trap 'rm -rf "$SCRATCH_ROOT"' EXIT
|
trap 'rm -rf "$SCRATCH_ROOT"' EXIT
|
||||||
# shellcheck source=lib/batch-run.sh
|
# The source= path below is repo-root-relative, matching scripts/install.sh:5 --
|
||||||
|
# NOT script-dir-relative. The source-path used to resolve it is the cwd
|
||||||
|
# pre-commit invokes the linter from, which is the repo root, so `lib/...`
|
||||||
|
# (resolving to a nonexistent tests/lib/) and `../scripts/...` (escaping the repo
|
||||||
|
# entirely) both fail. Both spellings were live until issue #97, and neither was
|
||||||
|
# visible: the resulting SC1091 is `info` while .pre-commit-config.yaml pins
|
||||||
|
# `--severity=warning`. A directive that does not resolve also blinds
|
||||||
|
# test-vale-wrap.sh's `sourced_files()` exemption, which reads these same
|
||||||
|
# directives to find array seeding that lives in the sourced file.
|
||||||
|
#
|
||||||
|
# Do not start a comment line here with the linter's name -- it is parsed as a
|
||||||
|
# directive and errors out (SC1073).
|
||||||
|
# shellcheck source=scripts/lib/batch-run.sh
|
||||||
source "$REPO_ROOT/scripts/lib/batch-run.sh"
|
source "$REPO_ROOT/scripts/lib/batch-run.sh"
|
||||||
|
|
||||||
declare -a batch_args=()
|
declare -a batch_args=()
|
||||||
|
|||||||
@@ -58,6 +58,45 @@ with open(path, 'w', encoding='utf-8') as fh:
|
|||||||
PYTHON
|
PYTHON
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# Vale masking, hoisted so the text-only cases below can use it. Case 12 keeps
|
||||||
|
# its own independent construction and its own loud failure if masking breaks —
|
||||||
|
# it is what proves this mechanism works, so it is not refactored onto this.
|
||||||
|
#
|
||||||
|
# Why: a script run with vale on PATH performs six `vale` invocations (one per
|
||||||
|
# probe path), and they are the suite's entire wall clock. The cases that assert
|
||||||
|
# a text-level finding — StylesPath, BasedOnStyles, per-rule overrides — reach
|
||||||
|
# their verdict through `grep` alone and gain nothing from paying for the
|
||||||
|
# probes. Masking vale is not merely cheaper for them, it is STRICTER: with vale
|
||||||
|
# present a dropped StylesPath also breaks the probe, so such a case would still
|
||||||
|
# exit 1 with the assertion under test deleted. Without vale, only the assertion
|
||||||
|
# under test can produce the failure.
|
||||||
|
#
|
||||||
|
# run_check falls back to an unmasked run rather than skipping when masking is
|
||||||
|
# not safely available, so a machine where this cannot work loses speed, never
|
||||||
|
# coverage. The utility probe matters as much as the vale probe: PATH_NO_VALE
|
||||||
|
# deletes a whole PATH entry, and if that entry also carried grep/diff/awk/cat
|
||||||
|
# the script would fail for an unrelated reason and every negative case below
|
||||||
|
# would pass vacuously.
|
||||||
|
VALE_DIR="$(dirname "$(command -v vale 2>/dev/null || echo /nonexistent/vale)")"
|
||||||
|
PATH_NO_VALE="$(printf '%s' "$PATH" | tr ':' '\n' | grep -vxF "$VALE_DIR" | paste -sd: -)"
|
||||||
|
VALE_MASKED=false
|
||||||
|
if ! PATH="$PATH_NO_VALE" bash -c 'command -v vale' >/dev/null 2>&1 \
|
||||||
|
&& PATH="$PATH_NO_VALE" bash -c \
|
||||||
|
'command -v grep && command -v diff && command -v awk && command -v cat' >/dev/null 2>&1; then
|
||||||
|
VALE_MASKED=true
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Runs the check with vale masked off PATH when that is safe. For text-only
|
||||||
|
# assertions ONLY — never for a case whose verdict depends on a glob probe
|
||||||
|
# actually running.
|
||||||
|
run_check_no_vale() {
|
||||||
|
if [[ "$VALE_MASKED" == true ]]; then
|
||||||
|
PATH="$PATH_NO_VALE" bash "$SCRIPT" "$@"
|
||||||
|
else
|
||||||
|
bash "$SCRIPT" "$@"
|
||||||
|
fi
|
||||||
|
}
|
||||||
|
|
||||||
# --- 1. Exits 0 when the two copies are in sync ---
|
# --- 1. Exits 0 when the two copies are in sync ---
|
||||||
echo ""
|
echo ""
|
||||||
echo "--- exits 0 when skill-audit and agent-audit copies are in sync ---"
|
echo "--- exits 0 when skill-audit and agent-audit copies are in sync ---"
|
||||||
@@ -171,9 +210,48 @@ else
|
|||||||
pass "exits non-zero when a .vale.ini is missing"
|
pass "exits non-zero when a .vale.ini is missing"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# --- 7b. Exits 1, saying so, when a .vale.ini is present but cannot be read ---
|
||||||
|
# Every assertion in that loop is a grep, and grep exits 2 on a read error: the
|
||||||
|
# two `grep -q` checks then misreport a file whose StylesPath and BasedOnStyles
|
||||||
|
# may be perfectly fine, and the override capture swallows the error into an
|
||||||
|
# empty result that reads as "no findings". So the exit code alone proves
|
||||||
|
# nothing here — the check already exits 1 either way, just with the wrong
|
||||||
|
# reason — and this case asserts the MESSAGE. Deleting the readability guard
|
||||||
|
# leaves the exit code at 1 and the diagnosis wrong, which is exactly the
|
||||||
|
# mutation the assertion below kills.
|
||||||
|
#
|
||||||
|
# The unreadable path is a DIRECTORY, not a mode-000 file, and that is the whole
|
||||||
|
# point of the case: `cat` on a directory fails for every uid, while a mode-000
|
||||||
|
# file is readable by root, which is what this repo's dev environment and its
|
||||||
|
# pre-push hooks run as. A permission-based fixture would pass or fail depending
|
||||||
|
# on the invoking uid; this one does not.
|
||||||
|
echo ""
|
||||||
|
echo "--- exits 1 and says so when a .vale.ini exists but cannot be read ---"
|
||||||
|
FIXTURE8B="$(make_fixture)"
|
||||||
|
FIXTURES+=("$FIXTURE8B")
|
||||||
|
UNREADABLE_INI="$FIXTURE8B/plugins/kyberforge/.apm/skills/skill-audit/assets/vale/.vale.ini"
|
||||||
|
rm -f "$UNREADABLE_INI"
|
||||||
|
mkdir -p "$UNREADABLE_INI"
|
||||||
|
UNREADABLE_OUT=""
|
||||||
|
UNREADABLE_RC=0
|
||||||
|
UNREADABLE_OUT="$(bash "$SCRIPT" "$FIXTURE8B" 2>&1)" || UNREADABLE_RC=$?
|
||||||
|
if [[ -e "$UNREADABLE_INI" ]] && cat "$UNREADABLE_INI" >/dev/null 2>&1; then
|
||||||
|
fail "the fixture's .vale.ini is still readable, so this case proves nothing about the unreadable branch"
|
||||||
|
elif [[ $UNREADABLE_RC -eq 0 ]]; then
|
||||||
|
fail "exited 0 when skill-audit's .vale.ini could not be read — expected exit 1"
|
||||||
|
elif ! printf '%s\n' "$UNREADABLE_OUT" | grep -q "could not be read"; then
|
||||||
|
fail "failed for the wrong reason on an unreadable .vale.ini — the readability guard did not fire, so the greps misdiagnosed it: $(printf '%s' "$UNREADABLE_OUT" | tr '\n' ' ')"
|
||||||
|
else
|
||||||
|
pass "exits non-zero and reports an unreadable .vale.ini as unreadable, not as missing or malformed"
|
||||||
|
fi
|
||||||
|
|
||||||
# --- 8. Exits 1 when the shared StylesPath line is dropped from either copy ---
|
# --- 8. Exits 1 when the shared StylesPath line is dropped from either copy ---
|
||||||
# StylesPath resolves relative to the .vale.ini, which is the only reason the
|
# StylesPath resolves relative to the .vale.ini, which is the only reason the
|
||||||
# bundled styles are found from a consuming repo's clone prefix.
|
# bundled styles are found from a consuming repo's clone prefix.
|
||||||
|
# Run with vale masked: a dropped StylesPath also stops vale finding the styles,
|
||||||
|
# so with vale on PATH the glob probe fails too and this case would still exit 1
|
||||||
|
# with the StylesPath assertion itself deleted. Masking makes the text assertion
|
||||||
|
# the only thing that can produce the verdict.
|
||||||
echo ""
|
echo ""
|
||||||
echo "--- exits 1 when StylesPath is missing from either .vale.ini ---"
|
echo "--- exits 1 when StylesPath is missing from either .vale.ini ---"
|
||||||
FIXTURE9="$(make_fixture)"
|
FIXTURE9="$(make_fixture)"
|
||||||
@@ -183,12 +261,12 @@ break_glob "$FIXTURE9/plugins/kyberforge/.apm/skills/skill-audit/assets/vale/.va
|
|||||||
'StylesPath = styles' 'StylesPath = elsewhere'
|
'StylesPath = styles' 'StylesPath = elsewhere'
|
||||||
break_glob "$FIXTURE10/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini" \
|
break_glob "$FIXTURE10/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini" \
|
||||||
'StylesPath = styles' 'StylesPath = elsewhere'
|
'StylesPath = styles' 'StylesPath = elsewhere'
|
||||||
if bash "$SCRIPT" "$FIXTURE9" > /dev/null 2>&1; then
|
if run_check_no_vale "$FIXTURE9" > /dev/null 2>&1; then
|
||||||
fail "exited 0 when skill-audit's .vale.ini lost StylesPath — expected exit 1"
|
fail "exited 0 when skill-audit's .vale.ini lost StylesPath — expected exit 1"
|
||||||
else
|
else
|
||||||
pass "exits non-zero when skill-audit's .vale.ini lost StylesPath"
|
pass "exits non-zero when skill-audit's .vale.ini lost StylesPath"
|
||||||
fi
|
fi
|
||||||
if bash "$SCRIPT" "$FIXTURE10" > /dev/null 2>&1; then
|
if run_check_no_vale "$FIXTURE10" > /dev/null 2>&1; then
|
||||||
fail "exited 0 when agent-audit's .vale.ini lost StylesPath — expected exit 1"
|
fail "exited 0 when agent-audit's .vale.ini lost StylesPath — expected exit 1"
|
||||||
else
|
else
|
||||||
pass "exits non-zero when agent-audit's .vale.ini lost StylesPath"
|
pass "exits non-zero when agent-audit's .vale.ini lost StylesPath"
|
||||||
@@ -203,7 +281,7 @@ FIXTURE11="$(make_fixture)"
|
|||||||
FIXTURES+=("$FIXTURE11")
|
FIXTURES+=("$FIXTURE11")
|
||||||
break_glob "$FIXTURE11/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini" \
|
break_glob "$FIXTURE11/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini" \
|
||||||
'BasedOnStyles = Kyberforge' 'BasedOnStyles = KyberforgeCopilot'
|
'BasedOnStyles = Kyberforge' 'BasedOnStyles = KyberforgeCopilot'
|
||||||
if bash "$SCRIPT" "$FIXTURE11" > /dev/null 2>&1; then
|
if run_check_no_vale "$FIXTURE11" > /dev/null 2>&1; then
|
||||||
fail "exited 0 when agent-audit's .vale.ini stopped naming Kyberforge — expected exit 1"
|
fail "exited 0 when agent-audit's .vale.ini stopped naming Kyberforge — expected exit 1"
|
||||||
else
|
else
|
||||||
pass "exits non-zero when a .vale.ini no longer names the Kyberforge style"
|
pass "exits non-zero when a .vale.ini no longer names the Kyberforge style"
|
||||||
@@ -246,7 +324,7 @@ while IFS= read -r override; do
|
|||||||
FIXTURE_OV="$(make_fixture)"
|
FIXTURE_OV="$(make_fixture)"
|
||||||
FIXTURES+=("$FIXTURE_OV")
|
FIXTURES+=("$FIXTURE_OV")
|
||||||
echo "$override" >> "$FIXTURE_OV/plugins/kyberforge/.apm/skills/skill-audit/assets/vale/.vale.ini"
|
echo "$override" >> "$FIXTURE_OV/plugins/kyberforge/.apm/skills/skill-audit/assets/vale/.vale.ini"
|
||||||
if bash "$SCRIPT" "$FIXTURE_OV" > /dev/null 2>&1; then
|
if run_check_no_vale "$FIXTURE_OV" > /dev/null 2>&1; then
|
||||||
fail "exited 0 with '$override' in skill-audit's .vale.ini -- expected exit 1"
|
fail "exited 0 with '$override' in skill-audit's .vale.ini -- expected exit 1"
|
||||||
else
|
else
|
||||||
pass "exits non-zero on '$override'"
|
pass "exits non-zero on '$override'"
|
||||||
@@ -277,7 +355,7 @@ FIXTURE_OV_AGENT="$(make_fixture)"
|
|||||||
FIXTURES+=("$FIXTURE_OV_AGENT")
|
FIXTURES+=("$FIXTURE_OV_AGENT")
|
||||||
echo "KyberforgeCopilot.ProactivePhrase = false" \
|
echo "KyberforgeCopilot.ProactivePhrase = false" \
|
||||||
>> "$FIXTURE_OV_AGENT/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini"
|
>> "$FIXTURE_OV_AGENT/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini"
|
||||||
if bash "$SCRIPT" "$FIXTURE_OV_AGENT" > /dev/null 2>&1; then
|
if run_check_no_vale "$FIXTURE_OV_AGENT" > /dev/null 2>&1; then
|
||||||
fail "exited 0 with 'KyberforgeCopilot.ProactivePhrase = false' in agent-audit's .vale.ini -- expected exit 1"
|
fail "exited 0 with 'KyberforgeCopilot.ProactivePhrase = false' in agent-audit's .vale.ini -- expected exit 1"
|
||||||
else
|
else
|
||||||
pass "exits non-zero when agent-audit's copy retires a KyberforgeCopilot rule"
|
pass "exits non-zero when agent-audit's copy retires a KyberforgeCopilot rule"
|
||||||
@@ -316,7 +394,7 @@ FIXTURE11C="$(make_fixture)"
|
|||||||
FIXTURES+=("$FIXTURE11C")
|
FIXTURES+=("$FIXTURE11C")
|
||||||
break_glob "$FIXTURE11C/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini" \
|
break_glob "$FIXTURE11C/plugins/kyberforge/.apm/skills/agent-audit/assets/vale/.vale.ini" \
|
||||||
'BasedOnStyles = Kyberforge, KyberforgeCopilot' 'BasedOnStyles = Kyberforge'
|
'BasedOnStyles = Kyberforge, KyberforgeCopilot' 'BasedOnStyles = Kyberforge'
|
||||||
if bash "$SCRIPT" "$FIXTURE11C" > /dev/null 2>&1; then
|
if run_check_no_vale "$FIXTURE11C" > /dev/null 2>&1; then
|
||||||
fail "exited 0 when KyberforgeCopilot was dropped from BasedOnStyles -- expected exit 1"
|
fail "exited 0 when KyberforgeCopilot was dropped from BasedOnStyles -- expected exit 1"
|
||||||
else
|
else
|
||||||
pass "exits non-zero when a shipped KyberforgeCopilot style is never loaded"
|
pass "exits non-zero when a shipped KyberforgeCopilot style is never loaded"
|
||||||
|
|||||||
@@ -17,7 +17,9 @@ fi
|
|||||||
# Minimal fixture exercising every mirrored category -- all five of
|
# Minimal fixture exercising every mirrored category -- all five of
|
||||||
# scripts/sync-plugin-content.sh's MIRROR_DIRS (agents, skills, commands,
|
# scripts/sync-plugin-content.sh's MIRROR_DIRS (agents, skills, commands,
|
||||||
# instructions, extensions) plus the merged hooks file -- without needing network
|
# instructions, extensions) plus the merged hooks file -- without needing network
|
||||||
# access (no apm.yml dependencies). Two deliberately-shaped skill subdirectories:
|
# access (no apm.yml dependencies). It also carries a .apm/prompts/ entry, which has
|
||||||
|
# no MIRROR_DIRS destination of its own: apm folds prompts into commands/. Two
|
||||||
|
# deliberately-shaped skill subdirectories:
|
||||||
#
|
#
|
||||||
# skills/hello/tests/ -- a dev-time fixture that must NOT be mirrored
|
# skills/hello/tests/ -- a dev-time fixture that must NOT be mirrored
|
||||||
# skills/hello/assets/templates/tests/ -- a template asset that MUST be mirrored
|
# skills/hello/assets/templates/tests/ -- a template asset that MUST be mirrored
|
||||||
@@ -34,7 +36,7 @@ make_fixture() {
|
|||||||
mkdir -p "$dir/.apm/skills/hello/tests" "$dir/.apm/skills/hello/scripts" \
|
mkdir -p "$dir/.apm/skills/hello/tests" "$dir/.apm/skills/hello/scripts" \
|
||||||
"$dir/.apm/skills/hello/assets/templates/tests" "$dir/.apm/agents" \
|
"$dir/.apm/skills/hello/assets/templates/tests" "$dir/.apm/agents" \
|
||||||
"$dir/.apm/hooks" "$dir/.apm/commands" "$dir/.apm/instructions" \
|
"$dir/.apm/hooks" "$dir/.apm/commands" "$dir/.apm/instructions" \
|
||||||
"$dir/.apm/extensions"
|
"$dir/.apm/extensions" "$dir/.apm/prompts"
|
||||||
cat > "$dir/apm.yml" <<'YAML'
|
cat > "$dir/apm.yml" <<'YAML'
|
||||||
name: fixture
|
name: fixture
|
||||||
version: 0.0.1
|
version: 0.0.1
|
||||||
@@ -81,6 +83,12 @@ EOF
|
|||||||
description: mycmd
|
description: mycmd
|
||||||
---
|
---
|
||||||
Do a thing.
|
Do a thing.
|
||||||
|
EOF
|
||||||
|
cat > "$dir/.apm/prompts/greet.prompt.md" <<'EOF'
|
||||||
|
---
|
||||||
|
description: greet
|
||||||
|
---
|
||||||
|
Greet the user.
|
||||||
EOF
|
EOF
|
||||||
cat > "$dir/.apm/instructions/style.instructions.md" <<'EOF'
|
cat > "$dir/.apm/instructions/style.instructions.md" <<'EOF'
|
||||||
---
|
---
|
||||||
@@ -539,6 +547,98 @@ else
|
|||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# --- 20. .apm/prompts/ is mirrored, via commands/ rather than a prompts/ of its own ---
|
||||||
|
# The script header lists prompts among the .apm/ directories it mirrors while
|
||||||
|
# MIRROR_DIRS has no `prompts` entry, which reads as a hole and has already been filed
|
||||||
|
# as one. It is not: MIRROR_DIRS names DESTINATION directories, and apm's exporter
|
||||||
|
# folds .apm/prompts/ into commands/ (renaming *.prompt.md to *.md) alongside
|
||||||
|
# .apm/commands/. This pins that mapping, so an apm upgrade that gave prompts a
|
||||||
|
# destination of its own — the one change that would genuinely need a MIRROR_DIRS
|
||||||
|
# entry — fails here instead of silently dropping the content.
|
||||||
|
echo ""
|
||||||
|
echo "--- .apm/prompts/ content arrives in commands/, not in a prompts/ directory ---"
|
||||||
|
FIXTURE20="$(make_fixture)"; track "$FIXTURE20"
|
||||||
|
bash "$SCRIPT" "$FIXTURE20" > /dev/null 2>&1
|
||||||
|
if [[ -f "$FIXTURE20/commands/greet.md" ]]; then
|
||||||
|
pass ".apm/prompts/greet.prompt.md is mirrored to commands/greet.md"
|
||||||
|
else
|
||||||
|
fail ".apm/prompts/ content never reached commands/ — apm's prompts mapping changed"
|
||||||
|
fi
|
||||||
|
if [[ ! -e "$FIXTURE20/prompts" ]]; then
|
||||||
|
pass "no prompts/ directory is produced at the plugin root"
|
||||||
|
else
|
||||||
|
fail "a prompts/ directory appeared at the plugin root — it now needs a MIRROR_DIRS entry"
|
||||||
|
fi
|
||||||
|
if bash "$SCRIPT" --check "$FIXTURE20" > /dev/null 2>&1; then
|
||||||
|
pass "--check is clean with .apm/prompts/ content present"
|
||||||
|
else
|
||||||
|
fail "--check reports drift on a freshly synced fixture carrying .apm/prompts/"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# --- 21. A stray file inside the generated hooks/ directory is drift ---
|
||||||
|
# hooks/ is mirror-owned output, so it is scoped like a MIRROR_DIRS destination and
|
||||||
|
# not like the plugin root (where README.md, docs/, bin/ and .mcp.json are all
|
||||||
|
# hand-authored and deliberately none of this script's business). Before hooks/ came
|
||||||
|
# under that ownership, --check exited 0 on a stray inside it and a real sync left the
|
||||||
|
# stray untouched — agreeing with each other, but agreeing on the wrong answer.
|
||||||
|
echo ""
|
||||||
|
echo "--- a stray file inside the generated hooks/ directory is reported and removed ---"
|
||||||
|
FIXTURE21="$(make_fixture)"; track "$FIXTURE21"
|
||||||
|
bash "$SCRIPT" "$FIXTURE21" > /dev/null 2>&1
|
||||||
|
if [[ ! -f "$FIXTURE21/hooks/hooks.json" ]]; then
|
||||||
|
fail "initial sync did not create hooks/hooks.json -- can't test the stray case"
|
||||||
|
else
|
||||||
|
printf '{"stray": true}\n' > "$FIXTURE21/hooks/extra.json"
|
||||||
|
if bash "$SCRIPT" --check "$FIXTURE21" > /dev/null 2>&1; then
|
||||||
|
fail "no drift reported for a stray file inside the generated hooks/ directory"
|
||||||
|
else
|
||||||
|
pass "a stray file inside hooks/ is reported as drift"
|
||||||
|
bash "$SCRIPT" "$FIXTURE21" > /dev/null 2>&1
|
||||||
|
if [[ ! -e "$FIXTURE21/hooks/extra.json" ]] && [[ -f "$FIXTURE21/hooks/hooks.json" ]]; then
|
||||||
|
pass "re-sync removes the stray and keeps hooks/hooks.json"
|
||||||
|
else
|
||||||
|
fail "re-sync did not clean the stray out of hooks/"
|
||||||
|
fi
|
||||||
|
if bash "$SCRIPT" --check "$FIXTURE21" > /dev/null 2>&1; then
|
||||||
|
pass "re-sync clears the stray-hooks-file drift"
|
||||||
|
else
|
||||||
|
fail "re-sync did not clear the stray-hooks-file drift"
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
|
||||||
|
# --- 22. An empty leftover hooks/ directory is drift, not an acceptable resting state ---
|
||||||
|
# When .apm/hooks/ produces no hooks.json, the correct mirror state is no hooks/
|
||||||
|
# directory at all — not an empty one. Empty leftovers are exactly what earlier
|
||||||
|
# revisions of this script left behind in this repo's own plugin roots.
|
||||||
|
echo ""
|
||||||
|
echo "--- an empty leftover hooks/ directory is reported and removed ---"
|
||||||
|
FIXTURE22="$(make_fixture)"; track "$FIXTURE22"
|
||||||
|
rm -rf "$FIXTURE22/.apm/hooks"
|
||||||
|
bash "$SCRIPT" "$FIXTURE22" > /dev/null 2>&1
|
||||||
|
if [[ -e "$FIXTURE22/hooks" ]]; then
|
||||||
|
fail "sync created hooks/ for a plugin whose .apm/hooks/ produces no hooks.json"
|
||||||
|
else
|
||||||
|
pass "no hooks/ directory when .apm/hooks/ produces nothing"
|
||||||
|
fi
|
||||||
|
mkdir -p "$FIXTURE22/hooks"
|
||||||
|
if bash "$SCRIPT" --check "$FIXTURE22" > /dev/null 2>&1; then
|
||||||
|
fail "no drift reported for an empty leftover hooks/ directory"
|
||||||
|
else
|
||||||
|
pass "an empty leftover hooks/ directory is reported as drift"
|
||||||
|
bash "$SCRIPT" "$FIXTURE22" > /dev/null 2>&1
|
||||||
|
if [[ ! -e "$FIXTURE22/hooks" ]]; then
|
||||||
|
pass "re-sync removes the empty leftover hooks/ directory"
|
||||||
|
else
|
||||||
|
fail "re-sync left the empty hooks/ directory in place"
|
||||||
|
fi
|
||||||
|
if bash "$SCRIPT" --check "$FIXTURE22" > /dev/null 2>&1; then
|
||||||
|
pass "re-sync clears the empty-hooks-directory drift"
|
||||||
|
else
|
||||||
|
fail "re-sync did not clear the empty-hooks-directory drift"
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
|
||||||
echo ""
|
echo ""
|
||||||
echo "Results: $PASS passed, $FAIL failed"
|
echo "Results: $PASS passed, $FAIL failed"
|
||||||
[[ $FAIL -eq 0 ]]
|
[[ $FAIL -eq 0 ]]
|
||||||
|
|||||||
@@ -452,21 +452,28 @@ fi
|
|||||||
# this branch introduced, whose own header (batch-run.sh:9-11) documents it as
|
# this branch introduced, whose own header (batch-run.sh:9-11) documents it as
|
||||||
# bash-3.2-safe — along with four other scripts/*.sh. Deriving the list means a
|
# bash-3.2-safe — along with four other scripts/*.sh. Deriving the list means a
|
||||||
# new script is covered the moment it lands. AGENTS.md names bash 3.2 as an
|
# new script is covered the moment it lands. AGENTS.md names bash 3.2 as an
|
||||||
# explicit repo target, so the scope is three globs, each floor-asserted below:
|
# explicit repo target, so the scope is four globs, each floor-asserted below:
|
||||||
# - scripts/**/*.sh — repo tooling and pre-commit hook scripts
|
# - scripts/**/*.sh — repo tooling and pre-commit hook scripts
|
||||||
# - tests/*.sh — the runners and every regression test
|
# - tests/*.sh — the runners and every regression test
|
||||||
# - plugins/*/.apm/**/*.sh — the scripts plugins ship to users
|
# - plugins/*/.apm/**/*.sh — the scripts plugins ship to users
|
||||||
|
# - providers/**/*.sh — the scripts install.sh deploys to user machines
|
||||||
# plugins/*/skills/** is deliberately NOT scanned: it is the generated mirror of
|
# plugins/*/skills/** is deliberately NOT scanned: it is the generated mirror of
|
||||||
# .apm/, so scanning both double-reports every finding, and mirror-vs-source
|
# .apm/, so scanning both double-reports every finding, and mirror-vs-source
|
||||||
# drift is already sync-plugin-content.sh --check's job. Scanning .apm/ is what
|
# drift is already sync-plugin-content.sh --check's job. Scanning .apm/ is what
|
||||||
# closed the gap where skill-audit's vale-wrap.sh was covered but agent-audit's
|
# closed the gap where skill-audit's vale-wrap.sh was covered but agent-audit's
|
||||||
# byte-identical copy of it was not.
|
# byte-identical copy of it was not.
|
||||||
# providers/**/*.sh is the one shipped script deliberately left out:
|
# providers/**/*.sh was excluded until issue #96: statusline-command.sh seeded
|
||||||
# providers/claude-code/statusline-command.sh seeds `parts=()` empty at :83 and
|
# `parts=()` empty and expanded it unguarded, a real latent hazard rather than a
|
||||||
# expands it unguarded at :96. That is a real latent hazard rather than a false
|
# false positive, in a file this case did not own. That expansion is now guarded,
|
||||||
# positive — it just cannot abort today because the file enables no `set -u`.
|
# so the glob is in the table below and the deployed provider scripts get the
|
||||||
# Fixing it is a change to a file this case does not own; once :96 uses the
|
# same coverage as scripts/, tests/ and plugins/*/.apm/. This is the glob most
|
||||||
# guarded form, add a `providers` glob to the table below.
|
# worth having: install.sh copies providers/ content onto every user machine, and
|
||||||
|
# it is also the glob most likely to look redundant to a future reader, because
|
||||||
|
# the hazard it catches is invisible on any modern bash — bash 4.4 stopped
|
||||||
|
# treating an empty-array expansion as unbound, so a bare "${a[@]}" under `set -u`
|
||||||
|
# runs clean on the maintainer's bash 5 and aborts only on macOS's bash 3.2.
|
||||||
|
# A runtime test cannot demonstrate that without a 3.2 binary; this static scan
|
||||||
|
# is the enforcement, which is why its floor must never be dropped to zero.
|
||||||
#
|
#
|
||||||
# Four constructs are checked, because the expansion scan cannot see any of the
|
# Four constructs are checked, because the expansion scan cannot see any of the
|
||||||
# other three:
|
# other three:
|
||||||
@@ -588,19 +595,28 @@ bash32_glob() {
|
|||||||
scripts) find "$REPO_ROOT/scripts" -name '*.sh' -type f ;;
|
scripts) find "$REPO_ROOT/scripts" -name '*.sh' -type f ;;
|
||||||
tests) find "$REPO_ROOT/tests" -maxdepth 1 -name '*.sh' -type f ;;
|
tests) find "$REPO_ROOT/tests" -maxdepth 1 -name '*.sh' -type f ;;
|
||||||
plugins) find "$REPO_ROOT/plugins" -path '*/.apm/*' -name '*.sh' -type f ;;
|
plugins) find "$REPO_ROOT/plugins" -path '*/.apm/*' -name '*.sh' -type f ;;
|
||||||
|
providers) find "$REPO_ROOT/providers" -name '*.sh' -type f ;;
|
||||||
*) echo "bash32_glob: unknown glob '$1'" >&2; return 1 ;;
|
*) echo "bash32_glob: unknown glob '$1'" >&2; return 1 ;;
|
||||||
esac
|
esac
|
||||||
}
|
}
|
||||||
# Floors are PER GLOB, not on the merged total. A single total floor cannot
|
# Floors are PER GLOB, not on the merged total. A single total floor cannot
|
||||||
# detect the failure this assertion exists to name: with 12 + 17 + 13 files,
|
# detect the failure this assertion exists to name: with 12 + 18 + 13 + 1 files,
|
||||||
# losing the whole `scripts` glob still leaves 30 and losing the whole `tests`
|
# losing the whole `scripts` glob still leaves 32 and losing the whole `tests`
|
||||||
# glob still leaves 25, so any total floor low enough to survive normal churn
|
# glob still leaves 26, so any total floor low enough to survive normal churn
|
||||||
# is too low to notice an entire glob silently resolving to nothing. Each floor
|
# is too low to notice an entire glob silently resolving to nothing. Each floor
|
||||||
# sits a little under its current count so ordinary file removal does not trip
|
# sits a little under its current count so ordinary file removal does not trip
|
||||||
# it, but a broken or renamed path does. Parallel arrays rather than an
|
# it, but a broken or renamed path does. Parallel arrays rather than an
|
||||||
# associative one — `declare -A` is bash 4.0+, which this very case forbids.
|
# associative one — `declare -A` is bash 4.0+, which this very case forbids.
|
||||||
BASH32_GLOB_NAMES=(scripts tests plugins)
|
#
|
||||||
BASH32_GLOB_FLOORS=(10 14 10)
|
# `providers` is floored at 1 rather than "current count minus slack" because it
|
||||||
|
# holds exactly one file: any slack at all would be a floor of 0, which passes on
|
||||||
|
# a renamed or deleted directory and is precisely the silent-zero failure the
|
||||||
|
# per-glob floors exist to catch. A floor of 1 means removing the last provider
|
||||||
|
# script trips this assertion — correct, because at that point the glob is dead
|
||||||
|
# weight and should be deleted from the table deliberately, not left to pass
|
||||||
|
# vacuously.
|
||||||
|
BASH32_GLOB_NAMES=(scripts tests plugins providers)
|
||||||
|
BASH32_GLOB_FLOORS=(10 14 10 1)
|
||||||
BASH32_SCRIPTS=()
|
BASH32_SCRIPTS=()
|
||||||
BASH32_IDX=0
|
BASH32_IDX=0
|
||||||
while [[ $BASH32_IDX -lt ${#BASH32_GLOB_NAMES[@]} ]]; do
|
while [[ $BASH32_IDX -lt ${#BASH32_GLOB_NAMES[@]} ]]; do
|
||||||
@@ -1091,6 +1107,92 @@ else
|
|||||||
pass "all 6 never-empty shell arrays are exempt and all 5 sometimes-empty ones are still flagged"
|
pass "all 6 never-empty shell arrays are exempt and all 5 sometimes-empty ones are still flagged"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# Case 27 pins the `sourced_files()` exemption against the failure mode that
|
||||||
|
# actually happened (issue #97 item 2): a stale source-directive path.
|
||||||
|
#
|
||||||
|
# The exemption exists because array seeding often lives in the sourced file
|
||||||
|
# rather than the sourcing one -- install.sh's DEPLOY_* come from
|
||||||
|
# deploy-manifest.sh -- so without it those expansions read as hazards. It works
|
||||||
|
# by parsing each file's own source-directive comments. That makes it silently
|
||||||
|
# dependent on those paths resolving: a directive naming a file that is not
|
||||||
|
# there does not error, it just contributes no seed file, and the exemption
|
||||||
|
# quietly stops covering what it was written to cover.
|
||||||
|
#
|
||||||
|
# tests/run-tests.sh shipped exactly that for the length of PR #95 -- a directive
|
||||||
|
# resolving to neither the repo root nor the script's own directory. Nothing
|
||||||
|
# caught it: shellcheck's own SC1091 is `info`, and .pre-commit-config.yaml pins
|
||||||
|
# `--severity=warning`. The live consequence was nil only because both of
|
||||||
|
# run-tests.sh's expansions were independently guarded.
|
||||||
|
#
|
||||||
|
# Part C is the assertion that would have caught it, and it is the reason this
|
||||||
|
# case is not just fixture theatre: it holds every real directive in the scanned
|
||||||
|
# corpus to the resolution rule, so the next stale one fails here rather than
|
||||||
|
# lying dormant. Parts A and B pin the mechanism Part C depends on -- that
|
||||||
|
# resolution is what switches the exemption on, and non-resolution silently
|
||||||
|
# switches it off.
|
||||||
|
echo ""
|
||||||
|
echo "--- every shellcheck source directive resolves, so no seeding exemption is silently off ---"
|
||||||
|
FIXTURE27="$(mktemp -d)"
|
||||||
|
new_fixture "$FIXTURE27"
|
||||||
|
# The seeded array lives ONLY in the sourced file, never in the sourcing one --
|
||||||
|
# that separation is the whole point of the exemption.
|
||||||
|
{
|
||||||
|
echo "#!/usr/bin/env bash"
|
||||||
|
echo "SEEDED27=(a b c)"
|
||||||
|
} > "$FIXTURE27/seed-lib.sh"
|
||||||
|
|
||||||
|
# Both fixture bodies are emitted through printf with the closing `]` passed as
|
||||||
|
# an argument, never written literally. This file is itself inside the `tests`
|
||||||
|
# glob, so a literal bare expansion here would be scanned as a hazard in
|
||||||
|
# test-vale-wrap.sh's own source -- the trailing-comment/inside-a-string caveat
|
||||||
|
# documented above strip_comments(). Splitting the token means the emitted
|
||||||
|
# fixture holds the real text while this file holds no bare spelling, the same
|
||||||
|
# trick the `npro[c]` rule uses.
|
||||||
|
EXPANSION27="$(printf 'echo "${SEEDED27[@%s}"' ']')"
|
||||||
|
|
||||||
|
# Part A: a directive that resolves against the script's own directory exempts
|
||||||
|
# the expansion, so no hazard is reported.
|
||||||
|
{
|
||||||
|
echo "#!/usr/bin/env bash"
|
||||||
|
echo "# shellcheck source=seed-lib.sh"
|
||||||
|
echo 'source "$(dirname "$0")/seed-lib.sh"'
|
||||||
|
echo "$EXPANSION27"
|
||||||
|
} > "$FIXTURE27/resolves.sh"
|
||||||
|
|
||||||
|
# Part B: byte-identical except the directive names a file that is not there.
|
||||||
|
{
|
||||||
|
echo "#!/usr/bin/env bash"
|
||||||
|
echo "# shellcheck source=nonexistent/seed-lib.sh"
|
||||||
|
echo 'source "$(dirname "$0")/seed-lib.sh"'
|
||||||
|
echo "$EXPANSION27"
|
||||||
|
} > "$FIXTURE27/stale.sh"
|
||||||
|
|
||||||
|
RESOLVES27="$(unguarded_expansions "$FIXTURE27/resolves.sh")"
|
||||||
|
STALE27="$(unguarded_expansions "$FIXTURE27/stale.sh")"
|
||||||
|
|
||||||
|
# Part C: no real directive in the derived corpus may fail to resolve. Counted
|
||||||
|
# rather than diffed, because sourced_files() reports resolution only by
|
||||||
|
# omission -- an unresolvable directive produces no output line at all.
|
||||||
|
UNRESOLVED27=""
|
||||||
|
for BASH32_SCRIPT in ${BASH32_SCRIPTS[@]+"${BASH32_SCRIPTS[@]}"}; do
|
||||||
|
DECLARED27="$(grep -cE '^[[:space:]]*#[[:space:]]*shellcheck[[:space:]]+source=[^[:space:]]+' "$BASH32_SCRIPT" || true)"
|
||||||
|
[[ "$DECLARED27" -gt 0 ]] || continue
|
||||||
|
RESOLVED27="$(sourced_files "$BASH32_SCRIPT" | grep -c . || true)"
|
||||||
|
if [[ "$RESOLVED27" -lt "$DECLARED27" ]]; then
|
||||||
|
UNRESOLVED27+="${BASH32_SCRIPT#"$REPO_ROOT"/} ($RESOLVED27/$DECLARED27) "
|
||||||
|
fi
|
||||||
|
done
|
||||||
|
|
||||||
|
if [[ -n "$RESOLVES27" ]]; then
|
||||||
|
fail "an array seeded only in a resolvable sourced file was reported as a hazard, so the seeding exemption is not reading source directives at all"
|
||||||
|
elif [[ -z "$STALE27" ]]; then
|
||||||
|
fail "an array seeded only in an UNRESOLVABLE sourced file was still exempted, so this case cannot detect a stale directive and Part C proves nothing"
|
||||||
|
elif [[ -n "$UNRESOLVED27" ]]; then
|
||||||
|
fail "shellcheck source directive(s) resolve to nothing, silently disarming the seeding exemption for those files: $UNRESOLVED27"
|
||||||
|
else
|
||||||
|
pass "the seeding exemption switches on only for resolvable directives, and every directive in the scanned corpus resolves"
|
||||||
|
fi
|
||||||
|
|
||||||
echo ""
|
echo ""
|
||||||
echo "Results: $PASS passed, $FAIL failed"
|
echo "Results: $PASS passed, $FAIL failed"
|
||||||
[[ $FAIL -eq 0 ]]
|
[[ $FAIL -eq 0 ]]
|
||||||
|
|||||||
Reference in New Issue
Block a user