diff --git a/bin/doctor b/bin/doctor new file mode 100755 index 0000000..ceab5df --- /dev/null +++ b/bin/doctor @@ -0,0 +1,185 @@ +#!/bin/sh +# doctor — report skills-library drift on this host. +# +# Written because every failure this fleet actually hit was SILENT: 243 +# dangling symlinks hidden from git by the **/.claude/skills/ ignore rule, a +# checkout that still resolved but was 20 skills behind, a loose .md that +# loaded in no harness, and two machines where the library was simply absent. +# None of it errored. Exit status here is the signal that was missing. +# +# doctor human report, non-zero exit on drift +# doctor --quiet print only when something is wrong (SessionStart hook) +# doctor --json machine-readable findings +# +# POSIX sh on purpose: it runs on macOS and Linux, before any toolchain is +# guaranteed present, and inside a session-start hook where latency shows. +# It does no network I/O and never shells out to git for a fetch. +# +# Test seams: $HOME drives harness paths, $DOCTOR_CHECKOUT names the checkout. + +set -u + +QUIET=0 +JSON=0 +for arg in "$@"; do + case "$arg" in + --quiet) QUIET=1 ;; + --json) JSON=1 ;; + -h|--help) + sed -n '2,20p' "$0" | sed 's/^# \{0,1\}//' + exit 0 ;; + *) printf 'doctor: unknown option %s\n' "$arg" >&2; exit 2 ;; + esac +done + +SELF_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) +SELF_CHECKOUT=$(CDPATH= cd -- "$SELF_DIR/.." && pwd) + +# The canonical checkout is whatever ~/.agents/skills resolves to — NOT the +# copy doctor happens to be running from. When invoked as a plugin hook, $0 +# lives in the plugin cache, which is legitimately a second copy: judging the +# harness links against it would flag every one of them as "outside". +if [ -n "${DOCTOR_CHECKOUT:-}" ]; then + CHECKOUT="$DOCTOR_CHECKOUT" +elif [ -d "$HOME/.agents/skills" ]; then + # Resolve the link FIRST, then take the parent. "cd X/.." resolves ".." + # lexically against the link path, which lands back in ~/.agents. + _resolved=$(CDPATH= cd -- "$HOME/.agents/skills" 2>/dev/null && pwd -P) + CHECKOUT=$(dirname -- "$_resolved") +else + CHECKOUT="$SELF_CHECKOUT" +fi +SKILLS_DIR="$CHECKOUT/skills" + +findings=0 +report="" + +# add +add() { + findings=$((findings + 1)) + if [ "$JSON" -eq 1 ]; then + report="${report}{\"severity\":\"$1\",\"check\":\"$2\",\"detail\":\"$(printf '%s' "$3" | sed 's/"/\\"/g')\"}," + else + report="${report}$1 $2: $3 +" + fi +} + +# count_skills — directories containing a SKILL.md, one level down. +count_skills() { + [ -d "$1" ] || { printf '0'; return; } + find "$1"/ -mindepth 2 -maxdepth 2 -name SKILL.md 2>/dev/null | wc -l | tr -d ' ' +} + +# --- the checkout itself ---------------------------------------------------- +if [ ! -d "$SKILLS_DIR" ]; then + add FAIL checkout "no skills/ directory at $CHECKOUT" + # Nothing downstream can be judged without it. + printf 'FAIL checkout: no skills/ directory at %s\n' "$CHECKOUT" >&2 + exit 1 +fi + +EXPECTED=$(count_skills "$SKILLS_DIR") +[ "$EXPECTED" -gt 0 ] || add FAIL checkout "skills/ holds no SKILL.md at all" + +# --- stray entries in the checkout ----------------------------------------- +# A loose foo.md where foo/SKILL.md was meant loads in NO harness and errors +# in none either. That is exactly how secret-hygiene.md sat dead for months. +for entry in "$SKILLS_DIR"/*; do + [ -e "$entry" ] || continue + name=$(basename "$entry") + if [ ! -d "$entry" ]; then + add FAIL stray "$SKILLS_DIR/$name is not a directory — it loads nowhere" + elif [ ! -f "$entry/SKILL.md" ]; then + add FAIL stray "$SKILLS_DIR/$name has no SKILL.md" + fi +done + +# --- harness wiring --------------------------------------------------------- +# Canonical location first; Pi reads it directly and the others link to it. +for dir in \ + "$HOME/.agents/skills" \ + "$HOME/.config/crush/skills" \ + "$HOME/.config/opencode/skills" +do + [ -L "$dir" ] || [ -d "$dir" ] || continue + + if [ -L "$dir" ] && [ ! -e "$dir" ]; then + add FAIL dangling "$dir -> $(readlink "$dir") (target does not exist)" + continue + fi + + # Resolve to a physical path and confirm it lands inside THIS checkout. + # A link that resolves to a different checkout is the dangerous case: it + # works, so nothing complains, while serving stale content. + resolved=$(CDPATH= cd -- "$dir" 2>/dev/null && pwd -P) + canonical=$(CDPATH= cd -- "$SKILLS_DIR" 2>/dev/null && pwd -P) + if [ -n "$resolved" ] && [ "$resolved" != "$canonical" ]; then + add FAIL outside "$dir resolves to $resolved, outside the active checkout $canonical" + continue + fi + + got=$(count_skills "$dir") + if [ "$got" != "$EXPECTED" ]; then + add FAIL count "$dir sees $got skills, checkout has $EXPECTED" + fi +done + +# --- dangling links anywhere under the harness dirs ------------------------- +for dir in "$HOME/.agents/skills" "$HOME/.config/crush/skills" \ + "$HOME/.config/opencode/skills" "$HOME/.claude/skills" +do + [ -d "$dir" ] || continue + # Plain glob, not find|while: a while loop on the right of a pipe runs in a + # subshell, so add() would increment a counter that dies with it and doctor + # would exit 0 having found faults. + for link in "$dir"/*; do + [ -L "$link" ] || continue + [ -e "$link" ] && continue + add FAIL dangling "$link -> $(readlink "$link") (target does not exist)" + done +done + +# --- Claude Code: plugin vs leftover per-skill symlinks --------------------- +# Plugin skills do NOT override same-named user skills — both stay visible. So +# leftover links from the pre-plugin era mean every skill appears twice. +CLAUDE_SKILLS="$HOME/.claude/skills" +if [ -d "$CLAUDE_SKILLS" ]; then + legacy=$(find "$CLAUDE_SKILLS"/ -maxdepth 1 -type l 2>/dev/null | wc -l | tr -d ' ') + if [ "$legacy" -gt 0 ]; then + add FAIL legacy "$CLAUDE_SKILLS holds $legacy per-skill symlink(s) from the pre-plugin wiring — remove them or every skill loads twice" + fi +fi + +# Two delivery mechanisms means two copies that can drift apart: the plugin +# (Claude Code) and the checkout the other harnesses link to. Same host, +# different skill sets, no error — compare them. +# Skipped when DOCTOR_CHECKOUT is set: that scopes doctor to an explicit +# target, so comparing it against whatever copy the script lives in is +# meaningless rather than informative. +if [ -z "${DOCTOR_CHECKOUT:-}" ] && [ -f "$SELF_DIR/../.claude-plugin/plugin.json" ] \ + && [ "$SELF_CHECKOUT" != "$CHECKOUT" ]; then + plugin_count=$(count_skills "$SELF_CHECKOUT/skills") + if [ "$plugin_count" != "$EXPECTED" ]; then + add FAIL drift "Claude Code plugin serves $plugin_count skills, the other harnesses see $EXPECTED — the two copies have diverged" + fi +fi + +INSTALLED="$HOME/.claude/plugins/installed_plugins.json" +if [ -f "$INSTALLED" ]; then + grep -q '"skills@mathias"' "$INSTALLED" 2>/dev/null \ + || add WARN plugin "skills@mathias is not installed for Claude Code — /plugin marketplace add https://git.d-ma.be/mathias/skills.git" +fi + +# --- output ----------------------------------------------------------------- +if [ "$JSON" -eq 1 ]; then + printf '{"checkout":"%s","expected":%s,"findings":[%s]}\n' \ + "$CHECKOUT" "$EXPECTED" "${report%,}" +elif [ "$findings" -gt 0 ]; then + printf 'skills doctor — %s finding(s), checkout %s\n\n%s\n' \ + "$findings" "$CHECKOUT" "$report" +elif [ "$QUIET" -eq 0 ]; then + printf 'skills doctor: clean — %s skills, checkout %s\n' "$EXPECTED" "$CHECKOUT" +fi + +[ "$findings" -eq 0 ] diff --git a/hooks/hooks.json b/hooks/hooks.json index b9cf8e2..8db511f 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -1,5 +1,17 @@ { "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "sh \"${CLAUDE_PLUGIN_ROOT}/bin/doctor\" --quiet", + "timeout": 5, + "statusMessage": "Checking skills wiring..." + } + ] + } + ], "UserPromptSubmit": [ { "hooks": [ diff --git a/tests/doctor_test.sh b/tests/doctor_test.sh new file mode 100644 index 0000000..f18eb8d --- /dev/null +++ b/tests/doctor_test.sh @@ -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 — builds $WORK//{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 — 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