The previous round widened path_manifest from the exec bit to full permission bits, and that made check-plugin-content-sync fail at pre-push on a pristine tree. hooks/hooks.json is not copied from the bundle -- sync_hooks_json writes it with printf, i.e. at the runtime umask -- while the real side comes from the checkout. On a umask-002 clone the two disagree, 664 vs 644, and no commit can reconcile them because git tracks no non-exec mode. The rule adopted: record a mode for a path this pipeline copies, never for one it writes. A copied path's mode traces to the same checkout on both sides, so comparing it means something; a written path's mode is the writer's umask on one side and the checkout's on the other, which are independent. That is the same rationale the directory exclusion already carried -- what broke was the premise that files are immune. Normalising instead was rejected: pinning the generated side cannot fix a checked-out side that is already 664. The unconditional chmod 644 in reinject_mcp_servers goes for the same reason; writing through the destination inode already closed the original 0600 bug. The mode coverage added for the two plugin.json manifests is removed rather than documented, because it measured nothing on any axis. In check mode the expected side is a cp -a of the real plugin root, so apm rewrites an existing inode and inherits its mode; and a symlinked manifest is copied as a symlink and written straight through, so both sides agreed no matter what. That symlink case is a real hazard -- the re-injection corrupts the link's target -- so it is now asserted directly instead. Also: an unparseable or non-object per-plugin plugin.json killed the manifest walk mid-loop; the source-less-entry guard closed only source: null and let every other malformed value through; the select it backstops was extracted so a test can exercise it independently, which nothing could before; and two more `|| pwd` fallbacks now hard-error -- with a decoy marketplace.json in $PWD, --all derived its plugin list from it. Tests: 63 -> 77 and 23 -> 31 assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
772 lines
34 KiB
Bash
772 lines
34 KiB
Bash
#!/usr/bin/env bash
|
|
set -euo pipefail
|
|
|
|
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
|
SCRIPT="$REPO_ROOT/scripts/check-manifests.sh"
|
|
PASS=0
|
|
FAIL=0
|
|
|
|
pass() { echo " PASS: $1"; PASS=$((PASS + 1)); }
|
|
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
|
|
|
|
# Several distinct faults all end in exit 1, and the bugs fixed below were precisely
|
|
# about the WRONG one being reported (a corrupt manifest blamed on six unlisted plugin
|
|
# directories, a legal manifest blamed for unresolvable paths). Exit-code-only
|
|
# assertions cannot see that, so these cases assert on the message text.
|
|
RUN_OUT=""
|
|
RUN_RC=0
|
|
run_script() { RUN_OUT="$(bash "$SCRIPT" "$1" 2>&1)" && RUN_RC=0 || RUN_RC=$?; }
|
|
|
|
# assert_fails_with <fixture> <label> <expected substring>...
|
|
assert_fails_with() {
|
|
local fixture="$1" label="$2"
|
|
shift 2
|
|
run_script "$fixture"
|
|
if [[ $RUN_RC -eq 0 ]]; then
|
|
fail "$label -- expected exit 1, got 0. Output: $RUN_OUT"
|
|
return
|
|
fi
|
|
local needle
|
|
for needle in "$@"; do
|
|
if [[ "$RUN_OUT" != *"$needle"* ]]; then
|
|
fail "$label -- exited $RUN_RC but message lacked '$needle'. Output: $RUN_OUT"
|
|
return
|
|
fi
|
|
done
|
|
pass "$label"
|
|
}
|
|
|
|
# assert_passes <fixture> <label>
|
|
assert_passes() {
|
|
run_script "$1"
|
|
if [[ $RUN_RC -eq 0 ]]; then
|
|
pass "$2"
|
|
else
|
|
fail "$2 -- expected exit 0, got $RUN_RC. Output: $RUN_OUT"
|
|
fi
|
|
}
|
|
|
|
# Writes a marketplace.json listing every "<name>=<source>" pair given.
|
|
write_marketplace() {
|
|
local dir="$1" entries="" pair name src
|
|
shift
|
|
for pair in "$@"; do
|
|
name="${pair%%=*}"
|
|
src="${pair#*=}"
|
|
entries+="${entries:+,}"$'\n'" { \"name\": \"$name\", \"source\": \"$src\" }"
|
|
done
|
|
mkdir -p "$dir/.claude-plugin"
|
|
printf '{\n "name": "test-marketplace",\n "plugins": [%s\n ]\n}\n' "$entries" > "$dir/.claude-plugin/marketplace.json"
|
|
}
|
|
|
|
# One trap over a registry rather than a fresh `trap 'rm -rf "$FIXTUREn"' EXIT`
|
|
# per fixture: each such trap REPLACES the previous one, so only the last
|
|
# fixture was ever cleaned and the rest leaked into TMPDIR every run. Same
|
|
# pattern as tests/test-check-vale-style-sync.sh and
|
|
# tests/test-check-scope-walkup-sync.sh; the emptiness guard is there because
|
|
# bash 3.2 treats "${arr[@]}" on an empty array as unbound under `set -u`.
|
|
FIXTURES=()
|
|
cleanup() { [[ ${#FIXTURES[@]} -eq 0 ]] || rm -rf "${FIXTURES[@]}"; }
|
|
trap cleanup EXIT
|
|
|
|
# Helper: make a minimal valid repo fixture with marketplace + plugin structure.
|
|
# Per ADR-0015/ADR-0017, the manifest check-manifests.sh validates is
|
|
# .claude-plugin/plugin.json (compiled output) -- not the root-level plugin.json,
|
|
# which was deleted repo-wide, and not the skills/hooks/mcpServers/agents pointer
|
|
# fields apm's compiler deliberately never populates (see scripts/check-manifests.sh's
|
|
# own header comment). Content-presence drift is scripts/sync-plugin-content.sh's job.
|
|
make_valid_fixture() {
|
|
local dir
|
|
dir="$(mktemp -d)"
|
|
mkdir -p "$dir/.claude-plugin"
|
|
mkdir -p "$dir/plugins/myplugin/.claude-plugin"
|
|
cat > "$dir/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "myplugin", "source": "./plugins/myplugin" }
|
|
]
|
|
}
|
|
JSON
|
|
cat > "$dir/plugins/myplugin/.claude-plugin/plugin.json" <<'JSON'
|
|
{
|
|
"name": "myplugin"
|
|
}
|
|
JSON
|
|
echo "$dir"
|
|
}
|
|
|
|
# --- 1. Exits 0 against valid repo structure ---
|
|
echo ""
|
|
echo "--- exits 0 when all references are valid ---"
|
|
FIXTURE="$(make_valid_fixture)"
|
|
FIXTURES+=("$FIXTURE")
|
|
if bash "$SCRIPT" "$FIXTURE" > /dev/null 2>&1; then
|
|
pass "exits 0 when all manifest references resolve"
|
|
else
|
|
fail "exited non-zero against a valid fixture"
|
|
fi
|
|
|
|
# --- 2. Exits 1 when plugin source dir is missing ---
|
|
echo ""
|
|
echo "--- exits 1 when plugin source directory missing ---"
|
|
FIXTURE2="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE2")
|
|
mkdir -p "$FIXTURE2/.claude-plugin"
|
|
cat > "$FIXTURE2/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "ghost", "source": "./plugins/ghost" }
|
|
]
|
|
}
|
|
JSON
|
|
if bash "$SCRIPT" "$FIXTURE2" > /dev/null 2>&1; then
|
|
fail "exited 0 when plugin source dir is missing — expected exit 1"
|
|
else
|
|
pass "exits non-zero when plugin source directory does not exist"
|
|
fi
|
|
|
|
# --- 3. Exits 1 when .claude-plugin/plugin.json is missing from plugin dir ---
|
|
echo ""
|
|
echo "--- exits 1 when .claude-plugin/plugin.json missing from plugin directory ---"
|
|
FIXTURE3="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE3")
|
|
mkdir -p "$FIXTURE3/.claude-plugin"
|
|
mkdir -p "$FIXTURE3/plugins/nomanifest"
|
|
cat > "$FIXTURE3/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "nomanifest", "source": "./plugins/nomanifest" }
|
|
]
|
|
}
|
|
JSON
|
|
if bash "$SCRIPT" "$FIXTURE3" > /dev/null 2>&1; then
|
|
fail "exited 0 when .claude-plugin/plugin.json is missing — expected exit 1"
|
|
else
|
|
pass "exits non-zero when .claude-plugin/plugin.json is missing from plugin directory"
|
|
fi
|
|
|
|
# --- 4. Exits 1 when a remote-source plugin entry's local plugin still lacks a manifest ---
|
|
# Remote sources (object-typed `source:`) are skipped entirely; only string (local path)
|
|
# sources are checked. This guards that a mixed marketplace.json still catches a broken
|
|
# local entry alongside a legitimately-skipped remote one.
|
|
echo ""
|
|
echo "--- exits 1 for a broken local entry even when a remote entry is present ---"
|
|
FIXTURE4="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE4")
|
|
mkdir -p "$FIXTURE4/.claude-plugin"
|
|
mkdir -p "$FIXTURE4/plugins/broken"
|
|
cat > "$FIXTURE4/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "remote-thing", "source": { "repo": "someorg/somerepo", "source": "github" } },
|
|
{ "name": "broken", "source": "./plugins/broken" }
|
|
]
|
|
}
|
|
JSON
|
|
if bash "$SCRIPT" "$FIXTURE4" > /dev/null 2>&1; then
|
|
fail "exited 0 with a broken local entry present — expected exit 1"
|
|
else
|
|
pass "exits non-zero for a broken local entry even alongside a skipped remote entry"
|
|
fi
|
|
|
|
# --- 5. Non-.apm/ plugin with a broken pointer field is caught by the fallback path ---
|
|
# apm-native plugins (.apm/ present) get their skills/hooks/mcpServers/agents
|
|
# pointer-field validation from sync-plugin-content.sh --check instead (see this
|
|
# script's header comment) -- but that script skips any plugin dir lacking .apm/
|
|
# outright, so a non-apm plugin's hand-authored plugin.json needs this script's own
|
|
# fallback validation to catch a broken pointer field.
|
|
echo ""
|
|
echo "--- catches a broken pointer field in a non-apm plugin's plugin.json ---"
|
|
FIXTURE5="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE5")
|
|
mkdir -p "$FIXTURE5/.claude-plugin"
|
|
mkdir -p "$FIXTURE5/plugins/legacy/.claude-plugin"
|
|
cat > "$FIXTURE5/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "legacy", "source": "./plugins/legacy" }
|
|
]
|
|
}
|
|
JSON
|
|
cat > "$FIXTURE5/plugins/legacy/.claude-plugin/plugin.json" <<'JSON'
|
|
{
|
|
"name": "legacy",
|
|
"skills": ["./skills/does-not-exist"]
|
|
}
|
|
JSON
|
|
if bash "$SCRIPT" "$FIXTURE5" > /dev/null 2>&1; then
|
|
fail "exited 0 for a non-apm plugin with a broken skills pointer -- expected exit 1"
|
|
else
|
|
pass "catches a broken skills pointer field in a non-apm (no .apm/) plugin.json"
|
|
fi
|
|
|
|
# --- 6. Non-.apm/ plugin with valid pointer fields still passes (no false positive) ---
|
|
echo ""
|
|
echo "--- a non-apm plugin with valid pointer fields still passes ---"
|
|
FIXTURE6="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE6")
|
|
mkdir -p "$FIXTURE6/.claude-plugin"
|
|
mkdir -p "$FIXTURE6/plugins/legacy-ok/.claude-plugin"
|
|
mkdir -p "$FIXTURE6/plugins/legacy-ok/skills/real-skill"
|
|
cat > "$FIXTURE6/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "legacy-ok", "source": "./plugins/legacy-ok" }
|
|
]
|
|
}
|
|
JSON
|
|
cat > "$FIXTURE6/plugins/legacy-ok/.claude-plugin/plugin.json" <<'JSON'
|
|
{
|
|
"name": "legacy-ok",
|
|
"skills": ["./skills/real-skill"]
|
|
}
|
|
JSON
|
|
if bash "$SCRIPT" "$FIXTURE6" > /dev/null 2>&1; then
|
|
pass "a non-apm plugin with a resolving skills pointer passes"
|
|
else
|
|
fail "exited non-zero for a non-apm plugin whose pointer fields all resolve"
|
|
fi
|
|
|
|
# --- 7. An .apm/ plugin with a broken pointer field is NOT caught here (delegated) ---
|
|
# Guards against the fallback path in finding #6 accidentally widening to also
|
|
# validate apm-native plugins, which would duplicate (and could disagree with)
|
|
# sync-plugin-content.sh --check's own drift detection.
|
|
echo ""
|
|
echo "--- an apm-native plugin's pointer fields are left to sync-plugin-content.sh --check ---"
|
|
FIXTURE7="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE7")
|
|
mkdir -p "$FIXTURE7/.claude-plugin"
|
|
mkdir -p "$FIXTURE7/plugins/apm-plugin/.claude-plugin"
|
|
mkdir -p "$FIXTURE7/plugins/apm-plugin/.apm"
|
|
cat > "$FIXTURE7/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "apm-plugin", "source": "./plugins/apm-plugin" }
|
|
]
|
|
}
|
|
JSON
|
|
cat > "$FIXTURE7/plugins/apm-plugin/.claude-plugin/plugin.json" <<'JSON'
|
|
{
|
|
"name": "apm-plugin",
|
|
"skills": ["./skills/does-not-exist"]
|
|
}
|
|
JSON
|
|
if bash "$SCRIPT" "$FIXTURE7" > /dev/null 2>&1; then
|
|
pass "an apm-native plugin (has .apm/) is not checked here, even with a broken pointer field"
|
|
else
|
|
fail "check-manifests.sh failed on an apm-native plugin -- pointer-field validation should be delegated, not duplicated"
|
|
fi
|
|
|
|
# --- 8. Disk -> marketplace: an apm package dir with no marketplace entry is caught ---
|
|
# Both this script and sync-plugin-content.sh --all derive their plugin set from
|
|
# marketplace.json, so before this check an unlisted plugins/<name>/ was skipped by
|
|
# every marketplace-derived gate at once while still being globbed by the
|
|
# validate-plugins pre-commit hook -- two different notions of "the plugin set".
|
|
# Per ADR-0015 marketplace.json is compiled from root apm.yml's marketplace.packages[],
|
|
# so an on-disk apm package missing from it is compiled-output drift.
|
|
echo ""
|
|
echo "--- exits 1 for a plugins/<name>/ apm package with no marketplace entry ---"
|
|
FIXTURE8="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE8")
|
|
mkdir -p "$FIXTURE8/.claude-plugin"
|
|
mkdir -p "$FIXTURE8/plugins/listed/.claude-plugin"
|
|
mkdir -p "$FIXTURE8/plugins/orphan/.claude-plugin" "$FIXTURE8/plugins/orphan/.apm/skills"
|
|
cat > "$FIXTURE8/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "listed", "source": "./plugins/listed" }
|
|
]
|
|
}
|
|
JSON
|
|
echo '{ "name": "listed" }' > "$FIXTURE8/plugins/listed/.claude-plugin/plugin.json"
|
|
echo '{ "name": "orphan" }' > "$FIXTURE8/plugins/orphan/.claude-plugin/plugin.json"
|
|
printf 'name: orphan\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE8/plugins/orphan/apm.yml"
|
|
if bash "$SCRIPT" "$FIXTURE8" > /dev/null 2>&1; then
|
|
fail "exited 0 for an on-disk apm package absent from marketplace.json -- expected exit 1"
|
|
else
|
|
pass "catches a plugins/<name>/ apm package that produced no marketplace entry"
|
|
fi
|
|
|
|
# --- 9. A plugins/<name>/ dir with none of the three plugin markers is not flagged ---
|
|
# The trigger is apm.yml || .apm/ || .claude-plugin/plugin.json -- broad enough to match
|
|
# the plugins/*/ set the validate-plugins hook globs, which is the disagreement this check
|
|
# closes. A directory carrying none of the three is scratch and stays out of scope.
|
|
echo ""
|
|
echo "--- a plugins/<name>/ directory with none of the three plugin markers is not flagged ---"
|
|
FIXTURE9="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE9")
|
|
mkdir -p "$FIXTURE9/.claude-plugin"
|
|
mkdir -p "$FIXTURE9/plugins/listed/.claude-plugin"
|
|
mkdir -p "$FIXTURE9/plugins/scratch/notes"
|
|
cat > "$FIXTURE9/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "listed", "source": "./plugins/listed" }
|
|
]
|
|
}
|
|
JSON
|
|
echo '{ "name": "listed" }' > "$FIXTURE9/plugins/listed/.claude-plugin/plugin.json"
|
|
if bash "$SCRIPT" "$FIXTURE9" > /dev/null 2>&1; then
|
|
pass "a plugins/<name>/ directory with no plugin markers is left alone"
|
|
else
|
|
fail "flagged a non-package directory under plugins/ -- expected exit 0"
|
|
fi
|
|
|
|
# --- 9b. Each of the three markers on its own is enough to trigger the check ---
|
|
# Keying only off apm.yml would leave a plugin dir carrying just .apm/ or just a
|
|
# compiled .claude-plugin/plugin.json invisible -- exactly the class of gap this
|
|
# check exists to close, since validate-plugins would still glob it.
|
|
marker_case() {
|
|
local label="$1" marker_setup="$2" dir
|
|
dir="$(mktemp -d)"
|
|
FIXTURES+=("$dir")
|
|
mkdir -p "$dir/.claude-plugin" "$dir/plugins/listed/.claude-plugin" "$dir/plugins/orphan"
|
|
cat > "$dir/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "listed", "source": "./plugins/listed" }
|
|
]
|
|
}
|
|
JSON
|
|
echo '{ "name": "listed" }' > "$dir/plugins/listed/.claude-plugin/plugin.json"
|
|
case "$marker_setup" in
|
|
apm-dir) mkdir -p "$dir/plugins/orphan/.apm/skills" ;;
|
|
plugin-json)
|
|
mkdir -p "$dir/plugins/orphan/.claude-plugin"
|
|
echo '{ "name": "orphan" }' > "$dir/plugins/orphan/.claude-plugin/plugin.json"
|
|
;;
|
|
esac
|
|
if bash "$SCRIPT" "$dir" > /dev/null 2>&1; then
|
|
fail "an unlisted plugin dir carrying only $label was not flagged"
|
|
else
|
|
pass "an unlisted plugin dir carrying only $label is flagged"
|
|
fi
|
|
}
|
|
echo ""
|
|
echo "--- .apm/ alone and .claude-plugin/plugin.json alone each trigger the check ---"
|
|
marker_case ".apm/" apm-dir
|
|
marker_case ".claude-plugin/plugin.json" plugin-json
|
|
|
|
# --- 9c. A vendored plugin declared with a remote-object source: is already listed ---
|
|
# list_marketplace_local_plugins deliberately skips remote-object entries, so a
|
|
# path-only listed/unlisted match reported a missing entry for a directory whose
|
|
# entry is in fact right there -- telling the author to add what already exists.
|
|
# The name axis of the match closes that.
|
|
echo ""
|
|
echo "--- a vendored plugin whose marketplace entry uses a remote source: is not flagged ---"
|
|
FIXTURE9C="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE9C")
|
|
mkdir -p "$FIXTURE9C/.claude-plugin" "$FIXTURE9C/plugins/vendored/.claude-plugin"
|
|
cat > "$FIXTURE9C/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "vendored", "source": { "repo": "someorg/somerepo", "source": "github" } }
|
|
]
|
|
}
|
|
JSON
|
|
echo '{ "name": "vendored" }' > "$FIXTURE9C/plugins/vendored/.claude-plugin/plugin.json"
|
|
printf 'name: vendored\nversion: 1.2.3\ntype: skill\n' > "$FIXTURE9C/plugins/vendored/apm.yml"
|
|
if bash "$SCRIPT" "$FIXTURE9C" > /dev/null 2>&1; then
|
|
pass "a vendored dir matching a remote-source entry's name counts as listed"
|
|
else
|
|
fail "flagged a vendored plugin that already has a remote-source marketplace entry"
|
|
fi
|
|
|
|
# --- 9d. The name axis must NOT rescue an orphan via a LOCAL entry's name ---
|
|
# A local entry's name need not equal the basename of the directory it points at. An
|
|
# entry named "beta" pointing at ./plugins/alpha must not mark an unrelated, entirely
|
|
# unlisted plugins/beta/ as listed -- local entries match on their exact path, so
|
|
# extending the name fallback to them just reopens the gap this check exists to close.
|
|
echo ""
|
|
echo "--- a local entry's name does not rescue a same-named but unlisted directory ---"
|
|
FIXTURE9D="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE9D")
|
|
mkdir -p "$FIXTURE9D/.claude-plugin" "$FIXTURE9D/plugins/alpha/.claude-plugin" "$FIXTURE9D/plugins/beta"
|
|
cat > "$FIXTURE9D/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "beta", "source": "./plugins/alpha" }
|
|
]
|
|
}
|
|
JSON
|
|
echo '{ "name": "alpha" }' > "$FIXTURE9D/plugins/alpha/.claude-plugin/plugin.json"
|
|
printf 'name: beta\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE9D/plugins/beta/apm.yml"
|
|
if bash "$SCRIPT" "$FIXTURE9D" > /dev/null 2>&1; then
|
|
fail "an unlisted plugins/beta/ was rescued by an unrelated local entry named beta -- expected exit 1"
|
|
else
|
|
pass "an unlisted directory is not rescued by a local entry that merely shares its name"
|
|
fi
|
|
|
|
# --- 10. Marketplace `source:` spelling variants still count as "listed" ---
|
|
# The disk -> marketplace comparison canonicalizes both sides, so `plugins/x` and
|
|
# `./plugins/x/` must resolve to the same directory as the glob's `plugins/x/`.
|
|
#
|
|
# The entry names deliberately DIFFER from the directory basenames. With names equal to
|
|
# basenames this fixture proved nothing whenever the name axis was permissive: deleting
|
|
# the canonicalization entirely still left it passing, because the name match rescued it.
|
|
# Restricting the name axis to remote entries fixed that, but making the names differ is
|
|
# what keeps this assertion honest independently of that restriction.
|
|
echo ""
|
|
echo "--- a marketplace source without ./ or with a trailing slash still counts as listed ---"
|
|
FIXTURE10="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE10")
|
|
mkdir -p "$FIXTURE10/.claude-plugin"
|
|
mkdir -p "$FIXTURE10/plugins/bare/.claude-plugin" "$FIXTURE10/plugins/trailing/.claude-plugin"
|
|
cat > "$FIXTURE10/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "bare-entry", "source": "plugins/bare" },
|
|
{ "name": "trailing-entry", "source": "./plugins/trailing/" }
|
|
]
|
|
}
|
|
JSON
|
|
echo '{ "name": "bare" }' > "$FIXTURE10/plugins/bare/.claude-plugin/plugin.json"
|
|
echo '{ "name": "trailing" }' > "$FIXTURE10/plugins/trailing/.claude-plugin/plugin.json"
|
|
printf 'name: bare\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE10/plugins/bare/apm.yml"
|
|
printf 'name: trailing\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE10/plugins/trailing/apm.yml"
|
|
if bash "$SCRIPT" "$FIXTURE10" > /dev/null 2>&1; then
|
|
pass "source: spelling variants are canonicalized before the listed/unlisted comparison"
|
|
else
|
|
fail "flagged a listed plugin because its source: string was spelled differently"
|
|
fi
|
|
|
|
# --- 11. A marketplace entry with no `source` at all is rejected outright ---
|
|
# It used to disable BOTH directions of the check for that plugin at once:
|
|
# list_marketplace_local_plugins requires a string `source`, so the entry was skipped and
|
|
# its .claude-plugin/plugin.json never checked; and the disk -> marketplace name axis
|
|
# selected on `(.source | type) != "string"`, which is TRUE for null, so the same entry
|
|
# also marked its on-disk directory "listed". Net effect: a plugin with a broken manifest
|
|
# and a malformed entry passed clean, and silently dropped out of
|
|
# sync-plugin-content.sh --all's work list too, since that derives from the same helper.
|
|
echo ""
|
|
echo "--- a marketplace entry with no source: field is a hard error ---"
|
|
FIXTURE11="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE11")
|
|
mkdir -p "$FIXTURE11/.claude-plugin" "$FIXTURE11/plugins/lint"
|
|
cat > "$FIXTURE11/.claude-plugin/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "lint" }
|
|
]
|
|
}
|
|
JSON
|
|
printf 'name: lint\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE11/plugins/lint/apm.yml"
|
|
assert_fails_with "$FIXTURE11" \
|
|
"an entry with no source: is reported by name instead of silently disabling both checks" \
|
|
'`source` is neither a local path string nor a remote source object' 'lint (source: null)'
|
|
|
|
# --- 11b. Any other unclassifiable `source` is rejected the same way ---
|
|
# The guard used to test `.source == null` specifically, so every OTHER malformed value
|
|
# reached exactly the state the null case was fixed for: `"source": 42` passed the
|
|
# assert, was skipped by list_marketplace_local_plugins for not being a string, AND was
|
|
# rescued by the disk -> marketplace name axis (whose select was the denylist
|
|
# `(.source|type) != "string"`, true for a number). Verbatim the same defect, one value
|
|
# over. Only two shapes are classifiable -- a local path string and a remote source
|
|
# object -- so the guard is typed as "neither of those", not as a list of known-bad
|
|
# values.
|
|
echo ""
|
|
echo "--- a non-string, non-object source: is rejected by type, not by enumerating null ---"
|
|
for BAD_SOURCE in '42' '[]' 'true'; do
|
|
FIXTURE11B="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE11B")
|
|
mkdir -p "$FIXTURE11B/.claude-plugin" "$FIXTURE11B/plugins/lint"
|
|
printf '{ "name": "test-marketplace", "plugins": [ { "name": "lint", "source": %s } ] }\n' \
|
|
"$BAD_SOURCE" > "$FIXTURE11B/.claude-plugin/marketplace.json"
|
|
printf 'name: lint\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE11B/plugins/lint/apm.yml"
|
|
assert_fails_with "$FIXTURE11B" \
|
|
"a source: of $BAD_SOURCE is rejected instead of silently disabling both checks" \
|
|
'`source` is neither a local path string nor a remote source object' 'lint (source:'
|
|
done
|
|
|
|
# --- 11c. The disk -> marketplace name axis, exercised WITHOUT the precondition ---
|
|
# This is the one assertion that cannot go through bash "$SCRIPT": every malformed entry
|
|
# the select must reject is rejected first by assert_marketplace_manifest_usable, which
|
|
# exits before the select ever runs. So reverting the select alone left the whole suite
|
|
# green -- the code carried a comment claiming it "must not depend on that check running
|
|
# first", and nothing tested that independence. Call the function directly instead.
|
|
#
|
|
# The invariant: the name axis exists solely for a plugin vendored on disk under a
|
|
# REMOTE (object) `source:`, which has no local path to match on. Every other shape --
|
|
# a local string (which matches by path and needs no name fallback, see case 9d) and
|
|
# every unclassifiable value -- must produce no name at all.
|
|
echo ""
|
|
echo "--- list_marketplace_remote_plugin_names emits object-source names only ---"
|
|
# shellcheck source=scripts/lib/marketplace-plugins.sh
|
|
source "$REPO_ROOT/scripts/lib/marketplace-plugins.sh"
|
|
FIXTURE11C="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE11C")
|
|
cat > "$FIXTURE11C/marketplace.json" <<'JSON'
|
|
{
|
|
"name": "test-marketplace",
|
|
"plugins": [
|
|
{ "name": "remote-obj", "source": { "repo": "someorg/somerepo", "source": "github" } },
|
|
{ "name": "local-str", "source": "./plugins/local-str" },
|
|
{ "name": "null-src" },
|
|
{ "name": "explicit-null", "source": null },
|
|
{ "name": "number-src", "source": 42 },
|
|
{ "name": "array-src", "source": [] },
|
|
{ "name": "bool-src", "source": true }
|
|
]
|
|
}
|
|
JSON
|
|
NAMES11C="$(list_marketplace_remote_plugin_names "$FIXTURE11C/marketplace.json")"
|
|
if [[ "$NAMES11C" == "remote-obj" ]]; then
|
|
pass "only the remote object-source entry yields a name for the disk -> marketplace name axis"
|
|
else
|
|
fail "the name axis emitted $(printf '%s' "$NAMES11C" | tr '\n' ' ')— expected exactly 'remote-obj'; every other shape would rescue a same-named orphan directory"
|
|
fi
|
|
|
|
# --- 11d. A valid-JSON, non-object marketplace root is named, not left to crash jq ---
|
|
# `jq empty` passes on `[]`, `"x"` and `123`; the `.plugins` lookup on the next line then
|
|
# died with a raw `jq: error: Cannot index array with string "plugins"` and rc=5,
|
|
# attributed to nothing at all.
|
|
echo ""
|
|
echo "--- a valid-JSON non-object marketplace root is reported as such ---"
|
|
FIXTURE11D="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE11D")
|
|
mkdir -p "$FIXTURE11D/.claude-plugin" "$FIXTURE11D/plugins/one"
|
|
printf '[]\n' > "$FIXTURE11D/.claude-plugin/marketplace.json"
|
|
printf 'name: one\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE11D/plugins/one/apm.yml"
|
|
assert_fails_with "$FIXTURE11D" \
|
|
"a JSON array at the marketplace root is named as a root-shape error" \
|
|
'is a JSON array at its top level'
|
|
|
|
# --- 12. A `skills` string (a legal shape per the host docs) is resolved, not counted ---
|
|
# `jq '.skills | if . then length else 0 end'` is null-safe but not type-safe: on the
|
|
# string "./skills/x" it returned the CHARACTER count, and the `.skills[0]` that followed
|
|
# errored ("Cannot index string with number"), killing the whole script under `set -e`
|
|
# with no "Manifest check failed:" line -- and every plugin later in the marketplace
|
|
# unchecked. Both configuration.md references document `skills` as string | string[].
|
|
echo ""
|
|
echo "--- a string-valued skills field resolves instead of crashing the script ---"
|
|
FIXTURE12="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE12")
|
|
mkdir -p "$FIXTURE12/plugins/strskills/.claude-plugin" "$FIXTURE12/plugins/strskills/custom/skills"
|
|
write_marketplace "$FIXTURE12" "strskills=./plugins/strskills"
|
|
cat > "$FIXTURE12/plugins/strskills/.claude-plugin/plugin.json" <<'JSON'
|
|
{
|
|
"name": "strskills",
|
|
"skills": "./custom/skills/"
|
|
}
|
|
JSON
|
|
assert_passes "$FIXTURE12" "a resolving string-valued skills field passes"
|
|
|
|
# --- 13. A broken string `skills` is reported, and later plugins are still checked ---
|
|
# The mid-loop `set -e` abort meant a fault in the FIRST plugin hid every fault after it.
|
|
# The second entry here is broken in an unrelated way; both messages must appear.
|
|
echo ""
|
|
echo "--- a broken string skills field is reported without aborting the marketplace walk ---"
|
|
FIXTURE13="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE13")
|
|
mkdir -p "$FIXTURE13/plugins/first/.claude-plugin" "$FIXTURE13/plugins/second"
|
|
write_marketplace "$FIXTURE13" "first=./plugins/first" "second=./plugins/second"
|
|
cat > "$FIXTURE13/plugins/first/.claude-plugin/plugin.json" <<'JSON'
|
|
{
|
|
"name": "first",
|
|
"skills": "./skills/does-not-exist"
|
|
}
|
|
JSON
|
|
assert_fails_with "$FIXTURE13" \
|
|
"a broken string skills field is reported and the walk continues to later plugins" \
|
|
'skills path not found: ./skills/does-not-exist' \
|
|
"plugin 'second': .claude-plugin/plugin.json not found" \
|
|
'Manifest check failed: 2 error(s)'
|
|
|
|
# --- 14. A genuinely wrong-typed `skills` is named as such, walk still continues ---
|
|
echo ""
|
|
echo "--- a wrong-typed skills field is reported as a type error, not a missing path ---"
|
|
FIXTURE14="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE14")
|
|
mkdir -p "$FIXTURE14/plugins/first/.claude-plugin" "$FIXTURE14/plugins/second"
|
|
write_marketplace "$FIXTURE14" "first=./plugins/first" "second=./plugins/second"
|
|
cat > "$FIXTURE14/plugins/first/.claude-plugin/plugin.json" <<'JSON'
|
|
{
|
|
"name": "first",
|
|
"skills": 42
|
|
}
|
|
JSON
|
|
assert_fails_with "$FIXTURE14" \
|
|
"a wrong-typed skills field names the type and does not abort the walk" \
|
|
'skills must be a path string, an array of path strings, or an inline object, got number' \
|
|
"plugin 'second': .claude-plugin/plugin.json not found" \
|
|
'Manifest check failed: 2 error(s)'
|
|
|
|
# --- 15. Array- and object-valued pointer fields that resolve are not reported missing ---
|
|
# `ref=$(jq -r ".$field // empty")` returned the PRETTY-PRINTED JSON for an array or an
|
|
# object, which `[[ ! -e ]]` then rejected: a manifest whose paths all resolve was
|
|
# reported broken. Both host docs give `agents` as string | string[] and `hooks` /
|
|
# `mcpServers` as string | object (an inline definition, with no path to resolve).
|
|
echo ""
|
|
echo "--- array- and inline-object pointer fields that resolve are accepted ---"
|
|
FIXTURE15="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE15")
|
|
mkdir -p "$FIXTURE15/plugins/shapes/.claude-plugin" "$FIXTURE15/plugins/shapes/agents" "$FIXTURE15/plugins/shapes/skills/one"
|
|
touch "$FIXTURE15/plugins/shapes/agents/real.md"
|
|
write_marketplace "$FIXTURE15" "shapes=./plugins/shapes"
|
|
cat > "$FIXTURE15/plugins/shapes/.claude-plugin/plugin.json" <<'JSON'
|
|
{
|
|
"name": "shapes",
|
|
"skills": ["./skills/one"],
|
|
"agents": ["./agents/real.md"],
|
|
"hooks": { "PreToolUse": [{ "hooks": [{ "type": "command", "command": "true" }] }] },
|
|
"mcpServers": { "demo": { "command": "true" } }
|
|
}
|
|
JSON
|
|
assert_passes "$FIXTURE15" \
|
|
"an array-valued agents and an inline-object hooks/mcpServers are not reported missing"
|
|
|
|
# --- 16. A broken element inside an array-valued pointer field is still caught ---
|
|
# Guards the fix in #15 against over-correcting into "arrays are always fine".
|
|
echo ""
|
|
echo "--- a broken path inside an array-valued pointer field is still caught ---"
|
|
FIXTURE16="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE16")
|
|
mkdir -p "$FIXTURE16/plugins/shapes/.claude-plugin" "$FIXTURE16/plugins/shapes/agents"
|
|
touch "$FIXTURE16/plugins/shapes/agents/real.md"
|
|
write_marketplace "$FIXTURE16" "shapes=./plugins/shapes"
|
|
cat > "$FIXTURE16/plugins/shapes/.claude-plugin/plugin.json" <<'JSON'
|
|
{
|
|
"name": "shapes",
|
|
"agents": ["./agents/real.md", "./agents/ghost.md"]
|
|
}
|
|
JSON
|
|
assert_fails_with "$FIXTURE16" \
|
|
"a missing path in an array-valued agents field is reported with its own path" \
|
|
'agents path not found: ./agents/ghost.md'
|
|
|
|
# --- 17. An unparseable marketplace.json is reported as such, not as unlisted plugins ---
|
|
# The walk runs in a process substitution, so the helper's `set -e` abort on invalid JSON
|
|
# never reached the caller. The run still exited 1 -- backstopped by the disk -> marketplace
|
|
# pass -- but printed one "has no entry in .claude-plugin/marketplace.json ... add it to
|
|
# root apm.yml" per plugin directory, sending the reader to edit apm.yml when the actual
|
|
# fault was a corrupt manifest.
|
|
echo ""
|
|
echo "--- an unparseable marketplace.json is attributed to the manifest, not to the plugins ---"
|
|
FIXTURE17="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE17")
|
|
mkdir -p "$FIXTURE17/.claude-plugin" "$FIXTURE17/plugins/one/.claude-plugin" "$FIXTURE17/plugins/two/.claude-plugin"
|
|
printf '{ "name": "test-marketplace", "plugins": [ { "name": "one",\n' > "$FIXTURE17/.claude-plugin/marketplace.json"
|
|
echo '{ "name": "one" }' > "$FIXTURE17/plugins/one/.claude-plugin/plugin.json"
|
|
echo '{ "name": "two" }' > "$FIXTURE17/plugins/two/.claude-plugin/plugin.json"
|
|
run_script "$FIXTURE17"
|
|
if [[ $RUN_RC -eq 0 ]]; then
|
|
fail "exited 0 on an unparseable marketplace.json -- expected exit 1"
|
|
elif [[ "$RUN_OUT" != *"is not valid JSON"* ]]; then
|
|
fail "an unparseable marketplace.json was not named as such. Output: $RUN_OUT"
|
|
elif [[ "$RUN_OUT" == *"has no entry in .claude-plugin/marketplace.json"* ]]; then
|
|
fail "an unparseable marketplace.json was misreported as unlisted plugin directories. Output: $RUN_OUT"
|
|
else
|
|
pass "an unparseable marketplace.json is reported as invalid JSON, not as unlisted plugin directories"
|
|
fi
|
|
|
|
# --- 18. A missing marketplace.json with plugins on disk is drift, not an opt-out ---
|
|
# `[[ ! -f "$MARKETPLACE" ]] && exit 0` was the same empty-set-reads-as-pass shape as the
|
|
# rest: per ADR-0015 the manifest is compiled from root apm.yml, so its absence next to
|
|
# on-disk packages means the compiled output is missing, and every marketplace-derived
|
|
# gate walks an empty plugin set in silence.
|
|
echo ""
|
|
echo "--- a missing marketplace.json alongside on-disk plugin directories fails ---"
|
|
FIXTURE18="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE18")
|
|
mkdir -p "$FIXTURE18/plugins/orphan/.apm/skills"
|
|
assert_fails_with "$FIXTURE18" \
|
|
"a missing marketplace.json with plugin directories present is reported as drift" \
|
|
'.claude-plugin/marketplace.json does not exist' \
|
|
'plugins/orphan'
|
|
|
|
# --- 18b. A missing marketplace.json with nothing to check still exits 0 ---
|
|
# Guards the fix above against over-correcting into "always fail without a manifest":
|
|
# a repo with no plugin directories genuinely has nothing for this gate to check.
|
|
echo ""
|
|
echo "--- a missing marketplace.json with no plugin directories still exits 0 ---"
|
|
FIXTURE18B="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE18B")
|
|
mkdir -p "$FIXTURE18B/plugins/scratch/notes" "$FIXTURE18B/docs"
|
|
assert_passes "$FIXTURE18B" \
|
|
"no marketplace.json and no plugin-marked directories is a genuine no-op, not a failure"
|
|
|
|
# --- 19. An unparseable per-plugin plugin.json is attributed, and the walk continues ---
|
|
# check_pointer_field reads the manifest with bare `$(jq ...)` assignments, so under
|
|
# `set -e` a parse failure aborted the whole script mid-loop: rc=5, a raw
|
|
# `jq: parse error` on stderr, no `Manifest check failed:` summary, and every plugin
|
|
# later in the marketplace silently unchecked. That is the same failure class the
|
|
# marketplace's own `jq empty` precondition closes -- and .claude-plugin/plugin.json is
|
|
# equally generated output, so it is equally capable of being corrupt.
|
|
#
|
|
# The second entry is broken in an unrelated way; both messages plus the summary must
|
|
# appear, which is what proves the walk survived the first fault.
|
|
echo ""
|
|
echo "--- an unparseable plugin.json is reported and does not abort the marketplace walk ---"
|
|
FIXTURE19="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE19")
|
|
mkdir -p "$FIXTURE19/plugins/corrupt/.claude-plugin" "$FIXTURE19/plugins/second"
|
|
write_marketplace "$FIXTURE19" "corrupt=./plugins/corrupt" "second=./plugins/second"
|
|
printf '{ "name": "corrupt",\n' > "$FIXTURE19/plugins/corrupt/.claude-plugin/plugin.json"
|
|
assert_fails_with "$FIXTURE19" \
|
|
"an unparseable plugin.json is named and later plugins are still checked" \
|
|
"plugin 'corrupt': .claude-plugin/plugin.json is not valid JSON" \
|
|
"plugin 'second': .claude-plugin/plugin.json not found" \
|
|
'Manifest check failed: 2 error(s)'
|
|
|
|
# --- 19b. A valid-JSON but non-object plugin.json is caught too ---
|
|
# `jq empty` passes on `[]`; it is the `.skills` lookup on such a root that aborts
|
|
# ("Cannot index array with string"), not the parse -- so the parse check alone would
|
|
# leave this exact crash reachable.
|
|
echo ""
|
|
echo "--- a valid-JSON non-object plugin.json is reported, not left to crash jq ---"
|
|
FIXTURE19B="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE19B")
|
|
mkdir -p "$FIXTURE19B/plugins/arrayjson/.claude-plugin" "$FIXTURE19B/plugins/second"
|
|
write_marketplace "$FIXTURE19B" "arrayjson=./plugins/arrayjson" "second=./plugins/second"
|
|
printf '[]\n' > "$FIXTURE19B/plugins/arrayjson/.claude-plugin/plugin.json"
|
|
assert_fails_with "$FIXTURE19B" \
|
|
"a non-object plugin.json is named by type and later plugins are still checked" \
|
|
"plugin 'arrayjson': .claude-plugin/plugin.json is a JSON array at its top level" \
|
|
"plugin 'second': .claude-plugin/plugin.json not found" \
|
|
'Manifest check failed: 2 error(s)'
|
|
|
|
# --- 20. Run with no argument outside a worktree: refuse, do not guess $PWD ---
|
|
# Every path this script touches hangs off REPO_ROOT, and its exit-0 path is "no
|
|
# manifest and nothing on disk" -- so `|| pwd` made a run from an empty directory
|
|
# outside any worktree exit 0, silently, having inspected no repository at all. Same
|
|
# reasoning as scripts/sync-marketplace-mirror.sh, which dropped its fallback first.
|
|
#
|
|
# `env -u GIT_DIR -u GIT_WORK_TREE` because run-tests.sh runs as a pre-push hook and git
|
|
# hooks export both, which would re-target `git rev-parse --show-toplevel` at the LIVE
|
|
# repo from any cwd -- making this case pass for the wrong reason.
|
|
echo ""
|
|
echo "--- with no argument outside a git worktree, it refuses instead of guessing \$PWD ---"
|
|
FIXTURE20="$(mktemp -d)"
|
|
FIXTURES+=("$FIXTURE20")
|
|
if (cd "$FIXTURE20" && env -u GIT_DIR -u GIT_WORK_TREE git rev-parse --show-toplevel) >/dev/null 2>&1; then
|
|
fail "fixture precondition: $FIXTURE20 is inside a git worktree, so this case cannot test the no-worktree path"
|
|
else
|
|
RC20=0
|
|
OUT20="$(cd "$FIXTURE20" && env -u GIT_DIR -u GIT_WORK_TREE bash "$SCRIPT" 2>&1)" || RC20=$?
|
|
if [[ $RC20 -eq 0 ]]; then
|
|
fail "exited 0 from outside a worktree with no argument -- it checked nothing and said so to no one"
|
|
elif [[ "$OUT20" != *"not inside a git worktree"* ]]; then
|
|
fail "exited $RC20 outside a worktree but not for the stated reason. Output: $OUT20"
|
|
else
|
|
pass "refuses to guess \$PWD when it cannot locate the repository root"
|
|
fi
|
|
fi
|
|
|
|
echo ""
|
|
echo "Results: $PASS passed, $FAIL failed"
|
|
[[ $FAIL -eq 0 ]]
|