diff --git a/scripts/sync-plugin-content.sh b/scripts/sync-plugin-content.sh index 0adb9fd..0cd1ee2 100755 --- a/scripts/sync-plugin-content.sh +++ b/scripts/sync-plugin-content.sh @@ -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 diff --git a/tests/test-sync-plugin-content.sh b/tests/test-sync-plugin-content.sh index 69fe1b0..a3bf90f 100755 --- a/tests/test-sync-plugin-content.sh +++ b/tests/test-sync-plugin-content.sh @@ -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 ]]