From 0771fb2d376ca7d740f4ca1e7104fecc6a96f458 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Mon, 28 Sep 2026 18:22:44 +0000 Subject: [PATCH] fix(kyberforge): target-neutral hook token and accurate hook reach docs Switch the SessionStart hook to apm's target-neutral ${PLUGIN_ROOT} token. Two scratch packages differing only in the token deploy byte-identical SessionStart entries with apm 0.28.0, matching the committed .claude/settings.json, so the deployed output does not change. The test pin in tests/test-apm-current-hook.sh moves with it. Correct the claim that Copilot loads no hooks from kyberforge. targets: is package-wide, so apm also writes .github/hooks/kyberforge-hooks.json (nested shape passed through, runtime unverified) and merges into .codex/hooks.json when .codex/ exists. Recorded as accepted in an ADR-0019 amendment dated 2026-09-28; README and docs/hooks.md updated. Refs #94 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KkT7RSDwDbmrM9T34b6sTi --- ...tart-hook-keeps-the-apm-install-current.md | 27 +++++++++ plugins/kyberforge/.apm/hooks/hooks.json | 2 +- plugins/kyberforge/README.md | 4 +- plugins/kyberforge/docs/hooks.md | 56 ++++++++++--------- tests/test-apm-current-hook.sh | 10 ++-- 5 files changed, 65 insertions(+), 34 deletions(-) diff --git a/docs/adr/0019-session-start-hook-keeps-the-apm-install-current.md b/docs/adr/0019-session-start-hook-keeps-the-apm-install-current.md index e8ce5df..81dd203 100644 --- a/docs/adr/0019-session-start-hook-keeps-the-apm-install-current.md +++ b/docs/adr/0019-session-start-hook-keeps-the-apm-install-current.md @@ -124,6 +124,9 @@ A test pins the reference. > `plugins/kyberforge/hooks/` no longer exists at all — so `${CLAUDE_PLUGIN_ROOT}/hooks/...` still > names a path with nothing at it, now because the directory is gone rather than because a sync > emptied it. `tests/test-apm-current-hook.sh` still pins the literal string. +> +> *Superseded in part by the 2026-09-28 amendment below: the token is now `${PLUGIN_ROOT}`. The +> `.apm/`-path conclusion is unchanged.* **Session startup gets slower when the install is stale.** Measured: ~0.7 s for the `apm outdated` check when everything is current, ~10.4 s when six packages are behind and the refresh runs. The @@ -234,6 +237,30 @@ no hook at all, for the reasons already documented in `plugins/kyberforge/docs/h > lockfile there is nothing for `apm update` to refresh, so exiting silently is the correct > behaviour rather than a defensive measure aimed at a second installer. The Copilot CLI sentence is > unaffected. +> +> *The Copilot CLI sentence is superseded by the 2026-09-28 amendment below.* + +> **Amendment (2026-09-28) — target-neutral token; the hook reaches Copilot and Codex, accepted.** +> Two corrections from the apm 0.28.0 research pass behind `primitive-author` (issue #94; +> `plugins/kyberforge/docs/research/docs/microsoft-apm/hooks-primitive-schema.md`). +> +> *The token.* `hooks.json` now references `${PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh`. apm +> documents `${PLUGIN_ROOT}` as its target-neutral token and rewrites it exactly as it rewrites +> `${CLAUDE_PLUGIN_ROOT}`: two scratch packages differing only in the token deploy byte-identical +> `SessionStart` entries, matching the one committed in `.claude/settings.json`. The `.apm/`-path +> rule above is unchanged, and `tests/test-apm-current-hook.sh` pins the new literal. +> +> *The reach.* "Copilot CLI sees no hook at all" was wrong for apm installs. `targets:` is +> package-wide, and kyberforge declares `claude`, `copilot` and `codex`, so apm also writes the hook +> to `.github/hooks/kyberforge-hooks.json` — event renamed to `sessionStart`, path rewritten, +> `version: 1` added, the nested Claude shape otherwise passed through unreshaped — and merges it +> into `.codex/hooks.json` whenever `.codex/` exists. Whether Copilot CLI executes a nested entry or +> honours `matcher` is unverified. This is **accepted**: the hook's behaviour is Claude-specific (the +> `startup` matcher, `CLAUDE_PROJECT_DIR`, the `reloadSkills` output) and the `apm.lock.yaml` guard +> keeps it inert where there is nothing to refresh. Keeping it Claude-only the apm-native way would +> need a separate package declaring `target: claude` — the seventh-plugin alternative below, still +> rejected as disproportionate — because per-file routing (`claude-hooks.json`) is deprecated and +> narrowing kyberforge's own `targets:` would drop its skills from Copilot and Codex. **`scripts/git-hooks/` is now empty.** `post-push` and `test-post-push.sh` are deleted. `install.sh`'s copy block is generic and is kept; `test-git-hooks-install.sh` now synthesizes its diff --git a/plugins/kyberforge/.apm/hooks/hooks.json b/plugins/kyberforge/.apm/hooks/hooks.json index 4d1c7c4..6a6b1dd 100644 --- a/plugins/kyberforge/.apm/hooks/hooks.json +++ b/plugins/kyberforge/.apm/hooks/hooks.json @@ -4,7 +4,7 @@ { "hooks": [ { - "command": "${CLAUDE_PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh", + "command": "${PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh", "timeout": 380, "type": "command" } diff --git a/plugins/kyberforge/README.md b/plugins/kyberforge/README.md index fda9f08..478d461 100644 --- a/plugins/kyberforge/README.md +++ b/plugins/kyberforge/README.md @@ -31,9 +31,9 @@ Authoring source lives in `.apm/`; it is the only content source and the only th |---|---|---| | Skills | `.apm/skills/` | Slash commands available after install | | Agents | `.apm/agents/*.agent.md` | Role-based agents; one vendor-neutral `.agent.md` per agent (ADR-0016) | -| Hooks | `.apm/hooks/` | Event-triggered automation — Claude Code only, see below | +| Hooks | `.apm/hooks/` | Event-triggered automation — authored for Claude Code, see below | -**Hooks are Claude Code-only.** apm merges `.apm/hooks/*.json` into the consuming project's `.claude/settings.json` at install. Copilot CLI has no default hooks path — `agents` and `skills` default to `agents/` and `skills/`, but `hooks` defaults to nothing (`docs/research/docs/github-copilot-plugins/configuration.md:47`), so Copilot reads hooks only via an explicit pointer in a plugin manifest — and since ADR-0024 there is no per-plugin manifest to carry one. Copilot therefore loads no hooks from this plugin. Details, including why a pointer was the wrong fix even when a manifest existed, are in `docs/hooks.md`. +**Hooks are authored for Claude Code, but apm writes them for every package target.** apm merges `.apm/hooks/*.json` into the consuming project's `.claude/settings.json` at install. Because kyberforge also targets Copilot and Codex, apm writes the same hook to `.github/hooks/kyberforge-hooks.json` (nested shape passed through, not reshaped) and into `.codex/hooks.json` when `.codex/` exists. Whether those harnesses execute it is unverified; the hook's behaviour is Claude-specific and it exits silently without an `apm.lock.yaml`. Details are in `docs/hooks.md` and ADR-0019's 2026-09-28 amendment. ## Skills diff --git a/plugins/kyberforge/docs/hooks.md b/plugins/kyberforge/docs/hooks.md index 915ae74..2688d68 100644 --- a/plugins/kyberforge/docs/hooks.md +++ b/plugins/kyberforge/docs/hooks.md @@ -43,15 +43,17 @@ an event not listed here. ## 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 -`.apm/` path:** +Use `${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. `${PLUGIN_ROOT}` is apm's +target-neutral token; apm rewrites it exactly as it rewrites `${CLAUDE_PLUGIN_ROOT}` — verified +byte-identical in the deployed `.claude/settings.json` with apm 0.28.0 — so prefer it, and +`factory-audit` suggests it. **Address the script at its `.apm/` path:** ```json -"command": "${CLAUDE_PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh" +"command": "${PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh" ``` -The obvious-looking `${CLAUDE_PLUGIN_ROOT}/hooks/check-apm-current.sh` does not work, and fails +The obvious-looking `${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, 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 @@ -105,32 +107,32 @@ stages a genuinely outdated dependency against the **real** `apm` — a local gi `url..insteadOf` rewrites, so it needs no network — and replays that genuine output through the hook. -## GitHub Copilot CLI +## GitHub Copilot CLI and Codex -**Copilot loads no hooks from this plugin.** Two independent reasons, either one sufficient: +**apm writes this plugin's hook for Copilot and Codex too; whether they run it is unverified.** +kyberforge's `apm.yml` declares `targets: [claude, copilot, codex]`, and `targets:` is package-wide, +so the hook reaches every target the package does +(`plugins/kyberforge/docs/research/docs/microsoft-apm/hooks-primitive-schema.md`, verified against +apm 0.28.0): -- **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. +- **Copilot** gets `.github/hooks/kyberforge-hooks.json`, one file per source file. apm renames the + event (`SessionStart` → `sessionStart`), rewrites the script path, adds `version: 1`, and otherwise + passes the nested Claude shape through — it does **not** reshape it into Copilot's flat + `bash`/`powershell`/`timeoutSec` form. Whether Copilot CLI executes a nested entry, or honours + `matcher`, has not been verified. +- **Codex** gets the entry merged into `.codex/hooks.json`, but only when `.codex/` already exists; + otherwise nothing is written. -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. +This is accepted rather than fixed (ADR-0019, amendment 2026-09-28). The hook's behaviour is +Claude-specific anyway — the `startup` matcher, `CLAUDE_PROJECT_DIR`, and the `reloadSkills` +output — and the script exits silently without an `apm.lock.yaml`, so a harness that does run it is +unharmed. The only apm-native way to keep it Claude-only is a separate package whose `apm.yml` +declares `target: claude`; per-file target routing (`claude-hooks.json`) is deprecated, and +kyberforge cannot narrow its own `targets:` without dropping its skills from Copilot and Codex. -**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. +An earlier version of this section said Copilot loads no hooks from this plugin, because nothing +could point Copilot at a hooks file and apm did no per-target shaping. Both halves are superseded: +apm deploys the file into Copilot's hooks directory itself, and does rename events per target. ## Symlinks under `.apm/` do not survive, and nothing reports it diff --git a/tests/test-apm-current-hook.sh b/tests/test-apm-current-hook.sh index dffff6d..9dece5b 100755 --- a/tests/test-apm-current-hook.sh +++ b/tests/test-apm-current-hook.sh @@ -297,12 +297,14 @@ echo "--- hooks.json wiring ---" # --------------------------------------------------------------------------- # apm resolves script paths relative to the package root, and `apm pack` keeps -# only *.json from .apm/hooks/ — so a ${CLAUDE_PLUGIN_ROOT}/hooks/... reference -# points at a directory the script never reaches. It must be .apm/-relative. +# only *.json from .apm/hooks/ — so a ${PLUGIN_ROOT}/hooks/... reference +# points at a directory the script never reaches. It must be .apm/-relative, and +# it uses apm's target-neutral token, which apm rewrites identically to +# ${CLAUDE_PLUGIN_ROOT} for every target (ADR-0019, amendment 2026-09-28). referenced="$(python3 -c 'import json,sys; d=json.load(open(sys.argv[1])); print(d["hooks"]["SessionStart"][0]["hooks"][0]["command"])' "$HOOKS_JSON")" -[[ "$referenced" == '${CLAUDE_PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh' ]] \ +[[ "$referenced" == '${PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh' ]] \ && pass "hooks.json references the script at its .apm/ path" \ - || fail "hooks.json references '$referenced' — must be \${CLAUDE_PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh" + || fail "hooks.json references '$referenced' — must be \${PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh" [[ -x "$HOOK" ]] && pass "hook script is executable" || fail "hook script must be executable"