fix(kyberforge): catch plugin.json and hooks.json drift in sync-plugin-content.sh --check
--check's throwaway pack copy seeded .claude-plugin/plugin.json and
.github/plugin/plugin.json from the real plugin dir, then packed without
--force -- apm pack silently skips regenerating a plugin.json that already
exists, so the diff always compared the copy against itself and never caught
drift in the compiled name/version/description/mcpServers. --force is now
always passed; in check mode it forces regeneration inside the throwaway copy
only, which sync_plugin_manifest() then diffs against the real committed
manifest.
sync_hooks_json() returned early whenever .apm/hooks/ was missing, without
checking whether a stale hooks.json was still sitting at the plugin root from
a prior sync -- unlike sync_dir(), which already detects that kind of orphaned
mirrored output. It now mirrors sync_dir()'s shape: flagged as drift in
--check, removed on a real sync.
Running the corrected --check --all against this repo's own plugins surfaced
3 real orphans: plugins/{git,gitea,core}/hooks.json, empty stubs added in
4edaaac only to satisfy an old plugin.json pointer-field check that no longer
exists (their compiled plugin.json has never had a hooks field, and none of
the three ever had .apm/hooks/). Removed as part of this fix since they're
exactly the drift the corrected check now catches -- leaving them would break
the sync-plugin-content pre-push gate on this branch.
Also extracts two shared helpers into scripts/lib/, sourced by this script and
others so a future bug fix doesn't need hand-applying three times:
- marketplace-plugins.sh: walks marketplace.json for local plugin dirs (this
script's --all branch and check-manifests.sh had near-identical copies)
- batch-run.sh: the bounded-batch concurrent job runner (this script,
tests/run-tests.sh, and tests/run-bats.sh each hand-rolled the same
core-count-capped wait loop independently)
Extended tests/test-sync-plugin-content.sh with coverage for both drift cases
(plugin.json version-bump drift, orphaned-hooks.json drift), including that a
re-sync clears each.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
This commit is contained in:
53
scripts/lib/batch-run.sh
Normal file
53
scripts/lib/batch-run.sh
Normal file
@@ -0,0 +1,53 @@
|
||||
#!/usr/bin/env bash
|
||||
# Shared bounded-batch concurrent job runner. Sourced by
|
||||
# scripts/sync-plugin-content.sh, tests/run-tests.sh, and tests/run-bats.sh so
|
||||
# their concurrency-cap and per-item log/status handling can't silently
|
||||
# diverge -- previously the same batching logic (core-count cap, per-item
|
||||
# log/status files, batched `wait`) was hand-implemented independently in all
|
||||
# three places.
|
||||
#
|
||||
# Batches (not a rolling pool) because a bounded rolling pool needs `wait -n`,
|
||||
# which is bash 4.3+ -- all three callers are explicitly bash-3.2-safe.
|
||||
# `getconf` over `nproc` for the same reason: `nproc` doesn't exist on macOS.
|
||||
#
|
||||
# Not meant to be executed directly -- source it.
|
||||
|
||||
# batch_jobs_limit
|
||||
# Prints the concurrency cap to use for batching.
|
||||
batch_jobs_limit() {
|
||||
getconf _NPROCESSORS_ONLN 2>/dev/null || echo 4
|
||||
}
|
||||
|
||||
# batch_run <scratch_dir> <key1> <cmd1> [<key2> <cmd2> ...]
|
||||
#
|
||||
# For each key/cmd pair, backgrounds `eval "$cmd"` with its combined
|
||||
# stdout+stderr redirected to "<scratch_dir>/<key>.log", bounded to at most
|
||||
# batch_jobs_limit concurrent jobs (waiting out the current batch before
|
||||
# starting the next).
|
||||
#
|
||||
# Each <cmd> owns writing its own result to "<scratch_dir>/<key>.status" --
|
||||
# this helper only owns dispatch/throttling and log capture, not status
|
||||
# semantics. Callers differ on how they do that (capturing $? of an external
|
||||
# command with `|| rc=$?`, or a sync function writing its own status flag
|
||||
# directly) -- both patterns are preserved as-is by callers, not standardized
|
||||
# here, so existing error-handling behavior (including how each pattern
|
||||
# interacts with `set -e` in the caller) is unchanged by this extraction.
|
||||
batch_run() {
|
||||
local scratch_dir="$1"
|
||||
shift
|
||||
local jobs_limit running key cmd
|
||||
jobs_limit="$(batch_jobs_limit)"
|
||||
running=0
|
||||
|
||||
while [[ $# -gt 0 ]]; do
|
||||
key="$1" cmd="$2"
|
||||
shift 2
|
||||
(eval "$cmd") >"$scratch_dir/$key.log" 2>&1 &
|
||||
running=$((running + 1))
|
||||
if [[ $running -ge $jobs_limit ]]; then
|
||||
wait
|
||||
running=0
|
||||
fi
|
||||
done
|
||||
wait
|
||||
}
|
||||
28
scripts/lib/marketplace-plugins.sh
Normal file
28
scripts/lib/marketplace-plugins.sh
Normal file
@@ -0,0 +1,28 @@
|
||||
#!/usr/bin/env bash
|
||||
# Shared helper: enumerates the local (string-source) plugin entries declared in
|
||||
# .claude-plugin/marketplace.json. Sourced by scripts/sync-plugin-content.sh
|
||||
# (--all) and scripts/check-manifests.sh so a future marketplace.json schema
|
||||
# change (e.g. a new source type) only has to be handled in one place instead
|
||||
# of drifting between two hand-maintained copies of the same walk.
|
||||
#
|
||||
# Requires jq. Not meant to be executed directly -- source it.
|
||||
|
||||
# list_marketplace_local_plugins <repo_root> <marketplace_json_path>
|
||||
#
|
||||
# Prints one "<name>\t<absolute_plugin_dir>" line per local (string `source:`)
|
||||
# marketplace entry. Remote sources (github/git/npm objects) have no local
|
||||
# directory to walk and are skipped, matching both callers' prior behavior.
|
||||
list_marketplace_local_plugins() {
|
||||
local repo_root="$1" marketplace="$2"
|
||||
local plugin_count i name source_type source
|
||||
|
||||
plugin_count="$(jq '.plugins | length' "$marketplace")"
|
||||
for ((i = 0; i < plugin_count; i++)); do
|
||||
source_type="$(jq -r ".plugins[$i].source | type" "$marketplace")"
|
||||
[[ "$source_type" == "string" ]] || continue
|
||||
name="$(jq -r ".plugins[$i].name" "$marketplace")"
|
||||
source="$(jq -r ".plugins[$i].source" "$marketplace")"
|
||||
source="${source#./}"
|
||||
printf '%s\t%s\n' "$name" "$repo_root/$source"
|
||||
done
|
||||
}
|
||||
@@ -18,28 +18,34 @@ set -euo pipefail
|
||||
# generated in-place at the plugin root by a separate apm code path
|
||||
# (core/plugin_manifest.py, run as part of the same `apm pack` invocation, keyed off
|
||||
# cwd rather than -o), and .mcp.json is hand-authored at the plugin root per ADR-0015
|
||||
# (it is not an .apm/ primitive). Real-mode syncs pass --force so that path actually
|
||||
# refreshes both files from current apm.yml/.apm/ content -- apm pack silently skips
|
||||
# regenerating an existing plugin.json otherwise ("already exists; skipping plugin.json
|
||||
# generation"), which would let them go stale after a name/version/description edit.
|
||||
# (it is not an .apm/ primitive). Both real and check-mode syncs pass --force so that
|
||||
# path actually refreshes both files from current apm.yml/.apm/ content -- apm pack
|
||||
# silently skips regenerating an existing plugin.json otherwise ("already exists;
|
||||
# skipping plugin.json generation"), which would let them go stale after a
|
||||
# name/version/description edit (real mode) or let --check compare a copy against
|
||||
# itself and never see the drift (check mode; see sync_plugin_manifest below).
|
||||
#
|
||||
# apm's Copilot-ecosystem plugin.json builder omits mcpServers entirely -- its own
|
||||
# docstring calls it out-of-schema for Copilot, but this repo's researched Copilot
|
||||
# plugin schema docs (plugins/kyberforge/docs/research/docs/github-copilot-plugins/
|
||||
# configuration.md) document mcpServers as valid there. Real-mode syncs re-inject it
|
||||
# into .github/plugin/plugin.json from the plugin's own .mcp.json after apm pack runs
|
||||
# (see reinject_mcp_servers below); staleness there, like the rest of plugin.json, is
|
||||
# only fixed by the next real sync, not detected by --check.
|
||||
# configuration.md) document mcpServers as valid there. Both modes re-inject it into
|
||||
# .github/plugin/plugin.json from the plugin's own .mcp.json after apm pack runs (see
|
||||
# reinject_mcp_servers below) -- real mode into the plugin root directly, check mode
|
||||
# into the throwaway copy first so the manifest diff below sees the same content a
|
||||
# real sync would actually produce.
|
||||
#
|
||||
# apm pack also writes .claude-plugin/plugin.json and .github/plugin/plugin.json into
|
||||
# cwd whenever those files don't already exist yet -- regardless of --force -- so
|
||||
# --check (which must never mutate the real plugin root) never cds into plugin_dir
|
||||
# directly. It packs a throwaway copy instead (see sync_one's pack_cwd); only that
|
||||
# copy's manifest files, never the real ones, can get created as a first-write.
|
||||
# directly. It packs a throwaway copy instead (see sync_one's pack_cwd), forces
|
||||
# regeneration of both manifest files inside that copy, then diffs them
|
||||
# (sync_plugin_manifest) against the real plugin root's committed manifests to catch
|
||||
# drift in name/version/description/mcpServers -- only the copy's manifest files,
|
||||
# never the real ones, can get created as a first-write.
|
||||
#
|
||||
# hooks.json is only synced when .apm/hooks/ actually produces one -- a plugin with
|
||||
# no .apm/hooks/ content is left alone even if a root-level hooks.json already exists
|
||||
# (pre-existing scaffolding outside this script's concern).
|
||||
# hooks.json is mirrored like the other MIRROR_DIRS content: synced when .apm/hooks/
|
||||
# produces one, and removed (real mode) / flagged as drift (--check) when it no
|
||||
# longer does but a root-level hooks.json is still sitting there from a prior sync.
|
||||
#
|
||||
# tests/ subdirectories (e.g. .apm/skills/<name>/tests/*.bats) are excluded from the
|
||||
# mirror -- they are dev-time fixtures a plugin host never needs to discover, and several
|
||||
@@ -81,6 +87,12 @@ if ! command -v jq &>/dev/null; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
# shellcheck source=lib/marketplace-plugins.sh
|
||||
source "$SCRIPT_DIR/lib/marketplace-plugins.sh"
|
||||
# shellcheck source=lib/batch-run.sh
|
||||
source "$SCRIPT_DIR/lib/batch-run.sh"
|
||||
|
||||
# Convention subdirectories apm's plugin exporter can populate from .apm/.
|
||||
MIRROR_DIRS=(agents skills commands instructions extensions)
|
||||
|
||||
@@ -89,9 +101,11 @@ SCRATCH_ROOT="$(mktemp -d)"
|
||||
trap 'rm -rf "$SCRATCH_ROOT"' EXIT
|
||||
|
||||
if [[ "$ALL" -eq 1 ]]; then
|
||||
# Derives the plugin list from marketplace.json the same way
|
||||
# scripts/check-manifests.sh does, instead of hand-maintaining a duplicate list
|
||||
# at every call site (see .pre-commit-config.yaml's check-plugin-content-sync).
|
||||
# Derives the plugin list from marketplace.json via the shared
|
||||
# list_marketplace_local_plugins helper (scripts/lib/marketplace-plugins.sh),
|
||||
# the same one scripts/check-manifests.sh uses, instead of hand-maintaining a
|
||||
# duplicate walk at every call site (see .pre-commit-config.yaml's
|
||||
# check-plugin-content-sync).
|
||||
REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)"
|
||||
MARKETPLACE="$REPO_ROOT/.claude-plugin/marketplace.json"
|
||||
if [[ ! -f "$MARKETPLACE" ]]; then
|
||||
@@ -99,15 +113,9 @@ if [[ "$ALL" -eq 1 ]]; then
|
||||
exit 1
|
||||
fi
|
||||
declare -a plugin_dirs=()
|
||||
plugin_count="$(jq '.plugins | length' "$MARKETPLACE")"
|
||||
for ((i = 0; i < plugin_count; i++)); do
|
||||
source_type="$(jq -r ".plugins[$i].source | type" "$MARKETPLACE")"
|
||||
# Remote sources (github, git, npm objects) have no local directory to sync.
|
||||
[[ "$source_type" == "string" ]] || continue
|
||||
source="$(jq -r ".plugins[$i].source" "$MARKETPLACE")"
|
||||
source="${source#./}"
|
||||
plugin_dirs+=("$REPO_ROOT/$source")
|
||||
done
|
||||
while IFS=$'\t' read -r _name plugin_dir; do
|
||||
plugin_dirs+=("$plugin_dir")
|
||||
done < <(list_marketplace_local_plugins "$REPO_ROOT" "$MARKETPLACE")
|
||||
else
|
||||
declare -a plugin_dirs=("$@")
|
||||
fi
|
||||
@@ -169,27 +177,37 @@ sync_hooks_json() {
|
||||
local plugin_dir="$1" bundle_dir="$2"
|
||||
local src="$bundle_dir/hooks.json" dst="$plugin_dir/hooks.json"
|
||||
|
||||
# No .apm/hooks/ content -- hooks.json (if any) is out of scope for this script.
|
||||
[[ -f "$src" ]] || return 0
|
||||
|
||||
if [[ "$CHECK" -eq 1 ]]; then
|
||||
local normalized_src
|
||||
normalized_src="$(mktemp)"
|
||||
normalize_trailing_newline "$src" "$normalized_src"
|
||||
if [[ ! -f "$dst" ]] || ! diff -q "$normalized_src" "$dst" >/dev/null 2>&1; then
|
||||
echo "DRIFT $dst: out of sync with .apm/hooks/" >&2
|
||||
if [[ -f "$src" ]]; then
|
||||
local normalized_src
|
||||
normalized_src="$(mktemp)"
|
||||
normalize_trailing_newline "$src" "$normalized_src"
|
||||
if [[ ! -f "$dst" ]] || ! diff -q "$normalized_src" "$dst" >/dev/null 2>&1; then
|
||||
echo "DRIFT $dst: out of sync with .apm/hooks/" >&2
|
||||
FAIL=1
|
||||
fi
|
||||
rm -f "$normalized_src"
|
||||
elif [[ -f "$dst" ]]; then
|
||||
# Mirrors sync_dir()'s orphan handling: .apm/hooks/ no longer produces a
|
||||
# hooks.json, but one is still sitting at $dst from a prior sync -- that's
|
||||
# drift (stale mirrored output), not "no .apm/hooks/ content" (which would
|
||||
# mean $dst never existed in the first place).
|
||||
echo "DRIFT $dst: stale, no longer produced from .apm/hooks/" >&2
|
||||
FAIL=1
|
||||
fi
|
||||
rm -f "$normalized_src"
|
||||
return 0
|
||||
fi
|
||||
|
||||
normalize_trailing_newline "$src" "$dst"
|
||||
if [[ -f "$src" ]]; then
|
||||
normalize_trailing_newline "$src" "$dst"
|
||||
elif [[ -f "$dst" ]]; then
|
||||
rm -f "$dst"
|
||||
fi
|
||||
}
|
||||
|
||||
reinject_mcp_servers() {
|
||||
local plugin_dir="$1"
|
||||
local mcp_src="$plugin_dir/.mcp.json" dst="$plugin_dir/.github/plugin/plugin.json"
|
||||
local plugin_dir="$1" target_dir="$2"
|
||||
local mcp_src="$plugin_dir/.mcp.json" dst="$target_dir/.github/plugin/plugin.json"
|
||||
[[ -f "$mcp_src" ]] || return 0
|
||||
[[ -f "$dst" ]] || return 0
|
||||
|
||||
@@ -205,6 +223,29 @@ reinject_mcp_servers() {
|
||||
mv "$tmp" "$dst"
|
||||
}
|
||||
|
||||
# --check-only: diffs a freshly-regenerated manifest file (in the throwaway
|
||||
# pack_cwd copy, produced by a --force'd apm pack) against the one actually
|
||||
# committed at the real plugin root. Real-mode syncs never need this -- there
|
||||
# pack_cwd IS plugin_dir, so --force already refreshed the real file in place.
|
||||
sync_plugin_manifest() {
|
||||
local plugin_dir="$1" pack_cwd="$2" rel="$3"
|
||||
local src="$pack_cwd/$rel" dst="$plugin_dir/$rel"
|
||||
|
||||
if [[ -f "$src" ]]; then
|
||||
if [[ ! -f "$dst" ]]; then
|
||||
echo "DRIFT $dst: missing (would be created by apm pack from apm.yml/.mcp.json)" >&2
|
||||
FAIL=1
|
||||
elif ! diff -q "$src" "$dst" >/dev/null 2>&1; then
|
||||
echo "DRIFT $dst: out of sync with apm.yml/.mcp.json" >&2
|
||||
diff "$src" "$dst" 2>&1 | sed 's/^/ /' >&2
|
||||
FAIL=1
|
||||
fi
|
||||
elif [[ -f "$dst" ]]; then
|
||||
echo "DRIFT $dst: stale, no longer produced by apm pack" >&2
|
||||
FAIL=1
|
||||
fi
|
||||
}
|
||||
|
||||
# Runs entirely inside a backgrounded subshell (see the dispatch loop below), so
|
||||
# FAIL here is that subshell's own copy -- it never touches the parent's FAIL
|
||||
# and must be handed back via status_file instead.
|
||||
@@ -232,9 +273,15 @@ sync_one() {
|
||||
mkdir -p "$scratch"
|
||||
pack_log="$(mktemp)"
|
||||
|
||||
local force_flag=()
|
||||
# --force always: in real mode it refreshes the real plugin.json in place
|
||||
# (pack_cwd IS plugin_dir there); in check mode it forces regeneration inside
|
||||
# the throwaway pack_cwd copy so sync_plugin_manifest below has a genuinely
|
||||
# fresh manifest to diff against the real one -- without --force, apm pack
|
||||
# would silently skip regenerating a plugin.json that already exists in the
|
||||
# copy (it was seeded from the real plugin_dir), so --check would always
|
||||
# compare the copy against itself and never see drift.
|
||||
local force_flag=(--force)
|
||||
if [[ "$CHECK" -eq 0 ]]; then
|
||||
force_flag=(--force)
|
||||
pack_cwd="$plugin_dir"
|
||||
else
|
||||
pack_cwd="$SCRATCH_ROOT/$name.checkcopy"
|
||||
@@ -266,7 +313,14 @@ sync_one() {
|
||||
done
|
||||
sync_hooks_json "$plugin_dir" "$bundle_dir"
|
||||
if [[ "$CHECK" -eq 0 ]]; then
|
||||
reinject_mcp_servers "$plugin_dir"
|
||||
reinject_mcp_servers "$plugin_dir" "$plugin_dir"
|
||||
else
|
||||
# Reinject into the throwaway copy too, so the manifest diff below compares
|
||||
# against what a real sync would actually produce (mcpServers included),
|
||||
# not apm's own Copilot-ecosystem output (which omits it).
|
||||
reinject_mcp_servers "$plugin_dir" "$pack_cwd"
|
||||
sync_plugin_manifest "$plugin_dir" "$pack_cwd" ".claude-plugin/plugin.json"
|
||||
sync_plugin_manifest "$plugin_dir" "$pack_cwd" ".github/plugin/plugin.json"
|
||||
fi
|
||||
echo "$FAIL" >"$status_file"
|
||||
}
|
||||
@@ -276,24 +330,16 @@ sync_one() {
|
||||
# than paying that startup cost N times serially. Output is buffered per plugin
|
||||
# (not streamed) so concurrent DRIFT/FAIL messages from different plugins never
|
||||
# interleave; it's flushed in stable $@ order once every job has finished.
|
||||
#
|
||||
# Batched (not a rolling pool) because a bounded rolling pool needs `wait -n`,
|
||||
# which is bash 4.3+ -- tests/run-tests.sh and tests/run-bats.sh in this same repo
|
||||
# are explicitly bash-3.2-safe, so this script matches their pattern for
|
||||
# consistency. `getconf` over `nproc` for the same reason: `nproc` doesn't exist
|
||||
# on macOS.
|
||||
JOBS_LIMIT="$(getconf _NPROCESSORS_ONLN 2>/dev/null || echo 4)"
|
||||
running=0
|
||||
# Dispatch/throttling itself is scripts/lib/batch-run.sh's batch_run (shared
|
||||
# with tests/run-tests.sh and tests/run-bats.sh) -- see that file for why this
|
||||
# is batched rather than a rolling `wait -n` pool.
|
||||
declare -a batch_args=()
|
||||
for plugin_dir in ${plugin_dirs[@]+"${plugin_dirs[@]}"}; do
|
||||
name="$(basename "${plugin_dir%/}")"
|
||||
(sync_one "$plugin_dir" "$SCRATCH_ROOT/$name.status") >"$SCRATCH_ROOT/$name.log" 2>&1 &
|
||||
running=$((running + 1))
|
||||
if [[ $running -ge $JOBS_LIMIT ]]; then
|
||||
wait
|
||||
running=0
|
||||
fi
|
||||
cmd="$(printf 'sync_one %q %q' "$plugin_dir" "$SCRATCH_ROOT/$name.status")"
|
||||
batch_args+=("$name" "$cmd")
|
||||
done
|
||||
wait
|
||||
batch_run "$SCRATCH_ROOT" ${batch_args[@]+"${batch_args[@]}"}
|
||||
|
||||
for plugin_dir in ${plugin_dirs[@]+"${plugin_dirs[@]}"}; do
|
||||
name="$(basename "${plugin_dir%/}")"
|
||||
|
||||
Reference in New Issue
Block a user