fix(kyberforge): restore pointer-field validation for non-apm plugins
Skills/hooks/mcpServers/agents pointer-field validation in plugin.json was fully delegated to sync-plugin-content.sh --check, but that script explicitly skips any plugin directory lacking .apm/ (it has nothing to compile there). A plugin with no .apm/ and a hand-authored plugin.json whose pointer field points at a missing path was therefore left uncovered by either check -- currently latent since every plugin in this repo has .apm/, but a real gap for the first non-apm plugin added. Restores a fallback validation path here for exactly that case (no .apm/ directory), reusing the pre-delegation logic this script used to run unconditionally. apm-native plugins keep relying on the delegated check so the two never duplicate (or disagree) on the same manifest. Also switches the marketplace.json walk to the shared scripts/lib/marketplace-plugins.sh helper introduced alongside sync-plugin-content.sh's matching --all branch, replacing the near-identical hand-duplicated loop this script's own header comment already flagged as a duplication risk. Adds fixtures: a non-apm plugin with a broken skills pointer (caught), a non-apm plugin with a valid pointer (no false positive), and an apm-native plugin with a broken pointer (left to the delegated check, not double-validated here). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
This commit is contained in:
@@ -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,33 +35,51 @@ 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
|
||||
|
||||
# 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
|
||||
exit 1
|
||||
|
||||
@@ -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 ]]
|
||||
|
||||
Reference in New Issue
Block a user