diff --git a/AGENTS.md b/AGENTS.md index 3aeb20d..fd7b3c0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,7 +28,7 @@ Fall back to raw shell only when no skill covers it. ## Session rules - **Do not add repo-owned keys to `.claude/settings.json`.** apm treats it as its own deployed artifact and `apm audit --ci` replays the install and diffs, so anything apm would not have written is permanent drift that fails the `apm-audit-ci` pre-push hook. A hook you want here is authored in `plugins//.apm/hooks/` and deployed by apm, never hand-written into that file. The `SessionStart` entry already in it is exactly that: kyberforge authors it in `plugins/kyberforge/.apm/hooks/hooks.json` and apm merges it in, so it is apm's own output, it is what the replay expects, and it belongs in the commit — do not strip it (ADR-0019). Machine-specific settings go in the gitignored `.claude/settings.local.json`; shared enforcement goes in `.pre-commit-config.yaml`. -- **`apm.lock.yaml` turning up modified is expected, not a bug.** kyberforge's `SessionStart` hook keeps the install current on launch and rewrites the lock in the process (ADR-0019). On `main`, commit or discard it deliberately; on a feature branch, discard it (`git checkout -- apm.lock.yaml`, then `apm install`) — the refresh resolved against `main`, not the branch. +- **`apm.lock.yaml` turning up modified is expected, not a bug.** kyberforge's `SessionStart` hook keeps the install current on launch and rewrites the lock in the process (ADR-0019). On `main`, commit or discard it deliberately. On a feature branch, discard it (`git checkout -- apm.lock.yaml`, then `apm install`). This keeps unrelated lock churn out of the branch diff and keeps `apm pack --check-clean` consistent with the committed lock. The session then runs the older `main` that the lock records, which is accepted on a branch, and the next session start refreshes again. - **A `.apm/` edit is not live until it is on the remote's `main`.** The six dependencies resolve from the holocron remote, unpinned against the default branch, so pushing a feature branch does not deploy it (ADR-0019). `apm install` deploys from the lock; `apm update` is what re-resolves refs. - **No pre-push hook needs the network.** Root `apm.yml`'s marketplace has no remote package entries, so every hook resolves locally. - **This repo and Gitea are the only source of truth.** All project state, decisions, and working conventions live here. Do not use an external memory system for this project — cached state diverges from the repo and you get a split brain. Before answering any design or architecture question, check `docs/adr/` for an existing decision. diff --git a/README.md b/README.md index cf207d8..13d0159 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ pre-commit install -t pre-commit -t commit-msg -t pre-push ## Keeping the install current -The six dependencies in root `apm.yml` are unpinned against the default branch, so deployed skills go stale whenever anyone merges. kyberforge's `SessionStart` hook keeps the install current automatically on launch, rewriting `apm.lock.yaml` in the process — an unexplained modification to it after opening a session is expected, not a bug. On `main`, commit or discard it deliberately; on a feature branch, discard it (`git checkout -- apm.lock.yaml`, then `apm install`), because the refresh resolves against `main` and the lock now records `main`'s commit, not the branch's. Mechanism and rationale: `docs/adr/0019-session-start-hook-keeps-the-apm-install-current.md`. +The six dependencies in root `apm.yml` are unpinned against the default branch, so deployed skills go stale whenever anyone merges. kyberforge's `SessionStart` hook keeps the install current automatically on launch, rewriting `apm.lock.yaml` in the process — an unexplained modification to it after opening a session is expected, not a bug. On `main`, commit or discard it deliberately. On a feature branch, discard it (`git checkout -- apm.lock.yaml`, then `apm install`). The committed lock records a `main` commit too, just an older one. Discarding keeps lock churn unrelated to the branch out of its diff, and keeps the deployed tree consistent with the committed lock that `apm pack --check-clean` reads. The trade-off: the session then runs the older `main` the lock records, which is accepted on a feature branch. The discard also lasts only until the next session start, when the hook finds the lock behind `main` and refreshes again. Mechanism and rationale: `docs/adr/0019-session-start-hook-keeps-the-apm-install-current.md`. Note the difference between the two commands: 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 f9352d3..7f13d1d 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 @@ -152,18 +152,33 @@ 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`. Two visible -> effects follow. 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`. And the rewritten `apm.lock.yaml` records `main`'s commit, -> so on a branch it should be discarded (`git checkout -- apm.lock.yaml`, then `apm install` to -> bring the deployed tree back in line with the lock, or `apm pack --check-clean` refuses to run), -> not committed. 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. Both effects end when the branch merges; apm removes a server that is no -> longer declared on its next update. +> 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 diff --git a/plugins/kyberforge/.apm/hooks/check-apm-current.sh b/plugins/kyberforge/.apm/hooks/check-apm-current.sh index ba989fd..1475303 100755 --- a/plugins/kyberforge/.apm/hooks/check-apm-current.sh +++ b/plugins/kyberforge/.apm/hooks/check-apm-current.sh @@ -51,8 +51,25 @@ emit() { printf '{"hookSpecificOutput":{"hookEventName":"SessionStart","reloadSkills":%s,"additionalContext":"%s"}}\n' "$1" "$2" } +# What to do with the rewritten lock depends on the branch (ADR-0019): on the +# default branch it is a real update to commit or discard; on a feature branch it +# is churn unrelated to the branch and should be discarded. The branch name only +# selects between fixed strings and is never interpolated. Outside a git checkout, +# or on a detached HEAD, the neutral advice stands. +lock_advice="commit it or discard it deliberately." +current_branch="$(git symbolic-ref --short -q HEAD 2> /dev/null || true)" +if [[ -n "$current_branch" ]]; then + default_branch="$(git symbolic-ref --short -q refs/remotes/origin/HEAD 2> /dev/null || true)" + default_branch="${default_branch#origin/}" + if [[ "$current_branch" == "${default_branch:-main}" ]]; then + lock_advice="this is the default branch, so commit it or discard it deliberately." + else + lock_advice="this is a feature branch, so discard it: git checkout -- apm.lock.yaml && apm install" + fi +fi + if timeout 300 apm update --yes > /dev/null 2>&1; then - emit true "apm install was ${stale_count} package(s) behind the remote default branch and has been refreshed automatically; skills and agents were redeployed and re-scanned. apm.lock.yaml has been rewritten and is now a modified file in the working tree - commit it or discard it deliberately." + emit true "apm install was ${stale_count} package(s) behind the remote default branch and has been refreshed automatically; skills and agents were redeployed and re-scanned. apm.lock.yaml has been rewritten and is now a modified file in the working tree - ${lock_advice}" else emit false "apm install is ${stale_count} package(s) behind the remote default branch and the automatic refresh failed. Deployed skills and agents may be stale. Run: apm update --yes" fi diff --git a/tests/test-apm-current-hook.sh b/tests/test-apm-current-hook.sh index 8e1acb2..7f29cbb 100755 --- a/tests/test-apm-current-hook.sh +++ b/tests/test-apm-current-hook.sh @@ -21,6 +21,11 @@ command -v python3 > /dev/null 2>&1 || { echo "python3 required"; exit 77; } FAKE_BIN="$(mktemp -d)" WORK="$(mktemp -d)" trap 'rm -rf "$FAKE_BIN" "$WORK"' EXIT +# The hook asks git which branch it is on. Stop git's discovery at the temp +# root so a $TMPDIR that happens to sit inside a checkout cannot leak a branch +# into the fixtures that are meant to be outside one. +export GIT_CEILING_DIRECTORIES +GIT_CEILING_DIRECTORIES="$(dirname "$WORK")" # Mock `apm`. $1 chooses what `apm outdated` reports; $2 the exit code of # `apm update`. Sentinel files record whether update was actually invoked and @@ -124,6 +129,55 @@ else fail "emits valid JSON" fi +# --------------------------------------------------------------------------- +echo "" +echo "--- lock advice follows the branch ---" +# --------------------------------------------------------------------------- + +# ADR-0019: on the default branch the rewritten lock is a real update to commit or +# discard; on a feature branch it is unrelated churn to discard. Outside a git +# checkout (the fixture above) the neutral advice stands. +advice_of() { json_field additionalContext <<< "$1"; } +grep -q "commit it or discard it deliberately" <<< "$(advice_of "$out")" \ + && pass "gives neutral lock advice outside a git checkout" \ + || fail "outside a git checkout the advice should stay neutral" + +if command -v git > /dev/null 2>&1; then + REPO="$WORK/repo" + mkdir -p "$REPO" + git -C "$REPO" init -q -b main + git -C "$REPO" -c user.email=probe@example.invalid -c user.name=probe \ + commit -q --allow-empty -m init + touch "$REPO/apm.lock.yaml" + + out="$(run_hook_in "$REPO" "$REPO")" + grep -q "default branch, so commit it or discard it deliberately" <<< "$(advice_of "$out")" \ + && pass "on main, says to commit or discard the lock deliberately" \ + || fail "on main the advice should be commit-or-discard: $(advice_of "$out")" + + git -C "$REPO" checkout -q -b feature/x + out="$(run_hook_in "$REPO" "$REPO")" + advice="$(advice_of "$out")" + grep -qF "feature branch, so discard it: git checkout -- apm.lock.yaml && apm install" <<< "$advice" \ + && pass "on a feature branch, says to discard the lock and reinstall" \ + || fail "on a feature branch the advice should be discard-and-install: $advice" + grep -q "commit it" <<< "$advice" \ + && fail "on a feature branch the advice must not suggest committing the lock" \ + || pass "on a feature branch, does not suggest committing the lock" + echo "$out" | python3 -m json.tool > /dev/null 2>&1 \ + && pass "feature-branch notice is valid JSON" || fail "feature-branch notice broke the JSON" + + # A remote whose default branch is not `main` is honoured via origin/HEAD. + git -C "$REPO" update-ref refs/remotes/origin/feature/x HEAD + git -C "$REPO" symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/feature/x + out="$(run_hook_in "$REPO" "$REPO")" + grep -q "default branch, so commit it" <<< "$(advice_of "$out")" \ + && pass "reads the default branch from origin/HEAD when it is set" \ + || fail "should treat origin/HEAD's branch as the default: $(advice_of "$out")" +else + echo " (git not on PATH — branch-specific advice cases not run)" +fi + # --------------------------------------------------------------------------- echo "" echo "--- stale, refresh fails ---"