diff --git a/docs/adr/0017-plugin-content-mirror-bridges-apm-to-host-discovery.md b/docs/adr/0017-plugin-content-mirror-bridges-apm-to-host-discovery.md index 448fc5c..ad7856a 100644 --- a/docs/adr/0017-plugin-content-mirror-bridges-apm-to-host-discovery.md +++ b/docs/adr/0017-plugin-content-mirror-bridges-apm-to-host-discovery.md @@ -86,7 +86,11 @@ governance status as `.claude-plugin/plugin.json`/`marketplace.json`: `.pre-commit-config.yaml` as hook id `check-plugin-content-sync` by a parallel workstream on issue #90) — the same enforcement model `check-manifests.sh` already applies to the other compiled-output category. `--check` alone is not the gate: the script requires either `--all` or - an explicit list of plugin directories, and run bare it prints usage and exits 1. + an explicit list of plugin directories, and run bare it prints usage and exits 1. `--all` derives + its work list from `marketplace.json`, a generated file, so it asserts its own coverage against + that list: it fails if it verified fewer plugins than the marketplace declares, not merely if it + verified none. A listed plugin whose `.apm/` has gone missing is skipped by the per-plugin sync + and would otherwise let the gate report success over a shrinking work list. - Verified two ways before landing: `claude plugin validate --strict` passes on all 6 real (non-scratch) plugin directories, and a live behavioral test (`claude --plugin-dir plugins/kyberforge -p "list your skills and agents"`) against the real @@ -217,6 +221,94 @@ convention path. (`hooks` was never in `build_plugin_manifest`'s strip list at a corrected mechanism note under "Considered options".) Writing to the convention path is what makes the no-pointer premise true here rather than something to work around. +Read "the host finds it by auto-discovery" above as **Claude Code**, not both hosts. Copilot has no +default for `hooks` and so discovers none — a real gap, examined and deliberately left open in the +next amendment. + +## Amendment (2026-08-14): no `hooks` pointer is re-injected for Copilot — the gap stays documented + +PR #95's review found a third field, and it looks like the `mcpServers` amendment's exact twin: +`plugins/kyberforge/docs/research/docs/github-copilot-plugins/configuration.md:47` types `hooks` as +a `plugin.json` field, **"string or object"**, with **no default** — so Copilot has no convention +path to scan — and `jq 'has("hooks")'` returns `false` for all six `plugins/*/.github/plugin/plugin.json`. +Copilot therefore resolves **zero hooks from every plugin in this repo**. The facts are not in +dispute; the remedy is. + +State the mechanism correctly first, because it differs from `mcpServers` and the amendment above +depends on that distinction. `mcpServers` is *actively removed* — `build_plugin_manifest` runs +`manifest.pop("mcpServers", None)` on every Copilot manifest. `hooks` was **never in that strip +list** (its field list is `("agents", "skills", "commands", "instructions")`, and the loop is dead +code besides — see "Considered options"). This is an absence apm never fills, not a removal to +reverse. + +**Decision: do not re-inject. Document the gap.** The `mcpServers` exception was granted on three +conditions, and `hooks` meets only two of them: + +1. *A documented host schema field.* Met — `hooks` is in Copilot's own field table. +2. *apm has no input that produces it.* Met — `apm.yml` has no key for it. +3. *The payload is correct for the host regardless of content.* **Not met**, and this is the whole + difference. `.mcp.json` is one host-agnostic format that both ecosystems read, so the string + `".mcp.json"` is a true statement about the file no matter what is in it. Hooks have no such + shared format: Claude Code reads + `{"hooks": {"PreToolUse": [{"matcher": ..., "hooks": [...]}]}}` while Copilot requires + `{"version": 1, "hooks": {"sessionStart": [{"type": "command", "bash": ..., "powershell": ...}]}}` + — a mandatory `version`, lowercase and differently-named lifecycle events, and per-shell script + keys. apm's exporter merges `.apm/hooks/*.json` into **exactly one** `hooks.json` with no + per-target shaping (`_collect_hooks_from_apm`, `apm_cli/bundle/plugin_exporter.py`), and that one + file also sits at Claude Code's convention path, where Claude Code will read it whatever it + contains. So there is exactly one file and two incompatible readers of it. + +A `hooks` pointer would therefore assert that a Claude-shaped file is Copilot-shaped. That trades an +*incomplete* manifest for a *wrong* one, which is the opposite of the `mcpServers` amendment's +reasoning ("shipping the manifest as apm produces it would ship a manifest known to be incomplete"). + +The "it changes nothing today, so it is zero-risk and correct-by-construction for the first real +hook" argument does not survive the same check, in both halves. It is not inert today: both +`hooks/hooks.json` files are `{"hooks": {}}`, which lacks the `version: 1` Copilot's schema +requires, so a pointer would name a file invalid against the schema it is being pointed at from — +a change from "declares no hooks" to "declares hooks, at an invalid file". And it is not +correct-by-construction later: whoever writes the first real hook writes it in one of the two +shapes, and the pointer is wrong in the Claude-shaped case (the case that actually happens, since +Claude Code auto-discovers the same file and is what these hooks are authored against) while the +Copilot-shaped case breaks Claude Code instead. No content makes both readers correct. + +What would change this decision is upstream, not local: apm emitting a per-target hooks file (at +which point a pointer names a file genuinely shaped for its reader), or the two hook schemas +converging. Until then the honest artifact is a documented gap, recorded for authors in +`plugins/kyberforge/docs/hooks.md` and pinned by a test asserting the Copilot manifest carries no +`hooks` key — so that adding one is a deliberate act that has to confront the schema mismatch, +rather than a plausible-looking one-liner nobody re-derives. + +This does not weaken the `mcpServers` amendment. That exception was narrow on purpose, and this is +what its third condition was for. + +## Amendment (2026-08-14): symlinks under `.apm/` are dropped, and are now reported + +apm's bundle exporter filters symlinks out of the bundle entirely — `f.is_file() and not +f.is_symlink()` in `_collect_flat` and `_collect_recursive`, and the same test in +`_collect_hooks_from_apm` (`apm_cli/bundle/plugin_exporter.py`). It emits no warning. A symlink +placed under a plugin's `.apm/` therefore never reaches the mirror, and until now nothing said so. + +This was **silent content loss, not drift**, and that distinction is why no existing gate caught it. +Every other check in `sync-plugin-content.sh` compares the live mirror against a freshly synced +copy — and both sides are built from that same bundle. The symlink is absent from both, they agree, +and `--check` exits 0. There is no mismatch to detect, only an absence with nothing left to +mismatch against. Reproduced on a fixture: `ln -s real.md link.md` under `.apm/skills/hello/` +produced a mirror with no `link.md` and a `--check` at exit 0. + +`check_apm_symlinks()` therefore reads the `.apm/` **source** tree directly — the only place the +loss is visible — and reports each symlink in both modes, failing the run. It is reported rather +than resolved: dereferencing and copying the target would make a real sync emit content the bundle +does not contain, which is precisely the "reimplement apm's mapping outside apm" this ADR rejects. +Telling the author is the in-contract half. + +The scan covers only the `.apm/` directories apm's exporter actually reads +(`agents`, `skills`, `prompts`, `commands`, `instructions`, `extensions`, `hooks`), and carves out +`//tests` to match the mirror's own exclusion — that subtree is not mirrored whether +or not it holds a symlink, so nothing is lost there. The carve-out is depth-scoped for the same +reason the `tests/` exclusion is: a symlink under `assets/templates/tests` sits in content the +mirror does carry, and is reported. + ## Consequences - Git now tracks real, visible duplication: `.apm/skills//SKILL.md` and diff --git a/plugins/kyberforge/docs/hooks.md b/plugins/kyberforge/docs/hooks.md index b10769a..65fc4ea 100644 --- a/plugins/kyberforge/docs/hooks.md +++ b/plugins/kyberforge/docs/hooks.md @@ -73,17 +73,48 @@ scan, and `jq 'has("hooks")'` returns `false` for all six `.github/plugin/plugin `apm pack` emits. With the pointer gone and no auto-discovery to fall back on, the Copilot ecosystem sees zero hooks. -The effect is the twin of the `mcpServers` gap that ADR-0017's 2026-08-13 amendment re-injects for: -same "string or object" type, same absence of a default, same outcome of a Copilot manifest with no -pointer. The *mechanism* differs, and ADR-0017 is explicit about it — `mcpServers` is actively -stripped by `build_plugin_manifest`, whereas `hooks` "was never in `build_plugin_manifest`'s strip -list at all"; it is simply never emitted, because `apm.yml` has no key that produces one. So this is -an absence apm never fills, not a removal to reverse. It is harmless **today only** because both -generated files are empty (`{"hooks": {}}`), so nothing is being lost. The first real hook authored -under `.apm/hooks/` would ship to Claude Code and silently not to Copilot. +The effect looks like the twin of the `mcpServers` gap that ADR-0017's 2026-08-13 amendment +re-injects for: same "string or object" type, same absence of a default, same outcome of a Copilot +manifest with no pointer. The *mechanism* differs, and ADR-0017 is explicit about it — `mcpServers` +is actively stripped by `build_plugin_manifest`, whereas `hooks` "was never in +`build_plugin_manifest`'s strip list at all"; it is simply never emitted, because `apm.yml` has no +key that produces one. So this is an absence apm never fills, not a removal to reverse. -Follow-up, not fixed here: decide whether `sync-plugin-content.sh` should re-inject a `hooks` pointer -into `.github/plugin/plugin.json` the way `reinject_mcp_servers()` already does for `mcpServers`. That -decision has a prerequisite — the two hook schemas differ (see above), so a single `hooks/hooks.json` -pointed at from both manifests would hand Copilot a file shaped for Claude Code. Reconcile the schemas -before wiring the pointer. +## Why no `hooks` pointer is injected + +**Decided (2026-08-14, PR #95): the gap stays documented rather than patched.** `sync-plugin-content.sh` +does *not* re-inject a `hooks` pointer into `.github/plugin/plugin.json`, and a test pins that +absence. Full reasoning is in ADR-0017's "no `hooks` pointer" amendment; the short version, because +the one-line fix looks obvious and someone will propose it again: + +The `mcpServers` re-injection is safe because `.mcp.json` is **one format both ecosystems read**, so +the pointer is a true statement about the file whatever it contains. Hooks have no shared format. +Compare the two structures above: Claude Code wants `PreToolUse` with `matcher` objects; Copilot +requires `version: 1`, lowercase event names, and per-shell `bash`/`powershell` keys. And apm merges +`.apm/hooks/*.json` into **exactly one** `hooks.json` with no per-target shaping — the same file +Claude Code convention-scans. One file, two incompatible readers. + +So a pointer would tell Copilot that a Claude-shaped file is Copilot-shaped: an incomplete manifest +traded for a wrong one. It is not inert even today — `{"hooks": {}}` has no `version: 1`, so the +pointer would name a file invalid against the very schema it is pointed at from. And it does not +become correct later: whoever writes the first real hook writes it in one shape, and it is the +Claude shape in practice, since Claude Code auto-discovers the same file and is what hooks here are +authored against. + +**What this costs you:** a hook authored under `.apm/hooks/` reaches Claude Code and not Copilot. +That is a real limitation, and it is the accepted one until apm emits a per-target hooks file or the +two schemas converge. If you need a Copilot hook today, raise it — it needs an upstream change or a +second authoring path, not a pointer. + +## Symlinks under `.apm/` do not survive + +Do not author any file under `plugins/kyberforge/.apm/` as a symlink. apm's bundle exporter filters +symlinks out of the bundle entirely and says nothing, so the file never reaches the mirror. Since +`sync-plugin-content.sh` builds both sides of its drift comparison from that same bundle, the loss +used to be invisible to `--check` as well. `check_apm_symlinks()` now reads the `.apm/` source tree +directly and fails the sync with the offending path — replace the symlink with a regular file. + +It stays quiet about one place: `.apm///tests/`, the dev-fixture directory the +mirror excludes anyway (a symlink there loses nothing, because nothing under it is mirrored). A +`tests/` deeper than that — `assets/templates/tests/`, a scaffolding asset the mirror does carry — +is reported like anywhere else. See ADR-0017's symlink amendment. diff --git a/scripts/sync-plugin-content.sh b/scripts/sync-plugin-content.sh index 2d0b245..028ad85 100755 --- a/scripts/sync-plugin-content.sh +++ b/scripts/sync-plugin-content.sh @@ -56,11 +56,33 @@ set -euo pipefail # `hooks/hooks.json` "at the plugin root, not inside .claude-plugin/" # (plugins/kyberforge/docs/research/docs/claude-code-plugins/configuration.md's "Plugin # Directory Layout" table; quoted verbatim in ADR-0017's own root-cause analysis), and -# the compiled plugin.json carries no `hooks` pointer to override that -- apm's -# build_plugin_manifest strips pointer fields unconditionally, and re-injecting one is -# the option ADR-0017 explicitly rejected. A root-level hooks.json (this script's own -# pre-fix output shape) is therefore scanned by nothing at all, and is deleted as stale -# by a real sync / reported as drift by --check. +# the compiled plugin.json carries no `hooks` pointer to override that. apm emits none: +# `hooks` is not in build_plugin_manifest's strip list at all (that list is +# agents/skills/commands/instructions, and it is dead code besides -- see ADR-0017's +# "Considered options"); apm.yml simply has no key that produces one. A root-level +# hooks.json (this script's own pre-fix output shape) is therefore scanned by nothing at +# all, and is deleted as stale by a real sync / reported as drift by --check. +# +# NO `hooks` POINTER IS RE-INJECTED into .github/plugin/plugin.json, deliberately, and +# this is NOT the same call as mcpServers above. Copilot types `hooks` "string or object" +# with no default, exactly like mcpServers, so Copilot resolves no hooks from any plugin +# here -- but the two ecosystems' hooks FILE FORMATS are mutually incompatible (Claude: +# `{"hooks": {"PreToolUse": [{matcher, hooks:[...]}]}}`; Copilot: `{"version": 1, +# "hooks": {"sessionStart": [{type, bash, powershell, ...}]}}`), and apm's exporter +# merges .apm/hooks/*.json into exactly ONE hooks.json with no per-target shaping +# (_collect_hooks_from_apm, apm_cli/bundle/plugin_exporter.py). A pointer would therefore +# assert that a Claude-shaped file is Copilot-shaped. .mcp.json carries no such claim -- +# it is one host-agnostic format both ecosystems read. See ADR-0017's 2026-08-14 +# "no `hooks` pointer" amendment; plugins/kyberforge/docs/hooks.md carries the +# author-facing version. +# +# SYMLINKS UNDER .apm/ ARE NOT MIRRORED and cannot be: apm's bundle exporter filters +# every symlink out of the bundle it produces (`f.is_file() and not f.is_symlink()` in +# _collect_flat/_collect_recursive, and the same test in _collect_hooks_from_apm), with +# no warning. Nothing downstream of the bundle can see the omission -- both sides of +# --check's diff are built from that same bundle, so sync and --check agree the symlink +# never existed. check_apm_symlinks below therefore reads the .apm/ SOURCE tree directly, +# which is the only place the loss is visible, and reports it in both modes. # # tests/ subdirectories (e.g. .apm/skills//tests/*.bats) are excluded from the # mirror -- they are dev-time fixtures a plugin host never needs to discover, and several @@ -131,6 +153,14 @@ source "$SCRIPT_DIR/lib/batch-run.sh" # directory apm never emits and no plugin host ever scans. MIRROR_DIRS=(agents skills commands instructions extensions) +# The .apm/ SOURCE directories apm's exporter reads to build the bundle -- the input +# side of MIRROR_DIRS, and deliberately a different list: `prompts` is here because +# apm reads it (folding it into commands/), and `hooks` is here because +# _collect_hooks_from_apm reads it. Used only by check_apm_symlinks, which needs to +# know which parts of .apm/ are mirror INPUT: a symlink under a directory apm never +# reads loses nothing and must not be reported as loss. +APM_SOURCE_DIRS=(agents skills prompts commands instructions extensions hooks) + # 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. @@ -241,6 +271,49 @@ normalize_trailing_newline() { printf '%s\n' "$(cat "$1")" >"$2" } +# Reports every symlink under the .apm/ directories apm's exporter reads. Runs in +# BOTH modes, and is the one check here that reads the .apm/ source tree rather than +# comparing two bundle-derived trees. +# +# It has to: apm drops symlinks from the bundle silently (see the header), so by the +# time either mode has a bundle the symlink is already gone from both sides of every +# comparison. check_dir, check_file and check_path_modes all diff the live mirror +# against a freshly synced copy -- neither side has the file, they agree, and --check +# exits 0 while the author's content is simply not there. That is the entire failure +# mode: not a mismatch, an absence with nothing left to mismatch against. Verified on +# a fixture -- `ln -s real.md link.md` under .apm/skills/hello/ produced a mirror with +# no link.md and a --check at exit 0. +# +# Reported rather than resolved (no dereference-and-copy): the mirror's contract is +# that it is `apm pack`'s output, and materializing a file apm chose not to export +# would make a real sync produce content the bundle does not contain -- exactly the +# "reimplement the mapping outside apm" that ADR-0017 rejects. Telling the author is +# the cheap, in-contract half. +# +# /tests is carved out to match sync_dir's own depth-scoped exclusion: that +# subtree is not mirrored whether or not it holds a symlink, so nothing is lost there. +# The carve-out is on the SECOND path segment specifically, mirroring sync_dir's +# `-mindepth 2 -maxdepth 2`; a `tests` deeper than that (assets/templates/tests) IS +# mirrored, so a symlink in it is real loss and is reported. +check_apm_symlinks() { + local apm_dir="$1" + local d src link rel rest + + for d in "${APM_SOURCE_DIRS[@]}"; do + src="$apm_dir/$d" + [[ -d "$src" ]] || continue + while IFS= read -r link; do + rel="${link#"$src"/}" + rest="${rel#*/}" + if [[ "$rest" != "$rel" ]] && { [[ "$rest" == "tests" ]] || [[ "$rest" == tests/* ]]; }; then + continue + fi + echo "FAIL $link: symlink under .apm/ -- apm's bundle exporter drops symlinks from the bundle entirely, so this content never reaches the mirror and no diff can see it missing. Replace it with a regular file." >&2 + FAIL=1 + done < <(find "$src" -type l -print 2>/dev/null | LC_ALL=C sort) + done +} + # Real-mode mirror write. There is no --check branch here on purpose: check mode # calls this same function against a throwaway copy of the plugin root and diffs # the result (see sync_one), so the tests/ exclusion below is the only copy of @@ -553,9 +626,16 @@ sync_plugin_manifest() { # FAIL here is that subshell's own copy -- it never touches the parent's FAIL # and must be handed back via status_file instead. sync_one() { - local plugin_dir="${1%/}" status_file="$2" + local plugin_dir="${1%/}" status_file="$2" verified_file="$3" local apm_dir="$plugin_dir/.apm" FAIL=0 + # "This plugin's mirror was actually synced/checked." Flipped to 1 only at the + # very bottom, so every early return below -- nonexistent directory, no .apm/, + # apm pack failure, no bundle -- leaves it 0. --all compares the count of these + # against the number of plugins marketplace.json declared (see the dispatch + # loop's aggregation); an exit-status-only handshake cannot express "ran, but + # verified nothing", which is exactly what the SKIP below is. + echo 0 >"$verified_file" if [[ ! -d "$plugin_dir" ]]; then echo "FAIL $plugin_dir: plugin directory does not exist" >&2 @@ -570,6 +650,10 @@ sync_one() { return 0 fi + # Before the pack, not after: this reads the .apm/ source tree, and it is the + # only report that survives apm's silent symlink filtering (see the header). + check_apm_symlinks "$apm_dir" + local name scratch bundle_dir pack_log pack_cwd name="$(basename "$plugin_dir")" scratch="$SCRATCH_ROOT/$name" @@ -656,6 +740,7 @@ sync_one() { sync_plugin_manifest "$plugin_dir" "$pack_cwd" ".claude-plugin/plugin.json" sync_plugin_manifest "$plugin_dir" "$pack_cwd" ".github/plugin/plugin.json" fi + echo 1 >"$verified_file" echo "$FAIL" >"$status_file" } @@ -670,18 +755,50 @@ sync_one() { declare -a batch_args=() for plugin_dir in ${plugin_dirs[@]+"${plugin_dirs[@]}"}; do name="$(basename "${plugin_dir%/}")" - cmd="$(printf 'sync_one %q %q' "$plugin_dir" "$SCRATCH_ROOT/$name.status")" + cmd="$(printf 'sync_one %q %q %q' "$plugin_dir" "$SCRATCH_ROOT/$name.status" \ + "$SCRATCH_ROOT/$name.verified")" batch_args+=("$name" "$cmd") done batch_run "$SCRATCH_ROOT" ${batch_args[@]+"${batch_args[@]}"} +declare -a unverified=() for plugin_dir in ${plugin_dirs[@]+"${plugin_dirs[@]}"}; do name="$(basename "${plugin_dir%/}")" cat "$SCRATCH_ROOT/$name.log" >&2 status="$(cat "$SCRATCH_ROOT/$name.status" 2>/dev/null || echo 1)" [[ "$status" -ne 0 ]] && FAIL=1 + # Missing file reads as 0 (unverified), matching the status file's `|| echo 1` + # default: a job whose marker never got written did not verify anything. + if [[ "$(cat "$SCRATCH_ROOT/$name.verified" 2>/dev/null || echo 0)" != "1" ]]; then + unverified+=("$plugin_dir") + fi done +# --all's second floor, and the one the zero-plugin floor above cannot express. +# That floor rejects "the marketplace yielded no plugins"; this rejects "the +# marketplace yielded N and only M were actually verified". The gap between them +# is sync_one's SKIP path: a plugin directory with no .apm/ reports status 0 and +# checks nothing, so --all printed one SKIP line among the noise and exited 0 +# having verified fewer plugins than it listed. --all is a pre-push gate over a +# GENERATED work list, so "checked fewer than declared" has to be a failure. +# +# There is no legitimate state in this repo where a listed local plugin lacks +# .apm/: ADR-0015 made .apm/ the sole authoring source for every plugin here, and +# ADR-0017's mirror is defined as that directory's compiled output, so a local +# marketplace entry without one is drift in one of the two -- either the directory +# lost its .apm/, or marketplace.json still lists a package that is no longer one. +# Both need a human, and neither is fixed by re-running the sync, so this is +# reported separately from the drift hint below rather than folded into it. +# +# SKIP stays a skip when plugin directories are named EXPLICITLY on the command +# line: there the caller chose the work list and a non-apm directory is their +# business, not a generated file's drift. +UNVERIFIED=0 +if [[ "$ALL" -eq 1 ]] && [[ ${#unverified[@]} -gt 0 ]]; then + echo "Error: --all verified $(( ${#plugin_dirs[@]} - ${#unverified[@]} )) of the ${#plugin_dirs[@]} local plugin entries $MARKETPLACE declares; unverified: ${unverified[*]}. A listed plugin that cannot be checked (typically: its .apm/ is gone, which sync_one skips) is drift, not a pass -- restore its .apm/, or drop the entry from root apm.yml's marketplace.packages[] and recompile." >&2 + UNVERIFIED=1 +fi + if [[ "$FAIL" -ne 0 ]]; then if [[ "$CHECK" -eq 1 ]]; then if [[ "$ALL" -eq 1 ]]; then @@ -692,3 +809,5 @@ if [[ "$FAIL" -ne 0 ]]; then fi exit 1 fi + +[[ "$UNVERIFIED" -eq 0 ]] || exit 1 diff --git a/tests/test-sync-plugin-content.sh b/tests/test-sync-plugin-content.sh index be880f5..b83e756 100755 --- a/tests/test-sync-plugin-content.sh +++ b/tests/test-sync-plugin-content.sh @@ -1090,6 +1090,202 @@ else esac fi +# --- 29. A symlink under .apm/ is reported, in both modes --- +# apm's bundle exporter filters every symlink out of the bundle it builds +# (`f.is_file() and not f.is_symlink()` in _collect_flat/_collect_recursive, +# apm_cli/bundle/plugin_exporter.py) with no warning. Every other check here diffs +# the live mirror against a freshly synced copy, and BOTH are built from that same +# bundle — so the symlink is absent on both sides, they agree, and --check exits 0 +# while the author's content is simply gone. Not a mismatch: an absence with nothing +# left to mismatch against. Verified before the fix: `ln -s real.md link.md` under +# .apm/skills/hello/ produced a mirror with no link.md and a --check at exit 0. +# +# Message-asserted, not exit-code-asserted: an unsynced fixture exits 1 anyway, so a +# bare non-zero would pass with the detection deleted. +echo "" +echo "--- a symlink under .apm/ is reported as lost content in both modes ---" +FIXTURE29="$(make_fixture)"; track "$FIXTURE29" +printf 'real content\n' > "$FIXTURE29/.apm/skills/hello/real.md" +ln -s real.md "$FIXTURE29/.apm/skills/hello/link.md" +RCSYNC29=0 +SYNC29="$(bash "$SCRIPT" "$FIXTURE29" 2>&1)" || RCSYNC29=$? +CHECK29="$(bash "$SCRIPT" --check "$FIXTURE29" 2>&1 || true)" +for MODE29 in sync check; do + case "$MODE29" in + sync) OUT29="$SYNC29" ;; + check) OUT29="$CHECK29" ;; + esac + case "$OUT29" in + *"$FIXTURE29/.apm/skills/hello/link.md: symlink under .apm/"*) + pass "$MODE29 mode reports the symlink under .apm/ by path" ;; + *) + fail "$MODE29 mode did not report the symlink under .apm/ — apm drops it silently and no diff can see it: $OUT29" ;; + esac +done +# The loss is real, not theoretical: assert the mirror genuinely lacks it, so this +# case still means something if apm ever starts exporting symlinks. +if [[ ! -e "$FIXTURE29/skills/hello/link.md" ]]; then + pass "the symlink is indeed absent from the mirror (nothing else could have caught it)" +else + fail "the symlink reached the mirror — apm's exporter no longer drops it, so this report is now wrong" +fi +# The real sync above exited non-zero too (its rc, not a fresh run). Sync and --check +# agreeing is this script's core contract, and a real sync that "succeeds" while +# dropping content breaks it. +if [[ "$RCSYNC29" -ne 0 ]]; then + pass "a real sync exits non-zero rather than reporting success over dropped content" +else + fail "a real sync exited 0 while silently dropping .apm/ content — sync and --check must agree" +fi +rm -f "$FIXTURE29/.apm/skills/hello/link.md" +if bash "$SCRIPT" "$FIXTURE29" > /dev/null 2>&1 && bash "$SCRIPT" --check "$FIXTURE29" > /dev/null 2>&1; then + pass "removing the symlink clears the report in both modes" +else + fail "the symlink report survived its removal" +fi + +# --- 29b. The report is scoped to content the mirror would actually carry --- +# sync_dir strips //tests from the mirror outright, so a symlink in +# there loses nothing and reporting it would be a false alarm demanding a pointless +# edit. A `tests` DEEPER than that is a template asset the mirror does carry (case +# 3b), so a symlink in it is real loss. Same depth boundary, both directions — +# a carve-out asserted in only one direction passes on "report nothing, ever". +echo "" +echo "--- the symlink report follows the mirror's own tests/ depth boundary ---" +FIXTURE29B="$(make_fixture)"; track "$FIXTURE29B" +printf 'x\n' > "$FIXTURE29B/.apm/skills/hello/tests/real.txt" +ln -s real.txt "$FIXTURE29B/.apm/skills/hello/tests/link.txt" +bash "$SCRIPT" "$FIXTURE29B" > /dev/null 2>&1 +RC29B=0 +OUT29B="$(bash "$SCRIPT" --check "$FIXTURE29B" 2>&1)" || RC29B=$? +case "$OUT29B" in + *"tests/link.txt: symlink under .apm/"*) + fail "a symlink under the un-mirrored /tests/ was reported — nothing is lost there" ;; + *) + pass "a symlink under /tests/ is not reported (that subtree is not mirrored)" ;; +esac +# Exit code as well as message, from that same run: a carve-out that suppresses the +# line but still fails the gate is not a carve-out. +if [[ "$RC29B" -eq 0 ]]; then + pass "--check is clean with a symlink confined to the un-mirrored tests/ fixture dir" +else + fail "--check reported drift for a symlink under the un-mirrored /tests/: $OUT29B" +fi +printf 'y\n' > "$FIXTURE29B/.apm/skills/hello/assets/templates/tests/real.txt" +ln -s real.txt "$FIXTURE29B/.apm/skills/hello/assets/templates/tests/link.txt" +OUT29B2="$(bash "$SCRIPT" --check "$FIXTURE29B" 2>&1 || true)" +case "$OUT29B2" in + *"assets/templates/tests/link.txt: symlink under .apm/"*) + pass "a symlink under the mirrored assets/templates/tests/ IS reported" ;; + *) + fail "a symlink under the mirrored assets/templates/tests/ was not reported — the carve-out is depth-agnostic and swallows real loss: $OUT29B2" ;; +esac + +# --- 30. --all fails when it verified fewer plugins than the marketplace declares --- +# The zero-plugin floor (case 28) rejects "the marketplace yielded nothing"; it cannot +# see "it yielded N and only M were checked". sync_one SKIPs a plugin directory with no +# .apm/ at status 0, so --all printed one SKIP line and exited 0 having verified one +# plugin fewer than it listed — a pre-push gate over a GENERATED work list silently +# covering less than it claims. +echo "" +echo "--- --check --all fails when a listed plugin cannot be verified ---" +# A repo fixture with real plugin directories, unlike case 28's marketplace-only one. +make_repo_with_plugins() { + local dir + dir="$(mktemp -d)" + dir="$(cd "$dir" && pwd -P)" + if ! env -u GIT_DIR -u GIT_WORK_TREE git -C "$dir" init -q >/dev/null 2>&1; then + echo "make_repo_with_plugins: 'git init' failed in $dir" >&2 + exit 1 + fi + mkdir -p "$dir/.claude-plugin" "$dir/plugins" + printf '%s' '{"plugins":[{"name":"good","source":"./plugins/good"},{"name":"bare","source":"./plugins/bare"}]}' \ + > "$dir/.claude-plugin/marketplace.json" + local p + for p in good bare; do + local src + src="$(make_fixture)"; track "$src" + mv "$src" "$dir/plugins/$p" + done + echo "$dir" +} +REPO30="$(make_repo_with_plugins)"; track "$REPO30" +# Sync both first, so the ONLY thing --all can complain about below is the count. +bash "$SCRIPT" "$REPO30/plugins/good" "$REPO30/plugins/bare" > /dev/null 2>&1 +if (cd "$REPO30" && env -u GIT_DIR -u GIT_WORK_TREE bash "$SCRIPT" --check --all > /dev/null 2>&1); then + pass "--check --all is clean when every declared plugin is verifiable (baseline)" +else + fail "--check --all reported drift on a freshly synced two-plugin repo — cannot test the count case" +fi +# Now take one listed plugin's .apm/ away: it is still declared, still on disk, and +# now unverifiable. Its mirror is left in place, so no other check has anything to say. +rm -rf "$REPO30/plugins/bare/.apm" +RC30=0 +OUT30="$(cd "$REPO30" && env -u GIT_DIR -u GIT_WORK_TREE bash "$SCRIPT" --check --all 2>&1)" || RC30=$? +case "$RC30:$OUT30" in + 0:*) + fail "--check --all exited 0 having verified 1 of the 2 plugins its marketplace declares" ;; + *"verified 1 of the 2 local plugin entries"*) + pass "--check --all fails and names how many of the declared plugins it actually verified" ;; + *) + fail "--check --all exited $RC30 but not for the under-count reason: $OUT30" ;; +esac +# The message must name the plugin, not just the arithmetic — a count alone leaves the +# reader diffing marketplace.json against a directory listing by hand. +case "$OUT30" in + *"unverified: $REPO30/plugins/bare"*) + pass "the failure names the unverified plugin directory" ;; + *) + fail "the failure did not name the unverified plugin directory: $OUT30" ;; +esac +# ...and an explicitly-named plugin dir with no .apm/ stays a skip (case 6): there the +# caller chose the work list, so a non-apm directory is their business, not drift in a +# generated file. +if bash "$SCRIPT" --check "$REPO30/plugins/bare" > /dev/null 2>&1; then + pass "the same directory named explicitly is still a clean skip, not a failure" +else + fail "an explicitly-named plugin dir with no .apm/ now fails — case 6's skip contract is broken" +fi + +# --- 31. No `hooks` pointer is injected into the Copilot manifest --- +# Copilot types `hooks` "string or object" with NO default (github-copilot-plugins/ +# configuration.md:47), exactly like mcpServers — so Copilot resolves no hooks from any +# plugin here, and re-injecting a pointer the way reinject_mcp_servers() does for +# mcpServers looks like the obvious twin fix. It is not, and this case pins the +# difference: apm merges .apm/hooks/*.json into exactly ONE hooks.json with no +# per-target shaping, while the two ecosystems' hook file formats are mutually +# incompatible (Claude: `{"hooks":{"PreToolUse":[{matcher,hooks}]}}`; Copilot: +# `{"version":1,"hooks":{"sessionStart":[{type,bash,powershell}]}}`). A pointer would +# assert that a Claude-shaped file is Copilot-shaped — a wrong manifest in place of an +# incomplete one. .mcp.json carries no such claim: it is one format both hosts read. +# See ADR-0017's "no `hooks` pointer" amendment and plugins/kyberforge/docs/hooks.md. +echo "" +echo "--- the generated Copilot manifest carries no hooks pointer ---" +FIXTURE31="$(make_fixture_with_mcp '{"mcpServers":{"demo":{"command":"demo-server","type":"stdio"}}}')"; track "$FIXTURE31" +mkdir -p "$FIXTURE31/.apm/hooks" +cat > "$FIXTURE31/.apm/hooks/hooks.json" <<'EOF' +{"hooks": {"PreToolUse": [{"matcher": "Bash", "hooks": [{"type": "command", "command": "true"}]}]}} +EOF +bash "$SCRIPT" "$FIXTURE31" > /dev/null 2>&1 +if [[ ! -f "$FIXTURE31/hooks/hooks.json" ]]; then + fail "sync produced no hooks/hooks.json — cannot test the hooks-pointer decision" +elif [[ ! -f "$FIXTURE31/.github/plugin/plugin.json" ]]; then + fail "sync produced no .github/plugin/plugin.json — cannot test the hooks-pointer decision" +else + pass "a non-empty .apm/hooks/ produces hooks/hooks.json (Claude Code's convention path)" + if jq -e 'has("hooks") | not' "$FIXTURE31/.github/plugin/plugin.json" > /dev/null 2>&1; then + pass "no hooks pointer in .github/plugin/plugin.json, even with a real hook present" + else + fail "a hooks pointer was injected into the Copilot manifest (got: $(jq -c '.hooks' "$FIXTURE31/.github/plugin/plugin.json" 2>/dev/null)) — it would point Copilot at a Claude-shaped hooks file. Reconcile the two hook schemas first; see ADR-0017" + fi + # The Claude manifest needs none either: hooks/hooks.json IS its convention path. + if jq -e 'has("hooks") | not' "$FIXTURE31/.claude-plugin/plugin.json" > /dev/null 2>&1; then + pass "no hooks pointer in .claude-plugin/plugin.json either — the convention path needs none" + else + fail "a hooks pointer appeared in the Claude manifest, which convention-scans hooks/hooks.json already" + fi +fi + echo "" echo "Results: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]]