chore: delete the check-manifests pre-commit hook
Six pre-push hooks were validating overlapping sets of the same manifests. check-manifests (marketplace.json/plugin.json path checks) is redundant with validate-plugins (claude plugin validate) and apm-pack-check-clean, which already cover the same ground. Deletes the check-manifests hook entry, scripts/check-manifests.sh (282 lines), and tests/test-check-manifests.sh (771 lines). scripts/lib/marketplace-plugins.sh is kept — it is still sourced by sync-plugin-content.sh. Updates the now-stale check-manifests.sh mentions and hook counts in README.md and docs/spec/gates.md. The apm-audit-ci and apm-marketplace-check hooks named in the same finding are left untouched — the audit flags them as needing a separate decision. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
This commit is contained in:
@@ -1,282 +0,0 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
# Validates that marketplace.json's local plugin entries resolve to a real directory
|
||||
# containing a .claude-plugin/plugin.json. Run from repo root or pass REPO_ROOT as arg.
|
||||
#
|
||||
# Per ADR-0015, apm.yml is the authoring source and .claude-plugin/plugin.json is
|
||||
# compiled output with no skills/hooks/mcpServers/agents pointer fields (apm's plugin.json
|
||||
# builder deliberately omits them -- Claude Code auto-discovers those convention
|
||||
# directories, so listing them would be redundant/invalid). For a plugin with an .apm/
|
||||
# directory, this script no longer checks those pointer fields itself; that's
|
||||
# scripts/sync-plugin-content.sh --check's job (drift between .apm/ and the flat
|
||||
# plugin-root mirror), wired as its own pre-push hook.
|
||||
#
|
||||
# sync-plugin-content.sh --check explicitly skips any plugin directory lacking .apm/
|
||||
# (an apm-native package it has nothing to compile), so that delegation leaves a real
|
||||
# gap for a non-apm plugin whose hand-authored plugin.json still uses the old
|
||||
# skills/hooks/mcpServers/agents pointer-field convention: nothing would check whether
|
||||
# those paths resolve. The fallback block below restores that check, but only for
|
||||
# plugins without .apm/ -- apm-native plugins keep relying on the delegation above so
|
||||
# the two checks don't duplicate (and disagree) on the same manifest.
|
||||
#
|
||||
# Both of the above walk marketplace.json -> disk. Nothing walked disk -> marketplace,
|
||||
# so a plugins/<name>/ directory that never made it into marketplace.json was invisible
|
||||
# to every marketplace-derived gate at once (this script and sync-plugin-content.sh
|
||||
# --all both derive their plugin set from marketplace.json). The final block below
|
||||
# closes that direction: per ADR-0015 marketplace.json is compiled output of root
|
||||
# apm.yml's marketplace.packages[], so an on-disk apm package with no entry is
|
||||
# compiled-output drift of exactly the kind ADR-0017 wires pre-push gates for -- and it
|
||||
# is the same plugin set the validate-plugins pre-commit hook already globs as
|
||||
# plugins/*/.
|
||||
#
|
||||
# Every pass above reads its plugin set out of marketplace.json, so anything that makes
|
||||
# that file yield nothing -- absent, unparseable, a non-object root, or an entry whose
|
||||
# `source` is neither a path string nor a remote object -- used to read as "clean"
|
||||
# rather than "unchecked". The same is true one level down, of a per-plugin
|
||||
# .claude-plugin/plugin.json that does not parse: it aborted the walk mid-loop and left
|
||||
# every later plugin silently unchecked. The guards below turn each of those into an
|
||||
# explicit, attributable failure instead, because a vacuous pass is the one result a gate
|
||||
# must never produce.
|
||||
|
||||
# Hard error, not a `|| pwd` fallback, for the reason spelled out in
|
||||
# scripts/sync-marketplace-mirror.sh: every path below hangs off REPO_ROOT, and the
|
||||
# exit-0 path is "nothing on disk and no manifest", so a REPO_ROOT pointing somewhere
|
||||
# that is not this repo reports "clean" over a tree it never looked at. Run this from
|
||||
# an empty directory outside any worktree and the fallback made that the literal
|
||||
# outcome -- rev-parse failed, REPO_ROOT became $PWD, no plugins/ and no
|
||||
# marketplace.json were found, exit 0, silent.
|
||||
if [[ -n "${1:-}" ]]; then
|
||||
REPO_ROOT="$1"
|
||||
elif ! REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null)" || [[ -z "$REPO_ROOT" ]]; then
|
||||
echo "Error: not inside a git worktree -- cannot locate the repository root, and guessing \$PWD would let this check report \"clean\" over a tree it never inspected. Run it from within the repository, or pass the repo root as an argument." >&2
|
||||
exit 1
|
||||
fi
|
||||
FAIL=0
|
||||
|
||||
err() { echo " FAIL: $1" >&2; FAIL=$((FAIL + 1)); }
|
||||
|
||||
if ! command -v jq &>/dev/null; then
|
||||
echo "Error: jq is required but not installed" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
MARKETPLACE="$REPO_ROOT/.claude-plugin/marketplace.json"
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
# 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"
|
||||
|
||||
# Candidate plugin directories on disk. The trigger is any of the three markers that
|
||||
# make a directory a plugin rather than scratch -- apm.yml (the ADR-0015 authoring
|
||||
# source), .apm/ (its content tree), or a compiled .claude-plugin/plugin.json. Matching
|
||||
# all three keeps this set aligned with the plugins/*/ glob the validate-plugins
|
||||
# pre-commit hook uses, which is the disagreement the disk -> marketplace pass below
|
||||
# exists to close; a directory with none of them is scratch and stays out of scope.
|
||||
#
|
||||
# It is collected before the marketplace is read because a missing marketplace.json is
|
||||
# only "nothing to check" when there is also nothing on disk to check against it.
|
||||
PLUGIN_DIRS=()
|
||||
for candidate in "$REPO_ROOT"/plugins/*/; do
|
||||
candidate="${candidate%/}"
|
||||
[[ -d "$candidate" ]] || continue
|
||||
if [[ ! -f "$candidate/apm.yml" && ! -d "$candidate/.apm" && ! -f "$candidate/.claude-plugin/plugin.json" ]]; then
|
||||
continue
|
||||
fi
|
||||
PLUGIN_DIRS+=("$candidate")
|
||||
done
|
||||
|
||||
# An absent marketplace.json used to exit 0 unconditionally -- the same empty-set-reads-
|
||||
# as-pass shape this script's other passes were fixed for. Per ADR-0015 the manifest is
|
||||
# compiled output of root apm.yml's marketplace.packages[], so its absence alongside
|
||||
# on-disk packages is drift, not an opt-out: it leaves every marketplace-derived gate
|
||||
# (this one and sync-plugin-content.sh --all) walking an empty plugin set in silence.
|
||||
if [[ ! -f "$MARKETPLACE" ]]; then
|
||||
if [[ ${#PLUGIN_DIRS[@]} -eq 0 ]]; then
|
||||
exit 0
|
||||
fi
|
||||
listing=""
|
||||
# Guarded expansion even though the check above makes an empty array unreachable
|
||||
# here: bash 3.2 under `set -u` aborts on a bare expansion of an empty array, and
|
||||
# tests/test-vale-wrap.sh's bash32_glob scan is line-based, so a guard two lines up
|
||||
# cannot clear it. Same form as the disk -> marketplace loop below.
|
||||
for candidate in ${PLUGIN_DIRS[@]+"${PLUGIN_DIRS[@]}"}; do
|
||||
listing+="${listing:+, }${candidate#"$REPO_ROOT"/}"
|
||||
done
|
||||
err ".claude-plugin/marketplace.json does not exist, but plugins/ holds ${#PLUGIN_DIRS[@]} plugin directory/ies ($listing) — every marketplace-derived check (this one, and sync-plugin-content.sh --all) silently walks an empty plugin set without it. Recompile the manifests from root apm.yml with \`apm pack\`."
|
||||
echo "Manifest check failed: $FAIL error(s)" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Preconditions the marketplace walk below cannot report on itself: it runs inside a
|
||||
# process substitution, so an abort in there is swallowed (see the helper's comment).
|
||||
assert_marketplace_manifest_usable "$MARKETPLACE"
|
||||
|
||||
# Validates one plugin.json pointer field against disk, for the non-apm fallback below.
|
||||
#
|
||||
# check_pointer_field <plugin_name> <plugin_dir> <field> <test_flag>
|
||||
#
|
||||
# test_flag is `test`'s: -d where only a directory is meaningful, -e otherwise.
|
||||
#
|
||||
# Per the vendored host docs (plugins/kyberforge/docs/research/docs/
|
||||
# claude-code-plugins/configuration.md and .../github-copilot-plugins/configuration.md)
|
||||
# these fields are legally `string | string[] | object`. Reading them with
|
||||
# `jq -r ".$field // empty"` collapsed the array and object shapes to their
|
||||
# pretty-printed JSON text, which then matched no path on disk -- a manifest that
|
||||
# resolves fine reported as broken. Reading `.skills | length` was worse than wrong: on
|
||||
# a (legal) string value it returned the character count, and the `.skills[$i]` that
|
||||
# followed aborted the whole script mid-loop under `set -e` with no summary line, so
|
||||
# every plugin later in the marketplace went unchecked.
|
||||
#
|
||||
# The bare `$(jq ...)` assignments below are safe only because the caller has already
|
||||
# established that $manifest parses AND that its root is an object (see the
|
||||
# precondition in the marketplace walk). Do not call this without that check: `set -e`
|
||||
# turns any jq failure in here into the same silent mid-loop abort described above.
|
||||
check_pointer_field() {
|
||||
local name="$1" plugin_dir="$2" field="$3" test_flag="$4"
|
||||
local manifest="$plugin_dir/.claude-plugin/plugin.json"
|
||||
local field_type count i elem_type
|
||||
|
||||
field_type="$(jq -r ".${field} | type" "$manifest")"
|
||||
case "$field_type" in
|
||||
null) ;;
|
||||
# An inline definition (a hooks or mcpServers object written straight into the
|
||||
# manifest) declares no path, so there is nothing on disk to resolve.
|
||||
object) ;;
|
||||
string)
|
||||
check_pointer_path "$name" "$plugin_dir" "$field" "$(jq -r ".${field}" "$manifest")" "$test_flag"
|
||||
;;
|
||||
array)
|
||||
count="$(jq ".${field} | length" "$manifest")"
|
||||
for ((i = 0; i < count; i++)); do
|
||||
elem_type="$(jq -r ".${field}[$i] | type" "$manifest")"
|
||||
if [[ "$elem_type" != "string" ]]; then
|
||||
err "plugin '$name': ${field}[$i] must be a path string, got $elem_type"
|
||||
continue
|
||||
fi
|
||||
check_pointer_path "$name" "$plugin_dir" "$field" "$(jq -r ".${field}[$i]" "$manifest")" "$test_flag"
|
||||
done
|
||||
;;
|
||||
*)
|
||||
err "plugin '$name': $field must be a path string, an array of path strings, or an inline object, got $field_type"
|
||||
;;
|
||||
esac
|
||||
}
|
||||
|
||||
check_pointer_path() {
|
||||
local name="$1" plugin_dir="$2" field="$3" ref="$4" test_flag="$5"
|
||||
local full_path="$plugin_dir/$ref"
|
||||
full_path="${full_path%/}"
|
||||
if ! test "$test_flag" "$full_path"; then
|
||||
err "plugin '$name': $field path not found: $ref"
|
||||
fi
|
||||
}
|
||||
|
||||
# Every local plugin directory marketplace.json claimed, canonicalized, so the
|
||||
# disk -> marketplace pass below can tell "listed" from "unlisted" regardless of how
|
||||
# the `source:` string was spelled (./plugins/x, plugins/x, plugins/x/).
|
||||
SEEN_PLUGIN_DIRS=()
|
||||
|
||||
while IFS=$'\t' read -r name plugin_dir; do
|
||||
source_rel="${plugin_dir#"$REPO_ROOT"/}"
|
||||
|
||||
if [[ ! -d "$plugin_dir" ]]; then
|
||||
err "plugin '$name': source directory not found: $source_rel"
|
||||
continue
|
||||
fi
|
||||
# -P so a plugin directory reached through a symlink compares equal to the same
|
||||
# directory reached directly; the disk-side walk below resolves the same way.
|
||||
SEEN_PLUGIN_DIRS+=("$(cd "$plugin_dir" && pwd -P)")
|
||||
|
||||
manifest="$plugin_dir/.claude-plugin/plugin.json"
|
||||
if [[ ! -f "$manifest" ]]; then
|
||||
err "plugin '$name': .claude-plugin/plugin.json not found in $source_rel"
|
||||
continue
|
||||
fi
|
||||
|
||||
# apm-native plugin: pointer-field validation is sync-plugin-content.sh --check's
|
||||
# job (see header comment above).
|
||||
[[ -d "$plugin_dir/.apm" ]] && continue
|
||||
|
||||
# Precondition for check_pointer_field, which reads the manifest with bare
|
||||
# `field_type="$(jq ... )"` assignments. Under `set -e` a jq failure in one of
|
||||
# those aborts the whole script mid-loop: rc=5, a raw `jq: parse error` and no
|
||||
# `Manifest check failed:` summary, with every later plugin left unchecked --
|
||||
# the same failure class the marketplace's own `jq empty` precondition closes,
|
||||
# for a file that is equally generated output. Both shapes have to be caught
|
||||
# here: `jq empty` passes on a valid non-object document like `[]` or `123`, and
|
||||
# it is the `.skills` lookup on such a root ("Cannot index array with string")
|
||||
# that aborts, not the parse.
|
||||
if ! jq empty "$manifest" >/dev/null 2>&1; then
|
||||
err "plugin '$name': .claude-plugin/plugin.json is not valid JSON — it is compiled output, so recompile it with \`apm pack\`."
|
||||
continue
|
||||
fi
|
||||
manifest_type="$(jq -r 'type' "$manifest")"
|
||||
if [[ "$manifest_type" != "object" ]]; then
|
||||
err "plugin '$name': .claude-plugin/plugin.json is a JSON $manifest_type at its top level; expected an object."
|
||||
continue
|
||||
fi
|
||||
|
||||
# Fallback for a non-apm plugin: validate that any skills/hooks/mcpServers/agents
|
||||
# pointer fields in its hand-authored plugin.json still resolve to real paths.
|
||||
# skills/agents point at directories; hooks/mcpServers may point at a file.
|
||||
check_pointer_field "$name" "$plugin_dir" skills -d
|
||||
check_pointer_field "$name" "$plugin_dir" agents -e
|
||||
check_pointer_field "$name" "$plugin_dir" hooks -e
|
||||
check_pointer_field "$name" "$plugin_dir" mcpServers -e
|
||||
done < <(list_marketplace_local_plugins "$REPO_ROOT" "$MARKETPLACE")
|
||||
|
||||
# Disk -> marketplace, over the PLUGIN_DIRS candidate set collected above.
|
||||
#
|
||||
# A candidate counts as listed if it is either a directory some local entry pointed at
|
||||
# (path match, canonicalized above) or a directory whose name matches a REMOTE entry's
|
||||
# name. The name axis exists only for a plugin vendored on disk but declared with the
|
||||
# remote-object `source:` shape: list_marketplace_local_plugins deliberately skips those,
|
||||
# so a path-only match would report a missing entry that is in fact already there.
|
||||
#
|
||||
# It is restricted to non-string sources on purpose. Applied to local entries too, the
|
||||
# name axis silently rescues genuine orphans, because a local entry's name need not equal
|
||||
# the basename of the directory it points at: an entry named "beta" pointing at
|
||||
# ./plugins/alpha would mark an unrelated, entirely unlisted plugins/beta/ as listed.
|
||||
# Local entries already have an exact path to match on, so they need no name fallback.
|
||||
#
|
||||
# The select is an allowlist of the object shape, not a denylist of the string one --
|
||||
# see list_marketplace_remote_plugin_names in scripts/lib/marketplace-plugins.sh, which
|
||||
# owns it, and tests/test-check-manifests.sh, which exercises it directly against
|
||||
# malformed entries rather than through this caller (where
|
||||
# assert_marketplace_manifest_usable rejects them first, and so would mask a regression
|
||||
# in the select itself).
|
||||
MARKETPLACE_NAMES=()
|
||||
while IFS= read -r entry_name; do
|
||||
[[ -n "$entry_name" ]] && MARKETPLACE_NAMES+=("$entry_name")
|
||||
done < <(list_marketplace_remote_plugin_names "$MARKETPLACE")
|
||||
|
||||
for candidate in ${PLUGIN_DIRS[@]+"${PLUGIN_DIRS[@]}"}; do
|
||||
candidate_abs="$(cd "$candidate" && pwd -P)"
|
||||
candidate_name="$(basename "$candidate")"
|
||||
listed=0
|
||||
for seen in ${SEEN_PLUGIN_DIRS[@]+"${SEEN_PLUGIN_DIRS[@]}"}; do
|
||||
if [[ "$seen" == "$candidate_abs" ]]; then
|
||||
listed=1
|
||||
break
|
||||
fi
|
||||
done
|
||||
if [[ $listed -eq 0 ]]; then
|
||||
for entry_name in ${MARKETPLACE_NAMES[@]+"${MARKETPLACE_NAMES[@]}"}; do
|
||||
if [[ "$entry_name" == "$candidate_name" ]]; then
|
||||
listed=1
|
||||
break
|
||||
fi
|
||||
done
|
||||
fi
|
||||
|
||||
if [[ $listed -eq 0 ]]; then
|
||||
err "plugin directory '${candidate#"$REPO_ROOT"/}' has no entry in .claude-plugin/marketplace.json — it is skipped by every marketplace-derived check (this one, and sync-plugin-content.sh --all) while still being globbed by the validate-plugins hook. Add it to root apm.yml's marketplace.packages[] and recompile the manifests."
|
||||
fi
|
||||
done
|
||||
|
||||
if [[ $FAIL -gt 0 ]]; then
|
||||
echo "Manifest check failed: $FAIL error(s)" >&2
|
||||
exit 1
|
||||
fi
|
||||
Reference in New Issue
Block a user