fix(kyberforge): detect a single stale package at SessionStart
apm prints "1 outdated dependency found" in the singular when exactly one package is behind (apm_cli/commands/outdated.py). check-apm-current.sh matched only "outdated dependencies found", so one stale package was invisible: the hook exited 0 silently and no refresh ran. With six packages merging independently, one-behind is the ordinary case, so the freshness mechanism failed most often in the situation it exists for. Three further defects in the same hook: - The host timeout was below the script's own budget. hooks.json declared 320s while the script allows `timeout 60` plus `timeout 300` = 360s, so a slow remote let the host kill the hook mid-update and leave .claude/skills/ half-deployed with nothing emitted. Now 380. A test asserts the invariant rather than the literal: it sums every `timeout N` parsed out of the script and requires hooks.json to exceed it, so changing either side alone fails. - The lockfile guard was cwd-relative, so a session opened in a subdirectory no-opped silently and ran both apm calls against the wrong directory. Now anchored on CLAUDE_PROJECT_DIR, falling back to the cwd so the hook stays inert under a host that does not set it. - Every assertion mocked apm, so the suite was green over code that could not detect its own most common trigger. That blind spot is what hid the singular/plural bug, and it is the same shape as the deleted post-push tests. The suite now stages a genuinely outdated dependency against a local git remote — offline, via url.<path>.insteadOf, so the pass-under-unshare property survives — runs the real `apm outdated`, and replays its output through the real hook. Reverting the grep to plural-only fails it. 23 -> 35 assertions. Each fix mutation-tested individually. kyberforge stays at 1.5.0: it is untagged, so this changes what 1.5.0 ships rather than superseding it, and executables.allow needs no edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
This commit is contained in:
@@ -13,18 +13,36 @@
|
||||
# Inert in any project that does not consume packages through apm.
|
||||
set -uo pipefail
|
||||
|
||||
# Anchor on the project root, not the session's cwd. Claude Code exports
|
||||
# CLAUDE_PROJECT_DIR for SessionStart hooks; a session opened in a subdirectory
|
||||
# would otherwise miss the lockfile, no-op silently, and — worse — run the apm
|
||||
# calls below against that wrong directory. Fall back to the cwd when the
|
||||
# variable is absent, which keeps the hook inert-but-harmless under a host that
|
||||
# does not set it.
|
||||
project_dir="${CLAUDE_PROJECT_DIR:-$PWD}"
|
||||
|
||||
# No lockfile means nothing was installed through apm here — e.g. a host that
|
||||
# installed this plugin natively. Say nothing and cost nothing.
|
||||
[[ -f apm.lock.yaml ]] || exit 0
|
||||
[[ -f "$project_dir/apm.lock.yaml" ]] || exit 0
|
||||
command -v apm > /dev/null 2>&1 || exit 0
|
||||
|
||||
# Every apm call below must see the same directory the guard just checked —
|
||||
# `apm outdated` and `apm update` both resolve the lockfile from the cwd.
|
||||
cd "$project_dir" || exit 0
|
||||
|
||||
# `apm outdated` exits 0 whether or not anything is stale, so the answer has to
|
||||
# come from its output. ~0.7s against six remote refs; a hung remote must not
|
||||
# hold the session open.
|
||||
#
|
||||
# There is no --json/machine-readable flag on `apm outdated` (verified against
|
||||
# apm 0.28.0), so the phrase match is forced rather than chosen. Note the
|
||||
# singular: apm prints "1 outdated dependency found" when exactly one package is
|
||||
# behind, so matching only "dependencies" would silently miss a one-package
|
||||
# drift. tests/test-apm-current-hook.sh pins both spellings against the real apm.
|
||||
outdated_output="$(timeout 60 apm outdated 2>&1)" || exit 0
|
||||
grep -q "outdated dependencies found" <<< "$outdated_output" || exit 0
|
||||
grep -qE 'outdated dependenc(y|ies) found' <<< "$outdated_output" || exit 0
|
||||
|
||||
stale_count="$(grep -oE '[0-9]+ outdated dependencies found' <<< "$outdated_output" | grep -oE '^[0-9]+' || true)"
|
||||
stale_count="$(grep -oE '[0-9]+ outdated dependenc(y|ies) found' <<< "$outdated_output" | grep -oE '^[0-9]+' || true)"
|
||||
[[ "$stale_count" =~ ^[0-9]+$ ]] || stale_count="some"
|
||||
|
||||
# Only ever emit fixed text plus a digit-checked count — never interpolate
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
"hooks": [
|
||||
{
|
||||
"command": "${CLAUDE_PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh",
|
||||
"timeout": 320,
|
||||
"timeout": 380,
|
||||
"type": "command"
|
||||
}
|
||||
],
|
||||
|
||||
@@ -83,9 +83,33 @@ Note that apm's **executable-trust gate is off** unless the consuming project's
|
||||
|
||||
`check-apm-current.sh` keeps an apm-consumed install level with its remote: it runs `apm outdated`,
|
||||
and if anything is behind, runs `apm update --yes` and returns `reloadSkills: true` so the running
|
||||
session picks up the redeployed content. It exits silently when there is no `apm.lock.yaml` in the
|
||||
working directory, which makes it inert for any host that installed this plugin natively rather than
|
||||
through apm. Rationale, measurements, and the failure modes are in ADR-0019.
|
||||
session picks up the redeployed content. Rationale, measurements, and the failure modes are in
|
||||
ADR-0019.
|
||||
|
||||
**Where it looks for the lockfile.** The hook resolves a project directory as `${CLAUDE_PROJECT_DIR}`
|
||||
when the host exports it (Claude Code does, for SessionStart hooks) and the current directory
|
||||
otherwise, then exits silently unless that directory holds an `apm.lock.yaml` — which is what makes
|
||||
it inert for any host that installed this plugin natively rather than through apm. Both `apm`
|
||||
invocations run against the same resolved directory. The earlier spelling checked a bare
|
||||
`apm.lock.yaml` against the session's cwd, so a session opened in a subdirectory of an
|
||||
apm-consuming repo no-opped silently. Keep the cwd fallback: a host that sets no
|
||||
`CLAUDE_PROJECT_DIR` must still get inert-but-harmless behaviour, not an unset-variable error.
|
||||
|
||||
**The `timeout` in `hooks.json` must exceed the script's own budget.** The script spends at most
|
||||
`timeout 60 apm outdated` plus `timeout 300 apm update`; the hook entry declares `timeout: 380`, the
|
||||
sum plus a buffer. Set it lower and a slow remote gets the hook SIGKILLed mid-`apm update`, leaving a
|
||||
partially redeployed `.claude/skills/` and emitting no notice — precisely the silent failure the hook
|
||||
exists to prevent. `tests/test-apm-current-hook.sh` pins the relationship (host timeout > sum of the
|
||||
script's internal timeouts) rather than the literal, so raising either side alone fails the suite.
|
||||
|
||||
**Staleness is detected by matching apm's summary line, and both spellings count.** `apm outdated`
|
||||
has no `--json` or otherwise machine-readable output (verified against apm 0.28.0), so the hook
|
||||
greps its text. apm prints `1 outdated dependency found` in the singular when exactly one package is
|
||||
behind and `N outdated dependencies found` otherwise; matching only the plural silently misses a
|
||||
one-package drift. Because a mocked `apm` would keep a reworded release invisible, the test suite
|
||||
stages a genuinely outdated dependency against the **real** `apm` — a local git repo reached through
|
||||
`url.<path>.insteadOf` rewrites, so it needs no network — and replays that genuine output through the
|
||||
hook.
|
||||
|
||||
## GitHub Copilot CLI
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
"hooks": [
|
||||
{
|
||||
"command": "${CLAUDE_PLUGIN_ROOT}/.apm/hooks/check-apm-current.sh",
|
||||
"timeout": 320,
|
||||
"timeout": 380,
|
||||
"type": "command"
|
||||
}
|
||||
],
|
||||
|
||||
@@ -23,11 +23,14 @@ WORK="$(mktemp -d)"
|
||||
trap 'rm -rf "$FAKE_BIN" "$WORK"' EXIT
|
||||
|
||||
# Mock `apm`. $1 chooses what `apm outdated` reports; $2 the exit code of
|
||||
# `apm update`. A sentinel file records whether update was actually invoked.
|
||||
# `apm update`. Sentinel files record whether update was actually invoked and
|
||||
# which directory the calls ran in — the hook must run them against the same
|
||||
# directory its lockfile guard checked, not the session's cwd.
|
||||
make_apm() {
|
||||
local outdated_line="$1" update_exit="$2"
|
||||
cat > "$FAKE_BIN/apm" << EOF
|
||||
#!/usr/bin/env bash
|
||||
pwd > "$WORK/apm-cwd"
|
||||
case "\$1" in
|
||||
outdated) echo "$outdated_line"; exit 0 ;;
|
||||
update) touch "$WORK/update-was-called"; exit $update_exit ;;
|
||||
@@ -37,7 +40,22 @@ EOF
|
||||
chmod +x "$FAKE_BIN/apm"
|
||||
}
|
||||
|
||||
run_hook() { (cd "$WORK" && PATH="$FAKE_BIN:$PATH" bash "$HOOK" 2>/dev/null); }
|
||||
# CLAUDE_PROJECT_DIR is cleared rather than merely left alone: a session in this
|
||||
# repo exports it, and an inherited value would point every case at the real
|
||||
# repo root (which has a real apm.lock.yaml) instead of the fixture. The
|
||||
# project-directory cases below set it deliberately.
|
||||
run_hook() { (cd "$WORK" && env -u CLAUDE_PROJECT_DIR PATH="$FAKE_BIN:$PATH" bash "$HOOK" 2>/dev/null); }
|
||||
|
||||
# Same, with an explicit cwd and CLAUDE_PROJECT_DIR. $1 is the cwd; $2 the value
|
||||
# for CLAUDE_PROJECT_DIR, or the literal `-` to leave it unset.
|
||||
run_hook_in() {
|
||||
local cwd="$1" project_dir="$2"
|
||||
if [[ "$project_dir" == "-" ]]; then
|
||||
(cd "$cwd" && env -u CLAUDE_PROJECT_DIR PATH="$FAKE_BIN:$PATH" bash "$HOOK" 2>/dev/null)
|
||||
else
|
||||
(cd "$cwd" && env CLAUDE_PROJECT_DIR="$project_dir" PATH="$FAKE_BIN:$PATH" bash "$HOOK" 2>/dev/null)
|
||||
fi
|
||||
}
|
||||
|
||||
json_field() { python3 -c 'import json,sys; print(json.load(sys.stdin)["hookSpecificOutput"][sys.argv[1]])' "$1"; }
|
||||
|
||||
@@ -134,6 +152,54 @@ out="$(run_hook)"
|
||||
echo "$out" | python3 -m json.tool > /dev/null 2>&1 \
|
||||
&& pass "still emits valid JSON when the count cannot be parsed" || fail "JSON broke on an unparseable count"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
echo ""
|
||||
echo "--- anchors on the project root, not the session cwd ---"
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# `[[ -f apm.lock.yaml ]]` resolves against the cwd, and a session started in a
|
||||
# subdirectory of an apm-consuming repo therefore no-opped silently — and would
|
||||
# have run `apm outdated`/`apm update` against that wrong directory had the
|
||||
# guard passed. Claude Code exports CLAUDE_PROJECT_DIR for SessionStart hooks,
|
||||
# so that is the anchor; the cwd is only the fallback.
|
||||
ELSEWHERE="$WORK/elsewhere"
|
||||
mkdir -p "$ELSEWHERE"
|
||||
rm -f "$ELSEWHERE/apm.lock.yaml"
|
||||
|
||||
make_apm "[!] 6 outdated dependencies found" 0
|
||||
rm -f "$WORK/update-was-called" "$WORK/apm-cwd"
|
||||
out="$(run_hook_in "$ELSEWHERE" "$WORK")"
|
||||
[[ -f "$WORK/update-was-called" ]] \
|
||||
&& pass "finds the lockfile via CLAUDE_PROJECT_DIR when the cwd is elsewhere" \
|
||||
|| fail "a session started in a subdirectory must still see the project's lockfile"
|
||||
[[ "$(cat "$WORK/apm-cwd" 2>/dev/null)" == "$WORK" ]] \
|
||||
&& pass "runs apm in the directory the guard checked, not the cwd" \
|
||||
|| fail "apm ran in '$(cat "$WORK/apm-cwd" 2>/dev/null)' — must run in the resolved project directory"
|
||||
echo "$out" | json_field additionalContext | grep -q "6 package" \
|
||||
&& pass "reports the count found via CLAUDE_PROJECT_DIR" || fail "should report the count"
|
||||
|
||||
# The fallback is not cosmetic: a host that installed this plugin natively sets
|
||||
# no CLAUDE_PROJECT_DIR, and the hook must stay inert-but-harmless there rather
|
||||
# than erroring on an unset variable (the script runs under `set -u`).
|
||||
rm -f "$WORK/update-was-called" "$WORK/apm-cwd"
|
||||
out="$(run_hook_in "$WORK" "-")"
|
||||
[[ -f "$WORK/update-was-called" ]] \
|
||||
&& pass "falls back to the cwd when CLAUDE_PROJECT_DIR is unset" \
|
||||
|| fail "must still work with no CLAUDE_PROJECT_DIR in the environment"
|
||||
[[ "$(cat "$WORK/apm-cwd" 2>/dev/null)" == "$WORK" ]] \
|
||||
&& pass "runs apm in the cwd under the fallback" \
|
||||
|| fail "apm ran in '$(cat "$WORK/apm-cwd" 2>/dev/null)' — should be the cwd"
|
||||
|
||||
rm -f "$WORK/update-was-called" "$WORK/apm-cwd"
|
||||
out="$(run_hook_in "$ELSEWHERE" "$ELSEWHERE")"; rc=$?
|
||||
[[ $rc -eq 0 ]] && pass "exits 0 when neither the project dir nor the cwd has a lockfile" \
|
||||
|| fail "should exit 0 when there is no lockfile anywhere"
|
||||
[[ -z "$out" ]] && pass "stays silent when neither the project dir nor the cwd has a lockfile" \
|
||||
|| fail "should stay silent when there is no lockfile anywhere"
|
||||
[[ ! -f "$WORK/update-was-called" ]] \
|
||||
&& pass "does not run apm update when there is no lockfile anywhere" \
|
||||
|| fail "must not touch a project that does not use apm"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
echo ""
|
||||
echo "--- hooks.json wiring ---"
|
||||
@@ -153,7 +219,150 @@ matcher="$(python3 -c 'import json,sys; d=json.load(open(sys.argv[1])); print(d[
|
||||
[[ "$matcher" == "startup" ]] && pass "fires on startup only" \
|
||||
|| fail "matcher is '$matcher' — resume/clear/compact would re-run this every compaction"
|
||||
|
||||
# The host's timeout must strictly exceed everything the script can spend, or
|
||||
# the host SIGKILLs the hook mid-`apm update` and leaves a half-redeployed
|
||||
# .claude/skills/ with no notice emitted — the silent failure this hook exists
|
||||
# to prevent. Asserted as an invariant over both files rather than against a
|
||||
# literal, so raising either internal `timeout` without raising the host budget
|
||||
# fails here instead of reintroducing the gap quietly.
|
||||
#
|
||||
# Every `timeout N` in the script counts, comments included: a stray "timeout
|
||||
# 300" in prose only makes this stricter, which is the safe direction.
|
||||
script_budget=0
|
||||
timeout_count=0
|
||||
while read -r n; do
|
||||
[[ -n "$n" ]] || continue
|
||||
script_budget=$((script_budget + n))
|
||||
timeout_count=$((timeout_count + 1))
|
||||
done < <(grep -oE '\btimeout [0-9]+\b' "$HOOK" | grep -oE '[0-9]+')
|
||||
|
||||
hook_timeout="$(python3 -c 'import json,sys; d=json.load(open(sys.argv[1])); print(d["hooks"]["SessionStart"][0]["hooks"][0]["timeout"])' "$HOOKS_JSON")"
|
||||
|
||||
if [[ $timeout_count -eq 0 ]]; then
|
||||
fail "found no 'timeout N' in $HOOK — the budget assertion below would be vacuous"
|
||||
else
|
||||
pass "parsed $timeout_count internal timeout(s) totalling ${script_budget}s from the hook script"
|
||||
[[ $hook_timeout -gt $script_budget ]] \
|
||||
&& pass "hooks.json timeout (${hook_timeout}s) exceeds the script's own budget (${script_budget}s)" \
|
||||
|| fail "hooks.json timeout is ${hook_timeout}s but the script can spend ${script_budget}s — the host would SIGKILL it mid-update"
|
||||
fi
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
echo ""
|
||||
echo "--- the greped phrase, against the real apm ---"
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# Everything above mocks `apm`, so an apm release that reworded its summary line
|
||||
# would revert this hook to its pre-fix behaviour with a fully green suite.
|
||||
# `apm outdated` has no --json/machine-readable flag (verified against 0.28.0),
|
||||
# so the phrase match cannot be replaced — it can only be pinned.
|
||||
#
|
||||
# The probe stages a genuinely outdated dependency with no network: a local git
|
||||
# repo stands in for the upstream, reached through `url.<path>.insteadOf`
|
||||
# rewrites of every URL spelling apm may build (it picks ssh or https depending
|
||||
# on ambient auth config, so all three are mapped). apm appends `.git` to the
|
||||
# repo URL, which is why the local repo is named `upstream.git` and the rewrite
|
||||
# target omits the suffix. HOME is redirected so no user-level apm cache or
|
||||
# credential state can influence the result.
|
||||
#
|
||||
# The genuine output is then replayed into the real hook through the mock, so
|
||||
# what is asserted is the hook's own matching logic against real apm text —
|
||||
# no pattern is duplicated here to drift out of sync.
|
||||
SKIP_REASON=""
|
||||
if ! command -v apm > /dev/null 2>&1 || ! command -v git > /dev/null 2>&1; then
|
||||
SKIP_REASON="SKIP: apm and git are both required to verify the hook's phrase match against real \`apm outdated\` output — everything above ran, this axis did not"
|
||||
echo " $SKIP_REASON"
|
||||
else
|
||||
PROBE="$(mktemp -d)"
|
||||
trap 'rm -rf "$FAKE_BIN" "$WORK" "$PROBE"' EXIT
|
||||
|
||||
UPSTREAM="$PROBE/upstream.git"
|
||||
git init -q "$UPSTREAM"
|
||||
git -C "$UPSTREAM" -c user.email=probe@example.invalid -c user.name=probe \
|
||||
commit -q --allow-empty -m one
|
||||
LOCKED_SHA="$(git -C "$UPSTREAM" rev-parse HEAD)"
|
||||
git -C "$UPSTREAM" -c user.email=probe@example.invalid -c user.name=probe \
|
||||
commit -q --allow-empty -m two
|
||||
BRANCH="$(git -C "$UPSTREAM" symbolic-ref --short HEAD)"
|
||||
|
||||
{
|
||||
for repo in alpha beta; do
|
||||
printf '[url "%s/upstream"]\n' "$PROBE"
|
||||
printf '\tinsteadOf = git@apm-probe.invalid:probe/%s\n' "$repo"
|
||||
printf '\tinsteadOf = https://apm-probe.invalid/probe/%s\n' "$repo"
|
||||
printf '\tinsteadOf = ssh://git@apm-probe.invalid/probe/%s\n' "$repo"
|
||||
done
|
||||
} > "$PROBE/gitconfig"
|
||||
|
||||
mkdir -p "$PROBE/consumer" "$PROBE/home"
|
||||
|
||||
# $1 = how many stale dependencies to stage. Writes a lockfile and echoes what
|
||||
# the real `apm outdated` printed for it.
|
||||
real_apm_outdated() {
|
||||
local want="$1" repo
|
||||
{
|
||||
echo "lockfile_version: '1'"
|
||||
echo "generated_at: '2026-01-01T00:00:00+00:00'"
|
||||
echo "apm_version: 0.0.0"
|
||||
echo "dependencies:"
|
||||
for repo in $( [[ "$want" == 1 ]] && echo alpha || echo alpha beta ); do
|
||||
echo "- host: apm-probe.invalid"
|
||||
echo " name: probe-$repo"
|
||||
echo " package_type: apm_package"
|
||||
echo " repo_url: probe/$repo"
|
||||
echo " resolved_ref: $BRANCH"
|
||||
echo " resolved_commit: $LOCKED_SHA"
|
||||
echo " version: 1.0.0"
|
||||
done
|
||||
echo "deployments: []"
|
||||
} > "$PROBE/consumer/apm.lock.yaml"
|
||||
(
|
||||
cd "$PROBE/consumer" &&
|
||||
env HOME="$PROBE/home" \
|
||||
GIT_CONFIG_GLOBAL="$PROBE/gitconfig" \
|
||||
GIT_CONFIG_NOSYSTEM=1 \
|
||||
GIT_TERMINAL_PROMPT=0 \
|
||||
apm outdated 2>&1
|
||||
)
|
||||
}
|
||||
|
||||
# Replay genuine output through the hook. A harness that stages nothing would
|
||||
# make every assertion below vacuously true, so the staged row is checked
|
||||
# first and a failure to stage is a FAIL, not a quiet pass.
|
||||
for want in 1 2; do
|
||||
genuine="$(real_apm_outdated "$want" || true)"
|
||||
if ! grep -q "outdated" <<< "$genuine"; then
|
||||
fail "probe staged no outdated dependency against the real apm (harness broken, not the hook): $(tr '\n' ' ' <<< "$genuine" | cut -c1-160)"
|
||||
continue
|
||||
fi
|
||||
printf '%s\n' "$genuine" > "$PROBE/genuine-$want.txt"
|
||||
cat > "$FAKE_BIN/apm" << EOF
|
||||
#!/usr/bin/env bash
|
||||
pwd > "$WORK/apm-cwd"
|
||||
case "\$1" in
|
||||
outdated) cat "$PROBE/genuine-$want.txt"; exit 0 ;;
|
||||
update) touch "$WORK/update-was-called"; exit 0 ;;
|
||||
esac
|
||||
exit 0
|
||||
EOF
|
||||
chmod +x "$FAKE_BIN/apm"
|
||||
rm -f "$WORK/update-was-called"
|
||||
out="$(run_hook)"
|
||||
if [[ -n "$out" ]] && echo "$out" | json_field additionalContext 2>/dev/null | grep -q "$want package"; then
|
||||
pass "detects staleness in real \`apm outdated\` output and counts $want package(s)"
|
||||
else
|
||||
fail "real apm reported $want outdated dependency/dependencies but the hook did not act on it — apm reworded its summary line. Real output: $(tr '\n' ' ' < "$PROBE/genuine-$want.txt" | tail -c 120)"
|
||||
fi
|
||||
done
|
||||
fi
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
[[ $FAIL -eq 0 ]] || exit 1
|
||||
# A skip only after everything runnable has run and passed: the mocked axis is
|
||||
# still worth executing on a machine without apm, but the suite must not read
|
||||
# as green when the real-apm axis was not verified. run-tests.sh reports 77 as
|
||||
# SKIPPED and, at pre-push (--strict), as a setup error naming this reason.
|
||||
[[ -z "$SKIP_REASON" ]] || exit 77
|
||||
exit 0
|
||||
|
||||
Reference in New Issue
Block a user