Files
mathias c25175e4b3
CI / Check / Test (push) Successful in 4s
CI / Tag release (push) Successful in 1s
feat(agents): ship pr-gatekeeper from the repo, not one host's ~/.claude (#51)
pr-gatekeeper blocked dispatch#75 five times. Every block was a real,
executable escape — twelve pathspec bypasses, arbitrary command
execution via `git -c alias`, `--upload-pack='rm …'`, a write primitive
through `--output=`, `${IFS}` word-splitting past the policy, and a path
denylist that had been a no-op in the live flow since it was written.
task check was green on all five.

The agent definition existed in no repository. A code search across the
estate found only references to it — comments in dispatch source, a
local-dev sprint doc — while the file itself lived at
~/.claude/agents/pr-gatekeeper.md on koala alone: unversioned, on one
host, one rm from gone. local-dev AGENT.md rule 8 now requires routing
control code through it, so doctrine depended on an artifact with no
history.

PORTED, PLUS WHAT THE FIVE ROUNDS ACTUALLY TAUGHT

The file already had flip-testing. Three habits that did the finding were
implied at best, and are now explicit (§4b):

  - Verify by EXECUTION, not by reading the implementation. Every finding
    came from running the command in a scratch repo and looking at the
    filesystem. The matcher said one thing and the filesystem another,
    five times.
  - Read the SURVIVORS. After an allow/deny sweep, read what is still
    permitted, one at a time. This caught `git commit -F /etc/passwd` —
    exfiltration wearing a message flag — which neither the review nor
    the differential found, because both implementations agreed and both
    were wrong. Agreement is not correctness.
  - Grade a tool on its EXIT CODE, never by grepping output. A crashed
    linter greps as clean.

Plus rule 9 in §4: a test whose name claims more than it proves is worse
than no test, because the name wins in the reader's head.

AND THE INCIDENT THIS AGENT CAUSED (§4c)

