fix(scripts): bring the generated hooks/ directory under the mirror's ownership
`checked_paths` covered hooks/hooks.json but not the hooks/ directory holding it, so a stray file dropped inside, or an empty hooks/ left behind once .apm/hooks/ stopped producing anything, was invisible to --check. Check and sync agreed in both cases, so the invariant held -- but a stray in a directory the mirror owns should be drift, exactly as it is inside skills/ or agents/. A stray at the PLUGIN root stays out of scope by design: README.md, docs/, bin/, .mcp.json are hand-authored. hooks/ is now wiped and rebuilt like every MIRROR_DIRS destination, and the directory is listed in checked_paths so the recursive manifest sees one-sided entries. Issue #97 item 4 reports `prompts` as documented-but-unmirrored. That is refuted: MIRROR_DIRS lists DESTINATION directories, and apm folds .apm/prompts/ into commands/ (renaming *.prompt.md to *.md), verified empirically. A plugin adding .apm/prompts/ is mirrored today; adding a `prompts` entry would name an output directory apm never emits. Pinned with a characterization test that fires if that mapping ever changes, plus a comment so it is not refiled. Guards the new wipe with ${target_dir:?}: `set -u` aborts on an unset variable but not an empty one, which would make it `rm -rf /hooks`. Refs #97 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
This commit is contained in:
@@ -46,6 +46,12 @@ set -euo pipefail
|
||||
# The merged hooks file 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 mirrored copy is still sitting there from a prior sync.
|
||||
# The hooks/ directory holding it is owned outright by the mirror the same way every
|
||||
# MIRROR_DIRS destination is -- a real sync wipes and rebuilds it, so a stray file
|
||||
# dropped inside, or the empty directory left behind once .apm/hooks/ stops producing
|
||||
# a hooks.json, is cleaned up rather than preserved. (A stray at the PLUGIN ROOT is a
|
||||
# different matter and stays out of scope by design: README.md, docs/, bin/, .mcp.json
|
||||
# and friends are hand-authored there.)
|
||||
# It lands at hooks/hooks.json, not at the plugin root: Claude Code convention-scans
|
||||
# `hooks/hooks.json` "at the plugin root, not inside .claude-plugin/"
|
||||
# (plugins/kyberforge/docs/research/docs/claude-code-plugins/configuration.md's "Plugin
|
||||
@@ -106,17 +112,30 @@ if ! command -v jq &>/dev/null; then
|
||||
fi
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
# shellcheck source=lib/marketplace-plugins.sh
|
||||
# Both source= paths below are repo-root-relative, not script-dir-relative -- see
|
||||
# tests/run-tests.sh for why the script-dir spelling silently fails to resolve.
|
||||
# shellcheck source=scripts/lib/marketplace-plugins.sh
|
||||
source "$SCRIPT_DIR/lib/marketplace-plugins.sh"
|
||||
# shellcheck source=lib/batch-run.sh
|
||||
# shellcheck source=scripts/lib/batch-run.sh
|
||||
source "$SCRIPT_DIR/lib/batch-run.sh"
|
||||
|
||||
# Convention subdirectories apm's plugin exporter can populate from .apm/.
|
||||
#
|
||||
# These are DESTINATION directory names at the plugin root, not .apm/ source
|
||||
# directory names -- the two lists are deliberately not the same, so do not "fix"
|
||||
# this by pasting in the header comment's .apm/ list. In particular .apm/prompts/
|
||||
# has no destination of its own: apm's exporter folds it into commands/ together
|
||||
# with .apm/commands/, renaming *.prompt.md to *.md (ADR-0017's mapping table;
|
||||
# re-verified empirically against apm 0.28.0, and pinned by the prompts test in
|
||||
# tests/test-sync-plugin-content.sh). Adding `prompts` here would name an output
|
||||
# directory apm never emits and no plugin host ever scans.
|
||||
MIRROR_DIRS=(agents skills commands instructions extensions)
|
||||
|
||||
# Where the merged hooks file lands (Claude Code's convention-scanned path), and
|
||||
# the pre-fix root-level path a real sync now cleans up as stale.
|
||||
HOOKS_REL="hooks/hooks.json"
|
||||
# The generated hooks directory, the merged hooks file inside it (Claude Code's
|
||||
# convention-scanned path), and the pre-fix root-level path a real sync now cleans
|
||||
# up as stale.
|
||||
HOOKS_DIR_REL="hooks"
|
||||
HOOKS_REL="$HOOKS_DIR_REL/hooks.json"
|
||||
LEGACY_HOOKS_REL="hooks.json"
|
||||
|
||||
FAIL=0
|
||||
@@ -202,15 +221,24 @@ sync_hooks_json() {
|
||||
rm -f "$legacy"
|
||||
fi
|
||||
|
||||
# hooks/ is generated output the mirror owns outright, exactly like every
|
||||
# MIRROR_DIRS destination -- so wipe it wholesale and rebuild, rather than
|
||||
# editing hooks.json in place and leaving whatever else happens to be in there.
|
||||
# sync_dir gets this for free from its own rm -rf; hooks/ has to spell it out
|
||||
# because its one legitimate occupant is written by name instead of copied as a
|
||||
# tree. The wipe covers both a stray file someone dropped alongside hooks.json
|
||||
# and the case where .apm/hooks/ stops producing a hooks.json at all: the
|
||||
# directory then ends up absent, not present-and-empty.
|
||||
#
|
||||
# ${target_dir:?} rather than a bare expansion: `set -u` aborts on an UNSET
|
||||
# variable but not an empty one, and an empty $target_dir would make this
|
||||
# `rm -rf /hooks`. The other rm -rf calls here take a $dst built by their
|
||||
# caller; this one is the only place a bare parameter is the whole prefix.
|
||||
rm -rf "${target_dir:?}/$HOOKS_DIR_REL"
|
||||
|
||||
if [[ -f "$src" ]]; then
|
||||
mkdir -p "$target_dir/hooks"
|
||||
mkdir -p "$target_dir/$HOOKS_DIR_REL"
|
||||
normalize_trailing_newline "$src" "$dst"
|
||||
elif [[ -f "$dst" ]]; then
|
||||
# .apm/hooks/ no longer produces a hooks.json, but one is still sitting at
|
||||
# $dst from a prior sync -- that's stale mirrored output, not "no .apm/hooks/
|
||||
# content" (which would mean $dst never existed in the first place).
|
||||
rm -f "$dst"
|
||||
rmdir "$target_dir/hooks" 2>/dev/null || true
|
||||
fi
|
||||
}
|
||||
|
||||
@@ -290,6 +318,14 @@ path_manifest() {
|
||||
# symlink to identical content, both leave --check at exit 0 while a real sync
|
||||
# silently repairs them -- check and sync disagreeing, which is the one thing this
|
||||
# gate exists to prevent. Compare an explicit type+exec-bit manifest as well.
|
||||
#
|
||||
# The manifest is a full recursive listing, so it is also what makes an entry that
|
||||
# exists on only one side visible. That is the sole coverage the generated hooks/
|
||||
# directory gets for anything other than hooks.json itself (which check_file above
|
||||
# handles by name): a stray file inside hooks/, or hooks/ still sitting there empty
|
||||
# after .apm/hooks/ stopped producing anything, shows up here as a manifest line
|
||||
# present on one side only. Both are things sync_hooks_json's rm -rf removes, so
|
||||
# both have to be drift.
|
||||
check_path_modes() {
|
||||
local plugin_dir="$1" expected_dir="$2"
|
||||
shift 2
|
||||
@@ -299,7 +335,7 @@ check_path_modes() {
|
||||
path_manifest "$expected_dir" "$@" >"$expected"
|
||||
path_manifest "$plugin_dir" "$@" >"$actual"
|
||||
if ! diff -q "$expected" "$actual" >/dev/null 2>&1; then
|
||||
echo "DRIFT $plugin_dir: mirrored file types/modes differ from a fresh sync" >&2
|
||||
echo "DRIFT $plugin_dir: mirrored paths/types/modes differ from a fresh sync" >&2
|
||||
diff "$expected" "$actual" 2>&1 | sed 's/^/ /' >&2 || true
|
||||
FAIL=1
|
||||
fi
|
||||
@@ -426,7 +462,10 @@ sync_one() {
|
||||
# not apm's own Copilot-ecosystem output (which omits it).
|
||||
reinject_mcp_servers "$plugin_dir" "$pack_cwd"
|
||||
local -a checked_paths
|
||||
checked_paths=("${MIRROR_DIRS[@]}" "$HOOKS_REL" "$LEGACY_HOOKS_REL")
|
||||
# $HOOKS_DIR_REL, not $HOOKS_REL: the manifest comparison has to see the whole
|
||||
# generated directory (see check_path_modes), and listing it recursively already
|
||||
# covers hooks/hooks.json.
|
||||
checked_paths=("${MIRROR_DIRS[@]}" "$HOOKS_DIR_REL" "$LEGACY_HOOKS_REL")
|
||||
for d in "${MIRROR_DIRS[@]}"; do
|
||||
check_dir "$plugin_dir" "$pack_cwd" "$d"
|
||||
done
|
||||
|
||||
@@ -17,7 +17,9 @@ fi
|
||||
# Minimal fixture exercising every mirrored category -- all five of
|
||||
# scripts/sync-plugin-content.sh's MIRROR_DIRS (agents, skills, commands,
|
||||
# instructions, extensions) plus the merged hooks file -- without needing network
|
||||
# access (no apm.yml dependencies). Two deliberately-shaped skill subdirectories:
|
||||
# access (no apm.yml dependencies). It also carries a .apm/prompts/ entry, which has
|
||||
# no MIRROR_DIRS destination of its own: apm folds prompts into commands/. Two
|
||||
# deliberately-shaped skill subdirectories:
|
||||
#
|
||||
# skills/hello/tests/ -- a dev-time fixture that must NOT be mirrored
|
||||
# skills/hello/assets/templates/tests/ -- a template asset that MUST be mirrored
|
||||
@@ -34,7 +36,7 @@ make_fixture() {
|
||||
mkdir -p "$dir/.apm/skills/hello/tests" "$dir/.apm/skills/hello/scripts" \
|
||||
"$dir/.apm/skills/hello/assets/templates/tests" "$dir/.apm/agents" \
|
||||
"$dir/.apm/hooks" "$dir/.apm/commands" "$dir/.apm/instructions" \
|
||||
"$dir/.apm/extensions"
|
||||
"$dir/.apm/extensions" "$dir/.apm/prompts"
|
||||
cat > "$dir/apm.yml" <<'YAML'
|
||||
name: fixture
|
||||
version: 0.0.1
|
||||
@@ -81,6 +83,12 @@ EOF
|
||||
description: mycmd
|
||||
---
|
||||
Do a thing.
|
||||
EOF
|
||||
cat > "$dir/.apm/prompts/greet.prompt.md" <<'EOF'
|
||||
---
|
||||
description: greet
|
||||
---
|
||||
Greet the user.
|
||||
EOF
|
||||
cat > "$dir/.apm/instructions/style.instructions.md" <<'EOF'
|
||||
---
|
||||
@@ -539,6 +547,98 @@ else
|
||||
fi
|
||||
fi
|
||||
|
||||
# --- 20. .apm/prompts/ is mirrored, via commands/ rather than a prompts/ of its own ---
|
||||
# The script header lists prompts among the .apm/ directories it mirrors while
|
||||
# MIRROR_DIRS has no `prompts` entry, which reads as a hole and has already been filed
|
||||
# as one. It is not: MIRROR_DIRS names DESTINATION directories, and apm's exporter
|
||||
# folds .apm/prompts/ into commands/ (renaming *.prompt.md to *.md) alongside
|
||||
# .apm/commands/. This pins that mapping, so an apm upgrade that gave prompts a
|
||||
# destination of its own — the one change that would genuinely need a MIRROR_DIRS
|
||||
# entry — fails here instead of silently dropping the content.
|
||||
echo ""
|
||||
echo "--- .apm/prompts/ content arrives in commands/, not in a prompts/ directory ---"
|
||||
FIXTURE20="$(make_fixture)"; track "$FIXTURE20"
|
||||
bash "$SCRIPT" "$FIXTURE20" > /dev/null 2>&1
|
||||
if [[ -f "$FIXTURE20/commands/greet.md" ]]; then
|
||||
pass ".apm/prompts/greet.prompt.md is mirrored to commands/greet.md"
|
||||
else
|
||||
fail ".apm/prompts/ content never reached commands/ — apm's prompts mapping changed"
|
||||
fi
|
||||
if [[ ! -e "$FIXTURE20/prompts" ]]; then
|
||||
pass "no prompts/ directory is produced at the plugin root"
|
||||
else
|
||||
fail "a prompts/ directory appeared at the plugin root — it now needs a MIRROR_DIRS entry"
|
||||
fi
|
||||
if bash "$SCRIPT" --check "$FIXTURE20" > /dev/null 2>&1; then
|
||||
pass "--check is clean with .apm/prompts/ content present"
|
||||
else
|
||||
fail "--check reports drift on a freshly synced fixture carrying .apm/prompts/"
|
||||
fi
|
||||
|
||||
# --- 21. A stray file inside the generated hooks/ directory is drift ---
|
||||
# hooks/ is mirror-owned output, so it is scoped like a MIRROR_DIRS destination and
|
||||
# not like the plugin root (where README.md, docs/, bin/ and .mcp.json are all
|
||||
# hand-authored and deliberately none of this script's business). Before hooks/ came
|
||||
# under that ownership, --check exited 0 on a stray inside it and a real sync left the
|
||||
# stray untouched — agreeing with each other, but agreeing on the wrong answer.
|
||||
echo ""
|
||||
echo "--- a stray file inside the generated hooks/ directory is reported and removed ---"
|
||||
FIXTURE21="$(make_fixture)"; track "$FIXTURE21"
|
||||
bash "$SCRIPT" "$FIXTURE21" > /dev/null 2>&1
|
||||
if [[ ! -f "$FIXTURE21/hooks/hooks.json" ]]; then
|
||||
fail "initial sync did not create hooks/hooks.json -- can't test the stray case"
|
||||
else
|
||||
printf '{"stray": true}\n' > "$FIXTURE21/hooks/extra.json"
|
||||
if bash "$SCRIPT" --check "$FIXTURE21" > /dev/null 2>&1; then
|
||||
fail "no drift reported for a stray file inside the generated hooks/ directory"
|
||||
else
|
||||
pass "a stray file inside hooks/ is reported as drift"
|
||||
bash "$SCRIPT" "$FIXTURE21" > /dev/null 2>&1
|
||||
if [[ ! -e "$FIXTURE21/hooks/extra.json" ]] && [[ -f "$FIXTURE21/hooks/hooks.json" ]]; then
|
||||
pass "re-sync removes the stray and keeps hooks/hooks.json"
|
||||
else
|
||||
fail "re-sync did not clean the stray out of hooks/"
|
||||
fi
|
||||
if bash "$SCRIPT" --check "$FIXTURE21" > /dev/null 2>&1; then
|
||||
pass "re-sync clears the stray-hooks-file drift"
|
||||
else
|
||||
fail "re-sync did not clear the stray-hooks-file drift"
|
||||
fi
|
||||
fi
|
||||
fi
|
||||
|
||||
# --- 22. An empty leftover hooks/ directory is drift, not an acceptable resting state ---
|
||||
# When .apm/hooks/ produces no hooks.json, the correct mirror state is no hooks/
|
||||
# directory at all — not an empty one. Empty leftovers are exactly what earlier
|
||||
# revisions of this script left behind in this repo's own plugin roots.
|
||||
echo ""
|
||||
echo "--- an empty leftover hooks/ directory is reported and removed ---"
|
||||
FIXTURE22="$(make_fixture)"; track "$FIXTURE22"
|
||||
rm -rf "$FIXTURE22/.apm/hooks"
|
||||
bash "$SCRIPT" "$FIXTURE22" > /dev/null 2>&1
|
||||
if [[ -e "$FIXTURE22/hooks" ]]; then
|
||||
fail "sync created hooks/ for a plugin whose .apm/hooks/ produces no hooks.json"
|
||||
else
|
||||
pass "no hooks/ directory when .apm/hooks/ produces nothing"
|
||||
fi
|
||||
mkdir -p "$FIXTURE22/hooks"
|
||||
if bash "$SCRIPT" --check "$FIXTURE22" > /dev/null 2>&1; then
|
||||
fail "no drift reported for an empty leftover hooks/ directory"
|
||||
else
|
||||
pass "an empty leftover hooks/ directory is reported as drift"
|
||||
bash "$SCRIPT" "$FIXTURE22" > /dev/null 2>&1
|
||||
if [[ ! -e "$FIXTURE22/hooks" ]]; then
|
||||
pass "re-sync removes the empty leftover hooks/ directory"
|
||||
else
|
||||
fail "re-sync left the empty hooks/ directory in place"
|
||||
fi
|
||||
if bash "$SCRIPT" --check "$FIXTURE22" > /dev/null 2>&1; then
|
||||
pass "re-sync clears the empty-hooks-directory drift"
|
||||
else
|
||||
fail "re-sync did not clear the empty-hooks-directory drift"
|
||||
fi
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
|
||||
Reference in New Issue
Block a user