feat(doctor): fail loud on skills drift, wired to SessionStart

Every failure this fleet actually hit was silent. 243 dangling symlinks were
invisible to git because **/.claude/skills/ is in the global gitignore. A
checkout 20 skills behind still resolved, so nothing complained. A loose
secret-hygiene.md loaded in no harness and errored in none. Two machines had
the library missing entirely. Exit status is the signal that was absent.

bin/doctor reports, with a non-zero exit:
  - dangling links, anywhere under the harness dirs
  - links that RESOLVE but land outside the active checkout — the dangerous
    case, because it works while serving stale content
  - skill-count mismatch between a harness and the checkout
  - stray entries where <name>/SKILL.md was expected
  - leftover per-skill Claude symlinks (plugin skills do not override
    same-named user skills, so leftovers mean everything loads twice)
  - divergence between the plugin copy and the copy the other harnesses link to

Wired as a SessionStart hook with --quiet, so a healthy host prints nothing and
a drifting one says so at the top of the next session. That beats a timer:
no launchd/systemd wiring, and it reaches every host that installs the plugin.
27ms on koala, no network, no git fetch.

Two design points found by the tests, both worth keeping:
  - The canonical checkout is whatever ~/.agents/skills resolves to, NOT the
    copy doctor runs from. As a plugin hook $0 lives in the plugin cache, which
    is legitimately a second copy; judging harness links against it flagged
    every one of them as "outside".
  - "cd X/.." resolves ".." lexically against the link path and lands back in
    ~/.agents. Resolve the link first, then take the parent.

tests/doctor_test.sh drives seven fixtures through $HOME and $DOCTOR_CHECKOUT,
so no test touches the real host. Each asserts both the non-zero exit and that
the report names the fault — an exit-code-only assertion would pass while the
message said nothing useful.

