docs: correct the apm-only change after a five-agent review
A five-agent review of718c79aandd2480b8found no skill, agent or hook regressions (39 skills before and after) and confirmed both hook removals are genuinely moot -- verified against the tree, not taken on the commit's word. It did find one functional regression (fixed separately) and this documentation drift. Counting errors, all from a git pathspec `*` crossing `/`: - 17 .bats files shipped to consumers is really 10; 17 counted tracked paths merely containing /tests/, one of them a template asset - "roughly 88s off every push" is ~92.4s; 88 omitted validate-plugins - "roughly 70% of each plugin remains live" holds only for kyberforge; the real spread is 44.3% (bin) to 70.6%, now a table - the pre-push enforcement row was half-corrected: 33 entries stood unstruck (now 27) and 14 -> 11 switched counting basis mid-sentence - the root .claude-plugin/plugin.json was described as "kept"; it has never been tracked gates.md said "Ten hooks" above a nine-row table (11 was decremented for one removal, not two), and "both need the claude CLI" for one remaining validator. Its pretty-format-json exclude rationale claimed six alternations expanding to sixteen files in a passage headed "Mind which number you are quoting" -- four alternations, two live files; the two dead ones are dropped from the pattern. check-useless-excludes could not catch this: it only flags an exclude matching nothing at all. ADR-0024 cited ADR-0006 for a patch-bump rule it does not contain and which ADR-0015 explicitly retired; stated apm's marketplace probe order backwards (.claude-plugin/ is the last candidate, not the first, so the earlier .github/plugin/ deletion only demoted resolution); undercounted apm's skill-deploying targets as seven when there are fifteen; and never recorded that validate-plugins was removed. The symlink hedge is resolved: apm_cli/security/gate.py's ignore_non_content() drops symlinks silently on deploy while apm_modules/ materialization dereferences them, so content survives that far and vanishes at install. Accepted with no replacement guard, per decision -- kyberforge/docs/hooks.md previously asserted a guard that had been deleted with its script. Four plugin READMEs still advertised `claude plugin install`; ADRs 0001, 0006, 0013, 0014, 0015 and 0019 described deleted machinery in the present tense, 0019 most consequentially as the live justification for the SessionStart hook's .apm/ path. CONTEXT.md's "apm package" entry forbade "plugin" while using it in its own body, and "Output profile" lost the antecedent for "one catalogue serves both". run-tests.sh gains the .claude/skills/ exclusion run-bats.sh already had. Latent today -- no test-*.sh lives under any .apm/skills/*/tests/ -- but apm now deploys those directories, so one would be discovered twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
This commit is contained in:
@@ -4,7 +4,7 @@ Plugin documentation. Not read automatically by Claude Code or GitHub Copilot CL
|
||||
|
||||
| Path | Purpose |
|
||||
|------|---------|
|
||||
| `hooks.md` | Where this plugin's hooks are authored (`.apm/hooks/`), where they are generated to (`hooks/hooks.json`), and the Claude Code and Copilot CLI schemas |
|
||||
| `hooks.md` | Where this plugin's hooks are authored (`.apm/hooks/`), what apm deploys them into (`.claude/settings.json`), and the Claude Code and Copilot CLI schemas |
|
||||
|
||||
Everything else under this directory is research material.
|
||||
|
||||
|
||||
@@ -1,23 +1,21 @@
|
||||
# Hooks
|
||||
|
||||
Reference for this plugin's hook definitions: where to edit them, where they end up, and what the
|
||||
host reads.
|
||||
|
||||
This document lives in `docs/` rather than next to the hooks it describes. `plugins/kyberforge/hooks/`
|
||||
is a **generated mirror** — `scripts/sync-plugin-content.sh` runs `rm -rf` on it before every
|
||||
rebuild, so any hand-written file placed there is deleted on the next sync with no drift warning
|
||||
(a prior copy of this document was lost exactly that way). See ADR-0017.
|
||||
Reference for this plugin's hook definitions: where to edit them, what apm does with them, and what
|
||||
the host ends up reading.
|
||||
|
||||
## Where to edit
|
||||
|
||||
Author hooks in `plugins/kyberforge/.apm/hooks/*.json`. `apm pack --format plugin` merges every
|
||||
file in that directory into a single `hooks.json`, which `sync-plugin-content.sh` copies to
|
||||
`plugins/kyberforge/hooks/hooks.json` — the path Claude Code convention-scans. Never edit the
|
||||
mirrored file; the `check-plugin-content-sync` pre-push hook reports it as drift.
|
||||
Author hooks in `plugins/kyberforge/.apm/hooks/*.json`. `.apm/` is the only content source and
|
||||
`apm install` is the only supported install path (ADR-0024) — there is no generated mirror at the
|
||||
plugin root and no per-plugin `plugin.json`, so `.apm/hooks/` is both where you edit and what ships.
|
||||
|
||||
## Claude Code structure
|
||||
apm merges every `*.json` in that directory into a single hook definition and writes the event
|
||||
bindings into the consuming project's `.claude/settings.json`; see "Deployed shape" below. Scripts a
|
||||
hook invokes live in the same directory, alongside the JSON that references them.
|
||||
|
||||
`hooks/hooks.json` is read by Claude Code. Structure:
|
||||
## Hook file structure
|
||||
|
||||
The shape Claude Code reads, and therefore the shape to author under `.apm/hooks/`:
|
||||
|
||||
```json
|
||||
{
|
||||
@@ -43,7 +41,7 @@ only the per-agent `hooks` field, not the plugin-level set. Treat the five names
|
||||
this repo has verified, not as the schema. Check Claude Code's own hooks documentation before wiring
|
||||
an event not listed here.
|
||||
|
||||
## Referencing a script — use the `.apm/` path, not the mirror
|
||||
## Referencing a script — use the `.apm/` path
|
||||
|
||||
Use `${CLAUDE_PLUGIN_ROOT}` to reference scripts inside this plugin; the plugin runs from a cache or
|
||||
`apm_modules/` path after install, not its original repo location. **Address the script at its
|
||||
@@ -55,15 +53,9 @@ Use `${CLAUDE_PLUGIN_ROOT}` to reference scripts inside this plugin; the plugin
|
||||
|
||||
The obvious-looking `${CLAUDE_PLUGIN_ROOT}/hooks/check-apm-current.sh` does not work, and fails
|
||||
quietly enough to be worth spelling out. apm resolves the placeholder against the installed package
|
||||
root, where `hooks/` is the **generated mirror** — and `apm pack` merges only `*.json` out of
|
||||
`.apm/hooks/`, dropping every non-JSON file. So the mirror contains `hooks.json` and nothing else.
|
||||
apm prints `Hook script not found: .../hooks/check-apm-current.sh` and then deploys the hook anyway,
|
||||
pointing at a path with no file behind it.
|
||||
|
||||
Nor can the script be hand-placed in `plugins/kyberforge/hooks/` to satisfy that path:
|
||||
`sync-plugin-content.sh` runs `rm -rf` on the directory before every rebuild (ADR-0017), so it would
|
||||
be deleted on the next sync with no drift warning — the same trap that ate this document's
|
||||
predecessor.
|
||||
root, and there is no `hooks/` directory there at all — the package's content is `.apm/`. apm prints
|
||||
`Hook script not found: .../hooks/check-apm-current.sh` and then deploys the hook anyway, pointing at
|
||||
a path with no file behind it.
|
||||
|
||||
`tests/test-apm-current-hook.sh` pins the reference so a well-meaning "simplification" back to
|
||||
`hooks/` fails the suite rather than silently disabling the hook.
|
||||
@@ -77,7 +69,9 @@ tracked in a `.claude/apm-hooks.json` sidecar, so an uninstall removes them with
|
||||
hand-authored hooks. Both `.claude/hooks/` and the sidecar are gitignored install output.
|
||||
|
||||
Note that apm's **executable-trust gate is off** unless the consuming project's `apm.yml` has an
|
||||
`executables:` block — without one, package hooks deploy with no prompt. See ADR-0019.
|
||||
`executables:` block — without one, package hooks deploy with no prompt. The allow key is
|
||||
version-pinned (`kyberforge#<version>`), so a version bump on one side alone stops the hook
|
||||
deploying; `check-executables-allow-sync` is the pre-push gate that catches it. See ADR-0019.
|
||||
|
||||
## The SessionStart hook
|
||||
|
||||
@@ -89,11 +83,11 @@ ADR-0019.
|
||||
**Where it looks for the lockfile.** The hook resolves a project directory as `${CLAUDE_PROJECT_DIR}`
|
||||
when the host exports it (Claude Code does, for SessionStart hooks) and the current directory
|
||||
otherwise, then exits silently unless that directory holds an `apm.lock.yaml` — which is what makes
|
||||
it inert for any host that installed this plugin natively rather than through apm. Both `apm`
|
||||
invocations run against the same resolved directory. The earlier spelling checked a bare
|
||||
`apm.lock.yaml` against the session's cwd, so a session opened in a subdirectory of an
|
||||
apm-consuming repo no-opped silently. Keep the cwd fallback: a host that sets no
|
||||
`CLAUDE_PROJECT_DIR` must still get inert-but-harmless behaviour, not an unset-variable error.
|
||||
it inert in any project that does not consume packages through apm. Both `apm` invocations run
|
||||
against the same resolved directory. The earlier spelling checked a bare `apm.lock.yaml` against the
|
||||
session's cwd, so a session opened in a subdirectory of an apm-consuming repo no-opped silently.
|
||||
Keep the cwd fallback: a host that sets no `CLAUDE_PROJECT_DIR` must still get inert-but-harmless
|
||||
behaviour, not an unset-variable error.
|
||||
|
||||
**The `timeout` in `hooks.json` must exceed the script's own budget.** The script spends at most
|
||||
`timeout 60 apm outdated` plus `timeout 300 apm update`; the hook entry declares `timeout: 380`, the
|
||||
@@ -113,74 +107,45 @@ hook.
|
||||
|
||||
## GitHub Copilot CLI
|
||||
|
||||
Copilot reads a differently-shaped `hooks.json`: `version: 1` is required, each entry is
|
||||
`type: "command"` with separate `bash` and `powershell` scripts, and the lifecycle points are
|
||||
lowercase and differently named (`sessionStart`, `sessionEnd`, `userPromptSubmitted`, `preToolUse`,
|
||||
`postToolUse`, `errorOccurred`, `agentStop`). See
|
||||
`docs/research/docs/github-copilot-plugins/configuration.md`.
|
||||
**Copilot loads no hooks from this plugin.** Two independent reasons, either one sufficient:
|
||||
|
||||
There is no separate Copilot hooks file at this plugin root, and — as things stand — **Copilot
|
||||
resolves to no hooks file at all.** Two corrections to an earlier revision of this document, which
|
||||
got both halves of this wrong:
|
||||
- **Nothing can point Copilot at a hooks file.** Copilot types `hooks` as a `plugin.json` field of
|
||||
type "string or object" with **no default**
|
||||
(`docs/research/docs/github-copilot-plugins/configuration.md:47`), so there is no convention path
|
||||
for it to scan — it reads hooks only via an explicit pointer. Since ADR-0024 there is no
|
||||
per-plugin Copilot manifest at all, so there is nothing to carry that pointer.
|
||||
- **The two ecosystems do not share a hooks format.** Copilot reads a differently-shaped
|
||||
`hooks.json`: `version: 1` is required, each entry is `type: "command"` with separate `bash` and
|
||||
`powershell` scripts, and the lifecycle points are lowercase and differently named (`sessionStart`,
|
||||
`sessionEnd`, `userPromptSubmitted`, `preToolUse`, `postToolUse`, `errorOccurred`, `agentStop`).
|
||||
See `docs/research/docs/github-copilot-plugins/configuration.md`. apm merges `.apm/hooks/*.json`
|
||||
into one definition with no per-target shaping, and that definition is Claude-shaped.
|
||||
|
||||
**The deleted root `hooks.json` was not a stale sync artifact.** `plugins/kyberforge/hooks.json` was
|
||||
added in `2287ddc` (2026-06-20), the commit that created the plugin, well before
|
||||
`scripts/sync-plugin-content.sh` existed; `plugins/lint/hooks.json` arrived the same way in
|
||||
`f326df4`. Main's Copilot manifest `plugins/kyberforge/plugin.json` declared `"hooks": "hooks.json"`,
|
||||
and `plugins/lint/plugin.json` did the same — these were deliberately pointed-at Copilot hooks files,
|
||||
not leftovers. The sync (`38f1ba4`) later took ownership of that path, and ADR-0017's 2026-08-14
|
||||
amendment moved the generated file to `hooks/hooks.json` because that, not the plugin root, is the
|
||||
path Claude Code convention-scans.
|
||||
|
||||
**Only Claude Code resolves to `hooks/hooks.json`.** Claude Code finds it by auto-discovery.
|
||||
Copilot does not: `docs/research/docs/github-copilot-plugins/configuration.md:47` types `hooks` as a
|
||||
`plugin.json` field of type "string or object" with **no default**, so there is no convention path to
|
||||
scan, and `jq 'has("hooks")'` returns `false` for all six `.github/plugin/plugin.json` files that
|
||||
`apm pack` emits. With the pointer gone and no auto-discovery to fall back on, the Copilot ecosystem
|
||||
sees zero hooks.
|
||||
|
||||
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.
|
||||
|
||||
## 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.
|
||||
The second reason is why "just add a pointer" was rejected even while a Copilot manifest existed: a
|
||||
pointer would tell Copilot that a Claude-shaped file is Copilot-shaped, trading an incomplete
|
||||
manifest for a wrong one. ADR-0024 consequence 7 records that the question is now moot — the
|
||||
manifest it argued about is gone — but the schema mismatch it turned on is not, and it is what any
|
||||
future Copilot hooks support has to solve.
|
||||
|
||||
**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
|
||||
## Symlinks under `.apm/` do not survive, and nothing reports it
|
||||
|
||||
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.
|
||||
Do not author any file under `plugins/kyberforge/.apm/` as a symlink. apm's copy path filters
|
||||
symlinks out silently: `ignore_non_content()` in `apm_cli/security/gate.py` is a
|
||||
`shutil.copytree` ignore callback that drops every entry answering `is_symlink()`, commented
|
||||
"Excludes symlinks (security)". The file never reaches the consumer's install, and no warning is
|
||||
printed at any point.
|
||||
|
||||
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.
|
||||
**No gate catches this.** The pre-push check that used to read the `.apm/` tree and fail on the
|
||||
offending path was deleted along with the content mirror, and the decision was taken not to replace
|
||||
it (ADR-0024 consequence 7). This document is the only thing standing between a symlink and silent
|
||||
content loss. No symlink exists under any `.apm/` today; add one and it is dropped on deploy with
|
||||
nothing reporting it. Replace it with a regular file.
|
||||
|
||||
The old exemption for `.apm/<category>/<name>/tests/` no longer applies either. That directory was
|
||||
exempt only because the mirror excluded it; apm deploys it like any other content (ADR-0024
|
||||
consequence 2), so a symlink there loses content the same as anywhere else.
|
||||
|
||||
Reference in New Issue
Block a user