diff --git a/scripts/check-manifests.sh b/scripts/check-manifests.sh index 40ee6ce..930d23e 100755 --- a/scripts/check-manifests.sh +++ b/scripts/check-manifests.sh @@ -7,9 +7,18 @@ set -euo pipefail # 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). This script no longer checks -# those fields; that's now scripts/sync-plugin-content.sh --check's job (drift between -# .apm/ and the flat plugin-root mirror), wired as its own pre-push hook. +# 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. REPO_ROOT="${1:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}" FAIL=0 @@ -26,32 +35,50 @@ if [[ ! -f "$MARKETPLACE" ]]; then exit 0 fi -plugin_count=$(jq '.plugins | length' "$MARKETPLACE") +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib/marketplace-plugins.sh +source "$SCRIPT_DIR/lib/marketplace-plugins.sh" -for ((i = 0; i < plugin_count; i++)); do - name=$(jq -r ".plugins[$i].name" "$MARKETPLACE") - source_type=$(jq -r ".plugins[$i].source | type" "$MARKETPLACE") - - # Remote sources (github, git, npm objects) have no local directory to check - if [[ "$source_type" != "string" ]]; then - continue - fi - - source=$(jq -r ".plugins[$i].source" "$MARKETPLACE") - source="${source#./}" - plugin_dir="$REPO_ROOT/$source" +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" + err "plugin '$name': source directory not found: $source_rel" continue fi manifest="$plugin_dir/.claude-plugin/plugin.json" if [[ ! -f "$manifest" ]]; then - err "plugin '$name': .claude-plugin/plugin.json not found in $source" + err "plugin '$name': .claude-plugin/plugin.json not found in $source_rel" continue fi -done + + # apm-native plugin: pointer-field validation is sync-plugin-content.sh --check's + # job (see header comment above). + [[ -d "$plugin_dir/.apm" ]] && continue + + # 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. + skill_count=$(jq '.skills | if . then length else 0 end' "$manifest") + for ((s = 0; s < skill_count; s++)); do + skill_path=$(jq -r ".skills[$s]" "$manifest") + full_path="$plugin_dir/$skill_path" + full_path="${full_path%/}" + if [[ ! -d "$full_path" ]]; then + err "plugin '$name': skills path not found: $skill_path" + fi + done + + for field in hooks mcpServers agents; do + ref=$(jq -r ".${field} // empty" "$manifest") + [[ -z "$ref" ]] && continue + full_path="$plugin_dir/$ref" + full_path="${full_path%/}" + if [[ ! -e "$full_path" ]]; then + err "plugin '$name': $field path not found: $ref" + fi + done +done < <(list_marketplace_local_plugins "$REPO_ROOT" "$MARKETPLACE") if [[ $FAIL -gt 0 ]]; then echo "Manifest check failed: $FAIL error(s)" >&2 diff --git a/tests/test-check-manifests.sh b/tests/test-check-manifests.sh index 633fc95..8180ff8 100644 --- a/tests/test-check-manifests.sh +++ b/tests/test-check-manifests.sh @@ -113,6 +113,97 @@ 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)" +trap 'rm -rf "$FIXTURE5"' EXIT +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)" +trap 'rm -rf "$FIXTURE6"' EXIT +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)" +trap 'rm -rf "$FIXTURE7"' EXIT +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 + echo "" echo "Results: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]]