ci: audit every apm package, and validate the agents the validator governs
apm-audit-ci ran against root apm.yml alone, so it audited none of the six plugin packages, and its description claimed a lockfile/policy/hidden-content gate while delivering one vacuous check. It now loops all seven manifests, and the description says only what runs. Proven load-bearing: a malformed dependency in plugins/lint/apm.yml passed the old root-only entry at exit 0 and passed apm pack --check-clean too, because that gate never parses plugin dependencies; the loop catches it and names the file. policy.fetch_failure_default: block was considered and rejected. apm's org-policy discovery understands github.com and Azure DevOps; this repo's remote is self-hosted Gitea, so no policy source is discoverable and the setting makes the hook exit 1 on every push forever. Fail-closed is right when there is a control to fail closed on -- a permanently red gate is one people learn to SKIP=, which is worse than an accurate description. agent-audit's validate.sh had never run against the four real .apm/agents files it governs, only against synthetic fixtures. That is why an amended ADR-0016 and a validator that still rejected the field it blessed could disagree unnoticed until someone ran it by hand. check-apm-agents-valid.sh closes it, deriving the expected set from git ls-files rather than a count, failing on zero discovered files, and replaying validate.sh's own reason under each failing filename. Also makes the pretty-format-json exclude consistently root-anchored: it mixed (^|/) for five paths with ^ for one, so a nested fixture at .../.claude-plugin/plugin.json was silently exempt from formatting. Pre-push goes 12 repo-defined to 13, 14 total to 15; AGENTS.md's counts, hook list and offline-skip note are updated to match. The new hook needs no network. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
This commit is contained in:
@@ -28,7 +28,16 @@ repos:
|
||||
- id: pretty-format-json
|
||||
stages: ['pre-commit']
|
||||
args: [--autofix]
|
||||
exclude: '(^|/)(\.claude-plugin/plugin\.json|\.github/plugin/plugin\.json|\.claude-plugin/marketplace\.json|\.github/plugin/marketplace\.json)$|^\.agents/plugins/marketplace\.json$'
|
||||
# Every generated manifest lives at a KNOWN path, so every alternative is
|
||||
# root-anchored and spells that path out. This was five `(^|/)`
|
||||
# any-depth alternatives plus one `^` root-only one -- a mixture with no
|
||||
# rationale, under which a fixture or vendored tree containing
|
||||
# `.../.claude-plugin/plugin.json` would have been silently excluded from
|
||||
# formatting while an equivalent `.../.agents/plugins/marketplace.json`
|
||||
# would not. All fifteen real files (3 root marketplace manifests, 2 per
|
||||
# plugin x 6 plugins) match; anything else is hand-authored and gets
|
||||
# formatted.
|
||||
exclude: '^(\.claude-plugin/marketplace\.json|\.agents/plugins/marketplace\.json|\.github/plugin/marketplace\.json|plugins/[^/]+/\.claude-plugin/plugin\.json|plugins/[^/]+/\.github/plugin/plugin\.json)$'
|
||||
- id: check-yaml
|
||||
stages: ['pre-commit']
|
||||
- id: trailing-whitespace
|
||||
@@ -91,12 +100,66 @@ repos:
|
||||
|
||||
- id: apm-audit-ci
|
||||
name: apm audit --ci
|
||||
description: apm's own producer-side lockfile/policy/hidden-content integrity gate, per apm's documented recommended CI block (see docs/research/docs/microsoft-apm/testing-and-validation.md)
|
||||
entry: apm audit --ci
|
||||
description: Run apm's producer-side CI gate over the root manifest AND each of the six plugin packages. Verifies exactly two things per manifest -- apm.yml parses as a valid APM manifest (manifest-parse), and, if it declares dependencies, apm.lock.yaml exists and is consistent (lockfile-exists). It does NOT enforce an org policy and does NOT scan for hidden Unicode; see the comment below for why. Reference:plugins/kyberforge/.apm/skills/apm-workflow/references/audit.md
|
||||
entry: bash -c 'for d in . plugins/*/; do (cd "$d" && apm audit --ci) || { echo "apm audit --ci failed in $d" >&2; exit 1; }; done'
|
||||
language: system
|
||||
stages: [pre-push]
|
||||
pass_filenames: false
|
||||
always_run: true
|
||||
# The description above deliberately claims less than this hook's old one
|
||||
# did ("lockfile/policy/hidden-content integrity"), because two of those
|
||||
# three were never happening:
|
||||
#
|
||||
# * POLICY. `apm audit --ci` discovers an org policy from the git remote,
|
||||
# and apm's discovery only understands github.com and Azure DevOps.
|
||||
# This repo's remote is a self-hosted Gitea, so discovery resolves
|
||||
# nothing and the run prints `No org policy found at unknown;
|
||||
# enforcement skipped`. apm's own message suggests
|
||||
# `policy.fetch_failure_default=block` in apm.yml "to fail closed" --
|
||||
# that was tried on a scratch copy and REJECTED: it does not make the
|
||||
# check meaningful, it makes it permanently red. `apm audit --ci` then
|
||||
# exits 1 with `No org policy found at unknown
|
||||
# (policy.fetch_failure_default=block)` on every push, because there is
|
||||
# no org policy to find and no supported way for this remote to serve
|
||||
# one. A gate that can never go green is not a gate. Revisit if this
|
||||
# repo ever gains a policy source apm can actually reach.
|
||||
# * HIDDEN CONTENT. The hidden-Unicode scan is plain `apm audit`, not
|
||||
# `apm audit --ci` (the two are different modes, and --ci refuses to
|
||||
# combine with --file/--strip/--dry-run/PACKAGE). Plain `apm audit`
|
||||
# here reports `No apm.lock.yaml found -- nothing to scan` and exits 0,
|
||||
# so adding it would buy a second vacuous check, not coverage.
|
||||
#
|
||||
# What IS left is worth keeping, and is now run against seven manifests
|
||||
# instead of one. lockfile-exists is conditional -- it is vacuous while
|
||||
# every apm.yml declares `dependencies: {apm: [], mcp: []}`, and it arms
|
||||
# itself the moment one does not (verified: adding a git dependency to
|
||||
# plugins/lint/apm.yml fails with `apm.yml declares dependencies but
|
||||
# apm.lock.yaml is absent`). manifest-parse is unconditional and fires on
|
||||
# any malformed manifest (verified: a dependency entry missing its
|
||||
# git/path/registry field fails with `Cannot parse apm.yml`). Running the
|
||||
# six plugin packages is what makes either reachable for them at all --
|
||||
# the root-only invocation audits the marketplace manifest and nothing
|
||||
# else. Costs ~0.5s per package, needs no network (checked under
|
||||
# `unshare -rn`), so this does NOT join apm-marketplace-check and
|
||||
# apm-pack-check-clean on the offline SKIP= list.
|
||||
|
||||
- id: check-apm-agents-valid
|
||||
name: Validate real APM agent files
|
||||
description: Run agent-audit's validate.sh over every plugins/*/.apm/agents/*.agent.md file in this repo -- the artifacts it governs, not fixtures
|
||||
entry: bash scripts/check-apm-agents-valid.sh
|
||||
language: system
|
||||
stages: [pre-push]
|
||||
pass_filenames: false
|
||||
always_run: true
|
||||
# validate.sh was previously exercised only by check-scope-walkup-sync,
|
||||
# and only against synthetic mktemp fixtures -- it had never run against
|
||||
# the four agent files it governs. That is how ADR-0016 could be amended
|
||||
# to bless a `disallowedTools` frontmatter field while validate.sh's
|
||||
# allowlist still rejected it: the spec and its enforcer disagreed and
|
||||
# every gate stayed green. The expected file set is derived from
|
||||
# `git ls-files` (the pattern tests/run-bats.sh established) rather than
|
||||
# a hardcoded count, and discovering zero files is an error, not a pass.
|
||||
# Needs no network.
|
||||
|
||||
- id: apm-pack-check-clean
|
||||
name: apm pack --check-clean
|
||||
|
||||
10
AGENTS.md
10
AGENTS.md
@@ -35,16 +35,18 @@ Fall back to raw shell only when no skill covers it.
|
||||
|
||||
## Setup and testing
|
||||
|
||||
- Install git hooks via `git:pc-run`, wiring all three stages — this repo's `.pre-commit-config.yaml` has no `default_install_hook_types`, so a plain install silently skips `commit-msg` (Conventional Commits) and `pre-push` (the 12-hook gate described below).
|
||||
- Install the `apm` CLI — four pre-push hooks shell out to it: `apm-marketplace-check`, `apm-audit-ci`, `apm-pack-check-clean`, and `check-plugin-content-sync` (via `scripts/sync-plugin-content.sh`, which wraps `apm pack`). The first three are bare `apm …` hook entries, so without it the push dies with an unhelpful "command not found". Use `kyberforge:apm-install`, or `curl -sSL https://aka.ms/apm-unix | sh`; verify with `apm --version`.
|
||||
- Install git hooks via `git:pc-run`, wiring all three stages — this repo's `.pre-commit-config.yaml` has no `default_install_hook_types`, so a plain install silently skips `commit-msg` (Conventional Commits) and `pre-push` (the 13-hook gate described below).
|
||||
- Install the `apm` CLI — four pre-push hooks shell out to it: `apm-marketplace-check`, `apm-audit-ci`, `apm-pack-check-clean`, and `check-plugin-content-sync` (via `scripts/sync-plugin-content.sh`, which wraps `apm pack`). `apm-marketplace-check` and `apm-pack-check-clean` are bare `apm …` hook entries and `apm-audit-ci` is a `bash -c` loop calling `apm` once per package, so without it the push dies with an unhelpful "command not found". Use `kyberforge:apm-install`, or `curl -sSL https://aka.ms/apm-unix | sh`; verify with `apm --version`.
|
||||
- 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).
|
||||
- `vale` is also a **pre-push** dependency, not only pre-commit. `check-vale-style-sync` runs six glob-coverage probes by invoking `vale --config` — they are the only assertions in it that catch a `.vale.ini` glob typo, the failure mode where every text-level check stays clean while vale lints zero files. Missing `vale` is therefore a hard failure there. The opt-out is `CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1`, and it is **not** `SKIP=`: the hook still runs and still asserts everything verifiable from file text, but the six probes do not, and its summary says so explicitly — `Vale style sync check passed (text-level only, vale unavailable): … 0 glob probe(s) verified`. Use it only on a machine that genuinely cannot install `vale`, and read that summary line as "the glob axis was not checked", not as a pass.
|
||||
- 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.
|
||||
- A suite that exits 77 because a dependency is missing is reported as SKIPPED, and does **not** fail an ad-hoc run. The pre-push hook invokes the same script as `--strict` (`RUN_TESTS_STRICT=1` is equivalent), where a skip **does** fail the push: at pre-push a skip means one of the dependencies above is absent on this machine, so the gate would otherwise report success having run fewer suites than it appears to. Without vale, for instance, three suites skip (`test-check-vale-style-sync.sh`, `test-vale-hooks-consumer.sh`, `test-vale-wrap.sh`) and the strict failure names each one and what to install.
|
||||
- `tests/run-bats.sh` derives the set of `.bats` files it expects from `git ls-files`, so a `.bats` file deleted from the worktree but still tracked in the index fails the run rather than silently shrinking the suite. Remove one with `git rm` (or stage the deletion) when the removal is intentional; an untracked new `.bats` file is picked up and needs no ceremony.
|
||||
- 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.
|
||||
- **Two** pre-push hooks need the network, for one shared reason: root `apm.yml`'s `marketplace.packages[]` contains exactly one remote entry (`mattpocock-skills`, `source: mattpocock/skills`), and resolving it needs a `git ls-remote`. `apm-marketplace-check` resolves every entry and is `always_run`, so it fails with `No cached refs (offline)`. `apm-pack-check-clean` (`apm pack --check-versions --check-clean --dry-run`) re-resolves the same entry and fails with `Error: Git network timeout during ls-remote`. Pinning the entry to an exact version does **not** remove the call — an exact pin still ls-remotes. `--offline` rescues neither. To push without a network, skip both using pre-commit's own mechanism: `SKIP=apm-marketplace-check,apm-pack-check-clean git push`. Skip those two alone — verified under `unshare -rn`, the other ten pre-push hooks pass offline because they are real local checks, and adding one of them to `SKIP` disarms it silently.
|
||||
- Pushing runs 13 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`), artifact validators (`check-apm-agents-valid`, which runs agent-audit's `validate.sh` over every real `plugins/*/.apm/agents/*.agent.md`), 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 **15**, not 13: 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-audit-ci` runs `apm audit --ci` once per manifest — the root one and each of the six plugin packages — because the root-only invocation audits the marketplace manifest and **nothing else**, and `apm-pack-check-clean` does not parse plugin `dependencies:` blocks either (verified: a malformed one passes `apm pack --check-versions --check-clean --dry-run` and fails `apm audit --ci` in that package's directory). It verifies two things and claims no more: each `apm.yml` parses as a valid APM manifest, and any package declaring dependencies has a consistent `apm.lock.yaml`. It does **not** enforce an org policy — apm discovers one from the git remote and only understands github.com and Azure DevOps, so against this repo's self-hosted Gitea remote it prints `No org policy found at unknown; enforcement skipped`. Do **not** "fix" that with `policy.fetch_failure_default: block` in `apm.yml`: it was tested and rejected, because with no reachable policy source it makes the hook exit 1 on every push forever.
|
||||
- `check-apm-agents-valid` derives its expected agent-file set from `git ls-files` (same pattern as `tests/run-bats.sh`), so an agent file deleted from the worktree but still tracked fails the run, and discovering zero agent files is an error rather than a pass. An untracked new agent file is still validated — the derivation is one-directional on purpose, so uncommitted work is not blocked but also cannot bypass the gate.
|
||||
- **Two** pre-push hooks need the network, for one shared reason: root `apm.yml`'s `marketplace.packages[]` contains exactly one remote entry (`mattpocock-skills`, `source: mattpocock/skills`), and resolving it needs a `git ls-remote`. `apm-marketplace-check` resolves every entry and is `always_run`, so it fails with `No cached refs (offline)`. `apm-pack-check-clean` (`apm pack --check-versions --check-clean --dry-run`) re-resolves the same entry and fails with `Error: Git network timeout during ls-remote`. Pinning the entry to an exact version does **not** remove the call — an exact pin still ls-remotes. `--offline` rescues neither. To push without a network, skip both using pre-commit's own mechanism: `SKIP=apm-marketplace-check,apm-pack-check-clean git push`. Skip those two alone — verified under `unshare -rn`, the other eleven pre-push hooks pass offline because they are real local checks, and adding one of them to `SKIP` disarms it silently. `apm-audit-ci` calls `apm` too but stays local: its org-policy discovery resolves nothing on this remote before any network call, so it does not join the pair above.
|
||||
- Author commits with `git:git-commits` — it validates Conventional Commits (enforced at `commit-msg`) for you.
|
||||
|
||||
## Key documents
|
||||
|
||||
161
scripts/check-apm-agents-valid.sh
Executable file
161
scripts/check-apm-agents-valid.sh
Executable file
@@ -0,0 +1,161 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
# Run agent-audit's validate.sh over every REAL plugin-scope agent file in this
|
||||
# repo (plugins/*/.apm/agents/*.agent.md).
|
||||
#
|
||||
# Why this exists: validate.sh was previously exercised only by
|
||||
# scripts/check-scope-walkup-sync.sh, and only against synthetic fixtures built
|
||||
# in mktemp trees. It had never once run against the four agent files it
|
||||
# actually governs. That is how ADR-0016 could be amended to bless a
|
||||
# `disallowedTools` frontmatter field while validate.sh's allowlist still
|
||||
# rejected it -- the spec and its enforcer disagreed, every gate stayed green,
|
||||
# and the contradiction only surfaced when someone ran the validator by hand.
|
||||
#
|
||||
# A validator that checks fixtures but never artifacts is the same
|
||||
# green-because-nothing-was-checked shape as a suite that runs on an empty file
|
||||
# set. This closes it: the artifacts are the input.
|
||||
#
|
||||
# Run from repo root, or pass REPO_ROOT as the first argument (tests do).
|
||||
# Needs no network.
|
||||
|
||||
REPO_ROOT="${1:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"
|
||||
|
||||
# A nonexistent REPO_ROOT must fail loudly rather than fall through to the
|
||||
# zero-files floor below with a confusing message -- a typo'd or stale path is a
|
||||
# different problem from a repo that genuinely has no agents, and the fix
|
||||
# differs too.
|
||||
if [[ ! -d "$REPO_ROOT" ]]; then
|
||||
echo "APM agent validation failed: REPO_ROOT '$REPO_ROOT' is not a directory." >&2
|
||||
exit 1
|
||||
fi
|
||||
REPO_ROOT="$(cd "$REPO_ROOT" && pwd)"
|
||||
|
||||
VALIDATE="$REPO_ROOT/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate.sh"
|
||||
|
||||
# The validator's own absence is a hard failure, never a skip. If validate.sh
|
||||
# moves or is deleted, every assertion below evaporates and the hook would
|
||||
# otherwise exit 0 having validated nothing -- indistinguishable, from
|
||||
# pre-commit's silent-on-pass output, from a run where all four agents passed.
|
||||
if [[ ! -f "$VALIDATE" ]]; then
|
||||
echo "APM agent validation failed: validator not found at $VALIDATE — this script's path has gone stale, so no agent file was checked. Update it to wherever agent-audit's validate.sh now lives." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# validate.sh is a bash wrapper around a heredoc'd python3 program. Without
|
||||
# python3 it dies with a bare "command not found" per file and no pointer, which
|
||||
# reads like a validation failure rather than a missing dependency. Fail closed,
|
||||
# but say which it is.
|
||||
if ! command -v python3 >/dev/null 2>&1; then
|
||||
echo "APM agent validation failed: python3 not found on PATH — agent-audit's validate.sh is a python3 program and cannot run. Install python3; this gate does not degrade to a pass." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# --- Discover the agent files ---
|
||||
# `-not -path` mirrors tests/run-bats.sh: worktrees under .claude/ are other
|
||||
# checkouts of this same repo, not additional content.
|
||||
AGENT_FILES=()
|
||||
while IFS= read -r f; do
|
||||
[[ -n "$f" ]] && AGENT_FILES+=("$f")
|
||||
done < <(
|
||||
find "$REPO_ROOT/plugins" -type f -name '*.agent.md' \
|
||||
-path '*/.apm/agents/*' \
|
||||
-not -path '*/.claude/worktrees/*' \
|
||||
2>/dev/null | sort
|
||||
)
|
||||
|
||||
# --- Derive the EXPECTED set from the index, not from a hardcoded count ---
|
||||
# Same reasoning as tests/run-bats.sh: a magic number goes stale the moment a
|
||||
# plugin gains or loses an agent, and slack in a floor is exactly where a
|
||||
# silently-deleted file hides. `git ls-files` needs no maintenance -- a newly
|
||||
# `git add`ed agent file joins the expectation immediately.
|
||||
#
|
||||
# Direction matters, and it is the same direction run-bats.sh uses: every
|
||||
# TRACKED file must have been discovered, but a discovered file need not be
|
||||
# tracked. An untracked new agent file is ordinary work in progress (and is
|
||||
# still validated below), while a file that vanished from the worktree without
|
||||
# leaving the index is an accident and fails here. A deliberate `git rm` leaves
|
||||
# the index, so intentional removal passes.
|
||||
#
|
||||
# The exact-equality check on --show-toplevel keeps this off the mktemp fixture
|
||||
# trees in tests/test-check-apm-agents-valid.sh, which resolve no worktree. That
|
||||
# degradation is announced rather than silent, and the zero-file floor below is
|
||||
# unconditional regardless.
|
||||
EXPECTED_FILES=()
|
||||
DERIVED=false
|
||||
GIT_TOPLEVEL="$(git -C "$REPO_ROOT" rev-parse --show-toplevel 2>/dev/null || true)"
|
||||
if [[ -n "$GIT_TOPLEVEL" && "$GIT_TOPLEVEL" == "$REPO_ROOT" ]]; then
|
||||
DERIVED=true
|
||||
while IFS= read -r f; do
|
||||
[[ -n "$f" ]] && EXPECTED_FILES+=("$REPO_ROOT/$f")
|
||||
done < <(
|
||||
git -C "$REPO_ROOT" ls-files -- 'plugins/*/.apm/agents/*.agent.md' \
|
||||
| grep -Ev '(^|/)\.claude/worktrees/' \
|
||||
| sort || true
|
||||
)
|
||||
else
|
||||
echo "Note: $REPO_ROOT is not a git worktree root, so the expected agent file set could not be derived from the index — only the zero-file floor below applies" >&2
|
||||
fi
|
||||
|
||||
if [[ "$DERIVED" == true && ${#EXPECTED_FILES[@]} -gt 0 ]]; then
|
||||
MISSING=()
|
||||
for expected in ${EXPECTED_FILES[@]+"${EXPECTED_FILES[@]}"}; do
|
||||
found=false
|
||||
for actual in ${AGENT_FILES[@]+"${AGENT_FILES[@]}"}; do
|
||||
if [[ "$actual" == "$expected" ]]; then
|
||||
found=true
|
||||
break
|
||||
fi
|
||||
done
|
||||
[[ "$found" == true ]] || MISSING+=("${expected#"$REPO_ROOT"/}")
|
||||
done
|
||||
if [[ ${#MISSING[@]} -gt 0 ]]; then
|
||||
echo "APM agent validation failed: ${#MISSING[@]} of ${#EXPECTED_FILES[@]} tracked agent file(s) were not discovered under $REPO_ROOT — they were deleted without being removed from the index, or this script's search path no longer reaches them:" >&2
|
||||
for m in ${MISSING[@]+"${MISSING[@]}"}; do
|
||||
echo " $m" >&2
|
||||
done
|
||||
exit 1
|
||||
fi
|
||||
fi
|
||||
|
||||
# Unconditional floor, separate from the derived check above: a tree with
|
||||
# nothing tracked (a tarball export, a fresh scaffold, a moved plugins/ root)
|
||||
# still must not validate an empty set and call it green. Zero files is an
|
||||
# error, not a pass -- that is the entire defect this script was written to
|
||||
# close, one level up.
|
||||
if [[ ${#AGENT_FILES[@]} -eq 0 ]]; then
|
||||
echo "APM agent validation failed: found 0 plugin-scope agent file(s) under $REPO_ROOT/plugins — the search path is wrong or every .apm/agents/ directory has been emptied. Zero files is never a pass." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# --- Validate ---
|
||||
FAIL=0
|
||||
FAILED_FILES=()
|
||||
for f in ${AGENT_FILES[@]+"${AGENT_FILES[@]}"}; do
|
||||
rel="${f#"$REPO_ROOT"/}"
|
||||
# validate.sh prints its FAIL lines on stdout and its own errors on stderr;
|
||||
# both are captured and replayed under the filename so the reason travels with
|
||||
# the file that caused it. pre-commit shows a failing hook's output verbatim,
|
||||
# so this is what a developer reads.
|
||||
out=""
|
||||
rc=0
|
||||
out="$(bash "$VALIDATE" "$f" 2>&1)" || rc=$?
|
||||
if [[ "$rc" -ne 0 ]]; then
|
||||
FAIL=1
|
||||
FAILED_FILES+=("$rel")
|
||||
echo "FAIL: $rel (validate.sh exit $rc)" >&2
|
||||
if [[ -n "$out" ]]; then
|
||||
printf '%s\n' "$out" | sed 's/^/ /' >&2
|
||||
else
|
||||
echo " (validate.sh produced no output — see its exit code above; 2 means script error, e.g. a missing references/field-inventory.md)" >&2
|
||||
fi
|
||||
fi
|
||||
done
|
||||
|
||||
if [[ "$FAIL" -ne 0 ]]; then
|
||||
echo "" >&2
|
||||
echo "APM agent validation failed: ${#FAILED_FILES[@]} of ${#AGENT_FILES[@]} agent file(s) did not pass agent-audit's validate.sh." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "APM agent validation passed: ${#AGENT_FILES[@]} plugin-scope agent file(s) validated against agent-audit's validate.sh."
|
||||
264
tests/test-check-apm-agents-valid.sh
Executable file
264
tests/test-check-apm-agents-valid.sh
Executable file
@@ -0,0 +1,264 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
# Tests for scripts/check-apm-agents-valid.sh — the gate that runs agent-audit's
|
||||
# validate.sh over the repo's REAL plugin-scope agent files.
|
||||
#
|
||||
# Case 1 runs against the real repo. Every other case runs against a synthetic
|
||||
# fixture, for the same reason scripts/check-scope-walkup-sync.sh's tests do: the
|
||||
# RED cases have to mutate an agent file, and mutating the real tree from a test
|
||||
# is not on.
|
||||
#
|
||||
# The point of case 1b is that case 1's exit 0 is EARNED. Exit 0 is also what
|
||||
# this script would print if it validated nothing at all, which is the exact
|
||||
# defect it exists to close — so the clean run's own count is asserted against
|
||||
# the index rather than taken on trust.
|
||||
|
||||
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
||||
SCRIPT="$REPO_ROOT/scripts/check-apm-agents-valid.sh"
|
||||
PASS=0
|
||||
FAIL=0
|
||||
|
||||
pass() { echo " PASS: $1"; PASS=$((PASS + 1)); }
|
||||
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
|
||||
|
||||
# validate.sh is a python3 program. Without python3 the script under test fails
|
||||
# closed by design, which is correct behavior but makes every case here assert
|
||||
# the same missing-dependency message instead of what it is meant to assert.
|
||||
if ! command -v python3 >/dev/null 2>&1; then
|
||||
echo "SKIP: python3 is not installed — agent-audit's validate.sh cannot run, so these cases would only re-assert the missing-dependency guard"
|
||||
exit 77
|
||||
fi
|
||||
|
||||
FIXTURES=()
|
||||
cleanup() { [[ ${#FIXTURES[@]} -eq 0 ]] || rm -rf "${FIXTURES[@]}"; }
|
||||
trap cleanup EXIT
|
||||
|
||||
# Per-run scratch dir for captured output. tests/run-tests.sh fans test scripts
|
||||
# out concurrently, so a fixed path under the shared system temp directory is
|
||||
# mutable state shared between two simultaneous runs.
|
||||
RUN_TMP="$(mktemp -d)"
|
||||
FIXTURES+=("$RUN_TMP")
|
||||
|
||||
# Builds a minimal REPO_ROOT: agent-audit's validator and the field inventory it
|
||||
# reads at load time, plus one plugin carrying a valid agent file. The plugin's
|
||||
# apm.yml needs a top-level `type:` line — that is the marker validate.sh's
|
||||
# walk-up uses to resolve plugin scope, and without it the fixture would resolve
|
||||
# to project scope and fail looking for a .github/agents counterpart.
|
||||
#
|
||||
# `pwd -P` because the script under test compares its REPO_ROOT against
|
||||
# `git rev-parse --show-toplevel`, which is always physical. On a platform where
|
||||
# the system temp dir is a symlink (macOS /tmp -> /private/tmp) a logical path
|
||||
# would silently fail that equality and take the un-derived branch, quietly
|
||||
# turning case 4 into a no-op.
|
||||
make_fixture() {
|
||||
local dir
|
||||
dir="$(cd "$(mktemp -d)" && pwd -P)"
|
||||
local aa="$dir/plugins/kyberforge/.apm/skills/agent-audit"
|
||||
mkdir -p "$aa/scripts" "$aa/references" "$dir/plugins/lint/.apm/agents"
|
||||
cp "$REPO_ROOT/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate.sh" "$aa/scripts/"
|
||||
cp "$REPO_ROOT/plugins/kyberforge/.apm/skills/agent-audit/references/field-inventory.md" "$aa/references/"
|
||||
cat > "$dir/plugins/lint/apm.yml" <<'YAML'
|
||||
name: lint
|
||||
version: 0.0.1
|
||||
type: hybrid
|
||||
YAML
|
||||
cat > "$dir/plugins/lint/.apm/agents/lint-runner.agent.md" <<'MD'
|
||||
---
|
||||
name: lint-runner
|
||||
description: Runs a linter sweep over a target scope and reports findings back to the caller.
|
||||
---
|
||||
|
||||
Run the linter over the scope the caller names and report what it found.
|
||||
MD
|
||||
echo "$dir"
|
||||
}
|
||||
|
||||
# --- 1. Exits 0 against this repo's real agent files ---
|
||||
echo ""
|
||||
echo "--- exits 0 against this repo's real agent files ---"
|
||||
if bash "$SCRIPT" "$REPO_ROOT" > "$RUN_TMP/clean.out" 2>&1; then
|
||||
pass "exits 0 against this repo's four real plugin-scope agent files"
|
||||
else
|
||||
fail "exited non-zero against this repo's real (already-fixed) agent files"
|
||||
sed 's/^/ /' "$RUN_TMP/clean.out"
|
||||
fi
|
||||
|
||||
# --- 1b. That exit 0 was earned: the count matches the index, and is non-zero ---
|
||||
echo ""
|
||||
echo "--- the clean run's reported count matches git ls-files ---"
|
||||
TRACKED_COUNT="$(git -C "$REPO_ROOT" ls-files -- 'plugins/*/.apm/agents/*.agent.md' | grep -c . || true)"
|
||||
REPORTED_COUNT="$(sed -n 's/^APM agent validation passed: \([0-9]\{1,\}\) plugin-scope.*/\1/p' "$RUN_TMP/clean.out")"
|
||||
if [[ -z "$REPORTED_COUNT" ]]; then
|
||||
fail "the clean run printed no 'APM agent validation passed: N ...' summary line — the script's contract with this test is gone"
|
||||
sed 's/^/ /' "$RUN_TMP/clean.out"
|
||||
elif [[ "$TRACKED_COUNT" -eq 0 ]]; then
|
||||
fail "git ls-files found 0 tracked agent files — this test's own expectation is broken, not the script's"
|
||||
elif [[ "$REPORTED_COUNT" -ne "$TRACKED_COUNT" ]]; then
|
||||
fail "the clean run validated $REPORTED_COUNT file(s) but $TRACKED_COUNT are tracked"
|
||||
else
|
||||
pass "validated $REPORTED_COUNT file(s), matching the $TRACKED_COUNT tracked in the index"
|
||||
fi
|
||||
|
||||
# --- 2. An invalid agent file fails, and both the file and the reason are named ---
|
||||
echo ""
|
||||
echo "--- an invalid agent file fails, naming the file and the reason ---"
|
||||
FIX2="$(make_fixture)"
|
||||
FIXTURES+=("$FIX2")
|
||||
# `tools:` is deliberately absent from field-inventory.md's apm-agent-allowlist:
|
||||
# its value shape differs per harness and apm compile copies frontmatter verbatim
|
||||
# to every target (ADR-0016).
|
||||
python3 - "$FIX2/plugins/lint/.apm/agents/lint-runner.agent.md" <<'PY'
|
||||
import sys
|
||||
p = sys.argv[1]
|
||||
s = open(p).read()
|
||||
open(p, 'w').write(s.replace('---\n', '---\ntools: Read, Write\n', 1))
|
||||
PY
|
||||
if bash "$SCRIPT" "$FIX2" > "$RUN_TMP/invalid.out" 2>&1; then
|
||||
fail "an agent file with a non-allowlisted frontmatter field still exited 0"
|
||||
sed 's/^/ /' "$RUN_TMP/invalid.out"
|
||||
elif ! grep -q 'plugins/lint/\.apm/agents/lint-runner\.agent\.md' "$RUN_TMP/invalid.out"; then
|
||||
fail "failed as expected but did not name the offending file"
|
||||
sed 's/^/ /' "$RUN_TMP/invalid.out"
|
||||
elif ! grep -q "field 'tools' is not in the vendor-neutral APM agent allowlist" "$RUN_TMP/invalid.out"; then
|
||||
fail "failed as expected and named the file but did not carry validate.sh's reason through"
|
||||
sed 's/^/ /' "$RUN_TMP/invalid.out"
|
||||
else
|
||||
pass "an invalid agent file exits 1, naming both the file and validate.sh's reason"
|
||||
fi
|
||||
|
||||
# --- 3. Zero discovered files is an error, not a pass ---
|
||||
echo ""
|
||||
echo "--- zero discovered agent files is an error ---"
|
||||
FIX3="$(make_fixture)"
|
||||
FIXTURES+=("$FIX3")
|
||||
rm -f "$FIX3/plugins/lint/.apm/agents/lint-runner.agent.md"
|
||||
if bash "$SCRIPT" "$FIX3" > "$RUN_TMP/empty.out" 2>&1; then
|
||||
fail "a tree with zero agent files exited 0 — the floor is gone and the gate is vacuous"
|
||||
sed 's/^/ /' "$RUN_TMP/empty.out"
|
||||
elif ! grep -q 'found 0 plugin-scope agent file' "$RUN_TMP/empty.out"; then
|
||||
fail "a tree with zero agent files exited non-zero but not for the zero-file reason"
|
||||
sed 's/^/ /' "$RUN_TMP/empty.out"
|
||||
else
|
||||
pass "a tree with zero agent files exits 1 and says so"
|
||||
fi
|
||||
|
||||
# --- 4. A tracked file missing from the worktree fails, derived from the index ---
|
||||
# This is the check a hardcoded count cannot make: the file is gone but the count
|
||||
# of what remains would still look plausible.
|
||||
echo ""
|
||||
echo "--- a tracked-but-deleted agent file fails against the derived expectation ---"
|
||||
FIX4="$(make_fixture)"
|
||||
FIXTURES+=("$FIX4")
|
||||
cat > "$FIX4/plugins/lint/.apm/agents/second-agent.agent.md" <<'MD'
|
||||
---
|
||||
name: second-agent
|
||||
description: A second agent, present only so its deletion leaves a plausible-looking non-empty set behind.
|
||||
---
|
||||
|
||||
Do the second thing.
|
||||
MD
|
||||
git -C "$FIX4" init -q
|
||||
git -C "$FIX4" add -A
|
||||
git -C "$FIX4" -c user.email=t@example.invalid -c user.name=t commit -qm "fixture"
|
||||
rm -f "$FIX4/plugins/lint/.apm/agents/second-agent.agent.md"
|
||||
if bash "$SCRIPT" "$FIX4" > "$RUN_TMP/missing.out" 2>&1; then
|
||||
fail "a tracked agent file deleted from the worktree still exited 0"
|
||||
sed 's/^/ /' "$RUN_TMP/missing.out"
|
||||
elif grep -q 'not a git worktree root' "$RUN_TMP/missing.out"; then
|
||||
fail "the fixture did not resolve as its own git worktree root, so the derived check never ran"
|
||||
sed 's/^/ /' "$RUN_TMP/missing.out"
|
||||
elif ! grep -q 'second-agent\.agent\.md' "$RUN_TMP/missing.out"; then
|
||||
fail "failed as expected but did not name the tracked file that went missing"
|
||||
sed 's/^/ /' "$RUN_TMP/missing.out"
|
||||
else
|
||||
pass "a tracked agent file deleted from the worktree exits 1 and is named"
|
||||
fi
|
||||
|
||||
# --- 5. An untracked agent file is still validated ---
|
||||
# The derived expectation is one-directional on purpose (tracked ⊆ discovered).
|
||||
# Work in progress must not fail the gate for being uncommitted — but it must
|
||||
# still be validated, or the gate would be trivially bypassed by not committing.
|
||||
echo ""
|
||||
echo "--- an untracked, invalid agent file still fails the gate ---"
|
||||
FIX5="$(make_fixture)"
|
||||
FIXTURES+=("$FIX5")
|
||||
git -C "$FIX5" init -q
|
||||
git -C "$FIX5" add -A
|
||||
git -C "$FIX5" -c user.email=t@example.invalid -c user.name=t commit -qm "fixture"
|
||||
cat > "$FIX5/plugins/lint/.apm/agents/wip-agent.agent.md" <<'MD'
|
||||
---
|
||||
name: wip-agent
|
||||
tools: Read, Write
|
||||
description: An uncommitted work-in-progress agent carrying a non-allowlisted field.
|
||||
---
|
||||
|
||||
Do the work-in-progress thing.
|
||||
MD
|
||||
if bash "$SCRIPT" "$FIX5" > "$RUN_TMP/untracked.out" 2>&1; then
|
||||
fail "an untracked, invalid agent file was not validated — the gate can be bypassed by not committing"
|
||||
sed 's/^/ /' "$RUN_TMP/untracked.out"
|
||||
elif ! grep -q 'wip-agent\.agent\.md' "$RUN_TMP/untracked.out"; then
|
||||
fail "failed but did not name the untracked file"
|
||||
sed 's/^/ /' "$RUN_TMP/untracked.out"
|
||||
else
|
||||
pass "an untracked, invalid agent file exits 1 and is named"
|
||||
fi
|
||||
|
||||
# --- 5b. An untracked but VALID agent file does not fail ---
|
||||
echo ""
|
||||
echo "--- an untracked, valid agent file passes ---"
|
||||
FIX5B="$(make_fixture)"
|
||||
FIXTURES+=("$FIX5B")
|
||||
git -C "$FIX5B" init -q
|
||||
git -C "$FIX5B" add -A
|
||||
git -C "$FIX5B" -c user.email=t@example.invalid -c user.name=t commit -qm "fixture"
|
||||
cat > "$FIX5B/plugins/lint/.apm/agents/wip-ok.agent.md" <<'MD'
|
||||
---
|
||||
name: wip-ok
|
||||
description: An uncommitted work-in-progress agent that is nonetheless entirely valid.
|
||||
---
|
||||
|
||||
Do the valid work-in-progress thing.
|
||||
MD
|
||||
if bash "$SCRIPT" "$FIX5B" > "$RUN_TMP/untracked-ok.out" 2>&1; then
|
||||
pass "an untracked but valid agent file does not fail the gate"
|
||||
else
|
||||
fail "an untracked but valid agent file failed the gate — uncommitted work must not be an error"
|
||||
sed 's/^/ /' "$RUN_TMP/untracked-ok.out"
|
||||
fi
|
||||
|
||||
# --- 6. A nonexistent REPO_ROOT fails loudly ---
|
||||
echo ""
|
||||
echo "--- a nonexistent REPO_ROOT fails loudly ---"
|
||||
if bash "$SCRIPT" "$RUN_TMP/does-not-exist" > "$RUN_TMP/norepo.out" 2>&1; then
|
||||
fail "a nonexistent REPO_ROOT exited 0"
|
||||
sed 's/^/ /' "$RUN_TMP/norepo.out"
|
||||
elif ! grep -q 'is not a directory' "$RUN_TMP/norepo.out"; then
|
||||
fail "a nonexistent REPO_ROOT failed for the wrong reason"
|
||||
sed 's/^/ /' "$RUN_TMP/norepo.out"
|
||||
else
|
||||
pass "a nonexistent REPO_ROOT exits 1 and says which path it was"
|
||||
fi
|
||||
|
||||
# --- 7. A missing validator is a hard failure, never a silent pass ---
|
||||
# The whole gate is void without validate.sh, and exit 0 here would be
|
||||
# indistinguishable from a run where every agent passed.
|
||||
echo ""
|
||||
echo "--- a missing validate.sh fails rather than validating nothing ---"
|
||||
FIX7="$(make_fixture)"
|
||||
FIXTURES+=("$FIX7")
|
||||
rm -f "$FIX7/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate.sh"
|
||||
if bash "$SCRIPT" "$FIX7" > "$RUN_TMP/novalidator.out" 2>&1; then
|
||||
fail "a missing validate.sh exited 0 — the gate silently validated nothing"
|
||||
sed 's/^/ /' "$RUN_TMP/novalidator.out"
|
||||
elif ! grep -q 'validator not found' "$RUN_TMP/novalidator.out"; then
|
||||
fail "a missing validate.sh failed for the wrong reason"
|
||||
sed 's/^/ /' "$RUN_TMP/novalidator.out"
|
||||
else
|
||||
pass "a missing validate.sh exits 1 and names the stale path"
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
Reference in New Issue
Block a user