A five-agent review of today's seven commits found no executable
regressions and no dangling references, but a set of documents still
asserting, in present tense, machinery that ADR-0024 and its commits
removed. This corrects them in place, keeping the original text as the
historical record wherever the repo's amendment convention applies.
LESSONS.md: the 2026-06-21 entry prescribed a `claude plugin validate`
sweep that now fails on every plugin, so it is marked superseded with
the surviving gates named. The 2026-08-09 entry gained a recurrence
note: today's manifest deletion broke apm's MCP propagation exactly as
that lesson describes, and its prescribed repo-local grep could not
have caught it, because `plugin_parser.py` ships in the apm toolchain
installed outside this repository.
ADR-0019, ADR-0011 and ADR-0021: amendments extended to passages the
earlier correction passes stepped over -- a dead native-consumer guard,
Consequences bullets still calling for a `plugins/gitea/.mcp.json` that
must not be recreated, and a drift-gate list naming a deleted script.
ADR-0021's list is down to one gate, not two: `apm audit --ci` never
read `description` and was never a drift gate.
architecture.md and enrichments.md: the self-containment constraint is
restated on its live source, the agentskills.io APM package-mode spec,
rather than on Claude Code's plugin cache-install, which ADR-0024
consequence 6 pins as a superseded rationale. releasing.md's pointer to
the deleted sync script is rewritten as history.
tests/run-bats.sh and scripts/lib/batch-run.sh: comment-only. The
`.claude/skills/` exclusion comment claimed a duplication that is not
live yet; apm does not strip `tests/`, and the deployed tree is empty
of them only because the lockfile still resolves the six dependencies
to a pre-ADR-0024 commit carrying the flat mirror. The exclusion is
correct but forward-looking, and now says so.
SIMPLIFICATION-AUDIT.md: reconciled against what the commits actually
did. Two closed findings recorded conclusions that ADR-0024 reversed
hours later; findings 1, 3, 31 and 35 carried prescriptions voided the
same day; finding 28 is now recorded as having moved backwards, with
docs/adr/ measured at +336 lines over the day. The section 1 headline
table is re-measured at a6434e0 and labelled with its basis. The
ADR-0012 contradiction between finding 2b and section 8 is resolved in
2b's favour after reading the ADR: only finding 24 is governed by it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
188 lines
13 KiB
Markdown
188 lines
13 KiB
Markdown
# A SessionStart hook keeps the apm install current, replacing a git hook that never ran
|
|
|
|
ADR-0018 switched this repo to consuming its own plugins through `apm install`, with the six
|
|
packages declared as unpinned git refs against the holocron remote's default branch. That decision
|
|
left a hole it named but did not fill: the deployed content goes stale the moment anyone merges,
|
|
and nothing detects it.
|
|
|
|
**Status: accepted (2026-08-14).**
|
|
|
|
## Context
|
|
|
|
The pre-existing answer was `scripts/git-hooks/post-push`, which pulled the marketplace clone and
|
|
ran `claude plugin update kyberforge`. Issue #78 filed it as a bug — the hook updated `kyberforge`
|
|
but not `gitea`, so gitea skills stayed pinned at a pre-refactor version after #67 merged.
|
|
|
|
The issue's premise was wrong in a way nobody had noticed for six weeks. **Git has no client-side
|
|
`post-push` hook.** `githooks(5)` does not list one, and git 2.39.5 does not invoke one.
|
|
`scripts/install.sh` copies every file in `scripts/git-hooks/` into `.git/hooks/`, so
|
|
`.git/hooks/post-push` existed on disk and looked installed. It had never fired. The hook did not
|
|
skip `gitea`; it skipped everything. Both tests that appeared to cover it — `test-post-push.sh` and
|
|
`test-git-hooks-install.sh` — asserted only that the script behaved correctly when invoked directly
|
|
and that install.sh copied the file. Neither asserted that git ever runs it.
|
|
|
|
That also makes the original framing wrong. Refreshing on push assumes the person who pushes is the
|
|
person who goes stale, which is backwards: your install goes stale when *someone else* merges, and a
|
|
push of your own is neither necessary nor sufficient for it to have happened.
|
|
|
|
## Decision
|
|
|
|
A `SessionStart` hook, shipped in `plugins/kyberforge/.apm/hooks/`, checks whether the install is
|
|
behind and refreshes it in place.
|
|
|
|
`SessionStart` is the correct trigger because the thing that goes stale is the skill content a
|
|
*session* loads, and that is the moment the staleness does damage. It also enables two things a git
|
|
hook structurally cannot do: `additionalContext` puts the notice into the agent's context rather
|
|
than terminal scrollback nobody reads, and `reloadSkills: true` makes the host re-scan the skill
|
|
directories after the hook returns, so a refresh lands in the running session without a restart.
|
|
|
|
apm's own lifecycle events (`pre-/post-install`, `pre-/post-update`, `pre-/post-uninstall`) were
|
|
rejected: they fire around apm operations already chosen, so they can announce a refresh but never
|
|
detect that one is needed.
|
|
|
|
Three sub-decisions:
|
|
|
|
- **Refresh automatically rather than report.** The hook runs `apm update --yes` and asks for a skill
|
|
reload. The rejected alternative was to report and let a human run it. Auto-refresh costs a
|
|
rewritten `apm.lock.yaml` — a committed file — appearing as an unexplained modification in the
|
|
working tree, on any branch, at any time. The emitted notice says so explicitly for that reason.
|
|
- **`plugins/kyberforge/.apm/hooks/`, not `.claude/settings.json`.** ADR-0018 established that apm
|
|
owns `.claude/settings.json` and that any repo-authored key in it is permanent `apm audit --ci`
|
|
drift. A hook shipped in a package is written into that file by apm itself, so it is apm's output
|
|
and does not drift. `.claude/settings.local.json` also works but is gitignored and machine-local,
|
|
which fails the requirement that this travel with the repo.
|
|
- **`startup` matcher only.** `resume`, `clear`, `compact` and `fork` would re-run the check on every
|
|
compaction, and a compaction is not an event after which the remote can have moved.
|
|
|
|
The executable-trust gate is switched on at the same time. Root `apm.yml` gains an `executables:`
|
|
block allowing kyberforge's hooks and bin.
|
|
|
|
## Consequences
|
|
|
|
**The gate is off until something turns it on, and this repo had it off.** `apm approve --list`
|
|
reports `Executable-trust gate disabled -- all executables deploy` until an `executables:` block
|
|
exists in `apm.yml`. Any hook, bin, or MCP primitive a dependency shipped would have deployed with
|
|
no prompt and no record. The block added here closes that for this repo; every other apm project on
|
|
this machine still has it open.
|
|
|
|
**The allow key is version-pinned, and that is a live failure mode.** apm writes
|
|
`kyberforge#1.5.0`, not `kyberforge` — and the release that ships this hook proved the point
|
|
immediately, since bumping kyberforge to 1.5.0 required editing the key in the same commit. A
|
|
kyberforge version bump makes the entry stop matching, the
|
|
gate blocks the hook, and the install silently stops refreshing — the exact failure this ADR exists
|
|
to end, reintroduced through the mechanism meant to secure it.
|
|
|
|
Matching is an exact dictionary lookup on the composed `name#version` string
|
|
(`apm_cli/security/executables.py`, `is_package_approved`), so there is no wildcard or
|
|
version-less key that would sidestep this — the key has to be edited on every bump, and the
|
|
question is only what catches a missed edit. A comment in the `executables:` block is not enough:
|
|
this repo gates generated-content drift, marketplace mirror drift and vale style drift
|
|
deterministically, and a silent-staleness failure is strictly worse than any of them. So
|
|
`scripts/check-executables-allow-sync.sh` runs at pre-push, parsing `version:` out of
|
|
`plugins/kyberforge/apm.yml` and asserting root `apm.yml` carries the matching
|
|
`kyberforge#<version>` key. The comment stays as the human-facing pointer; the hook is what
|
|
actually holds. It parses with PyYAML where importable and falls back to a two-shape scan
|
|
otherwise, so a missing pip package cannot become the thing that blocks every push.
|
|
|
|
**Trust is keyed on the version, not on the content.** `kyberforge#1.5.0` approves whatever
|
|
`check-apm-current.sh` contains at the moment it is fetched, not the bytes that were reviewed when
|
|
the key was written. Because the dependency is unpinned against the default branch and the hook
|
|
runs `apm update --yes` unattended, an edit to that script landing on `main` deploys and executes
|
|
on every contributor's machine at their next session start, with no second approval prompt and no
|
|
diff shown. The trust gate constrains *which package* may ship an executable; it does not constrain
|
|
what that executable does between version bumps. That is an accepted property of this design rather
|
|
than an oversight — the remote is self-hosted, push access to `main` is already sufficient to
|
|
change any skill body an agent will follow — but it is the reason the gate should not be read as a
|
|
supply-chain control. Pinning each dependency to a `ref:` is what would make it one, and ADR-0018
|
|
defers that until per-package release tags exist.
|
|
|
|
**A referenced hook script must be addressed at its `.apm/` path.** apm resolves
|
|
`${CLAUDE_PLUGIN_ROOT}/...` against the installed package root, and `apm pack` keeps only `*.json`
|
|
from `.apm/hooks/` when it builds the flat mirror. So `${CLAUDE_PLUGIN_ROOT}/hooks/check-apm-current.sh`
|
|
resolves to the mirror, where the script does not exist — verified, apm reports
|
|
`Hook script not found` and deploys a hook pointing at nothing. The working reference is
|
|
`${CLAUDE_PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh`. The script cannot simply be placed in
|
|
`plugins/kyberforge/hooks/` either: that directory is `rm -rf`'d by every content sync (ADR-0017).
|
|
A test pins the reference.
|
|
|
|
> **Amendment (2026-09-14) — the mirror half of that reasoning is gone; the conclusion is not.**
|
|
> ADR-0024 deleted the flat mirror and `scripts/sync-plugin-content.sh`, so neither the `apm pack`
|
|
> mirror-filtering behaviour nor the `rm -rf` content sync described above still happens. The
|
|
> reference must stay exactly as written, for the reason that survives independently: `.apm/` is the
|
|
> sole hand-edited authoring source (ADR-0015), it is what ships in the installed package root, and
|
|
> `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.
|
|
|
|
**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
|
|
hook declares `timeout: 380` to cover a cold multi-package fetch. That number is not free-standing:
|
|
the script imposes its own `timeout 60` on `apm outdated` and `timeout 300` on `apm update`, so the
|
|
host-side timeout has to exceed their sum or the host kills the hook mid-update and leaves
|
|
`.claude/skills/` half-deployed with no notice emitted. An earlier revision declared `320`, which
|
|
was below the 360 s the script can legitimately take. A test asserts the invariant rather than the
|
|
literal — it parses every `timeout N` out of the script, sums them, and requires the `hooks.json`
|
|
value to be larger — so changing either side without the other fails the suite.
|
|
|
|
**Reading a human-readable CLI for a control decision cost a silent failure, again.** `apm outdated`
|
|
has no `--json` or other machine-readable flag (confirmed against 0.28.0), so the hook must match
|
|
its prose. The first attempt matched `outdated dependencies found` — plural only. apm emits
|
|
`1 outdated dependency found` in the singular when exactly one package is behind
|
|
(`apm_cli/commands/outdated.py`), so a single stale package was invisible: the hook exited 0
|
|
silently and no refresh ran. With six packages merging independently, one-behind is the ordinary
|
|
case rather than an edge, which means the mechanism failed most often in exactly the situation it
|
|
exists for. The match is now `outdated dependenc(y|ies) found`.
|
|
|
|
The deeper lesson is the one `post-push` already taught and this repeated: every assertion about the
|
|
hook mocked `apm`, so the suite was green while the hook could not detect the common case. Mocks
|
|
verify the code against its author's belief about the interface, never the interface. The suite now
|
|
carries one probe that stages a genuinely outdated dependency against a local git remote — offline,
|
|
via `url.<path>.insteadOf`, so the twelve-hooks-pass-under-`unshare -rn` property survives — runs
|
|
the real `apm outdated`, and replays its genuine output through the real hook. Reverting the grep
|
|
to plural-only fails it.
|
|
|
|
**The hook cannot install itself.** Dependencies resolve from the remote, so the hook does not
|
|
deploy until this change is merged and `apm update` has run once against the new default branch.
|
|
Until then the repo has the mechanism in source and not in effect.
|
|
|
|
**`.claude/settings.json` stops being `{"hooks": {}}`.** apm merges the hook into it and tracks
|
|
ownership in a `.claude/apm-hooks.json` sidecar, with the script copied to
|
|
`.claude/hooks/<pkg>/`. The sidecar and the script directory are gitignored install output; the
|
|
settings file remains committed, now with apm-generated content in it. ADR-0018's statement that the
|
|
committed content is exactly `{"hooks": {}}` is superseded on that point only — the rule it was
|
|
protecting, that nothing repo-authored goes in that file, is unchanged.
|
|
|
|
**Native consumers are protected by a guard, not by the gate.** A host installing holocron through
|
|
`claude plugin install` auto-discovers `hooks/hooks.json` and does not consult apm's trust gate at
|
|
all. The script therefore exits silently when there is no `apm.lock.yaml` in the working directory,
|
|
which is what makes it inert in a repo that does not consume packages through apm. Copilot CLI sees
|
|
no hook at all, for the reasons already documented in `plugins/kyberforge/docs/hooks.md`.
|
|
|
|
> **Amendment (2026-09-14) — both halves of that paragraph are gone; the guard is kept on other
|
|
> grounds.** There is no `hooks/hooks.json` to auto-discover: `718c79a` deleted
|
|
> `plugins/kyberforge/hooks/` with the rest of the flat mirror, and the only hooks manifest left in
|
|
> the tree is `plugins/kyberforge/.apm/hooks/hooks.json`, which apm reads and a convention scan of
|
|
> the package root never sees. And there are no native consumers to protect: ADR-0024 ended
|
|
> `claude plugin install` support outright. The `apm.lock.yaml` guard itself stays, for the reason
|
|
> that survives both — kyberforge ships to any apm consumer, and in a working directory with no
|
|
> 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.
|
|
|
|
**`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
|
|
own fixture hook instead of depending on a real one existing, so the mechanism stays tested and can
|
|
be used again if a hook git actually invokes is ever wanted.
|
|
|
|
## Alternatives considered
|
|
|
|
- **A `post-merge` git hook.** Real, unlike `post-push`, and verified to fire on both a
|
|
fast-forward `git pull` and a `git pull --rebase`. Rejected as the primary mechanism because a
|
|
pull is the wrong signal, and because it cannot reload skills in a running session. It remains
|
|
the only option for a project that consumes apm packages without a Claude-family host.
|
|
- **Reporting instead of refreshing.** See the sub-decision above.
|
|
- **A seventh plugin holding only this hook**, to avoid shipping it to external kyberforge
|
|
consumers. Rejected as disproportionate: the `apm.lock.yaml` guard already makes the hook inert
|
|
for anyone not consuming through apm, and a package exists to be maintained, versioned, and
|
|
registered in the marketplace.
|