Bump-Type: minor

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BeAp5LTscnz2W7Ubt4eJ6m
This commit is contained in:
2026-08-23 14:34:32 +02:00
co-authored by Claude Opus 5
parent 747c6c7f4c
commit 88c54399fe
3 changed files with 321 additions and 0 deletions
+124
View File
@@ -0,0 +1,124 @@
#!/bin/sh
# Tests for bin/doctor.
#
# Every case here is a real state this fleet was found in on 2026-08-23, and
# every one of them was SILENT — nothing errored, git status was clean, the
# harness simply ran with the wrong skills or none at all. The point of doctor
# is that none of these can be silent again, so each fixture asserts both a
# non-zero exit AND that the report names the fault.
#
# Fixtures drive doctor through $HOME and $DOCTOR_CHECKOUT, so no test touches
# the real host.
set -u
ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
DOCTOR="$ROOT/bin/doctor"
pass=0
fail=0
WORK=${TMPDIR:-/tmp}/doctor_test.$$
if [ ! -f "$DOCTOR" ]; then
printf 'FAIL: %s does not exist\n' "$DOCTOR"
exit 1
fi
cleanup() { rm -rf "$WORK"; }
trap cleanup EXIT INT TERM
# new_fixture <name> — builds $WORK/<name>/{ck,home} with a 2-skill checkout
# and an empty fake HOME. Echoes the fixture root.
new_fixture() {
_f="$WORK/$1"
mkdir -p "$_f/ck/skills/alpha" "$_f/ck/skills/beta" "$_f/home"
printf -- '---\nname: alpha\ndescription: a\n---\n' > "$_f/ck/skills/alpha/SKILL.md"
printf -- '---\nname: beta\ndescription: b\n---\n' > "$_f/ck/skills/beta/SKILL.md"
printf '%s' "$_f"
}
# wire_clean <fixture> — the correct wiring: one canonical link, harnesses
# pointing at it.
wire_clean() {
mkdir -p "$1/home/.agents" "$1/home/.config/crush" "$1/home/.config/opencode"
ln -sfn "$1/ck/skills" "$1/home/.agents/skills"
ln -sfn "$1/home/.agents/skills" "$1/home/.config/crush/skills"
ln -sfn "$1/home/.agents/skills" "$1/home/.config/opencode/skills"
}
# assert_fails <fixture> <substring> <label>
# Note: doctor must be invoked directly here, not via a helper called in a
# command substitution — $? from inside $( ) belongs to the subshell and the
# exit status, which is the whole point of these assertions, would be lost.
assert_fails() {
_out=$(HOME="$1/home" DOCTOR_CHECKOUT="$1/ck" sh "$DOCTOR" 2>&1); rc=$?
if [ "$rc" -eq 0 ]; then
fail=$((fail + 1))
printf ' FAIL %s\n doctor exited 0; drift went undetected\n' "$3"
elif ! printf '%s' "$_out" | grep -qi -- "$2"; then
fail=$((fail + 1))
printf ' FAIL %s\n exited %s but report never mentions "%s"\n got: %s\n' "$3" "$rc" "$2" "$_out"
else
pass=$((pass + 1))
printf ' ok %s\n' "$3"
fi
}
assert_clean() {
_out=$(HOME="$1/home" DOCTOR_CHECKOUT="$1/ck" sh "$DOCTOR" 2>&1); rc=$?
if [ "$rc" -eq 0 ]; then
pass=$((pass + 1)); printf ' ok %s\n' "$2"
else
fail=$((fail + 1)); printf ' FAIL %s\n exited %s on a healthy fixture\n got: %s\n' "$2" "$rc" "$_out"
fi
}
# --- 1. healthy host stays silent -------------------------------------------
f=$(new_fixture clean); wire_clean "$f"
assert_clean "$f" 'clean fixture exits 0'
# --- 2. dangling link (the 243-link case, gitignored and invisible) ----------
f=$(new_fixture dangling); wire_clean "$f"
ln -sfn "$f/ck/skills-that-moved" "$f/home/.agents/skills"
assert_fails "$f" 'dangling' 'dangling canonical link is reported'
# --- 3. resolves, but to a DIFFERENT checkout (the silent-stale case) --------
# Worse than dangling: it works, so nothing complains, while serving content
# from a checkout that is months behind.
f=$(new_fixture stale)
mkdir -p "$f/other/skills/alpha" "$f/other/skills/beta"
printf -- '---\nname: alpha\ndescription: a\n---\n' > "$f/other/skills/alpha/SKILL.md"
printf -- '---\nname: beta\ndescription: b\n---\n' > "$f/other/skills/beta/SKILL.md"
wire_clean "$f"
ln -sfn "$f/other/skills" "$f/home/.agents/skills"
assert_fails "$f" 'outside' 'link into a foreign checkout is reported'
# --- 4. count mismatch (a harness seeing fewer skills than the checkout) -----
f=$(new_fixture count)
mkdir -p "$f/home/.config/crush/skills/alpha"
printf -- '---\nname: alpha\ndescription: a\n---\n' > "$f/home/.config/crush/skills/alpha/SKILL.md"
mkdir -p "$f/home/.agents"
ln -sfn "$f/ck/skills" "$f/home/.agents/skills"
assert_fails "$f" 'count' 'skill-count mismatch is reported'
# --- 5. stray entry (the loose secret-hygiene.md that loaded nowhere) --------
f=$(new_fixture stray); wire_clean "$f"
printf 'orphan\n' > "$f/ck/skills/loose-note.md"
assert_fails "$f" 'stray' 'stray non-directory entry is reported'
# --- 6. legacy Claude symlinks alongside the plugin (duplicate skills) -------
f=$(new_fixture dupe); wire_clean "$f"
mkdir -p "$f/home/.claude/skills"
ln -sfn "$f/ck/skills/alpha" "$f/home/.claude/skills/alpha"
assert_fails "$f" 'legacy' 'leftover per-skill Claude symlinks are reported'
# --- 7. dangling link inside a harness dir (not the canonical link itself) --
# Exercises the per-entry scan rather than the canonical-link check: this is
# the shape the 243 dead links actually had.
f=$(new_fixture ghost); wire_clean "$f"
mkdir -p "$f/home/.claude/skills"
ln -sfn "$f/ck/skills/vanished" "$f/home/.claude/skills/vanished"
assert_fails "$f" 'dangling' 'dangling link inside a harness dir is reported'
printf '\ndoctor: %d passed, %d failed\n' "$pass" "$fail"
[ "$fail" -eq 0 ]