feat(kyberforge): execute the plugin→APM conversion #95
@@ -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
|
||||
`<category>/<name>/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/<name>/SKILL.md` and
|
||||
|
||||
@@ -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/<category>/<name>/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.
|
||||
|
||||
@@ -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/<name>/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.
|
||||
#
|
||||
# <name>/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
|
||||
|
||||
@@ -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 <category>/<name>/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 <name>/tests/ was reported — nothing is lost there" ;;
|
||||
*)
|
||||
pass "a symlink under <name>/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 <name>/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 ]]
|
||||
|
||||
Reference in New Issue
Block a user