Files
holocron/docs/adr/0019-session-start-hook-keeps-the-apm-install-current.md
Defame1297 44bde9e9c9 docs(adr): correct the records the branch left describing deleted things
Seven ADRs described code that no longer exists or behaviour the gates do
not have. Where the wrong text came from main it carries a dated
correction; where this branch introduced it, it is fixed in place, because
main never published it and there is no record to preserve.

Fixed in place, branch-introduced:

- ADR-0021's 2026-09-14 correction asserted apm audit --ci "was never a
  drift gate at all". It is one: it replays the install and diffs. The
  claim contradicted this branch's own AGENTS.md and gates.md.
- ADR-0015 said unconditionally that no pre-push hook needs the network.
  The guarantee holds only once apm install has populated apm_modules/.
- ADR-0014's 2026-09-16 correction said restoring .pre-commit-hooks.yaml
  would ship a hook that fails for every consumer, because their checkout
  has no lib-boundary-resolver.sh. pre-commit clones the whole hook repo
  and skill-size-check.sh resolves the library from BASH_SOURCE, so the
  hook would work.
- ADR-0019's "twelve hooks pass under unshare -rn" matched neither HEAD
  (8) nor main (14), and stated the offline guarantee unconditionally.

Corrected, inherited from main:

- ADR-0022 and ADR-0013 named skill-frontmatter's pre-commit hook as the
  enforcer of mandatory metadata.version. That hook was deleted on this
  branch; the check lives in skill-size-check.sh.
- ADR-0022 enumerated the tip rule's carve-outs as a closed list and
  described a single merge-base. The gate also exempts a tree-identical
  skill and intersects every base from merge-base --all, and emits a third
  failure form. 8cfd54f said the documented behaviour did not change; it
  did. The gate is correct and is unchanged -- the record was not.
- ADR-0020's Decision still routed description overflow to README.md, its
  ADR-0025 amendment pointed the mirrored constants at validate.sh, which
  holds none, and its Enforcement table still named the two deleted
  validate.sh paths.
- ADR-0015's Status claimed every plugin's plugin.json is pack output;
  none exist.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2
2026-09-20 18:38:00 +00:00

19 KiB
Raw Permalink Blame History

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.

Correction (2026-09-19): the mechanism above is wrong for apm 0.28.0, verified in source. is_package_approved is an exact lookup, but install/exec_gate.py calls it across a candidate list that includes the version-blind name, materialize_exec_map stores each approved key under its version-blind name too, and _map_grants matches exact key, version-blind name, or any stored key sharing that name. So approving kyberforge#2.0.0 keeps covering kyberforge#2.1.0: a bump does not silently stop the hook deploying. Whether apm behaved this way when this ADR was written was not established. The decision stands — scripts/check-executables-allow-sync.sh is now justified by this repo's own requirement that the key track plugins/kyberforge/apm.yml's version:, not by an apm-level failure mode. docs/spec/gates.md carries the same correction.

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.

Amendment (2026-09-16) — the refresh is slower than first measured, still inside the budget. Re-measured: ~24–26 s for the same six-behind refresh, warm, on a LAN remote — well inside the 380 s above.

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.

Correction (2026-09-20): neither half of "the twelve-hooks-pass-under-unshare -rn property" is accurate. The count was never twelve: main declares 14 pre-push hooks and HEAD declares 8 — 10 counting the two repo: meta hooks, which set no stages and so run at every stage. And the property is conditional, not absolute: no pre-push hook needs the network once apm install has populated apm_modules/, but on a fresh clone apm-audit-ci's deployed-files-present fails outright and its drift and config-consistency install-replays clone from the holocron remote (README.md:89; docs/spec/gates.md, "Pushing without a network"). What the probe itself establishes is unchanged and is the point of the sentence: staging the outdated dependency against a local git remote via url.<path>.insteadOf adds no network call of its own.

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.

Amendment (2026-09-16) — on a feature branch, the refresh installs main, not the branch. Recorded after it happened. The dependencies resolve against the remote default branch, so a session opened on a branch that changes plugins/ loads main's content, refreshed or not — a branch's own .apm/ edits are live only once they are on the remote's main. Content the branch removes comes back in the deployed install: on docs/simplification-audit a refresh redeployed main's skill-audit and agent-audit over the branch's merged factory-audit, and re-materialised main's plugins/bin/.mcp.json into apm_modules/, so the gitignored root .mcp.json regained the obsidian server the branch deleted — invisible to git status. Skipping the refresh off the default branch was considered and rejected: it would not make the branch live, only freeze the session on an older main — the silent staleness this ADR exists to prevent. The redeployed content goes away once the branch merges. For the server, the next apm update or apm install that resolves a tree no longer declaring it removes it from .mcp.json: both commands call MCPIntegrator.remove_stale for every server listed under the lock's mcp_servers that no dependency declares any more (apm_cli/install/mcp/integration.py).

The rewritten apm.lock.yaml is a separate matter. It records main's current tip, but the branch's committed lock records a main commit too, just an older one, so committing the rewrite would not swap the branch for main. On a feature branch, discard it anyway (git checkout -- apm.lock.yaml, then apm install), for two reasons. First, it keeps lock churn that has nothing to do with the branch out of the branch's diff. Second, it keeps the deployed tree consistent with the committed lock that the apm-pack-check-clean pre-push hook reads. In this repo apm pack builds a bundle from the lock before its --check-clean gate runs, and it stops with a build error ("deployed files are missing on disk -- run 'apm install'") when a file the lock lists is absent (apm_cli/bundle/packer.py, pack_bundle). A refresh leaves the tree in that state whenever the newer main dropped a file the older lock still lists. Under --dry-run it checks only that each file exists, not its content hash. The cost of discarding is real: the session then runs the older main that the lock records, which is the staleness the rejected skip would have caused. That cost is accepted on a feature branch, and it does not last. At the next session start the hook finds the restored lock behind main and refreshes again.

.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.

Correction (2026-09-16) — the sidecar is committed, not ignored. apm keeps no ownership marker inside settings.json; it recognises its own entries by matching them against .claude/apm-hooks.json, then replaces them. With the sidecar gitignored, a fresh clone holds the committed SessionStart entry but no sidecar, so apm install treats the entry as user-owned, keeps it, and adds its own identical copy. apm audit --ci then reports settings.json drift and the apm-audit-ci pre-push hook fails. Reproduced on main (a712f2c) and on this branch with apm 0.28.0; committing the sidecar makes the install idempotent and the audit pass. The sidecar is apm output like the settings entry it describes, so it is committed for the same reason and changes only when the owning package is renamed or moved. .claude/hooks/ stays ignored.

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.