On 2026-09-19 a probe of this agent's own making printed /proc/$PPID/environ
and `ps e` raw, and a `git credential fill` consulted the host's real
store. Seven live credentials reached the transcript and need rotation
(infra#445). The finding was correct; the method was not. Versioning the
file without that lesson would have shipped the incident along with the
agent. So: a probe that reads an environment prints a count, never the
buffer; invented sentinels only; isolated HOME for anything touching
git's credential subsystem; prove a primitive and stop.

DOCTOR

New `shadow` check: a real file in ~/.claude/agents with the same name as
a shipped agent wins over the shipped one and nothing says so. Same
silent-drift shape as the existing `outside` and `unmanaged` checks — it
works, so nothing complains, while serving content nobody updates. A
symlink is managed wiring and is not flagged.

The host-local copy on koala has been moved aside, so the shipped file is
what runs once the plugin updates.

TESTS

14 assertions, each named for what it observes rather than what it
guarantees — a grep proves a phrase is present, not that a procedure is
followed, and naming it otherwise would be the defect rule 9 exists for.
Flip-tested both directions: removing the doctor check, and altering the
probe-safety phrase, each turn the suite red.

`agents` added to task check's root-entry allowlist. Leaving it out made
check_tolerates_unverifiable_test.sh fail, which is #23's stray-entry
guard working exactly as designed — an undeclared root entry loads in no
harness and errors in none either.

Closes #51

Bump-Type: minor
2026-09-28 23:30:37 +02:00

403 lines
17 KiB
Bash
Executable File

#!/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
# doctor --online ALSO ask origin for the latest tag (one network call)
#
# 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
ONLINE=0
for arg in "$@"; do
case "$arg" in
--quiet) QUIET=1 ;;
--json) JSON=1 ;;
--online) ONLINE=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".
AGENTS_DIR="$HOME/.agents/skills"
SOURCES_FILE="$(dirname -- "$AGENTS_DIR")/$(basename -- "$AGENTS_DIR")-sources"
if [ -n "${DOCTOR_CHECKOUT:-}" ]; then
CHECKOUT="$DOCTOR_CHECKOUT"
elif [ -f "$SOURCES_FILE" ]; then
# Since #27 the canonical location is a real directory fed by several
# sources, so it can no longer be resolved to a checkout. install.sh records
# the list; the first line is always this repo's skills/ directory.
CHECKOUT=$(dirname -- "$(head -n 1 "$SOURCES_FILE")")
elif [ -L "$AGENTS_DIR" ]; then
# Pre-#27 wiring: one link straight at the checkout. Resolve the link FIRST,
# then take the parent. "cd X/.." resolves ".." lexically against the link
# path, which lands back in ~/.agents.
_resolved=$(CDPATH= cd -- "$AGENTS_DIR" 2>/dev/null && pwd -P)
CHECKOUT=$(dirname -- "$_resolved")
else
CHECKOUT="$SELF_CHECKOUT"
fi
SKILLS_DIR="$CHECKOUT/skills"
findings=0
report=""
# add <severity> <check> <detail>
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 <dir> — directories containing a SKILL.md, one level down.
# -L on purpose: since #27 the entries are per-skill symlinks, and find does
# not descend into a symlinked directory without it.
count_skills() {
[ -d "$1" ] || { printf '0'; return; }
find -L "$1"/ -mindepth 2 -maxdepth 2 -name SKILL.md 2>/dev/null | wc -l | tr -d ' '
}
# under_known_source <physical-path> — 0 when the path lies inside a source
# install.sh recorded, or inside the active checkout.
under_known_source() {
_path=$1
_list="$SKILLS_DIR"
[ -f "$SOURCES_FILE" ] && _list="$_list
$(cat "$SOURCES_FILE")"
printf '%s\n' "$_list" | while IFS= read -r _src; do
[ -n "$_src" ] || continue
_real=$(CDPATH= cd -- "$_src" 2>/dev/null && pwd -P) || continue
[ -n "$_real" ] || continue
case "$_path" in "$_real"/*|"$_real") printf 'yes'; break ;; esac
done | grep -q yes
}
# --- 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
# --- the canonical directory --------------------------------------------------
# Pre-#27 this was a symlink at the checkout. It is a real directory now, one
# per-skill link per skill, fed by every configured source — that is what lets
# a second library (Anthropic's official set on flamingo) coexist instead of
# being displaced.
if [ -L "$AGENTS_DIR" ]; then
add WARN legacy "$AGENTS_DIR is the pre-#27 single-link wiring — re-run install.sh to adopt the multi-source layout"
elif [ -d "$AGENTS_DIR" ]; then
for entry in "$AGENTS_DIR"/*; do
[ -e "$entry" ] || [ -L "$entry" ] || continue
name=$(basename "$entry")
if [ ! -L "$entry" ]; then
add FAIL unmanaged "$AGENTS_DIR/$name is not a link into any source — nothing manages it and nothing updates it"
continue
fi
if [ ! -e "$entry" ]; then
add FAIL dangling "$entry -> $(readlink "$entry") (target does not exist)"
continue
fi
resolved=$(CDPATH= cd -- "$entry" 2>/dev/null && pwd -P)
if [ -n "$resolved" ] && ! under_known_source "$resolved"; then
add FAIL outside "$entry resolves to $resolved, an unknown source — it works, so nothing complains, while serving content nobody updates"
fi
done
# A per-skill layout can lose one skill without changing any total, so the
# invariant is per skill, not a count.
for dir in "$SKILLS_DIR"/*/; do
[ -f "$dir/SKILL.md" ] || continue
name=$(basename "$dir")
# Only skills that ship to the library belong here. A chat-only skill
# missing from a Crush directory is correct, not drift (#29).
surfaces=$(sed -n 's/^surfaces:[[:space:]]*//p' "$dir/SKILL.md" | head -1)
if [ -n "$surfaces" ] \
&& ! printf '%s' "$surfaces" | tr -d '[]' | tr ',' ' ' | grep -qw library; then
continue
fi
[ -L "$AGENTS_DIR/$name" ] \
|| add FAIL missing "$name is in the checkout but has no link in $AGENTS_DIR — re-run install.sh"
done
fi
# --- harness wiring ---------------------------------------------------------
# Crush and OpenCode keep their single directory link at the canonical dir;
# Pi reads it directly and needs no link of its own.
for dir in \
"$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
# A real directory where a link belongs holds content nothing manages: it
# cannot receive new skills and silently shadows the canonical dir. Since
# #27 the fix is not only "move it aside" — it can be added as an extra
# source instead, which is how flamingo keeps the official Anthropic set.
if [ ! -L "$dir" ] && [ -d "$dir" ]; then
add FAIL unmanaged "$dir is a real directory, not a link into the canonical dir — it holds $(count_skills "$dir") unmanaged skill(s); add it to SKILLS_EXTRA_SOURCES and re-run install.sh, or move it aside"
continue
fi
resolved=$(CDPATH= cd -- "$dir" 2>/dev/null && pwd -P)
canonical=$(CDPATH= cd -- "$AGENTS_DIR" 2>/dev/null && pwd -P)
if [ -n "$resolved" ] && [ -n "$canonical" ] && [ "$resolved" != "$canonical" ]; then
add FAIL outside "$dir resolves to $resolved, not the canonical $canonical"
fi
done
# --- dangling links anywhere under the harness dirs -------------------------
for dir in "$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
# --- Claude Code: which GENERATION of the plugin is installed ---------------
# Every check above is about symlink paths, which is the wrong shape for the
# plugin: it is the only channel Claude Code has, and Claude Code gates updates
# on plugin.json's version. A host that never picked up a release loads all the
# skills successfully and is simply a generation behind — silently. Found on
# koala 2026-08-29: installed 0.6.0, marketplace clone 0.7.0, repo 0.8.5,
# doctor said "clean" (#30).
#
# Offline by construction, like the rest of doctor: "available" means the
# newest manifest already on this host — the active checkout and any
# marketplace clone — never a fetch.
manifest_version() {
[ -f "$1" ] || return 1
grep -q '"name"[[:space:]]*:[[:space:]]*"skills"' "$1" || return 1
sed -n 's/.*"version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "$1" | head -1
}
# newer_version a b — echoes whichever sorts higher on major.minor.patch.
# Lives in bin/newer-version because `task check` needs the same ordering (#34)
# and a second copy is a second thing to get wrong.
newer_version() {
sh "$SELF_DIR/newer-version" "$1" "$2"
}
installed_v=''
if [ -f "$INSTALLED" ]; then
installed_v=$(sed -n '/"skills@mathias"/,/\]/p' "$INSTALLED" \
| sed -n 's/.*"version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' | head -1)
fi
available_v=''
for manifest in "$CHECKOUT/.claude-plugin/plugin.json" \
"$HOME"/.claude/plugins/marketplaces/*/.claude-plugin/plugin.json
do
v=$(manifest_version "$manifest") || continue
[ -n "$v" ] || continue
if [ -z "$available_v" ]; then
available_v=$v
else
available_v=$(newer_version "$available_v" "$v")
fi
done
if [ -n "$installed_v" ] && [ -n "$available_v" ] && [ "$installed_v" != "$available_v" ]; then
add FAIL plugin-version "Claude Code has skills@mathias $installed_v installed but $available_v is available on this host — /plugin update skills@mathias (a host stale enough to matter is running the old doctor, so the first detection may need this run by hand)"
fi
# --- optional: the release this host has never fetched ----------------------
# Every check above compares copies already present, so a host whose checkout,
# marketplace clone and installed plugin all agree reads as clean while sitting
# a release behind origin (#38). Closing that needs a network call, which the
# default run must not make — hence opt-in, for a scheduled per-host job rather
# than the SessionStart hook.
#
# $DOCTOR_LATEST_TAG_CMD is the test seam; it prints the newest remote tag.
if [ "$ONLINE" -eq 1 ]; then
if [ -n "${DOCTOR_LATEST_TAG_CMD:-}" ]; then
remote_tag=$(eval "$DOCTOR_LATEST_TAG_CMD" 2>/dev/null | tail -1 | tr -d '[:space:]')
else
# The URL is never printed: a remote rewritten by url.insteadOf can expand
# into a live token.
origin=$(git -C "$CHECKOUT" config --get remote.origin.url 2>/dev/null || true)
if [ -n "$origin" ]; then
remote_tag=$(git ls-remote --tags --refs "$origin" 2>/dev/null \
| sed 's#.*refs/tags/##' | sort -t. -k1,1n -k2,2n -k3,3n | tail -1 | tr -d '[:space:]')
else
remote_tag=''
fi
fi
if [ -z "$remote_tag" ]; then
# Not drift. A host that cannot reach origin has an unanswered question,
# not a stale library, and saying otherwise would make the two look alike.
add WARN offline "could not reach origin for the latest tag — this run proves nothing about staleness"
else
remote_v=${remote_tag#v}
if [ -n "$available_v" ] && [ "$remote_v" != "$available_v" ] \
&& [ "$(newer_version "$available_v" "$remote_v")" = "$remote_v" ]; then
add FAIL behind "this host has $available_v; origin has released $remote_v — run 'task update' and '/plugin update skills@mathias'"
fi
fi
fi
# --- toolchain: golangci-lint must be built with Go >= GOROOT ---------------
# infra#375, second recurrence. golangci-lint embeds the go/types export-data
# reader from the Go it was BUILT with. When GOROOT is newer, that reader
# cannot decode the stdlib and the linter dies:
#
# panic: file requires newer Go version go1.27 (application built with go1.26)
#
# An earlier form printed a typecheck error naming the user's own file, which
# sends people to debug code that is fine. It cost time in two repos before
# anyone recognised it as a host fault rather than a repo fault.
#
# It is invisible to CI -- the runner image has matching versions -- so it
# breaks exactly the local `task check` that AGENT.md requires before a push,
# and nothing else. That is why it belongs in doctor: the signal was missing,
# not the fix.
#
# Fails open on anything unparseable. A host with no Go toolchain is a normal
# host, not a broken one, and doctor runs in a SessionStart hook everywhere.
ver_lt() {
# 0 when $1 < $2 on major.minor. Deliberately not sort -V: this is POSIX sh
# that also runs on macOS, and the export-data format is minor-versioned, so
# patch level does not enter into it.
_a=${1%%.*}; _ar=${1#*.}; _am=${_ar%%.*}
_b=${2%%.*}; _br=${2#*.}; _bm=${_br%%.*}
case "$_a$_am$_b$_bm" in *[!0-9]*) return 1 ;; esac
[ "$_a" -lt "$_b" ] && return 0
[ "$_a" -gt "$_b" ] && return 1
[ "$_am" -lt "$_bm" ]
}
# --- agents shipped by the plugin, shadowed by host-local copies -------------
#
# skills#51: pr-gatekeeper lived ONLY at ~/.claude/agents/pr-gatekeeper.md for
# weeks — unversioned, on one host, while doctrine came to depend on it
# (local-dev AGENT.md rule 8). Now that the plugin ships agents/, a real file of
# the same name in ~/.claude/agents takes precedence over the shipped one and
# nothing says so. That is the same silent-drift shape as the `outside` and
# `unmanaged` checks above: it works, so nothing complains, while serving
# content nobody updates.
CLAUDE_AGENTS="$HOME/.claude/agents"
if [ -d "$CHECKOUT/agents" ]; then
for _shipped in "$CHECKOUT"/agents/*.md; do
[ -e "$_shipped" ] || continue
_name=$(basename -- "$_shipped")
_local="$CLAUDE_AGENTS/$_name"
[ -e "$_local" ] || continue
if [ -L "$_local" ]; then
continue # a link is managed wiring, not a shadow
fi
add FAIL shadow "$_local is a host-local copy of the plugin's agents/$_name — it wins over the shipped one, so the versioned file is not what runs; delete it"
done
fi
if command -v golangci-lint >/dev/null 2>&1 && command -v go >/dev/null 2>&1; then
lint_go=$(golangci-lint --version 2>/dev/null | sed -n 's/.*built with go\([0-9][0-9.]*\).*/\1/p' | head -1)
root_go=$(go version 2>/dev/null | sed -n 's/.*go version go\([0-9][0-9.]*\).*/\1/p' | head -1)
if [ -n "$lint_go" ] && [ -n "$root_go" ] && ver_lt "$lint_go" "$root_go"; then
add FAIL toolchain "golangci-lint built with go$lint_go but GOROOT is go$root_go — lint panics on every Go repo; reinstall it under the current toolchain (infra#375)"
fi
fi
# --- output -----------------------------------------------------------------
if [ "$JSON" -eq 1 ]; then
printf '{"checkout":"%s","expected":%s,"plugin":{"installed":"%s","available":"%s"},"findings":[%s]}\n' \
"$CHECKOUT" "$EXPECTED" "$installed_v" "$available_v" "${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 ]