AGENTS.md and CONTEXT.md asserted that the `<plugin>:<skill>` form "no longer resolves here". It does: ~/.claude.json still enables core, git, gitea, kyberforge and lint at user scope, which ADR-0018 left in place deliberately. Both names are live at once, so a working `gitea:gitea-prs` is the user-scope copy answering — not evidence that the apm install is broken and not something to "fix". ADR-0018 contradicted itself on this, claiming every namespaced reference went stale while its own "User scope is untouched" consequence said otherwise; recorded as a dated correction alongside the existing one. Bare names stay the documented default. Five stale pre-push hook counts updated for the new check-executables-allow-sync gate: 13 -> 14 repo-defined hooks, 15 -> 16 reported by pre-commit, eleven -> twelve passing offline. The gate reads two local manifests and makes no network call, so the SKIP pair for offline pushes stays exactly two. "Four pre-push hooks shell out to apm" is unchanged and still correct — the new hook parses YAML directly. ADR-0019 gains the timeout arithmetic, the singular/plural failure and why mocking every apm call hid it, and a consequence recording that the trust gate is keyed on version rather than content: an edit to a hook script landing on main deploys and executes unattended on the next session start, since the dependency is unpinned and the hook runs `apm update --yes`. That is accepted, not overlooked, but it is why the gate should not be read as a supply-chain control. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
168 lines
12 KiB
Markdown
168 lines
12 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.
|
|
|
|
**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`.
|
|
|
|
**`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.
|